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

attempt to deal with rounding issue when creating the swap chain #997

Merged
merged 4 commits into from
Dec 7, 2020

Conversation

blunted2night
Copy link
Contributor

@blunted2night blunted2night commented Dec 3, 2020

While resizing a window on a high DPI display, some window sizes cause an off by one error when recalculating the physical size of the window. This causes the swap chain to fail to resize until the window resized again to a "good" resolution. The issue is caused by the logical size being stored as an integer which looses precision when calculated from the physical size.

This patch attempts to solve the issue by storing the physical resolution of the window instead of the logical resolution. As part of the effort, width & height become logical_width & logical_height, scaled_width & scaled_height become physical_width & physical_height. This requires touching a number of things and will probably break downstream users. It also changes the type of the components of the logical size to f32 so it can better represent sizes that are a fraction of a pixel.

@blunted2night blunted2night force-pushed the high-dpi branch 2 times, most recently from c00c9d5 to 5f622a4 Compare December 4, 2020 05:32
@blunted2night blunted2night marked this pull request as ready for review December 4, 2020 15:51
@cart
Copy link
Member

cart commented Dec 4, 2020

I think logical_width vs physical_width is a clear distinction (and its the one winit uses), but it forces the average user to think about which one to use, when in basically every case it should be logical_width. I would prefer it if we renamed logical_width() to width() to bias toward people using the correct value / save them from the need to ponder over which width to use. I think the width() and physical_width() distinction is suitably clear.

crates/bevy_winit/src/winit_windows.rs Outdated Show resolved Hide resolved
crates/bevy_window/src/window.rs Show resolved Hide resolved
crates/bevy_winit/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_window/src/window.rs Show resolved Hide resolved
@Moxinilian Moxinilian added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Dec 4, 2020
@blunted2night blunted2night force-pushed the high-dpi branch 2 times, most recently from 7f2d68d to 1b6d1c4 Compare December 7, 2020 16:23
Nathan Jeffords added 4 commits December 7, 2020 12:04
* fix window size on creation
* rename `logical_width` & `logical_height` back to `width` & `height`
* update `Camera` trait to take width/height as `f32`
* update `WindowResized` to report 'f32' sizes instead if `u32`
* update `WindowCommand::SetResolution` to pass physical resolution instead of logical
* put `set_resolution` back, now taking `f32` instead of `u32`
* remove several unneeded `as f32`s now that `width` & `height` returns `f32`
* add back `logical_width` & `logical_height` so that code that wants full
  precision still has access, updated several places in engine that wanted
  `f32` to use these.
@cart
Copy link
Member

cart commented Dec 7, 2020

Looks good to me! Thanks 😄

@cart cart merged commit 3d386a7 into bevyengine:master Dec 7, 2020
@blunted2night blunted2night deleted the high-dpi branch December 7, 2020 21:49
@fopsdev fopsdev mentioned this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants