Fix rendering issue after window resize #1987

ArnaudValensi · 2021-09-12T03:22:15Z

At the initialization CORE.Window.currentFbo.width and CORE.Window.render.width was set to the variable passed from InitWindow which is the virtual width, but the framebuffer size created by glfw is width * dpi scale.

After resize CORE.Window.screen.width was set to the framebuffer size which is the physical width instead of the logical width (on all platform except macos).

ArnaudValensi · 2021-09-12T03:22:53Z

Fix issue #1982

ArnaudValensi · 2021-09-12T03:27:34Z

src/core.c

 }

 // Get current screen height
 int GetScreenHeight(void)
 {
-    return CORE.Window.currentFbo.height;
+    return CORE.Window.screen.height;
+}


I think we expect to get back the logical size from these functions, so it returns the screen size now.
I've created two more functions (GetRenderWidth/GetRenderHeight) in case we want the physical size.
They all return the same size on macos though, maybe I should add this the comment.

ArnaudValensi · 2021-09-12T03:28:41Z

src/core.c

-#endif
-        TRACELOG(LOG_INFO, "    > Render size:  %i x %i", CORE.Window.render.width, CORE.Window.render.height);
-        TRACELOG(LOG_INFO, "    > Screen size:  %i x %i", CORE.Window.screen.width, CORE.Window.screen.height);
-        TRACELOG(LOG_INFO, "    > Viewport offsets: %i, %i", CORE.Window.renderOffset.x, CORE.Window.renderOffset.y);


I moved these logs because we only have the real CORE.Window.render size later on.

ArnaudValensi · 2021-09-12T03:32:36Z

src/core.c

+    CORE.Window.currentFbo.width = fbWidth;
+    CORE.Window.currentFbo.height = fbHeight;
+    CORE.Window.render.width = CORE.Window.currentFbo.width;
+    CORE.Window.render.height = CORE.Window.currentFbo.height;


These four lines are the most important part. Before this change, CORE.Window.currentFbo and CORE.Window.render.width were filed with the width and height passed in parameter by the user to this function. It was not correct because when creating a window at a specific size, glfw create a framebuffer with the size window size * dpi scale factor (for all desktop platform except macos).

ArnaudValensi · 2021-09-12T03:33:15Z

src/core.c

+
+    // Initialize OpenGL context (states and resources)
+    // NOTE: CORE.Window.currentFbo.width and CORE.Window.currentFbo.height not used, just stored as globals in rlgl
+    rlglInit(CORE.Window.currentFbo.width, CORE.Window.currentFbo.height);


Moved this line here so we can call it with the correct values.

ArnaudValensi · 2021-09-12T03:34:41Z

src/core.c

-
+#else
+    CORE.Window.screen.width = width / windowScaleDPI.x;
+    CORE.Window.screen.height = height / windowScaleDPI.y;


This callback gives the physical size (framebuffer size), so we need to divide by the dpi scale factor.

raysan5 · 2021-09-12T08:32:46Z

@ArnaudValensi this PR needs to be reviewed very carefully because it can break other things. Some situations to review:

When user do not want to use HighDPI despite the monitor is HighDPI (not enabled FLAG_WINDOW_HIGHDPI)
When using BeginTextureMode(RenderTexture), currentFBO is stored for further retrieval (and the size is independent of the DPI because is defined by user)
When scaling the Window on HTML5 (EmscriptenResizeCallback)
Mouse/Touch input interaction when using HighDPI

ArnaudValensi · 2021-09-13T02:23:10Z

Ok, let me create a checklist:

When user do not want to use HighDPI despite the monitor is HighDPI (not enabled FLAG_WINDOW_HIGHDPI)
When scaling the Window on HTML5 (EmscriptenResizeCallback)
Mouse input interaction when using HighDPI on windows/linux/macos
Mouse/Touch input interaction when using HighDPI on web
When using BeginTextureMode(RenderTexture), currentFBO is stored for further retrieval (and the size is independent of the DPI because is defined by user)
Fix scissor
Touch input interaction when using HighDPI on mobile

raysan5 · 2021-09-13T08:04:24Z

@ArnaudValensi Just note that those are only some things that should be reviewed for this change, they could be working ok. I reviewed HighDPI several times in the past and I always found some side case where things do not work as expected.

For reference: #1086, #837, #814, #1931

When possible, I think it's a good idea to give control of the HighDPI to the user (so GetWindowScaleDPI() is provided).

ArnaudValensi · 2021-09-13T14:52:55Z

@raysan5 Ok, thank you for the references.

Maybe these recurring problems/regressions are a good sign that some automated testing could be nice to be implemented. It would make us more confident about the features and the changes we make. I know it is not easy because automated tests often need to be headless, but it is not impossible. We can take a look at how Monogame implemented their tests.
What do you think?

When possible, I think it's a good idea to give control of the HighDPI to the user (so GetWindowScaleDPI() is provided).

Are you suggesting to not take into account the dpi scaling factor in some cases?
Or are you suggesting that you are not very comfortable with this PR? (I would understand)

raysan5 · 2021-09-13T16:39:23Z

Hi @ArnaudValensi, I'm comfortable with this PR, HighDPI has given problems for some time (specially on macOS) but I'm a bit afraid of the side problems that could arise. HighDPI has been reviewed several times and it seems there is always some side case that breaks or does not work as expected.

For example, if HighDPI is enabled and user creates a (800, 600) Window, actually the internal framebuffer created could be (8001.5, 6001.5) but a novel user expects to work with the size of (800, 600) for the game logic. An advance user could be creating a RenderTexture of (800, 600) and notice that when drawing it, the Window is bigger (x1.5) than the RenderTexture, in those cases I think it's up to the user to manage RenderTexture drawing considering the GetWindowScaleDPI().

ArnaudValensi · 2021-09-14T00:18:56Z

@raysan5 I have made the following tests on windows/linux/macos/web:

eF9sbeaXPC.mp4

Everything is fine. The web platform doesn't seem to use hdpi scale factor at all, so I did not modify its code.
I also tried the web on mobile, but I did not try ios or android builds (I don't have an android phone).

I still need to check BeginTextureMode.

All the checks are made using this repository: https://github.com/ArnaudValensi/raylib-highdpi-issue
This raylib libraries corresponding to this PR are directly on the repository, so you can check by doing:
make run on window, linux or macos
make game-web && make run-web on any platform to build the web version. Docker is needed though.

ArnaudValensi · 2021-09-14T00:47:01Z

I've found a missing modification in EndTextureMode 8b1dd7a

I've checked the example textures\textures_mouse_painting.c and it works fine.

LoadRenderTexture(width, height) does not use the dpi scale factor, but like you said, we can use GetWindowScaleDPI() if needed.

I will check the scissor mode.

ArnaudValensi · 2021-09-14T01:24:51Z

I discovered a bug with the scissor mode, it was here before this PR:

xxZDdCzHM5.mp4

I fixed it in 9187180

UTD3LoGiil.mp4

It solves this issue #1931

ArnaudValensi · 2021-09-14T01:56:54Z

The julia example works fine with hdpi enabled (#837), I've tried on windows and macos.

My only left concern is that I don't know how the hdpi behave on mobile devices, but maybe it can be the scope of another PR if there is some trouble.

So, we might be good to go.
Also, if there is some other troubles around hdpi in the future, I can be available.

Here a repost of the checklist:

When user do not want to use HighDPI despite the monitor is HighDPI (not enabled FLAG_WINDOW_HIGHDPI)
When scaling the Window on HTML5 (EmscriptenResizeCallback)
Mouse input interaction when using HighDPI on windows/linux/macos
Mouse/Touch input interaction when using HighDPI on web
When using BeginTextureMode(RenderTexture), currentFBO is stored for further retrieval (and the size is independent of the DPI because is defined by user)
Fix scissor [Core] Scissor Mode & High DPI Issue #1931
Julia shader test [examples] shaders_julia_set doesn't fill screen on Macbook Retina #837
Touch input interaction when using HighDPI on mobile (I'm pretty confident with this)
Check resize on mobile (I don't know if it's relevant, but it might be an issue)

ArnaudValensi · 2021-09-14T03:14:24Z

I think for the scissor, we should only multiply by the scale if the hdpi flag is set. I will make the change Wednesday.

raysan5 · 2021-09-14T21:33:30Z

@ArnaudValensi Very nice! Thank you very much for the review of all the HighDPI involved issues and the detailed report! About resize event on mobile using a NativeActivity (Android), I think it's not relevant but it's important to support the touch input properly.

ArnaudValensi · 2021-09-19T22:38:23Z

@raysan5 I fully fixed the scissor, it now works with and without the high dpi flag.

I wanted to try if this PR works on Android but I was not able to compile raylib due to this issue #1997
I don't have the time to figure out why. But if compilation on Android works again, I can finalize the checks to validate this PR.

raysan5 · 2021-09-22T09:34:43Z

@ArnaudValensi I'm afraid I renamed this module and now changes can not be merged due to conflicts... Could you open a new PR with those changes for merging? By the way, Android compilation should be working again.

Sorry for the inconvenience.

ArnaudValensi · 2021-09-26T22:05:35Z

Ok, I will try to find the time this week.

ArnaudValensi · 2021-09-29T11:23:22Z

I have rebased this PR.
I still need to check on Android and we should be good.

raysan5 · 2021-09-29T21:28:31Z

@ArnaudValensi Nice! Thank you very much for reviewing it again! Please, let me know when ready to merge! :D

ArnaudValensi · 2021-10-07T22:57:58Z

I rebased one more time.

I tried to test on android but I didn't manage to. Unfortunately, I don't have more time to allocate to this project. I think this PR works on Android, but not 100% sure. @raysan5, I let you decide what to do.

raysan5 · 2021-10-10T20:34:48Z

@ArnaudValensi thank you very much for all the time you put on this improvement, I'm merging it and reviewing.

ArnaudValensi changed the title ~~Fix rendering issue after window resize #1982~~ Fix rendering issue after window resize Sep 12, 2021

ArnaudValensi commented Sep 12, 2021

View reviewed changes

ArnaudValensi mentioned this pull request Sep 12, 2021

[core] Bad GetScreenWidth/GetScreenHeight values after resizing windows with FLAG_WINDOW_HIGHDPI #1982

Closed

3 tasks

ArnaudValensi force-pushed the master branch from abcee04 to 8b1dd7a Compare September 14, 2021 00:41

raysan5 mentioned this pull request Sep 25, 2021

[core] BeginScissorMode() fails on macOS #1281

Closed

ArnaudValensi force-pushed the master branch from fe0e10d to 07bb1fd Compare September 29, 2021 11:22

Fix dpi issues

3e18bf2

ArnaudValensi force-pushed the master branch from 07bb1fd to 3e18bf2 Compare October 7, 2021 22:17

raysan5 merged commit 7c36f3f into raysan5:master Oct 10, 2021

This was referenced Nov 15, 2021

Fix scissor mode for high DPI screens #2141

Closed

[core] core_scissor_test not working properly on high DPI screen #2142

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix rendering issue after window resize #1987

Fix rendering issue after window resize #1987

ArnaudValensi commented Sep 12, 2021

ArnaudValensi commented Sep 12, 2021

ArnaudValensi Sep 12, 2021

ArnaudValensi Sep 12, 2021

ArnaudValensi Sep 12, 2021

ArnaudValensi Sep 12, 2021

ArnaudValensi Sep 12, 2021

raysan5 commented Sep 12, 2021 •

edited

Loading

ArnaudValensi commented Sep 13, 2021 •

edited

Loading

raysan5 commented Sep 13, 2021

ArnaudValensi commented Sep 13, 2021

raysan5 commented Sep 13, 2021

ArnaudValensi commented Sep 14, 2021 •

edited

Loading

ArnaudValensi commented Sep 14, 2021

ArnaudValensi commented Sep 14, 2021

ArnaudValensi commented Sep 14, 2021 •

edited

Loading

ArnaudValensi commented Sep 14, 2021

raysan5 commented Sep 14, 2021

ArnaudValensi commented Sep 19, 2021

raysan5 commented Sep 22, 2021

ArnaudValensi commented Sep 26, 2021

ArnaudValensi commented Sep 29, 2021

raysan5 commented Sep 29, 2021

ArnaudValensi commented Oct 7, 2021

raysan5 commented Oct 10, 2021

Fix rendering issue after window resize #1987

Fix rendering issue after window resize #1987

Conversation

ArnaudValensi commented Sep 12, 2021

ArnaudValensi commented Sep 12, 2021

ArnaudValensi Sep 12, 2021

Choose a reason for hiding this comment

ArnaudValensi Sep 12, 2021

Choose a reason for hiding this comment

ArnaudValensi Sep 12, 2021

Choose a reason for hiding this comment

ArnaudValensi Sep 12, 2021

Choose a reason for hiding this comment

ArnaudValensi Sep 12, 2021

Choose a reason for hiding this comment

raysan5 commented Sep 12, 2021 • edited Loading

ArnaudValensi commented Sep 13, 2021 • edited Loading

raysan5 commented Sep 13, 2021

ArnaudValensi commented Sep 13, 2021

raysan5 commented Sep 13, 2021

ArnaudValensi commented Sep 14, 2021 • edited Loading

ArnaudValensi commented Sep 14, 2021

ArnaudValensi commented Sep 14, 2021

ArnaudValensi commented Sep 14, 2021 • edited Loading

ArnaudValensi commented Sep 14, 2021

raysan5 commented Sep 14, 2021

ArnaudValensi commented Sep 19, 2021

raysan5 commented Sep 22, 2021

ArnaudValensi commented Sep 26, 2021

ArnaudValensi commented Sep 29, 2021

raysan5 commented Sep 29, 2021

ArnaudValensi commented Oct 7, 2021

raysan5 commented Oct 10, 2021

raysan5 commented Sep 12, 2021 •

edited

Loading

ArnaudValensi commented Sep 13, 2021 •

edited

Loading

ArnaudValensi commented Sep 14, 2021 •

edited

Loading

ArnaudValensi commented Sep 14, 2021 •

edited

Loading