Skip to content
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

update LWJGL3 to v3.3.4 #2314

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

update LWJGL3 to v3.3.4 #2314

wants to merge 2 commits into from

Conversation

stephengold
Copy link
Member

to address issue #2298

@stephengold stephengold added this to the Future Release milestone Oct 22, 2024
@stephengold stephengold linked an issue Oct 22, 2024 that may be closed by this pull request
@stephengold stephengold added the buildscript An issue with the buildscript label Oct 22, 2024
@JNightRider
Copy link
Contributor

JNightRider commented Oct 25, 2024

Hi @stephengold

I was doing some tests with the new version of lwjgl3.4, apparently there is a small problem, this conflict is related to Wayland, when a window position is set, the JME3 application breaks.

oct. 24, 2024 8:28:08 P. M. com.jme3.system.JmeDesktopSystem initialize
INFORMACIÓN: Running on jMonkeyEngine 3.8.0-SNAPSHOT
 * Branch: unknown
 * Git Hash: 
 * Build Date: 2024-10-24
oct. 24, 2024 8:28:08 P. M. com.jme3.app.LegacyApplication handleError
GRAVE: Wayland: The platform does not support setting the window position
java.lang.Exception: Wayland: The platform does not support setting the window position
	at com.jme3.system.lwjgl.LwjglWindow$1.invoke(LwjglWindow.java:236)
	at org.lwjgl.glfw.GLFWErrorCallbackI.callback(GLFWErrorCallbackI.java:43)
	at org.lwjgl.system.JNI.invokePV(Native Method)
	at org.lwjgl.glfw.GLFW.glfwSetWindowPos(GLFW.java:2536)
	at com.jme3.system.lwjgl.LwjglWindow.createContext(LwjglWindow.java:327)
	at com.jme3.system.lwjgl.LwjglWindow.initInThread(LwjglWindow.java:591)
	at com.jme3.system.lwjgl.LwjglWindow.run(LwjglWindow.java:714)
	at java.base/java.lang.Thread.run(Thread.java:840)

oct. 24, 2024 8:28:08 P. M. com.jme3.system.lwjgl.LwjglContext printContextInitInfo
INFORMACIÓN: LWJGL 3.3.4+7 context running on thread jME3 Main
 * Graphics Adapter: GLFW 3.5.0 Wayland X11 GLX Null EGL OSMesa monotonic shared
oct. 24, 2024 8:28:09 P. M. com.jme3.renderer.opengl.GLRenderer loadCapabilitiesCommon

According to lwjgl, this is incompatible with Wayland (apparently the previous version used x11 as the backend by default).
source: here

This leaves wayland users with a serious problem. Would wayland users have to manually configure the lwjgl backend from now on?

Test Code

import com.jme3.app.SimpleApplication;
import com.jme3.system.AppSettings;

public class JME3 extends SimpleApplication {

    public static void main(String[] args){
        JME3 app = new JME3();
        AppSettings settings = new AppSettings(true);
        settings.setCenterWindow(true);

        app.setShowSettings(false);
        app.start();
    }

    @Override
    public void simpleInitApp() {
    }
}

[ NOTE ]
I don't know if this is the right place to discuss this problem, I hope I don't bother anyone.

@stephengold
Copy link
Member Author

@JNightRider Thanks for pointing out this issue. Totally the right place to discuss it. And thanks for the test code.

@stephengold
Copy link
Member Author

@JNightRider quick question: Do stable releases of JME work with the Wayland platform, or only with X11?

@JNightRider
Copy link
Contributor

JNightRider commented Oct 25, 2024

Current versions 3.x.x-stable with lwjgl3.x < v3.4 work as they use the x11 backend (as long as we use XWayland as a bridge), however with the fixes in the new version of lwjgl jme3 no longer works in wayland...

If no window position is set it works without problems.

@stephengold
Copy link
Member Author

I see at least 3 options:

  1. Proceed with the LWJGL update even though it will cause some apps to crash on Wayland platforms.
  2. Update and add code to catch the exception on Wayland platforms.
  3. Delay the LWJGL update until LWJGL v3.3.5 is released, in the hope it will solve the issue.

Seeing as Spasi closed lwjgl3 issue 998 as "3rd party", I have little confidence in option 3.

@JNightRider
Copy link
Contributor

Seeing as Spasi closed lwjgl3 issue 998 as "3rd party", I have little confidence in option 3.

Option 3 is certainly not reliable

Update and add code to catch the exception on Wayland platforms.

I would like jme3 to continue working with wayland even though it doesn't support viewport positioning.

The problem is in the glfwSetWindowPos() method (class -> LwjglWindow - Line [327 - 333]), if jme3 is removed it works (I think something similar to this can be handled for wayland only)

Captura desde 2024-10-25 21-41-00

I can move (drag) the window without problems

@stephengold
Copy link
Member Author

@JNightRider:

if jme3 is removed it works

I don't understand what that means. Can you elaborate? Or provide a patch?

@JNightRider
Copy link
Contributor

JNightRider commented Oct 26, 2024

@stephengold I'm sorry for not explaining myself correctly...

If we avoided calling the glfwSetWindowPos() method when starting the context, the problem with wayland would not arise, of course this leaves wayland users without the option of positioning the window.

At the moment I can run jme3 if I add this small patch in the LwjglWindow class:

        if (!settings.isFullscreen() && glfwGetPlatform() != GLFW_PLATFORM_WAYLAND) {
            if (settings.getCenterWindow()) {
                // Center the window
                glfwSetWindowPos(window,
                        (videoMode.width() - requestWidth) / 2,
                        (videoMode.height() - requestHeight) / 2);
            } else {
                glfwSetWindowPos(window,
                        settings.getWindowXPosition(),
                        settings.getWindowYPosition());
            }
        }

glfwGetPlatform() != GLFW_PLATFORM_WAYLAND <- patch

Test class

I am using this class as a test, I can drag the window, resize and there is no problem (full screen is not a problem):

import com.jme3.app.SimpleApplication;
import com.jme3.material.Material;
import com.jme3.math.ColorRGBA;
import com.jme3.scene.Geometry;
import com.jme3.scene.shape.Box;
import com.jme3.system.AppSettings;

public class JME3 extends SimpleApplication {

    public static void main(String[] args){
        JME3 app = new JME3();
        AppSettings settings = new AppSettings(true);
        settings.setResizable(true);

        app.setShowSettings(false);
        app.setPauseOnLostFocus(false);
        app.setSettings(settings);
        app.start();
    }

    @Override
    public void simpleInitApp() {
        inputManager.setCursorVisible(true);
        getFlyByCamera().setDragToRotate(true);

        Box b = new Box(1, 1, 1);
        Geometry geom = new Geometry("Box", b);
        Material mat = new Material(assetManager, "Common/MatDefs/Misc/Unshaded.j3md");
        mat.setColor("Color", ColorRGBA.Blue);

        geom.setMaterial(mat);
        rootNode.attachChild(geom);
    }
}

It seems that this is not a lwjgl specific problem, in the same GLFW documentation it is indicated that this option is not supported on the wayland platform.

@stephengold
Copy link
Member Author

I like your patch. It seems more clear than simply catching the exception. I'll add it to this PR.

@stephengold
Copy link
Member Author

@JNightRider I'm unable to install Wayland for testing. Would you test this branch please?

@JNightRider
Copy link
Contributor

JNightRider commented Oct 28, 2024

Would you test this branch please?

@stephengold Everything works very well, I have not found any errors!

It's great that jme3 works with Wayland ❤️.

@stephengold
Copy link
Member Author

Great! Then I believe this is ready for integration.

@tonihele
Copy link
Contributor

tonihele commented Oct 29, 2024

One thing that came to mind, https://www.glfw.org/docs/latest/news.html.

Window hints for initial window position

GLFW now provides the GLFW_POSITION_X and GLFW_POSITION_Y window hints for specifying the initial position of the window. This removes the need to create a hidden window, move it and then show it. The default value of these hints is GLFW_ANY_POSITION, which selects the previous behavior.

For more information see Window position.

The above comes with this version now, so would it be possible to use these and have the feature work on Wayland?

On the more detailed documentation it says that the windowing system may also ignore these. So I don't know is it exactly the same as old, but sounds more simpler the way it works too.
https://www.glfw.org/docs/latest/window_guide.html#GLFW_POSITION_X
https://www.glfw.org/docs/latest/window_guide.html#window_pos

@JNightRider
Copy link
Contributor

Hi @tonihele, thank you very much for the suggestion

thanks to @tonihele for suggesting this alternative

GLFW_POSITION_X and GLFW_POSITION_Y: This is definitely a better solution for sales positioning, however it is still incompatible with Waylanda (it doesn't break jme3, it just ignores window positioning). That is to say with wayland it has no effect, it is fine and it would avoid the small patch (detect the platform)...

I overlooked something in this matter, now I have something else to report, setting icons in the window throws a serious error (related to wayland -> compatibility issues).

little suggestion

We can use glfwWindowHint(GLFW_POSITION_X, xPos); and glfwWindowHint(GLFW_POSITION_Y, yPos); methods to set the window position and avoid patching (I didn't try them on Windows or x11, if it works on wayland -> It has no effect but does not throw any error, it is like an alternative to the patch):

if (!settings.isFullscreen()) {
    if (settings.getCenterWindow()) {
        // Center the window
        glfwWindowHint(GLFW_POSITION_X,  (videoMode.width() - requestWidth) / 2);
        glfwWindowHint(GLFW_POSITION_Y, (videoMode.height() - requestHeight) / 2);
    } else {
        glfwWindowHint(GLFW_POSITION_X,  settings.getWindowXPosition());
        glfwWindowHint(GLFW_POSITION_Y, settings.getWindowYPosition());
    }
}

With icons, I think we have to patch it to avoid calling the glfwSetWindowIcon(window, icon); method and breaking jme in wayland.

protected void setWindowIcon(final AppSettings settings) {
    if (glfwGetPlatform() == GLFW_PLATFORM_WAYLAND) {
        return;
    }
    ...
}

Of course now there is no support for custom icons and sales positioning on platforms with wayland, I think this is fine since JME3 works with wayland (I don't want to port my small games to another engine or switch to x11 just for that).

I will perform more tests when you have time to avoid unexpected surprises.

@stephengold stephengold marked this pull request as draft October 30, 2024 07:39
@JNightRider
Copy link
Contributor

I was recently testing things with this branch, I can run jme3 on wayland without problems (except for the 2 known issues)

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

Successfully merging this pull request may close these issues.

update jme3-lwjgl3 to use LWJGL v3.3.4
3 participants