feat: Support high-DPI rendering in a cross-platform way

[csnover]

Problem description

ImHex looks bad on high-DPI displays (at least Linux/Wayland) and the scaling slider doesn’t work right (it just makes everything bigger, but still blurry).

Implementation description

GLFW 3.3 added support for DPI-aware scaling and so now does all the window manager work of making sure the window and framebuffer are sized appropriately. This patch hooks up the new GLFW 3.3 APIs to the recently added DPI event handling code, and gets rid of the platform-specific code that either appeared to reimplement what exists in GLFW (Windows) or tried to disable the automatic DPI-aware scaling (Linux/macOS).

The implementation distinguishes between the native scaling (physical/DPI scaling) and global scaling (logical/UI scaling) so removes the “Native” scaling option from the scaling control since now that just means the same thing as 1x global scaling. I did not see any place for settings migrations across versions so just detect and rewrite the setting if needed in the place where it is used to initialise the UI.

Textures are given scaling values so SVG are rendered at the correct resolution.

Screenshots

Please forgive my custom font.

Before:

After:

Additional things

I did test this against a multi-monitor setup with different per-screen DPI and the UI does adapt the scaling to each screen in the expected way. Code that was trying to resize the window on DPI changes is removed since that is already handled by GLFW and this just caused the window size to change incorrectly.

I did not test this against macOS, Windows, or X11, so maybe it is catastrophically broken there, but since this is not doing any platform-specific stuff, I would expect it to work OK. Please test and report back! (In particular, it’s possible some of the existing Windows mouse handling code in the WM_NCHITTEST window message is doing buggy stuff since it is calling getGlobalScale.)

This change needs GLFW 3.3 or later, so maybe things like DEBIAN/control.in and lib/third_party/imgui/custom/CMakeLists.txt should be updated to specify that, although even Ubuntu 20.04 includes GLFW 3.3, so probably it does not matter.

Fractional scaling does not look great; there are clearly some rounding errors somewhere that make the text glyphs look bad. I am not sure if this is something fixable in ImHex.

I am not sure that OS_WEB should be excluded from detecting native scaling since devicePixelRatio is certainly exposed in Web API but I left the code in the splash window functionally equivalent to before since I don’t have time to test that myself.

[csnover]

Looking through bug reports this probably fixes #1685, #1752, and #942.

[WerWolv]

Hey! Thank you very very much for the PR! This has been a pain point for ages.

I haven't tested it on Linux yet and I don't have a second screen for my macbook to test it there really but on Windows I think the scaling is getting canceled out by something else. Some scaling happens (as seen with the font) but the window content stays the same size. Previously with my Windows-specific implementation, the window content size doubled when moving the window to a screen with 200% DPI scaling

5JnugpI9pz.mp4

[csnover]

To make sure I understand, are you saying that on the 200% DPI screen that the entire window and its contents appear to be 50% smaller than they do on the 100% DPI screen, as if no DPI scaling is being applied? My expectation is that the apparent size of the window and UI elements remain the same across all screens, and that they are simply crisper on the high-DPI screen. Just want to make sure we’re on the same page about that, since the video looks how I would expect.

[WerWolv]

You can try with a program that supports high DPI scaling such as Firefox and compare it to ImHex with my and your DPI handling. All screenshots were taken on the same screen, first ones in the sets with 100% scaling, second ones with 200% scaling.

Firefox

ImHex with my DPI handling

ImHex with your changed DPI handling

You see with Firefox, the entire content and UI elements double in size (and also render at a higher resolution), in ImHex with your changes, the rendering resolution doubles from the looks of it but the actual content scaling stays the same.

For the record, I think your changes are great and absolutely the way to go compared to my platform-specific way, if content scaling is handled correctly

[csnover]

Huh, that’s odd. Does the log output say “Native scaling set to: 2.0” when it goes to the second monitor? I’m not sure offhand why it would not be scaling otherwise and will take a closer look.

[WerWolv]

It does say that yeah. When moving back and fourth between a 100% and 200% scale monitor, it will also switch between printing Native scaling set to: 1.0 and Native scaling set to: 2.0. So the detection and everything works fine, it just doesn't scale the UI correctly

[csnover]

I think I have a local reproducer so I’ll convert this to draft for a minute until I can figure it out.

[csnover]

It feels unreasonable to me that twelve years after the first MBPr and 23 years after the IBM T220 there are still UI libraries that do not expose fully device-independent coordinate systems to users. I am mostly finishing this PR out of spite at this point.

I have a working but still too hacked up local implementation. It modifies the GLFW ImGui backend so that ImGui sees a consistent device-independent coordinate system no matter what platform it runs on. This enables rendering at the correct pixel density without needing to do anything special inside application code, so it also should become possible now for all the _scaled stuff to go away and be replaced with a single API call to modify the scaling vector in the backend to adjust the UI zoom.

In order for this patch set to be finished and ready I need to:

1. Add another (sigh) abstraction layer to mediate calls between the application code and GLFW so that device-independent coordinates are the only kind that exist within the application code, instead of the current situation where there are framebuffer coordinates, screen coordinates, and scaled UI coordinates;
2. Fix some issues with the order of initialisation and glfw events;
3. Try to consolidate all changes into one or two places;
4. Hook texture loading to DPI change event so SVG re-render at the correct resolution when DPI changes.

[WerWolv]

Thank you!
I think we're kinda using GLFW for things it wasn't really designed for. It's more for games than for desktop applications I think.
In any case, making changes to the backend file is completely fine, I did some as well (marked with the IMHEX PATCH comments)
Also getting rid of the _scaled UDLs would be amazing

[csnover]

This is not ready to merge yet for these reasons:

1. There are crashes when the UI reloads after a settings change with evidence of memory corruption. I need to run with ASan to look at what is going on. (n.b. Redirecting stderr to /dev/null to hide library messages is bad; it hides actually important memory corruption messages from libc and sanitisers, and even breaks the --help output :-))
2. SVG textures still need to be listening for DPI changes, but are not yet.
3. The getXXXScale APIs are still in flux; once I figure out what is needed to simplify the UI scaling I will get rid of the excess.

However, I would be glad if you could do another check on Windows to see how it goes. Once I know that is working OK then I can hopefully finish these remaining tasks quickly.

[WerWolv]

Absolutely, I'll check again once I'm home

[csnover]

The crash on restart is not caused by this patch set. It also happens on current master (b6f0ee9). It is a crash in theme_manager.cpp:186 writing to a stale pointer. ASan is sufficiently upset that it will not give information on the address. Restarting ImHex without actually replacing the process in the presence of so much global state seems like a recipe for issues like this.

[csnover]

The crash is because registerStyleHandlers is using static initialised variables that contain pointers to ImGui style object. That object will get reset but the initialisation will not run a second time so the pointers all become garbage. I don’t know how this ever worked.

[WerWolv]

Thank you! I pushed a commit that should fix the crashes and also one that improves some flickering during the DPI change on Windows. It looks pretty good now here :)

Edit: Third one makes all content scale changes happen at the same time. Before, when dragging the window from one monitor to the other, it would first change the content scaling and only when dragging the window a little further it triggered the DPI changed event to do everything else

[csnover]

ScaleCoordinates is the ratio between the framebuffer scale and the content scale, not the content scale. I’ll add a comment to make it clearer. This change breaks macOS and Wayland.

[WerWolv]

Oh thanks, I didn't realize. Fun that it's different on different platforms.
I tried to use bd->ScaleCoordinates = ImVec2(scaleX / io.DisplayFramebufferScale.x, scaleY / io.DisplayFramebufferScale.y); before like you did but it caused the window content to be way too large or way too small

[WerWolv]

It looks correct on macOS too with my changes when playing with these settings here:

[csnover]

Hopefully the change I made to restore most of the earlier code doesn’t mean that macOS scaling is broken now, because I might die of exasperation. I added the missing explainer comment on why it is written the way it is, which hopefully is clear, and hopefully at least provides some clarity on the issue to find other viable alternative approaches if they are needed.

[csnover]

The change to redraw in the SetWindowContentScale callback introduces a frame of flicker on Wayland that wasn’t there before, but I guess if it makes the experience better on Win32 then it is worth it. Transitions over the monitor boundary rarely happen anyway. I guess this might be due to some apparent non-atomicity that I mention in the comment I added to the ImGui_ImplGlfw_NewFrame function, since just eyeballing it it looks an awful lot like it is painting only the lower-left quadrant, as if ImGui thinks the framebuffer is the same size as the window.

The bug could be in my code, or it could be UB in GLFW to paint from the callback (this would be weird but it’s not specified in their documentation as far as I can tell), or it could be a bug in ImGui, or it could be a bug in GLFW, or it could be some bug somewhere else in ImHex, or it could be a Wayland issue. My money is on GLFW, but I don’t know, and unfortunately I don’t have time or interest to spend to figure it out any more. So long as everything is crisp and appropriately sized on all platforms, and these changes are comprehensible enough to the next person who might pick it up and work on it that no regressions are introduced due to a misunderstanding, I am satisfied.

I appreciate your collaboration on this and testing on the platforms I don’t currently have access to. I also just retired my Mac and it is a weird experience to not be able to also test on that platform myself after ~15 years as my daily driver.

I reorganised the commits and did a push that resolves the outstanding items from my list of TODOs. The getGlobalScale function still exists because I don’t know what the Win32 code is doing that uses it exactly.

[WerWolv]

It looks like your force pushes reverted some of my changes so now the weirdness on Windows is back again and it also doesn't look good on Linux anymore.

I think there's some weirdness going on with the glfw events. Right now, the window content scale callback fires right away, as soon as the window is moved even just a pixel onto a screen with a different DPI. That immediately re-renders the font which makes it look blurry. Then only later on when the window is about halfway on the other screen, the window content's size gets updated (actual UI element size changes and also the window size itself changes). I think that's some issue between when the getter functions return a new value and when the callbacks get fired or we're just using different callbacks in different places to do the same thing but I couldn't figure out which ones.

I re-implemented the changes I did before now and made them work somewhat okay on Linux too but I think the underlying issue isn't fixed yet

[csnover]

Uch, that’s very frustrating, I’m sorry that happened due to the changes I made.

I am surprised that you are seeing issues on Linux since that is the platform I have been testing on, using KWin 5.27.11 and GLFW’s runtime switching with either XDG_SESSION_TYPE=wayland or =x11 to check both protocols. I did notice that fractional font scaling does not look good compared to other applications but it is already an improvement since before it was always 1x. I didn’t notice any other problems with the scaling.

In the macOS and Wayland GLFW backends it is very clear there can be no expectation that GLFW is going to have the correct framebuffer size at the time of the content scale callback, which is why the scaling calculation seems like it really needs to only be in NewFrame. Unfortunately this also means calling Window::fullFrame from the content scale callback is going to be broken on some platforms, which does seem to support my theory that the reason why I was noticing a flicker on Wayland, but not you on Win32, was because the FB ratio is wrong for that frame.

On X11, the content scale callback is never invoked, so it seems there is only a single scaling ratio that is always used (the highest DPI one in my experience) and then the window manager does downscaling if needed.

Based on my reading of the GLFW Win32 backend what you describe there sounds like the difference between WM_DPICHANGED and WM_SIZE events.

After thinking about it for a while, I wonder if the right thing to do is to stop trying to use any of these GLFW events other than window damage. At least, that probably should be the only place that Window::fullFrame should be called and metrics should be queried.