-
Notifications
You must be signed in to change notification settings - Fork 76
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 zero size crash #276
Fix zero size crash #276
Conversation
It could be due to headless rendering or Android. Android is very specific in which order surfaces and the rendering context is initialized. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable! This needs testing on Android maybe.
@@ -25,7 +25,9 @@ impl UpdateState for PanHandler { | |||
(self.window_position, self.start_window_position) | |||
{ | |||
let view_proj = view_state.view_projection(); | |||
let inverted_view_proj = view_proj.invert(); | |||
let Some(inverted_view_proj) = view_proj.invert() else { | |||
todo!("what to do here?") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically in this case we would skip the execution of update_state
. Apparently it can happen for zero-sized windows. So this should be handled here either with ignoring it or doing a .except()
. You usually can not click in a zero-sized window. The except should explain that.
@@ -22,7 +22,9 @@ impl UpdateState for ZoomHandler { | |||
self.zoom_delta = None; | |||
|
|||
let view_proj = view_state.view_projection(); | |||
let inverted_view_proj = view_proj.invert(); | |||
let Some(inverted_view_proj) = view_proj.invert() else { | |||
todo!("what to do here?") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above
@@ -164,7 +164,8 @@ pub enum Head { | |||
} | |||
|
|||
pub struct Surface { | |||
size: WindowSize, | |||
/// Only render if size is not None | |||
size: Option<WindowSize>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems reasonable. I think on android this will be initially None.
…y in view state otherwise.
@terrorfisch Hi! Are you still interested in pushing this forward? :) I think the conflicts should be rather easily resolvable. If not I can also assist. |
@terrorfisch I would still love to merge this :) |
maybe this is fixed by now. I upgraded winit and if ofc changed its whole API 😅 |
Closing in favor of #315 |
Fixes #274
💻 Examples
Minimize the window or resize it to
y==0
. This previously crashed the demo.✔️ PR Todo
todo!
s remaining that are not handled yet because I do not understand if these cases can occure at all. It feels like failable projection inversion is only a crutch.WindowSize
member ofSurface
the way to go to represent a minimized/non-existing render target