Fix GLUT window resizing when CSS scaling

[erik-larsen]

When using GLUT and the window is resized with the canvas dependent upon it due to CSS scaling, the result is a stretched canvas with blocky pixel scaling. Here's a CSS scaling example:

<style>
    canvas {
        position: fixed;
        width: 75%;
        height: 75%;
    }
</style>
<canvas id="canvas"></canvas>

While position fixed isn't strictly necessary, it more readily shows the problem as it makes the canvas size directly dependent upon the browser window. For comparison, SDL behaves properly in this same scenario.

Fix

Three issues were found:

1. On window resize, glutReshapeFunc is never called.

2. Even with glutReshapeFunc working, the dimensions passed to it do not include CSS scaling. Specifically, the canvas width and height are never updated with the canvas clientWidth and clientHeight, which does include scaling.

3. On GLUT program startup, glutMainLoop calls glutReshapeWindow, which is slightly problematic for the case of loading the page while already in fullscreen. This is a problem because, while an initial resize is needed on startup, glutReshapeWindow also forces an exit from fullscreen mode.

Here are the proposed fixes:

1. Register a new resize callback GLUT.reshapeHandler using window.addEventListener, replacing Browser.resizeListeners.push. Previous work in this area (see below) utilized resizeListeners, however this fix takes a different route that is self-contained and I think simpler:

   • Using window.addEventListener keeps the fix entirely within libglut.js, avoiding any libbrowser.js changes as in previous attempts. As well, updateResizeListeners doesn't pass CSS-scaled canvas dimensions, so changing updateResizeListeners implementation might be necessary and this could impact other non-GLUT clients, going beyond this GLUT-only fix.
   • Since glutInit already utilizes window.addEventListener for all other event handling, doing the same for the resize event seems consistent and simpler, as it avoids mixing event handling methods for GLUT.

2. Create a new resize callback function, GLUT.reshapeHandler, which does the following:

   • Updates canvas dimensions (via Browser.setCanvasSize) to canvas.clientWidth and clientHeight, so that CSS scaling is accounted for. If no CSS scaling is present, clientWidth and clientHeight match canvas.width and height, so these values are safe to use in all cases, scaling or not.
   • After updating the canvas size, pass clientWidth and clientHeight to glutReshapeFunc. This is needed so that GLUT reshape callbacks can properly update their viewport transform by calling glViewport with the actual canvas dimensions.

3. At GLUT startup in glutMainLoop, call GLUT.reshapeHandler instead of glutReshapeWindow.

   • As mentioned above, glutReshapeWindow has an unwanted side effect of always forcing an exit from fullscreen (and this is by design, according to the GLUT API).

Testing

Manual testing:

1. Window resizing with no CSS, CSS scaling, CSS pixel dimensions, and a mix of these for canvas width and height.
2. Entering and exiting fullscreen, and loading a page while already in fullscreen.
3. No DPI testing done (window.devicePixelRatio != 1), as GLUT is not currently DPI aware and this fix does not address it. I did confirm on Retina Mac that this fix doesn't make this issue any better or worse.

Automated tests:

1. Added test/browser/test_glut_resize.c, with tests to assert canvas size matches target size under various scenarios (no CSS, CSS scaling, CSS pixel dimensions, and a mix of these), as well as canvas size always matching canvas client size (clientWidth and clientHeight).
2. Since programmatic browser window resizing is not allowed for security reasons, these tests dispatch a resize event after each CSS style change as a workaround.
3. Also added tests to assert canvas size consistency after glutMainLoop and glutReshapeWindow API calls.

Related work

All the previous work in this area worked toward enabling GLUT resize callbacks via Emscripten’s built-in event handling (specifically Browser.resizeListeners and updateResizeListeners). As mentioned above, this fix takes a different approach that is entirely self-contained within libglut.js.

This 2013 commit added GLUT.reshapeFunc to Browser.resizeListeners, presumably to handle window resizing. However there is no test code with that commit, and as of current Emscripten, updateResizeListeners() is never called on window resizing with GLUT programs, so this code is currently a no-op.

Issue 7133 (that I logged in 2018, hi again!) got part of the way on a fix, but used glutReshapeWindow which has the previously mentioned side effect of exiting fullscreen. This was closed unresolved.

PR 9835 proposed a fix for 7133. Also closed unresolved, this fix involved modifying libbrowser.js in order to get resize callbacks to GLUT via resizeListeners. While this got resize callbacks working, in my testing it didn’t pass CSS-scaled canvas size in the callback (the all-important clientWidth and clientHeight).

I also looked at how SDL handles resizing, which uses resizeEventListeners, but decided the more straightforward fix was to use addEventListener. Last, I looked at GLFW CSS scaling test which was helpful in writing the automated tests and also to confirm that no DPI ratio work is addressed by this fix.

Fixes: #7133

[sbc100]

@ypujante will certainly have some thoughts in this area.

[sbc100]

In general I think moving away from Browser.resizeListeners and into a GLUT-specific logic seems like a regression since there will be less shared code, but I get the it might be needed in this case.

[erik-larsen]

I couldn't find a path using Browser.resizeListeners that doesn't widen the fix beyond GLUT. Given GLUT is on life support (?) a self-contained fix seems like the better way to go. As well, glutInit is already using window.addEventListener for all other event callbacks so this seems consistent with that.

[sbc100]

Fair enough. It looks like the use of Browser.resizeListeners was added in 6d6490e, but I don't see any specific rationale for it.

[ypujante]

I don't have any knowledge of GLUT so I am not sure what I can contribute.

I will say that there was a PR that I authored to restore CSS scaling that I had accidentally removed when working on Hi DPI support for libglfw.js. I am not sure if that can be of any help but I thought I would point to it (there is a test that you might want to look at, maybe it can help with this effort)

[erik-larsen]

Thanks - I did look at GLFW CSS scaling test which was helpful in writing the automated tests and also to confirm that no DPI ratio work is addressed by this fix. GLUT doesn't handle Hi DPI but this fix doesn't make it any better or worse.

[erik-larsen]

Looking into CI failures now.

[erik-larsen]

Added potential fix for CI tests.

[erik-larsen]

All of the CI errors related to this PR appear to be fixed. The remaining CI errors look unrelated to the fix, test_glut_resize.c, and GLUT in general:

test-windows
ninja not installed. An error occurred during installation: Error downloading 'ninja.1.13.1' from 'https://community.chocolatey.org/api/v2/package/ninja/1.13.1'.

test-other
FAIL [0.002s]: test_codesize_hello_dylink_all (test_other.other)

test-mac-arm64
FAIL: test_codesize_hello_dylink_all (test_other.other.test_codesize_hello_dylink_all)

test-browser-chrome-wasm64-4gb
[49122:49122:0717/193332.423191:ERROR:CONSOLE:130] "got top level error: exit(0)", source: blob:http://localhost:8888/7de7e610-10a0-46d6-a81e-4ec04042ecec (130)

test-browser-chrome

FAIL [1.920s]: test_gl_stride (test_browser.browser)
----------------------------------------------------------------------
TypeError: Cannot read properties of undefined (reading 'clearColor')
    at _glClearColor (http://localhost:8888/test.js:4040:49)

FAIL [1.495s]: test_webgl_multi_draw_arrays (test_browser.browser)
----------------------------------------------------------------------
TypeError: Cannot read properties of undefined (reading 'GLctx')
    at _emscripten_webgl_enable_WEBGL_multi_draw (http://localhost:8888/test.js:1833:107)

FAIL [0.975s]: test_webgl_multi_draw_arrays_instanced (test_browser.browser)
----------------------------------------------------------------------
TypeError: Cannot read properties of undefined (reading 'GLctx')
    at _emscripten_webgl_enable_WEBGL_multi_draw (http://localhost:8888/test.js:1833:107)

FAIL [1.074s]: test_webgl_multi_draw_elements (test_browser.browser)
----------------------------------------------------------------------
TypeError: Cannot read properties of undefined (reading 'GLctx')
    at _emscripten_webgl_enable_WEBGL_multi_draw (http://localhost:8888/test.js:1833:107)

FAIL [0.986s]: test_webgl_multi_draw_elements_instanced (test_browser.browser)
----------------------------------------------------------------------
TypeError: Cannot read properties of undefined (reading 'GLctx')
    at _emscripten_webgl_enable_WEBGL_multi_draw (http://localhost:8888/test.js:1833:107)

[erik-larsen]

I forgot to mention my motivation for seeing this fix in GLUT: I'm interested in preserving old OpenGL graphics demos. Emscripten is the perfect vehicle for this, as I can compile C and C++ to the web, and then the demos can be run forever more, on any device with a web browser. Without Emscripten, a lot of this code is lost to time.

[erik-larsen]

@kripken We discussed this issue back in 2018 and 2019. This new fix avoids any changes to libbrowser.js and non-GLUT code paths.

@sbc100 Your recommended code changes have been integrated and testing has passed (the latest CI test failures appear unrelated to these changes).

[sbc100]

Hmm.. I recently added test_codesize_hello_dylink_all and I think it might be too sensitive. I think I will revert that.

[sbc100]

Can you rebase (or merge) one more time.

[erik-larsen]

@sbc100 I see @kripken merged, are we good or is another merge needed?

[sbc100]

Looks like you need to do a emsdk install tot followed by ./tools/main/rebaseline_tests.py. if that doesn't work when you do I can take a look too.

[erik-larsen]

Okay, updated to emsdk tot and latest emscripten, ran ./tools/maint/rebaseline_tests.py. Results:

Ran 60 tests in 170.629s

OK (skipped=1)
Total core time: 2301.424s. Wallclock time: 170.629s. Parallelization: 13.49x.
processing test/code_size/test_codesize_cxx_ctors1.json
processing test/code_size/test_codesize_cxx_ctors2.json
processing test/code_size/test_codesize_cxx_except.json
processing test/code_size/test_codesize_cxx_mangle.json
processing test/code_size/test_codesize_cxx_noexcept.json
processing test/code_size/test_codesize_cxx_wasmfs.json
processing test/code_size/test_codesize_hello_O0.json
processing test/code_size/test_codesize_hello_dylink_all.json
processing test/code_size/test_unoptimized_code_size.json

Automatic rebaseline of codesize expectations. NFC

This is an automatic change generated by tools/maint/rebaseline_tests.py.

The following (9) test expectation files were updated by
running the tests with --rebaseline:

code_size/test_codesize_cxx_ctors1.json: 149256 => 149232 [-24 bytes / -0.02%]
code_size/test_codesize_cxx_ctors2.json: 148662 => 148638 [-24 bytes / -0.02%]
code_size/test_codesize_cxx_except.json: 194695 => 194671 [-24 bytes / -0.01%]
code_size/test_codesize_cxx_mangle.json: 258786 => 258762 [-24 bytes / -0.01%]
code_size/test_codesize_cxx_noexcept.json: 151674 => 151650 [-24 bytes / -0.02%]
code_size/test_codesize_cxx_wasmfs.json: 176935 => 176911 [-24 bytes / -0.01%]
code_size/test_codesize_hello_O0.json: 37467 => 37453 [-14 bytes / -0.04%]
code_size/test_codesize_hello_dylink_all.json: 844590 => 844684 [+94 bytes / +0.01%]
code_size/test_unoptimized_code_size.json: 175109 => 175067 [-42 bytes / -0.02%]

Average change: -0.01% (-0.04% - +0.01%)

Anything else I can do?

[sbc100]

That doesn't looks quite right... can you make sure you are up-to-date with the emscripten main branch. And also try emcc --clear-cache before you run the rebaseline script?

[erik-larsen]

Ok. Ran emcc --clear-cache and reran rebaseline:

Ran 60 tests in 187.014s

OK (skipped=1)
Total core time: 2500.130s. Wallclock time: 187.014s. Parallelization: 13.37x.
processing test/code_size/test_codesize_cxx_ctors1.json
processing test/code_size/test_codesize_cxx_ctors2.json
processing test/code_size/test_codesize_cxx_except.json
processing test/code_size/test_codesize_cxx_mangle.json
processing test/code_size/test_codesize_cxx_noexcept.json
processing test/code_size/test_codesize_cxx_wasmfs.json
processing test/code_size/test_codesize_hello_O0.json
processing test/code_size/test_codesize_hello_dylink_all.json
processing test/code_size/test_unoptimized_code_size.json

Automatic rebaseline of codesize expectations. NFC

This is an automatic change generated by tools/maint/rebaseline_tests.py.

The following (9) test expectation files were updated by
running the tests with --rebaseline:

code_size/test_codesize_cxx_ctors1.json: 149232 => 149256 [+24 bytes / +0.02%]
code_size/test_codesize_cxx_ctors2.json: 148638 => 148662 [+24 bytes / +0.02%]
code_size/test_codesize_cxx_except.json: 194671 => 194695 [+24 bytes / +0.01%]
code_size/test_codesize_cxx_mangle.json: 258762 => 258786 [+24 bytes / +0.01%]
code_size/test_codesize_cxx_noexcept.json: 151650 => 151674 [+24 bytes / +0.02%]
code_size/test_codesize_cxx_wasmfs.json: 176911 => 176935 [+24 bytes / +0.01%]
code_size/test_codesize_hello_O0.json: 37453 => 37467 [+14 bytes / +0.04%]
code_size/test_codesize_hello_dylink_all.json: 844684 => 844702 [+18 bytes / +0.00%]
code_size/test_unoptimized_code_size.json: 175067 => 175109 [+42 bytes / +0.02%]

Average change: +0.02% (+0.00% - +0.04%)

[sbc100]

I don't think this change should include changes to most of those test files. Something odd must be going on there.

[erik-larsen]

Yeah I don't see how a change to libglut.js would cause changes elsewhere like this. Maybe something with my environment? Tried on an intel iMac on Sonoma 14.7.6 and a Macbook with latest Sequoia - same diffs.

[erik-larsen]

Curious that the diffs of the second run exactly cancels out the first run? I'm not familiar with rebaseline_tests.py, would one run affect the next?

[sbc100]

If you are having trouble rebaselining I can update the PR for you with the correct expectations.

[erik-larsen]

Yes, please. This is my first PR, and I'm happy to learn how to rebaseline if you can point me to documentation, otherwise please have at it.