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

DPI change events in Sdl2App and GlfwApp #423

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

mosra
Copy link
Owner

@mosra mosra commented Feb 15, 2020

Related to #243. As usual, doesn't seem to be as straightforward as I would think, and I'm actually not sure about the workflow at all.

The simple goal at first was to detect when an app goes to a differently dense display and emitting a viewportEvent() so the app can rescale itself according to the new DPI. However, not so easy:

  • Unless the app tracks DPI scaling and window/framebuffer size on its own, there's no way for it to know what the viewport event is for -- did the window get resized or did the DPI scaling change? To fix that, I added a Type enum that differentiates between resize and DPI scaling change. An alternative could be having the DPI scaling event separate, however in 90% cases the app needs to do some relayouting based either on the new size or the new density (which also implies new size), so having two separate events for this would mean the app needs to duplicate all that code (ugh) or put it in a helper function that's called from both (also ugh).
    • Later there could be minimized/iconified/maximized/fullscreen event types maybe, which does sound useful
    • Other ideas? How does Qt handle this?
    • The Type enum feels kinda ugly, still .. rename to Cause? Rename this whole thing to windowEvent()?
  • The interaction with user-defined DPI scaling is not clear at all. Right now, the whatever DPI scaling the user specified gets overwritten and inevitably lost by this event, thus moving a custom-scaled app to another monitor and then back causes it to not be the same size again. Big problem -- what to do here?
    • Let the event handler code supply a new DPI value, via accept() or some such, and ignore the DPI change otherwise? How does the user code calculate it? What if the user just always wants the physical DPI (to have the content match some physical size or whatever) while they always get just the virtual DPI from the WM?
    • Remember some "scale factor" to the actual virtual DPI on startup and then always apply that? That would cause the window to be correctly sized when moved back to the original monitor, but what if such "scaled scaling" is completely bogus on the other one? How can the user control that?
    • Or remove these user overrides altogether, forcing everything to be exactly what the platform tells it to be? This scaling override is broken on macOS and web anyway, where the framebuffer size != window size.
  • Windows supply a "desired rectangle" in the WM_DPICHANGED event, expecting the app to use exactly that
    • Make it possible to actually use the hint on the user side (again passing that to accept()?)
    • There's no way to set window position right now, implement (this would ideally need a combined position+size)
    • If an user ignores/overrides this, what are the consequences of not doing so (apart from the feedback loops I saw)?
    • I can access this hint from within Sdl2App and use that to position+size the window, but can't do that from within GLFW as it doesn't propagate the needed parameters anywhere and instead does its own thing without any ability to override -- however when searching for how to catch WM_DPICHANGED in SDL (which is actually easy to grab when SysWMEvent is enabled), I discovered it's possible to "subclass" the WndProc (source, docs, getting the procedure out of a window handle), catch the relevant events, do what I need and pass the rest to GLFW. Ugly but possibly doable?
  • Resizing directly from inside the event handler on GLFW causes a feedback loop where the window switches between various sizes and DPIs several times and then ends up resized to something extremely wide at the end, how to prevent this? Is that due to window borders being differently thick?
    • OTOH there's a GLFW_SCALE_TO_MONITOR hint that makes the resizing somehow automagic (and without this feedback loop) but it's too automagic for my taste, and should be under app control instead -- i.e., the app might only work well on particular sizes and it needs to "snap" to one of them
    • GLFW is using the WM_DPICHANGED rectangle hint to position+size the window both when GLFW_SCALE_TO_MONITOR is used and when it isn't, but it only does something in the first case .. why?
  • Speaking about GLFW_SCALE_TO_MONITOR automagic, provide a way to do this on a per-event basis, depending on what the app actually needs? Again by supplying something to accept()?
  • SDL halts all event processing while the window is dragged, meaning the window only gets resized after it jumps to the other display, which is freaking bad. GLFW doesn't do that, resizing the window right when it flips over the edge, so I don't think this is a limitation of Windows API. Possibly related SO thread, a SDL bugreport from 2014(!)
  • I guess because of the different window border sizes, even under SDL whers is no feedback loop going back and forth between two monitors several times causes the window to not be sized as expected anymore, being a bunch of pixels off (803x607 and such), similar thing happening with glfw and GLFW_SCALE_TO_MONITOR. Not good for my OCD. Solutions? GLFW is doing some calculations in its WM_GETDPISCALEDSIZE handler in order to keep the window the same size when GLFW_SCALE_TO_MONITOR is not enabled, maybe I could get inspired by that to ensure proper scaling?
  • WM_DPICHANGED can be caught on SDL (which doesn't support it natively) by enabling SDL_SYSWMEVENT that's disabled by default. Should this one be propagated to anyEvent() as well? It generates a lot of noise, however not propagating it might be limiting for the user :/
  • All the above is extremely non-trivial and needs to be thoroughly documented

And then ... the final boss bosses:

TODO: docs
TODO: I don't like the extra flag in viewport event, should this be
  separate?
TODO: resizing directly from inside the event handler on GLFW causes a
  loop where the window switches between the DPIs several times and then
  gets resized to something extremely wide at the end, how to prevent
  this? Is that due to window borders being differently thick?
TODO: there's a GLFW_SCALE_TO_MONITOR that makes the resizing somehow
  automagic but it's too automagic for my taste, and should be under app
  control instead
TODO: SDL halts all event processing while the window is dragged,
  meaning the window only gets resized *after* it jumps to the other
  display, which is freaking bad
TODO: Windows supply a "desired window rectangle" in the WM_DPICHANGED
  event and it should probably get used, how to hammer it out of GLFW?
  Interesting is that GLFW actually calls this but it does nothing
  unless GLFW_SCALE_TO_MONITOR is enabled as well. Huh?
TODO: I guess because of the different window border sizes, even under
  SDL whers is no feedback loop going back and forth between two
  monitors several times causes the window to not be sized as expected
  anymore, being a bunch of pixels off (803x607 and such). Not good for
  my OCD. Solutions? GLFW is doing some calculations in its
  GLFW_SCALE_TO_MONITOR routine, maybe that could get used?
TODO: WM_DPICHANGED can be caught by enabling SDL_SYSWMEVENT that's
  disabled by default. Should this one be propagated to anyEvent() as
  well? It generates *a lot* of events, however not propagating it might
  be limiting for the user :/
@codecov-io
Copy link

codecov-io commented Feb 15, 2020

Codecov Report

Merging #423 into master will increase coverage by 3.65%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #423      +/-   ##
==========================================
+ Coverage   72.31%   75.96%   +3.65%     
==========================================
  Files         367      367              
  Lines       19120    18755     -365     
==========================================
+ Hits        13827    14248     +421     
+ Misses       5293     4507     -786
Impacted Files Coverage Δ
...numPlugins/AnyAudioImporter/importStaticPlugin.cpp 15.78% <0%> (-84.22%) ⬇️
...umPlugins/TgaImageConverter/importStaticPlugin.cpp 50% <0%> (-50%) ⬇️
...numPlugins/AnySceneImporter/importStaticPlugin.cpp 60% <0%> (-40%) ⬇️
src/Magnum/Vk/Enums.h 68.42% <0%> (-31.58%) ⬇️
...Plugins/MagnumFontConverter/importStaticPlugin.cpp 86.73% <0%> (-13.27%) ⬇️
src/Magnum/Math/Tags.h 87.87% <0%> (-12.13%) ⬇️
src/Magnum/SceneGraph/AbstractGroupedFeature.h 75% <0%> (-5%) ⬇️
src/Magnum/SceneGraph/AbstractFeature.hpp 66.66% <0%> (-4.77%) ⬇️
src/Magnum/Vk/Implementation/formatMapping.hpp 96.66% <0%> (-3.34%) ⬇️
src/Magnum/SceneGraph/Camera.h 61.53% <0%> (-2.75%) ⬇️
... and 161 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65743b5...3ea97a2. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants