Fix handling of WindowResolution change#24746
Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly ✨ |
|
Video of what the showcase example looks like with this PR for Mac: My opinion: the showcase behaves better than it does on main (double pressing between 0 and 2 doesn’t change the window multiple times), but I find it strange that the content doesn’t update until the window is manually resized (however, maybe that’s outside the scope of this PR? Is that what you’re referencing in “additional info” in the issue?). I also find it strange that the initial window is still double in physical size at 2560x1440. I assume this PR would have also changed what the initial window looks like? It isn’t clear from the PR description what the behavior of the showcase is before the change and after; I’m just going from what the reported bug was in the issue. Like, I would think that the initializing of the window would start with the 1280 x 720 resolution, and then pressing “1” would not change anything since nothing changes with the resolution (base scale stays at 2, physical resolution would stay the same). |
|
Huge thanks for testing! One remark up front: My OS's scale factor is I didn't touch the initial window settings (not directly at least). By default Bevy creates a window with 1280x720 with Could you please confirm that the initial window size has not changed relative to In the beginning of the video, you start manually resizing and then a sudden change in resolution happens. Is that triggered by a keyboard input, or not? The content not updating until a manual resize is exactly what I referred to in the Additional Information part. This is caused by the condition in the I'm aware that this doesn't address all your remarks. I'm going to investigate this further later today/tomorrow and come back with some commits or comments. |
I can confirm that the initial window size on main for me has not changed with this PR (Logical size [1280, 720], Physical size [2560, 1440], Base scale: 2, Scale override: None)
Yep, it was triggered by me pressing “0” specifically. I should’ve said what I did exactly 😅 but after manually resizing and then pressing 0, I cycle through pressing “0” “1” and “2”, then I press “0” and then “2”, then some frantic combo of “0” “1” and “2” (hah), and then I manually resize with a scale factor of 2.0 without a scale override to show the content updating upon resize. Then I press “0” and then “2” to show the content updating upon resize again.
Let me know when you’d like me to test again on my Mac, happy to help |
|
Thanks for clarifying the video. I thought I had already solved that and for me it worked, but it appears that Mac treats something differently. In the recent commits I've added some clarification comments for future developers. Could you please verify how this works now? My guess is that the scale changes are not going to work (I expect you'll need to resize it manually for it to scale properly, just as you've shown in the video) but I have an idea of what might be behind it, and it's in a slightly different part of the code. |
|
With the updated changes, the example behaves the same as when I tested previously. This time I was more intentional with the sequence of number inputs and it should be clear in the video what was pressed and how things change. Screen.Recording.2026-06-25.at.8.30.58.PM.movI did notice something though (if it helps, although this mightve been deducible previously). If I manually resize the window at given scale, and then change the scale factor via pressing one of the number keys, the content does update in. size (i.e. going at base scale 1, manually resizing window, then going to base scale 2 OR overriding the scale to 2 does update the content size.) BTW if the scale changes are a different thing that you’d rather tackle in a follow up PR, that’s fine with me since I think this is already an improvement from main. If anyone’s curious what this example is like on main: |
2e5af00 to
55b0ec7
Compare
|
The last two commits introduce a mechanism of writing a
I considered that, but I think this logic is coupled in such a way that it's better to fix this together. Please let me know @kfc35 how is it now. |
|
Just tested it out, and the manual resize is no longer required for the content to re-scale! Nice!! I think after you take out the |
|
(video proof of what happens now) |
…tion, use the requested scale_factor
…e main struct was surely changed
a171d05 to
55791e2
Compare
|
Could someone please force a rerun of the checks, or should I force it with a blank commit? |
|
Summarizing the development: The handling of Hi @alice-i-cecile, as you're assigned, would you be able to find some time to review this, or should I try to find someone else? |
|
I'll review this today :) |
Objective
Fixes two issues I've identified with changing window resolution (and scaling).
See #24724.
Solution (outdated, see the later comments)
WindowResolutionproperties, not the cached ones.WindowResizedMessage not only if the physical resolution has changed but additionally if the scaling has changed.Testing
Showcase
Click to view showcase