Fix glfw3 default hints being modified

[ypujante]

The code is doing: GLFW.hints = GLFW.defaultHints which is assigning the object GLFW.defaultHints to the GLFW.hints and not copying its value. As a result setting any hint after the fact (using glfwWindowHint) modifies the GLFW.defaultHints and as a result glfwDefaultWindowHints is not restoring the defaults.

The purpose of this PR is to fix this problem. I also added a test for it.

[sbc100]

lgtm with some comments

[sbc100]

I wonder if you can test this without using EM_JS and instead using glfwGetWindowParam?

Then maybe we can avoiding hardcoded values like 0x00021005 in the test?

[ypujante]

The issue is that glfwGetWindowParam is not part of the GLW3 api. According to the documentation it has been replaced by glfwGetWindowAttrib, but it requires a window as an argument which I don't have in the test because I never create a window.

[ypujante]

@sbc100 I have updated the PR according to your feedback.

Looking at the source code, I noticed this as well

this.attributes = GLFW.hints;

which is a similar "mistake" and should probably be fixed.

If you write a test code like this:

glfwDefaultWindowHints();
glfwWindowHint(GLFW_DEPTH_BITS, 16);
window = glfwCreateWindow(...)
assert(glfwGetWindowAttrib(window, GLFW_DEPTH_BITS) == 16);
// this changes the window hints and due to the shallow copy, 
// also the attributes of the window just created...
glfwWindowHint(GLFW_DEPTH_BITS, 24); 
assert(glfwGetWindowAttrib(window, GLFW_DEPTH_BITS) == 16); // this will fail...

Do you want me to fix it part of this PR as well?

// trivial fix...
this.attributes = Object.assign({}, GLFW.hints);

[ypujante]

@sbc100 I ended up fixing the attributes assignment as well since it is such a trivial fix (and added a test for it)

I don't understand why so many tests failed and they seem to be unrelated to my changes. The test that I added passes ok:

  test_glfw3_default_hints (test_browser.browser) ... cache:INFO: generating system asset: symbol_lists/bd12648a8db173908b13aa381a9bd5bab1858118.json... (this will be cached in "/root/cache/symbol_lists/bd12648a8db173908b13aa381a9bd5bab1858118.json" for subsequent builds)
cache:INFO:  - ok
ok (0.931s)

[sbc100]

Can you try merging (or rebasing) to get the latest changes from the main branch?

[sbc100]

lgtm

[ypujante]

The last update is already up to date with main. I will try again when there is a new commit.

[sbc100]

The last update is already up to date with main. I will try again when there is a new commit.

I just landed b7924c4. Can you rebase or merge on top of that?

[ypujante]

There has been some progress, but still some tests failure which I believe are unrelated to my changes:

FAIL [121.921s]: test_idbstore_sync_jspi (test_browser.browser)
FAIL [122.945s]: test_idbstore_sync_worker (test_browser.browser)
FAIL [121.052s]: test_in_flight_memfile_request_O0 (test_browser.browser)
FAIL [121.069s]: test_in_flight_memfile_request_O1 (test_browser.browser)
FAIL [121.922s]: test_in_flight_memfile_request_O2 (test_browser.browser)

[ypujante]

Everything is green! Yay!

[ypujante]

I fixed the small indentation issues and everything still green :)