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

InnerSizeWriter does not give a way to read the suggested size #3080

Closed
ecton opened this issue Sep 2, 2023 · 7 comments
Closed

InnerSizeWriter does not give a way to read the suggested size #3080

ecton opened this issue Sep 2, 2023 · 7 comments
Labels
B - regression S - api Design and usability

Comments

@ecton
Copy link

ecton commented Sep 2, 2023

In the previous ScaleFactorChanged variant, we had access to read the suggested size of the window. As of 0.29.1-beta (should this have been 0.29.0-beta.1 as it's a beta of 0.29.0 not 0.29.1?), there is not a way to read the suggested size. Since the event loop is invoked with the ScaleFactorChanged variant before the inner size is updated, there is no way to read the suggested size within ScaleFactorChanged anymore.

Can InnerSizeWriter have a function added to retrieve the suggested size?

@madsmtm madsmtm added S - api Design and usability B - regression labels Sep 2, 2023
@madsmtm
Copy link
Member

madsmtm commented Sep 2, 2023

I think it can, would you be up for adding it in a PR?

(Or maybe the intention is to just query the requested size with Window::inner_size, unsure, @kchibisov?)

@ecton
Copy link
Author

ecton commented Sep 2, 2023

I'm happy to submit a PR. I hesitated only due to needing to come up with a name for the function :) Maybe InnerSizeWriter::suggested_size? But I'll wait to hear whether the inner_size should be updated before the event callback instead.

@daxpedda
Copy link
Member

daxpedda commented Sep 3, 2023

I just confirmed that on Web Window::inner_size would return the suggested size.
But I'm in favor of the suggested_size(), it's descriptive, and implementing it as well.

@kchibisov
Copy link
Member

It was intentionally done like that, because you can query window and that the handle once done clone, won't be able to provide a size hints, because it's all invalid. The window is also not locked to size changes (and can't be, because it could change at any time on e.g. X11, because the size is on the server), so you're encouraged to use Window method if you want to do adjustments to the size. If you don't want to do anything, just ignore the event all together, winit will resize you if it thinks that you should get a resize.

@daxpedda
Copy link
Member

daxpedda commented Sep 4, 2023

It was intentionally done like that, because you can query window and that the handle once done clone, won't be able to provide a size hints, because it's all invalid.

Isn't this handled in request_inner_size() as well? Just return an error if the handle becomes invalid.

The window is also not locked to size changes (and can't be, because it could change at any time on e.g. X11, because the size is on the server), so you're encouraged to use Window method if you want to do adjustments to the size. If you don't want to do anything, just ignore the event all together, winit will resize you if it thinks that you should get a resize.

I don't understand what you are trying to say here. It seems to me that you are suggesting that setting the size on InnerSizeWriter is pointless? Not sure how this is related to a getter though.

@kchibisov
Copy link
Member

I don't understand what you are trying to say here. It seems to me that you are suggesting that setting the size on InnerSizeWriter is pointless? Not sure how this is related to a getter though.

yeah, it's sort of pointless, because it has no use, we just need a handler to do a sync adjustment, but you can get the size of the window just fine from the window itself. Having it on the writer would mean that it's a resize event, while in reality, it's not a resize event and you don't have to resize anything when getting this event.

@daxpedda
Copy link
Member

daxpedda commented Sep 6, 2023

I was just discussing with @kchibisov on IRC how we could remove the whole thing, but that requires more discussion. See #2921.

I would say that the ultimate solution is to remove the whole InnerSizeWriter, until then if you need the "suggested size" just use Window::inner_size().

@daxpedda daxpedda closed this as completed Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - regression S - api Design and usability
Development

No branches or pull requests

4 participants