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

[Fix] #20600 : Calling GLFW.setWindowSize with the screen size leads to browser error #20699

Merged
merged 2 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ See docs/process.md for more on how version tagging works.

3.1.49 (in development)
-----------------------
- The `glfwSetWindowSize` function no longer switches to fullscreen when the
width/height provided as parameters match the screen size. This behavior
now matches the behavior of SDL and glut. In order to switch to fullscreen,
the client code should invoke `Module.requestFullscreen(...)` from a user
triggered event otherwise the browser raises an error. (#20600)

3.1.48 - 11/05/23
-----------------
Expand Down
11 changes: 3 additions & 8 deletions src/library_glfw.js
Original file line number Diff line number Diff line change
Expand Up @@ -981,14 +981,9 @@ var LibraryGLFW = {
if (!win) return;

if (GLFW.active.id == win.id) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the Browser.exitFullscreen(); thing... should explictly setting the windows size also not exit fullscreen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good question. SDL does not do it. glut does it. In my opinion SDL is right. When Browser.requestFullscreen is called, a fullscreenchange listener is put in place to detect exiting full screen (the user pressing ESC, or whatever key triggers that).

Calling Browser.exitFullscreen(); might trigger an error for the same reason that Browser.requestFullscreen can (if the code calls glfwSetWindowSize while in fullscreen mode but not from a user triggered event).

I believe the proper way for the client code to handle this is to set a Module.onFullScreen callback which gets called whenever the browser goes in/out of fullscreen and then call glfwSetWindowSize.

I actually used this technique in my code this morning:

    EM_ASM(
        {
            RAYLIB = {}; // used to store width/height before fullscreen
            const cb = Module.requestFullscreen;
            Module.requestFullscreen = (pointerLock, resize) => {
                const canvas = document.getElementById('canvas');
                RAYLIB.screenWidth = canvas.clientWidth;
                RAYLIB.screenHeight = canvas.clientHeight;
                cb(pointerLock, resize);
            };
            Module.onFullscreen = Module.cwrap('FullscreenCallback', null, ['boolean']); // called by emscripten
        });

void FullscreenCallback(bool isFullscreen) {
    if(isFullscreen) {
       // ...
    }
    else  {
        glfwSetWindowSize(EM_ASM_INT(return RAYLIB.screenWidth;), EM_ASM_INT(return RAYLIB.screenHeight;));
    }
}

That being said, if you prefer to leave it, I believe the chances of getting an error are lower and probably have less consequences.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I think being consistent with SDL and glut makes more sense.

if (width == screen.width && height == screen.height) {
Browser.requestFullscreen();
} else {
Browser.exitFullscreen();
Browser.setCanvasSize(width, height);
win.width = width;
win.height = height;
}
Browser.setCanvasSize(width, height);
win.width = width;
win.height = height;
}

if (win.windowSizeFunc) {
Expand Down