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

Fix Window::set_inner_size() #2858

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

daxpedda
Copy link
Member

@daxpedda daxpedda commented Jun 7, 2023

So since #2778 properly decoupling internal and visual canvas size, we should make things consistent.

Before (#2778)

Using Window::set_inner_size() would apply physical pixels to the internal canvas size and logical pixel to the visual canvas size. The logical pixel would be applied to the canvas size as physical pixels by setting CSS width and height as px values.

E.g.: Window::set_inner_size(PhysicalSize::new(500, 500)) with a scale factor of 2. Would apply 500x500px to the the internal canvas size and 250x250px to the visual size. Window::inner_size() would report a size of 500x500px.

In the meanwhile the reported size by Window::inner_size() would be the

Currently (after #2778)

Same as before, but it would only apply the visual size, internal size wouldn't be touched at all.

After (this PR)

Window::set_inner_size() will apply the correct size to the canvas. E.g. Window::set_inner_size(PhysicalSize::new(500, 500)) will apply 500x500px to the visual size of the canvas, no matter the current scale factor.

@daxpedda daxpedda merged commit e220a75 into rust-windowing:master Jun 7, 2023
@Liamolucko
Copy link
Contributor

I'm against this change - I think the old behaviour was correct.

A CSS "pixel" is not a physical pixel, it's a logical pixel. On a screen with a scale factor of 2, one CSS pixel represents a 2x2 grid of physical pixels. So, setting the CSS size of the canvas to the logical size of the window was correct.

@daxpedda
Copy link
Member Author

Reverted in #2861.

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

Successfully merging this pull request may close these issues.

2 participants