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

Resize the surface at the correct time in examples #236

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Sep 1, 2024

The examples previously showed resizing just before rendering, which is inefficient, the user should only resize when needed. Tested on macOS.

I have not updated the winit_multithread example, as it was a bit more cumbersome, and I wouldn't be able to test it anyways, as it doesn't seem to work on macOS.

@madsmtm madsmtm added the documentation Improvements or additions to documentation label Sep 1, 2024
@madsmtm madsmtm requested a review from john01dav as a code owner September 1, 2024 05:08
@madsmtm madsmtm changed the title Resize the surface at the correct times in examples Resize the surface at the correct time in examples Sep 1, 2024
@madsmtm madsmtm requested review from notgull and removed request for john01dav September 1, 2024 05:13
Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does winit assume (for every backend) / guarantee that a surface is "always" created with "some" initial size, and always delivers a Resized event afterwards if it changed? Or otherwise, if it doesn't know/understand/have an initial size, always raises a Resized event?

examples/animation.rs Outdated Show resolved Hide resolved
examples/fruit.rs Outdated Show resolved Hide resolved
examples/rectangle.rs Outdated Show resolved Hide resolved
examples/animation.rs Outdated Show resolved Hide resolved
examples/animation.rs Outdated Show resolved Hide resolved
examples/rectangle.rs Show resolved Hide resolved
@madsmtm
Copy link
Member Author

madsmtm commented Sep 1, 2024

Does winit assume (for every backend) / guarantee that a surface is "always" created with "some" initial size, and always delivers a Resized event afterwards if it changed? Or otherwise, if it doesn't know/understand/have an initial size, always raises a Resized event?

Hmm, isn't this more of a Softbuffer issue? We're creating the surface from the window, which itself has an initial size (the examples use with_inner_size).

@madsmtm madsmtm force-pushed the madsmtm/proper-resize branch from e44e4c0 to 6bb305f Compare September 1, 2024 22:29
@MarijnS95
Copy link
Member

Hmm, isn't this more of a Softbuffer issue? We're creating the surface from the window, which itself has an initial size (the examples use with_inner_size).

It seems a bit surprising that creating a Softbuffer surface doesn't require an initial size, as if it's able to query/follow that from the underlying window handle - but then we have to forward the size explicitly whenever a resize event occurs.

Otherwise that would have been a valid reason to not have size queries on the Surface - the caller is always aware of the size already (and would reduce ambiguity).

@madsmtm
Copy link
Member Author

madsmtm commented Sep 3, 2024

Hmm, isn't this more of a Softbuffer issue? We're creating the surface from the window, which itself has an initial size (the examples use with_inner_size).

It seems a bit surprising that creating a Softbuffer surface doesn't require an initial size, as if it's able to query/follow that from the underlying window handle - but then we have to forward the size explicitly whenever a resize event occurs.

Well, on macOS/iOS at least I think we could fairly easily make the surface follow the size of the window automatically, but I don't know about the other platforms.

@MarijnS95
Copy link
Member

Well, on macOS/iOS at least I think we could fairly easily make the surface follow the size of the window automatically, but I don't know about the other platforms.

Same on Android, by requesting a buffer size of 0 (or never setting one). I'll move the rest of the planned reply to #237 and post it in a second!

Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but as discussed in #237 (comment) we might want to remove resize() entirely as this change in particular sets the presedent that the user has to forward the Resized event from winit to softbuffer (which some platforms can handle implicitly by following the window/surface size, or could be useful for explicit hardware scaling).

@madsmtm madsmtm merged commit 91db82d into master Sep 4, 2024
38 checks passed
@madsmtm madsmtm deleted the madsmtm/proper-resize branch September 4, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Development

Successfully merging this pull request may close these issues.

2 participants