Skip to content

Adding AWT Component Rendering capabilities #1150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jseinturier
Copy link
Contributor

@jseinturier jseinturier commented Jul 23, 2019

I propose within this PR an AWT component rendering within app state

This implementation differs from the previous one as it's relies on AppState (compatible with all JME application) and as it use only produced framebuffer (no link with native objects).

With this implementation, it is possible to use AWT component ad rendering target and so to integrate JMonkey rendering within AWT / Swing application. This capability was no more available with LWJGL3 as the version 3.1.x does not provide anymore AWT link.

This implementation is compatible with LWJGL 2 and 3 and so, elder implementation of AWT binding can be removed.

An example of how to use AWT Component rendering can be found within jme3-examples project at src/main/java/jme3tests/awt/AWTComponentAppStateSample.java

@pspeed42
Copy link
Contributor

Did you move a bunch of JME classes around as part of this commit? "...that's going to be a no from me, dawg."

Things that make PRs more palatable:

  1. change only what is needed or make a different PR for the unrelated things.
  2. things that are compatible with the future
  3. things that are required to do something specific (ie: couldn't have just implemented it in your own application that required the specific use-case.)

So, I'm not saying that the essence of this PR would never be accepted... but it certainly has a lot of things going against it even if the class moves were undone. (because that's a non-starter.)

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 30, 2019

Hello,

Yes, i've moved some very old classes that was not referenced by any other modules and that are now completely useless as for example the old AWT related classes cannot be used with LWJGL3.

I know that we have to keep compatibility with old release but these classes are no more used by anybody and we propose alternative that are working with all new libraries.

I can modify my changes and perform a commit without removing old classes if you want.

@pspeed42
Copy link
Contributor

As it is, it's not clear to me that any changes to JME are required to support what you want to do... because I can't tell which changes were required versus "because I felt like it".

@jseinturier
Copy link
Contributor Author

To be more clear, I'm using JME within a Swing application. The problem is that Lwjgl3 has removed all the bindings with AWT (there is no more AWT canvas) and so the AWT related classes within JME does not work with Lwjgl3.
I've decided to implement an AWT bindings that can render JME content within any AWT component using AppState. This solution enables both to integrate JME within any AWT/Swing application and to run with lwjgl2 and lwjgl3.

As i said, i can restore old classes for compatibilité purpose and integrate my new related AWT stuff without interferences.

@pspeed42
Copy link
Contributor

lwjgl3 has bugs with AWT on some platforms that they don't intend to fix. So basically, AWT and lwjgl3 are incompatible as I understand it. Even the JME settings dialog will cause apps to fail on the broken platforms.

As for your app state, does it require any changes to JME to run or can it work stand-alone with JME "as is"? That's the part that was unclear to me because of all of the unrelated collateral changes.

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 30, 2019

According LWJGL version, I'm using the LWJGL3 version because I'm working on Virtual Reality purposes (I'm one of the maintainers of the jme3-vr module) and so i can't use lwgjl2.

I'm not sure that the support of AWT is a bug within lwjgl3. It's just seems that they've stopped to support AWT link (they've removed all AWT related classes from the distribution)

For the JME changes, my appstate can run with JME "as is" i've just proposed to integrate my changes within the jme-desktop module because JME was proposing AWT link and i wanted to make it work again with all the LWJGL version.

@Ali-RS
Copy link
Member

Ali-RS commented Jul 30, 2019

Not sure if this is related so I am putting this here just in case.
@jseinturier there is an unresolved issue with LWJGL3 + GLFW and AWT in Linux with X11 graphical environment. Initializing an AWT window with a GLFW window causes a JVM crash.

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 30, 2019

@Ali-RS i've also experimented incompatibility between lwjgl3 ans AWT even on Windows. This is the reason why i've devlopped the classes that i propose to add to JME.

But if i understand correctly, you are integrating JME canvas using lwjgl3 within a Swing application ?

@Ali-RS
Copy link
Member

Ali-RS commented Jul 30, 2019

But if i understand correctly, you are integrating JME canvas using lwjgl3 within a Swing application ?

No, I mean displaying a Swing window while/before a GLFW window is running. For example JME settings dialog. I already made a hacky fix for settings dialog in #968

Edit:
But yet the crash ( X Error of failed request: RenderBadPicture (invalid Picture parameter)) happens when an Error Dialog shown:

public void showErrorDialog(String message) {

@jseinturier
Copy link
Contributor Author

OK thanks for the information but despite this bug it is not possible to integrate JME with AWT from lwjgl3, even if the bug you describe is not present.

@pspeed42
Copy link
Contributor

What needs to be changed in JME to support your app state? For example, if you provided your app state separately, what would have to be changed in JME to support it?

It feels like "nothing"... but I can't tell because of the 100 other unrelated and unnecessary changes.

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 31, 2019

Nothing has to be changed within JME as my classes can stay belong legacy ones. The only need is to integrate the new classes that are used by the appstate (AWTContext, AWTInput, ...)

The idea of this PR was just to restore JME / AWT linking that is no more functionnal when using jme3-lwjgl3 (the actual JME AWT classes does not work with jme3-lwjgl3).

This PR provide classes that are functionnal with both jme3-lwjgl3 and jme3-lwjgl with the same code.

If you want i can modify my PR in order to only add New files and not modifying JME existing ones.

@Ali-RS
Copy link
Member

Ali-RS commented Jul 31, 2019

btw, @jseinturier I tested the AWTComponentAppStateSample on my Linux and it works fine.

@empirephoenix
Copy link
Contributor

I currently run a LWJGL3 inside a Swing frame without issues on Arch, using the Canvas based integration. Might this be a nvidia properitary driver issue?

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 31, 2019

I currently run a LWJGL3 inside a Swing frame without issues on Arch, using the Canvas based integration. Might this be a nvidia properitary driver issue?

Hello,

Can you please tell me which code you are using because LWJGL3 (the one within the master branch) does no more provide AWT bindings.

I've modified the PR and now no previous JME class is affected by my modifications.

@empirephoenix
Copy link
Contributor

Here it is, it#s bit shitty quality, because its only a blend to j3o + and all to dds textures converter, so the actual application starts faster :)

	@Override
	public void startCompileApplication() {
		this.app = new CompileApplication();
		this.app.setShowSettings(false);
        app.createCanvas();
        app.startCanvas(true);

		final File outFolder = new File(CompileConfiguration.getInstance().getCompiledGit());

		this.app.setPaths(this.packagesfolder, outFolder);
		try {
			SwingUtilities.invokeAndWait(() -> {
				final JmeCanvasContext ctx = (JmeCanvasContext) ConfigFrame.this.app.getContext();
				ConfigFrame.this.jcan = ctx.getCanvas();
				final Dimension dim = new Dimension(CompileApplication.PREVIEW_DIMENSION, CompileApplication.PREVIEW_DIMENSION);
				ConfigFrame.this.jcan.setSize(dim);
				ConfigFrame.this.jcan.setPreferredSize(dim);
				ConfigFrame.this.jcan.setMaximumSize(dim);
				ConfigFrame.this.jcan.setMinimumSize(dim);

				ConfigFrame.this.getContentPane().add(ConfigFrame.this.jcan);
			});
		} catch (InvocationTargetException | InterruptedException e) {
			e.printStackTrace();
		}

		this.pack();
	}
public CompileApplication() {
		this.setSettings(new AppSettings(true));
		this.settings.setVSync(true);
		this.settings.setRenderer(AppSettings.LWJGL_OPENGL3);
		this.settings.setSamples(1);
		this.settings.setAudioRenderer(null);
		this.settings.setTitle("CompileApplication");
		this.settings.setHeight(CompileApplication.PREVIEW_DIMENSION);
		this.settings.setWidth(CompileApplication.PREVIEW_DIMENSION);
		this.settings.setFrameRate(60);
		// this.settings.setCustomRenderer(AwtPanelsContext.class);

		CompileApplication.instance = this;
		this.setDisplayStatView(false);
		this.setDisplayFps(false);
	}

@jseinturier
Copy link
Contributor Author

@empirephoenix I'm sorry but when i run the jme3test.awt.TestCanvas class from the jme3-examples module from the master branch i got this exception:

INFO JmeDesktopSystem 09:31:42 Running on jMonkeyEngine 3.3-jme3-awt-6864

  • Branch: jme3-awt
  • Git Hash: de092b9
  • Build Date: 2019-07-23
    SEVERE JmeDesktopSystem 09:31:42 CRITICAL ERROR: Context class is missing!
    Make sure jme3_lwjgl-ogl is on the classpath.
    java.lang.ClassNotFoundException: com.jme3.system.lwjgl.LwjglCanvas
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:583)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
    at java.base/java.lang.Class.forName0(Native Method)
    at java.base/java.lang.Class.forName(Class.java:315)
    at com.jme3.system.JmeDesktopSystem.newContextLwjgl(JmeDesktopSystem.java:196)
    at com.jme3.system.JmeDesktopSystem.newContext(JmeDesktopSystem.java:279)
    at com.jme3.system.JmeSystem.newContext(JmeSystem.java:159)
    at com.jme3.app.LegacyApplication.createCanvas(LegacyApplication.java:510)
    at jme3test.awt.TestCanvas.createCanvas(TestCanvas.java:217)
    at jme3test.awt.TestCanvas.main(TestCanvas.java:248)
    Exception in thread "main" java.lang.NullPointerException
    at com.jme3.system.JmeDesktopSystem.newContext(JmeDesktopSystem.java:280)
    at com.jme3.system.JmeSystem.newContext(JmeSystem.java:159)
    at com.jme3.app.LegacyApplication.createCanvas(LegacyApplication.java:510)
    at jme3test.awt.TestCanvas.createCanvas(TestCanvas.java:217)
    at jme3test.awt.TestCanvas.main(TestCanvas.java:248)

I precise that i've included jme3-lwjgl3 in my classpath and NOT jme3-lwjgl

@empirephoenix
Copy link
Contributor

empirephoenix commented Jul 31, 2019

That class is in master:
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-lwjgl/src/main/java/com/jme3/system/lwjgl/LwjglCanvas.java
Are you sure you use lwjgl3 dependency?
Can you show a dependencies report?

@jseinturier
Copy link
Contributor Author

That class is in master:
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-lwjgl/src/main/java/com/jme3/system/lwjgl/LwjglCanvas.java
Are you sure you use lwjgl3 dependency?
Can you show a dependencies report?

As you said this is within the master jme3-lwjgl but I'm using jme3-lwjgl3 within my app. I don't want to include both jme3-lwjgl and jme3-lwjgl3 (and i'm not sure that is safe to integrate the two modules)

@empirephoenix
Copy link
Contributor

empirephoenix commented Jul 31, 2019

Well I see your point, not even saying it's safe, just that it is my current workaround. Having a clean solution would be nice tho, so I think this PR makes sense.

Btw can MOUSE_BUTTON_TO_JME be changed to protected? eg to support more than the 3 default buttons via custom subclass?

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 31, 2019

Btw can MOUSE_BUTTON_TO_JME be changed to protected? eg to support more than the 3 default buttons via custom subclass?

Ok I'm on i will commit soon

@pspeed42
Copy link
Contributor

Note: "make this field protected" is not proper design. It's "now we have to support this map forever" design. If you want subclasses to be able to add their own then give them a protected method to do so.

But anyway, for some reason when I look at the "Files changed", I still see classes being moved around. I don't know if this is a limitation in the GIT interface or not. If this class does not require any changes to JME then maybe it's best kept separate? Those folks who don't mind taking the "copy the frame" hit can always copy it into their own project or we can make it a JME example or something?

Supporting all of these different AWT hacks seems to get us into trouble and they can almost never be used "as is" anyway.

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 31, 2019

Hello,

When i'm looking at the changed files, there is no more deleted classes. The moved ones are the classes i add with a previous commit that i've refactored in order to not interfere with legacy classes that had the same name, so i think that only my code is affected by the move.

My classes does not require changes within JME but can be seen as an new implementation of the AWT link that does not relies on specific version of LWGL and on a specific GUI system. This is the reason why i propose to integrate it within the JME desktop module.

From my point of view, AWT related stuff that is actually present within JME is no more maintained and only works with LWJGL2. I'm proposing an alternative to restore JME AWT capabilities with an actualized and maintained code (as i'm using it - as for the vr - i'm able to maintain these functionalities) that works on all LWJGL version on all systems.

@pspeed42
Copy link
Contributor

For some reason, github is still showing "Files changed 9"... and when I look I see package changes to .awt, etc..

image

AWTInputMouse wheel scale factor can be dynamically changed

AWT Component resizing is now enable to resize the JME rendering viewPort
@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 31, 2019

Ok, first, i've modified the AWTInputMouse in order to enable to set button mapping using accessor (no more static protected fields).

For the changed files:

  1. jme3-desktop/src/main/java/com/jme3/app/state/AWTComponentAppState.java
    It's a new class, the main one for the AWT rendering

  2. jme3-desktop/src/main/java/com/jme3/input/AWTInput.java → jme3-desktop/src/main/java/com/jme3/input/awt/AWTInput.java
    This class has been added by me but when i first commited it, i made a mistake on the package of the class, so i've changed it later. But this class has been added by me on a previous commit and PR

  3. jme3-desktop/src/main/java/com/jme3/input/AWTKeyInput.java → jme3-desktop/src/main/java/com/jme3/input/awt/AWTInputKeyboard.java
    The moved class has been added wihin my previous commit, i've refactored it as a class named AwtKeyInput was existing within the package. I've restored the original AwtKeyInput class and renamed mine from AWTKeyInput.java to AWTInputKeyboard.java

  4. jme3-desktop/src/main/java/com/jme3/input/AWTMouseInput.java → jme3-desktop/src/main/java/com/jme3/input/awt/AWTInputMouse.java
    The moved class has been added wihin my previous commit, i've refactored it as a class named AwtMouseInput was existing within the package. I've restored the original AwtMouseInput class and renamed mine from AWTMouseInput.java to AWTInputMouse.java

  5. jme3-desktop/src/main/java/com/jme3/system/AWTComponentRenderer.java → jme3-desktop/src/main/java/com/jme3/system/awt/AWTComponentRenderer.java
    I've moved this class from the same reason as 2. But this class have been added by me within last commit / PR and it is not a JME legacy one.

  6. jme3-desktop/src/main/java/com/jme3/system/AWTContext.java → jme3-desktop/src/main/java/com/jme3/system/awt/AWTContext.java
    Same explanation as 5.

  7. jme3-desktop/src/main/java/com/jme3/system/AWTFrameProcessor.java → jme3-desktop/src/main/java/com/jme3/system/awt/AWTFrameProcessor.java
    Same explanation as 5.

  8. jme3-desktop/src/main/java/com/jme3/system/AWTTaskExecutor.java → jme3-desktop/src/main/java/com/jme3/system/awt/AWTTaskExecutor.java
    Same explanation as 5.

  9. jme3-examples/src/main/java/jme3test/awt/AWTComponentAppStateSample.java
    This is a new class that i've committed for illustrating the use of AWTComponentAppState.

I can see what is annoying for you: i've moved some classes (4) from com.jme3.system to com.jme3.system.awt. We can reverse these changes but it is more consistent from my point of view to let the awt related stuff within an awt package. However, these 4 classes are only used internally and so the move cannot be seen by persons that are using JMonkey as a library. Only JME developpers will be affected and only in the case where they want to modify the AWT stuff.

@riccardobl
Copy link
Member

riccardobl commented Jul 31, 2019

How can you know that those classes are used only internally? They are public...there is nothing in place to stop people from using them in their code. I'm not necessarily against breaking changes, if they are justified, but i don't think they are in this case.

On top of that, it is generally much easier to review prs if they don't move/rename classes.

Also, i'd like to know if this will affect the current awt integration.

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 31, 2019

"How can you know that those classes are used only internally?"

I know it because I'm the one who add them and i don't think that other People is working on AWT integration. But you're right that internally used classes should not be declared as public.

"but i don't think they are in this case."
can you justify why these changes are not justified ?

"Also, i'd like to know if this will affect the current awt integration."
all my classes are independant from the current integration so no there is no board effect. However i just want to argue that actually the existing AWT integration is not working with lwjgl3 that is bundle with JME.

@riccardobl
Copy link
Member

I know it because I'm the one Who add them...

Yes ok. They are intended to be internal, but there is nothing enforcing it.

can you justify why these changes are not justifiés ?

They don't add anything meaningful. Note: I'm talking only about the refactoring, not about the whole pr.

is not working with lwjgl3 that is bundle with JME.

Yes i know, but the other integration is still preferable with lwjgl2, because this new one is copying the entire frame back and forth.

@jseinturier
Copy link
Contributor Author

jseinturier commented Jul 31, 2019

I don't say that i propose the best solution but only a functional one. My problem is that i need jme3-lwjgl3 and i need AWT binding... I thought that restoring AWT binding with JME for all possible configurations was interesting for the project. Even if there is a frame copy, performance is quite good.

My classes does not overwrite existing one and so, it is possible to use the legacy method with LWJGL2. For those who want AWT biding without having to take in account the version of LWJGL or the system (Windows, linux, ...), the PR i propose enable to answer their need. I think that the 2 solution can live together.

@riccardobl
Copy link
Member

Yeah, it's fine by me. But i still think that the refactoring part should be reverted.

@jseinturier
Copy link
Contributor Author

Up to you guys, the refactor was just to avoid having AWT related classes melted with other ones but if you prefer, i can revert the refactoring.

@tlf30
Copy link
Contributor

tlf30 commented Sep 27, 2019

@jseinturier seeing as the current roadmap for jme4 is to remove support for lwjgl 2, when I am at a good point in the jme4 repo, I will let you know and we can talk about applying this to the jme4 repo. In the meantime it would be useful for bookkeeping if you open this as an issue on the jme4 repo.
https://github.com/tlf30/jme4

@jorigin
Copy link

jorigin commented Apr 8, 2020

Hello,

I really need AWT capabilities to be integrated within JMonkey despite using LWJGL2 or LWJGL3, this PR is a solution for an independent AWT integration.

Can you please remind me why this PR is not accepted ?

Thanks a lot.

@openMonArch
Copy link

@jseinturier
Hello, our team is highliy intressted in your solution. We are using Jmonkey in an Eclipse/RCP/SWT-Application. We use the AwtPanelsContext to get jme3-lwjgl3 working. Our solution ist not very stable, because it does not run on linux and mac. So I would highly vote to get your solution into master. Because with an AppState soution it would be much easier for us.
Would it be possible to put everything awt related in a own module. like the blender module.
Thanks

@jseinturier
Copy link
Contributor Author

@jseinturier
Hello, our team is highliy intressted in your solution. We are using Jmonkey in an Eclipse/RCP/SWT-Application. We use the AwtPanelsContext to get jme3-lwjgl3 working. Our solution ist not very stable, because it does not run on linux and mac. So I would highly vote to get your solution into master. Because with an AppState soution it would be much easier for us.
Would it be possible to put everything awt related in a own module. like the blender module.
Thanks

Hello,

Thanks a lot for your support, i hope solution will be integrated soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants