shell/grabs: Adjust reactive popups when window is moved#1369
shell/grabs: Adjust reactive popups when window is moved#1369dcz-self wants to merge 1 commit intopop-os:masterfrom
Conversation
| toplevel, | ||
| // TODO: give some thought to round vs floor | ||
| (location + offset.as_global().to_f64()).to_i32_round(), | ||
| self.window_outputs.iter(), |
There was a problem hiding this comment.
The outputs are for finding the anchor point and the anchor point must be within the surface, so it will be within the same outputs as the window, right?
There was a problem hiding this comment.
The outputs are for figuring out, which output contains the toplevel/popup to constrain that accordingly (and not let them go off-screen). window_outputs should be fine here, as they contain every output the window currently overlaps with. 👍
Reactive XDG popups should be reconstrained if the window is moved: > When set reactive, the surface is reconstrained if the conditions used for constraining changed, e.g. the parent window moved.
Drakulix
left a comment
Drakulix
left a comment
There was a problem hiding this comment.
Thanks and sorry for looking at this so late.
Code-wise this seems all fine, can you update the code-comments I commented on to reflect the state of things better for merging? (Or remove them, where they were just questions.)
src/shell/grabs/moving.rs
Outdated
| .get::<SeatMoveGrabState>() | ||
| .map(|s| s.lock().unwrap()); | ||
| if let Some(grab_state) = borrow.as_mut().and_then(|s| s.as_mut()) { | ||
| // is this workspace location or output location? |
There was a problem hiding this comment.
Looking at the code, it should be Global (I wonder why this piece of code isn't storing it that way), which means global coordinate space across outputs. update_reactive_popups wants global, so this should be correct.
src/shell/grabs/moving.rs
Outdated
| WindowSurface::Wayland(toplevel) => { | ||
| update_reactive_popups( | ||
| toplevel, | ||
| // TODO: give some thought to round vs floor |
There was a problem hiding this comment.
Round is fine here. We use that in most of the code-base, so this would be consistent when fractional scaling is applied.
| toplevel, | ||
| // TODO: give some thought to round vs floor | ||
| (location + offset.as_global().to_f64()).to_i32_round(), | ||
| self.window_outputs.iter(), |
There was a problem hiding this comment.
The outputs are for figuring out, which output contains the toplevel/popup to constrain that accordingly (and not let them go off-screen). window_outputs should be fine here, as they contain every output the window currently overlaps with. 👍
src/shell/grabs/moving.rs
Outdated
| self.window_outputs.iter(), | ||
| ); | ||
| } | ||
| // Not sure what to do wth X11 surfaces. Perhaps their popups don't need adjustments. |
There was a problem hiding this comment.
They do, but since they have access to the global coordinate space unlike wayland clients, they do the re-adjustment themselves.
|
Thanks for taking a look, I pushed a version with improved comments (though my explanations are surface-deep). If you feel like more popup-related contributions, I can do them as soon as smithay gets Smithay/smithay#1745 |
2 participants
XDG popups with positioner::reactive should be reconstrained if the window is moved:
This can be tested with the example from https://codeberg.org/dcz/stiwri :
This is what it looks like. Laggy because of my computer, but also I think I'm redrawing the surface one frame late.
The popup is allowed to slide to the right if the space to the left is not big enough. Its allowed to flip down if the space above is not big enough.
Screencast_20250423_145018.webm
Before this change, you needed to drag the edge and resize for the popup to move.
This will be very useful for the input method protocol update I'm working on.