So I've been trying to get the breaking winit/glutin update to work with Alacritty and I've run into issues where changes would work on X11/i3, but I'd run into issues with Wayland. I've tested out a couple of different ways to handle this, but I think I've run into an issue with how the events are ordered.
This all heavily depends on the order of events coming in, I wasn't able to reproduce it when logging all events for example. So I'm trying to outline the issue, but I'm not sure if everything I'm saying is comeletely correct.
Currently it seems like winit sends Resize events first and sends DPR change events after that. On i3 I'm sometimes running into the scenario that I'm just receiving a DPR change event when moving a window between screens with different DPRs, but the window got scaled correctly already, so this DPR event is just useful for converting the next resize event to physical sizes. However on Sway when starting the window on a 2.0 DPI screen, the window seems to be first resized to a smaller version with 1.0 DPR and then I receive a 2.0 DPR event which requires that I actually scale the previous viewport with the new DPR.
These two behaviors seem to be in conflict with each other. One requiring that a standalone DPR change is ignored, the other requiring that the viewport should be resized.
The common behavior when both DPR and Resize events are received at the same time (polling until the queue is empty), seems to be that the DPR change event should be ignored. So it doesn't seem like the behavior of i3 is incorrect, if anything this should be related to Wayland?
Even though it might not be useful, here's the debug output I've generated which lead me to this error:
bug_logs.txt
If this is actually an issue with winit and not the way I've been making use of the events, there are several ways this could be changed. I think always sending DPR events before resize events would resolve this issue, even though it could potentially still cause timing issues. Always sending the DPR with a resize event would also allow converting logical to physical sizes without having to rely on correct event order, however it seems like DPR is not stored internally, so that might cause some overhead. These were just my first two thoughts, I'm sure there are other solutions too.
Okay, it looks like we have some internal inconsistency here.
The common behavior when both DPR and Resize events are received at the same time (polling until the queue is empty), seems to be that the DPR change event should be ignored.
My approach regarding the wayland backend was that, given the Resized event provide a LogicalSize, it and the HiDPIFactorChanged event are effectively independent, and either of them changing means that the physical size of the window changes.
As a typical example, a window (in a non-tiling-wm context) moving from a 1.0DPI screen to a 2.0DPI screen will only receive a HiDPIFactorChanged event. In case of a tiling wm, the wm would also likely resize the window as a consequence of the screen change to fit it in the tiling scheme, and a Resized event would also be generated.
But specifically, both these events affect two independent things (the logical size of the window, and its DPI factor), and as a result the order in which they are generated should not matter at all. The actual physical size of the window should always match the one computed from the last received logical size and the last received DPI factor.
At creation, winit cannot know which screen the window will appear on. So we first assume a 1.0 DPI factor, and then generate an HiDPIFactorChanged as soon as we know the DPI factor of the screen the window appears on, which can only happen after at least one drawing on wayland.
Apparently the X11 backend does not match this behavior. I'm not clear on how it behaves exactly, but it's apparently different.
What is the exact behavior of the x11 backend? (and the other backends as well, actually?)
After a rather long discussion on IRC, it appears this kind of things can happen depending on races in x11 with a tiling manager:
Considering two monitors with different DPI factors, a window tiled on one of them.
Resized event, computing the logical size of the original monitorHiDPIFactorChanged eventas a result, the Resized event contains a logical size that has been computed with the wrong dpi factor, causing it to generate wrong results if attempted to compute a physical size using the new DPI factor.
Apparently this behavior can be racy at the x11 level, so it may be difficult to correctly restore the illusion that everything is logical sizes as winit wanted to expose.
I commented on IRC but want to include here as well.
One additional thought -- GLFW exposes two "Resize" events; one is screen coordinates (logical), and one is framebuffer size (physical). In many cases, this may be sufficient for the application's needs. However, each of these events is passed a GLFWwindow* which can be used to get a GLFWmonitor* which can ultimately be used to get the current DPI for the window.
I'm not sure how GLFW's DPI scaling is regarded, but this seems like a reasonable approach. The raciness would be less of an issue if the physical size event was available. For an application like Alacritty, physical size would simply update framebuffer, and DPI change would update our physical font size. Both would trigger a draw. From what I understand, this would fully solve our problem. cc @chrisduerr to correct me, though ;).
Yeah, that sounds right. If we get physical and logical sizes, we could resolve all current issues. The only thing DPR is required for are font sizes and padding. So a pointer to the window wouldn't even be necessary for this usecase, since the way Alacritty is setup we wouldn't need to query anything on the window with logical and physical sizes available.
So a pointer to the window wouldn't even be necessary for this usecase, since the way Alacritty is setup we wouldn't need to query anything on the window with logical and physical sizes available.
Thanks for making this distinction; I meant to present these as alternatives in my first comment.
After some more digging, I'm being more and more convinced that the only way to be robust across platforms would be to merge the Resized and HiDPIFactorChanged events into a single event, giving the size and dpi factor anytime any of them changes.
That, or we need to introduce thing like "when x11 detects that the dpi changes but the physical size does not, it need to generate a Resized event with the new logical size", but I fear this would be a road to yet more corner cases in all platforms.
So, for consistency with the rest of the API, I'd propose making an event reporting both the logical size and the dpi factor, whenever any of them changes.
Given there is already the event loop rework coming as a large breaking change of the API, this is not a bad time to consider changing this as well in my opinion.
@vberger, my only fear is if merging the two events will lead to developers writing things like if old_width/height != new_width/height I'm the new unified event handler, for performance reasons. I could be misunderstanding how this will impact users however, feel free to ignore this comment.
My understand is that if an unified event is generated, that means that either the dpi and/or the logical size have changed, as a result it is very likely that the physical size have changed (given it is basically the product of both), and as a result that it is worth processing the new dimension information.
Overall, I doubt that events gereated like this were the contents are actually not of interest would occur often enough to be worth trying to avoid for performance reasons.
Most helpful comment
After some more digging, I'm being more and more convinced that the only way to be robust across platforms would be to merge the Resized and HiDPIFactorChanged events into a single event, giving the size and dpi factor anytime any of them changes.
That, or we need to introduce thing like "when x11 detects that the dpi changes but the physical size does not, it need to generate a Resized event with the new logical size", but I fear this would be a road to yet more corner cases in all platforms.
So, for consistency with the rest of the API, I'd propose making an event reporting both the logical size and the dpi factor, whenever any of them changes.
Given there is already the event loop rework coming as a large breaking change of the API, this is not a bad time to consider changing this as well in my opinion.