Changes in preparation for Issue 3372

[ypujante]

Following our discussion on #3372, I am opening this PR.

The idea is that raylib can safely use glfwGetFramebufferSize to retrieve the correct frame buffer size. Before my emscripten PR is merged, it simply returns width and height (see code) so the screenScale will be 1.

The change in WindowSizeCallback is to follow what happens during InitPlatform
The change in EmscriptenResizeCallback is to simply call SetWindowSize as it will trigger the callback. Setting the canvas size explicitly (which is what is done prior to this PR) is in my opinion not the right thing to do anyway because it bypasses glfw.

Once the emscripten changes have been pushed, 4k is enabled by doing something like this:

EM_ASM(
Module.setHiDPIAware(true);
);

which can be done directly in the client code. I can imagine that there could be a convenient API provided by raylib so that client code does not have to write something like this. It could also be tied to the flag FLAG_WINDOW_HIGHDPI if that was the path you would like to follow.

Although I did test my changes on my (own) local example, I would be more than happy to run deeper tests if you let me know how you proceed when you make this kind of changes so as to make sure I did not break anything.

Thank you

[ypujante]

I also want to add that if you would prefer to wait until the changes are pushed to emscripten, it is obviously no issue with me. At this stage I have no clue if/when it will be merged as it has been in "waiting for somebody to approve" for several days now.

[ghost]

@ypujante Noticed that the previous/current WindowSizeCallback() and EmscriptenResizeCallback() had breaks if fullscreen was active (L976, L1210). Have you tested/considered this scenario?

[ypujante]

@ubkp in my local test it seems to work, but I would definitely feel a lot more confident if I could test it with a piece of code I did not write to make sure that it works. Could you point me to a program/test that I can experiment with fullscreen mode?

[ghost]

@ypujante Since the ToggleFullscreen() implementation for rcore_web.c is still in progress (L141-L209), I've been invoking fullscreen from the shell file. The non-scaling one (equivalent to F11) just calls:

<button onclick="document.getElementById('canvas').requestFullscreen();">Fullscreen</button>

But there's another option through emscripten's fullscreen with:

void requestFullscreen(int scaleMode, int canvasResolutionScaleMode, int filteringMode)

This one can be tested directly on any of the examples, since they all use shell.html (L181). I briefly reviewed that sometime ago at #3231 (comment), see points 4 and 5.

Edit: correction.

[ypujante]

@ubkp thank you for your feedback. I will double check that fullscreen works as you point out. Note that emscripten provides a Module.requestFullscreen api so it would be better to invoke this rather than document.getElementById('canvas').requestFullscreen() because that is bypassing what emscripten does for fullscreen

[ypujante]

I have spent some more time looking at the (raylib) code and some are my preliminary findings (note that all my findings are based on the main branch latest as of this writing a92c34d, not my changes)

• First of all, the fact that now each platform is its own separate file make it about a million times easier to reason about the code so thank you for this effort!
• Digging into the low level details I realized one thing that I guess I had missed/misunderstood: raylib web essentially uses glfw to create the window and that is pretty much it. The proof is that running the example core_window_letterbox with a slight addition generates this:
  
• No matter how much I resize the window, make it fullscreen or whatnot, the size, as reported by glfw never changes (glfwGetWindowSize)
• Clicking on the "Fullscreen" button also does not invoke any glfw callback because false is provided for the resize parameter. Note that this would change if my emscripten changes are ever merged in...
• As I pointed out in a previous comment, the EmscriptenResizeCallback bypasses glfw by calling this emscripten_set_canvas_element_size("#canvas", width, height) instead of SetWindowSize(width, height). As a result glfw and raylib becomes disconnected in what is the size of the window.
• As another result, the callback WindowSizeCallback is never called (when executing core_window_letterbox) despite changing the window size
• The call SetWindowSize is implemented as glfwSetWindowSize(platform.handle, width, height) so that would be the only case when glfw is involved and of course after this call then glfwGetWindowSize returns the proper size.

I am not saying that bypassing glfw is a bad thing, but I feel like it should be consistent: if the EmscriptenResizeCallback bypasses glfw, then SetWindowSize should bypass it as well and essentially implement a logic similar to EmscriptenResizeCallback for consistency. Of course, the catch is that Module.requestFullscreen which is called when the Fullscreen button is clicked ends up going through the glfw layers...

I believe my changes are going the opposite direction: using glfw as the source of truth when it comes to size.

I will follow back with more findings...

[ypujante]

As you can see I have done a follow up PR

[raysan5]

@ypujante thanks for the detailed analysis. Afair, EmscriptenResizeCallback was implemented to overcome the limitations of GLFW implementation for Web (library_glfw.js). If this implementation can be more aligned to GLFW desktop, that's great... but also note that a browser canvas and a desktop windows work in a very different way, I can't see how that functionality can be aligned or the cost (in code complexity) of doing that.

Also, I personally would love to completely replace GLFW dependency on Web and just use the functionality already exposed by Emscripten, I think going through that middle-layer will generate more trouble than benefit in the future. It made sense in the past when all code was in a single file but at this moment, every platform can be controlled using its own base libraries.

[ypujante]

@raysan5 I would be happy to take a look at "replace GLFW dependency on Web and just use the functionality already exposed by Emscripten" if you want. That being said I don't want to spend a huge amount of time on this and then be turned down because the "PLATFORM_WEB is currently stable and that represent too many changes"

Just want to be clear about where I am standing...

I am closing this PR as well.

[orcmid]

@ypujante
Before making extensive changes, it is valuable to always check first, say with an issue the allows the changes to be anticipated and considered before you do work that ends up not accepted. And if the changes can be done in understandable smaller stages, even better.

If it's not already clear, large modifications create a burden on verification by the regular committers as well as Ramon. And then there is now more code to maintain. These are not small matters.

[ypujante]

@orcmid For the record, this PR as well as the other one, were not "out of the blue" but based on issue #3772 that was open well over a month ago including 53 comments and I have been working on it almost this entire time as I was getting feedback along the way. Also for the record, I was not expecting either PR, in their state, to be accepted. I understand that this is not my project and that the owner has the burden of maintaining it after the fact. I also believe that waiting for my emscripten changes to be merged will result in less changes to raylib which is obviously preferable for this project.

Like I said, if @raysan5 wants me to take a look at "replace GLFW dependency on Web and just use the functionality already exposed by Emscripten" given the amount of time I have already spent looking at emscripten, I think it might be easier for me and I would be more than happy to do it given his blessing.

[raysan5]

@ypujante At this moment I'm not aware of the cost and implications of replacing the GLFW usage by a direct Emscripten usage, if it simplifies the current implementation and reduces the amount of code and complexity (replacing the GLFW intermediate callbacks), I would go for it.

If you want to take a look to it and evaluate how it can be accomplished, you are welcome.