From c5fd2cd9891ecc0d2a1f40048d640eef4342a8d5 Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Thu, 4 Apr 2024 10:28:30 +0200 Subject: [PATCH] surface: commit subsurface state during parent commit --- src/ifs/wl_surface.rs | 29 ++--- src/ifs/wl_surface/commit_timeline.rs | 8 +- src/ifs/wl_surface/wl_subsurface.rs | 109 ++++++++++-------- src/ifs/wl_surface/x_surface.rs | 4 +- src/ifs/wl_surface/xdg_surface.rs | 2 +- src/ifs/wl_surface/zwlr_layer_surface_v1.rs | 2 +- src/it/tests.rs | 2 + src/it/tests/t0038_subsurface_parent_state.rs | 38 ++++++ .../screenshot_1.qoi | Bin 0 -> 8139 bytes .../screenshot_2.qoi | Bin 0 -> 8143 bytes 10 files changed, 125 insertions(+), 69 deletions(-) create mode 100644 src/it/tests/t0038_subsurface_parent_state.rs create mode 100644 src/it/tests/t0038_subsurface_parent_state/screenshot_1.qoi create mode 100644 src/it/tests/t0038_subsurface_parent_state/screenshot_2.qoi diff --git a/src/ifs/wl_surface.rs b/src/ifs/wl_surface.rs index 7e777197..c96e8cab 100644 --- a/src/ifs/wl_surface.rs +++ b/src/ifs/wl_surface.rs @@ -287,8 +287,8 @@ trait SurfaceExt { Ok(()) } - fn after_apply_commit(self: Rc, pending: &mut PendingState) { - let _ = pending; + fn after_apply_commit(self: Rc) { + // nothing } fn is_some(&self) -> bool { @@ -336,7 +336,7 @@ trait SurfaceExt { surface: &WlSurface, child: SubsurfaceId, consume: &mut dyn FnMut( - OccupiedEntry, + OccupiedEntry, ) -> Result<(), WlSurfaceError>, ) -> Result<(), WlSurfaceError> { surface.pending.borrow_mut().consume_child(child, consume) @@ -367,18 +367,17 @@ struct PendingState { xwayland_serial: Option, tearing: Option, content_type: Option>, - subsurface: Option>, xdg_surface: Option>, layer_surface: Option>, - subsurfaces: AHashMap, + subsurfaces: AHashMap, acquire_point: Option<(Rc, SyncObjPoint)>, release_point: Option<(Rc, SyncObjPoint)>, explicit_sync: bool, } -struct CommittedSubsurface { +struct AttachedSubsurfaceState { subsurface: Rc, - state: Box, + pending: PendingSubsurfaceData, } impl PendingState { @@ -445,13 +444,12 @@ impl PendingState { } }; } - merge_ext!(subsurface); merge_ext!(xdg_surface); merge_ext!(layer_surface); for (id, mut state) in next.subsurfaces.drain() { match self.subsurfaces.entry(id) { Entry::Occupied(mut o) => { - o.get_mut().state.merge(&mut state.state, client); + o.get_mut().pending.merge(&mut state.pending, client); } Entry::Vacant(v) => { v.insert(state); @@ -464,7 +462,7 @@ impl PendingState { &mut self, child: SubsurfaceId, consume: impl FnOnce( - OccupiedEntry, + OccupiedEntry, ) -> Result<(), WlSurfaceError>, ) -> Result<(), WlSurfaceError> { match self.subsurfaces.entry(child) { @@ -851,11 +849,8 @@ impl WlSurface { } fn apply_state(self: &Rc, pending: &mut PendingState) -> Result<(), WlSurfaceError> { - for (_, mut subsurface) in pending.subsurfaces.drain() { - subsurface - .subsurface - .surface - .apply_state(&mut subsurface.state)?; + for (_, pending) in &mut pending.subsurfaces { + pending.subsurface.apply_state(&mut pending.pending)?; } if self.destroyed.get() { return Ok(()); @@ -1056,7 +1051,7 @@ impl WlSurface { cursor.update_hardware_cursor(); } } - self.ext.get().after_apply_commit(pending); + self.ext.get().after_apply_commit(); self.client.state.damage(); Ok(()) } @@ -1274,7 +1269,7 @@ impl WlSurface { &self, child: SubsurfaceId, mut consume: impl FnMut( - OccupiedEntry, + OccupiedEntry, ) -> Result<(), WlSurfaceError>, ) -> Result<(), WlSurfaceError> { self.ext diff --git a/src/ifs/wl_surface/commit_timeline.rs b/src/ifs/wl_surface/commit_timeline.rs index 2ab8151e..07d72e20 100644 --- a/src/ifs/wl_surface/commit_timeline.rs +++ b/src/ifs/wl_surface/commit_timeline.rs @@ -269,7 +269,9 @@ fn consume_acquire_points(pending: &mut PendingState, points: &mut SmallVec<[Poi points.push(point); } for ss in pending.subsurfaces.values_mut() { - consume_acquire_points(&mut ss.state, points); + if let Some(state) = &mut ss.pending.state { + consume_acquire_points(state, points); + } } } @@ -290,6 +292,8 @@ fn set_effective_timeline( } } for ss in pending.subsurfaces.values() { - set_effective_timeline(&ss.subsurface.surface.commit_timeline, &ss.state, effective); + if let Some(state) = &ss.pending.state { + set_effective_timeline(&ss.subsurface.surface.commit_timeline, state, effective); + } } } diff --git a/src/ifs/wl_surface/wl_subsurface.rs b/src/ifs/wl_surface/wl_subsurface.rs index aedb7609..76eb4c4a 100644 --- a/src/ifs/wl_surface/wl_subsurface.rs +++ b/src/ifs/wl_surface/wl_subsurface.rs @@ -1,9 +1,9 @@ use { crate::{ - client::ClientError, + client::{Client, ClientError}, ifs::wl_surface::{ - CommitAction, CommittedSubsurface, PendingState, StackElement, SurfaceExt, SurfaceRole, - WlSurface, WlSurfaceError, WlSurfaceId, + AttachedSubsurfaceState, CommitAction, PendingState, StackElement, SurfaceExt, + SurfaceRole, WlSurface, WlSurfaceError, WlSurfaceId, }, leaks::Tracker, object::Object, @@ -13,13 +13,12 @@ use { clonecell::CloneCell, linkedlist::{LinkedNode, NodeRef}, numcell::NumCell, - option_ext::OptionExt, }, wire::{wl_subsurface::*, WlSubsurfaceId}, }, std::{ cell::{Cell, RefCell, RefMut}, - collections::hash_map::{Entry, OccupiedEntry}, + collections::hash_map::OccupiedEntry, mem, ops::Deref, rc::Rc, @@ -51,12 +50,20 @@ pub struct WlSubsurface { #[derive(Default)] pub struct PendingSubsurfaceData { + pub(super) state: Option>, node: Option>, position: Option<(i32, i32)>, } impl PendingSubsurfaceData { - pub fn merge(&mut self, next: &mut Self) { + pub fn merge(&mut self, next: &mut Self, client: &Rc) { + if let Some(mut new) = next.state.take() { + match &mut self.state { + Some(old) => old.merge(&mut new, client), + _ => self.state = Some(new), + } + } + macro_rules! opt { ($name:ident) => { if let Some(n) = next.$name.take() { @@ -107,12 +114,35 @@ impl WlSubsurface { } } - fn pending(&self) -> RefMut> { - RefMut::map(self.surface.pending.borrow_mut(), |m| { - m.subsurface.get_or_insert_default_ext() + fn pending<'a>(self: &'a Rc) -> RefMut<'a, PendingSubsurfaceData> { + RefMut::map(self.parent.pending.borrow_mut(), |m| { + &mut m + .subsurfaces + .entry(self.unique_id) + .or_insert_with(|| AttachedSubsurfaceState { + subsurface: self.clone(), + pending: Default::default(), + }) + .pending }) } + pub fn apply_state(&self, pending: &mut PendingSubsurfaceData) -> Result<(), WlSurfaceError> { + if let Some(state) = &mut pending.state.take() { + self.surface.apply_state(state)?; + } + if let Some(v) = pending.node.take() { + v.pending.set(false); + self.node.borrow_mut().replace(v); + } + if let Some((x, y)) = pending.position.take() { + self.position + .set(self.surface.buffer_abs_pos.get().at_point(x, y)); + self.parent.need_extents_update.set(true); + } + Ok(()) + } + pub fn install(self: &Rc) -> Result<(), WlSubsurfaceError> { if self.surface.id == self.parent.id { return Err(WlSubsurfaceError::OwnParent(self.surface.id)); @@ -158,9 +188,12 @@ impl WlSubsurface { let _req: Destroy = self.surface.client.parse(self, parser)?; self.surface.unset_ext(); self.parent.consume_pending_child(self.unique_id, |oe| { - self.surface.apply_state(&mut oe.remove().state) + let oe = oe.remove(); + if let Some(mut state) = oe.pending.state { + self.surface.apply_state(&mut state)?; + } + Ok(()) })?; - self.surface.pending.borrow_mut().subsurface.take(); *self.node.borrow_mut() = None; self.latest_node.take(); { @@ -184,8 +217,8 @@ impl WlSubsurface { Ok(()) } - fn set_position(&self, parser: MsgParser<'_, '_>) -> Result<(), WlSubsurfaceError> { - let req: SetPosition = self.surface.client.parse(self, parser)?; + fn set_position(self: &Rc, parser: MsgParser<'_, '_>) -> Result<(), WlSubsurfaceError> { + let req: SetPosition = self.surface.client.parse(&**self, parser)?; self.pending().position = Some((req.x, req.y)); Ok(()) } @@ -270,14 +303,12 @@ impl WlSubsurface { } fn on_desync(&self) -> Result<(), WlSurfaceError> { - let committed = self - .parent - .pending - .borrow_mut() - .subsurfaces - .remove(&self.unique_id); - if let Some(mut ps) = committed { - self.surface.apply_state(&mut ps.state)?; + let committed = &mut *self.parent.pending.borrow_mut(); + let committed = committed.subsurfaces.get_mut(&self.unique_id); + if let Some(ps) = committed { + if let Some(mut state) = ps.pending.state.take() { + self.surface.apply_state(&mut state)?; + } } Ok(()) } @@ -318,35 +349,17 @@ simple_add_obj!(WlSubsurface); impl SurfaceExt for WlSubsurface { fn commit_requested(self: Rc, pending: &mut Box) -> CommitAction { if self.sync() { - let mut parent_pending = self.parent.pending.borrow_mut(); - match parent_pending.subsurfaces.entry(self.unique_id) { - Entry::Occupied(mut o) => { - o.get_mut().state.merge(pending, &self.surface.client); - } - Entry::Vacant(v) => { - v.insert(CommittedSubsurface { - subsurface: self.clone(), - state: mem::take(&mut *pending), - }); - } + let mut parent_pending = self.pending(); + match &mut parent_pending.state { + None => parent_pending.state = Some(mem::take(&mut *pending)), + Some(state) => state.merge(pending, &self.surface.client), } return CommitAction::AbortCommit; } CommitAction::ContinueCommit } - fn after_apply_commit(self: Rc, pending: &mut PendingState) { - if let Some(pending) = &mut pending.subsurface { - if let Some(v) = pending.node.take() { - v.pending.set(false); - self.node.borrow_mut().replace(v); - } - if let Some((x, y)) = pending.position.take() { - self.position - .set(self.surface.buffer_abs_pos.get().at_point(x, y)); - self.parent.need_extents_update.set(true); - } - } + fn after_apply_commit(self: Rc) { let has_buffer = self.surface.buffer.is_some(); if self.had_buffer.replace(has_buffer) != has_buffer { if has_buffer { @@ -376,12 +389,16 @@ impl SurfaceExt for WlSubsurface { surface: &WlSurface, child: SubsurfaceId, consume: &mut dyn FnMut( - OccupiedEntry, + OccupiedEntry, ) -> Result<(), WlSurfaceError>, ) -> Result<(), WlSurfaceError> { self.parent .consume_pending_child(self.unique_id, |mut oe| { - oe.get_mut().state.consume_child(child, &mut *consume) + let oe = oe.get_mut(); + match &mut oe.pending.state { + Some(state) => state.consume_child(child, &mut *consume), + _ => Ok(()), + } })?; surface.pending.borrow_mut().consume_child(child, consume) } diff --git a/src/ifs/wl_surface/x_surface.rs b/src/ifs/wl_surface/x_surface.rs index 3da57314..eabfbf65 100644 --- a/src/ifs/wl_surface/x_surface.rs +++ b/src/ifs/wl_surface/x_surface.rs @@ -2,7 +2,7 @@ use { crate::{ ifs::wl_surface::{ x_surface::{xwayland_surface_v1::XwaylandSurfaceV1, xwindow::Xwindow}, - PendingState, SurfaceExt, WlSurface, WlSurfaceError, + SurfaceExt, WlSurface, WlSurfaceError, }, leaks::Tracker, tree::ToplevelNode, @@ -23,7 +23,7 @@ pub struct XSurface { } impl SurfaceExt for XSurface { - fn after_apply_commit(self: Rc, _pending: &mut PendingState) { + fn after_apply_commit(self: Rc) { if let Some(xwindow) = self.xwindow.get() { xwindow.map_status_changed(); } diff --git a/src/ifs/wl_surface/xdg_surface.rs b/src/ifs/wl_surface/xdg_surface.rs index 4bdf2965..5bb581b7 100644 --- a/src/ifs/wl_surface/xdg_surface.rs +++ b/src/ifs/wl_surface/xdg_surface.rs @@ -404,7 +404,7 @@ impl SurfaceExt for XdgSurface { Ok(()) } - fn after_apply_commit(self: Rc, _pending: &mut PendingState) { + fn after_apply_commit(self: Rc) { if let Some(ext) = self.ext.get() { ext.post_commit(); } diff --git a/src/ifs/wl_surface/zwlr_layer_surface_v1.rs b/src/ifs/wl_surface/zwlr_layer_surface_v1.rs index c5817af1..f70d0147 100644 --- a/src/ifs/wl_surface/zwlr_layer_surface_v1.rs +++ b/src/ifs/wl_surface/zwlr_layer_surface_v1.rs @@ -349,7 +349,7 @@ impl SurfaceExt for ZwlrLayerSurfaceV1 { Ok(()) } - fn after_apply_commit(self: Rc, _pending: &mut PendingState) { + fn after_apply_commit(self: Rc) { let buffer_is_some = self.surface.buffer.is_some(); let was_mapped = self.mapped.get(); if self.mapped.get() { diff --git a/src/it/tests.rs b/src/it/tests.rs index 9d6a138a..a8bdc8b2 100644 --- a/src/it/tests.rs +++ b/src/it/tests.rs @@ -69,6 +69,7 @@ mod t0034_workspace_restoration; mod t0035_scanout_feedback; mod t0036_idle; mod t0037_toplevel_drag; +mod t0038_subsurface_parent_state; pub trait TestCase: Sync { fn name(&self) -> &'static str; @@ -125,5 +126,6 @@ pub fn tests() -> Vec<&'static dyn TestCase> { t0035_scanout_feedback, t0036_idle, t0037_toplevel_drag, + t0038_subsurface_parent_state, } } diff --git a/src/it/tests/t0038_subsurface_parent_state.rs b/src/it/tests/t0038_subsurface_parent_state.rs new file mode 100644 index 00000000..b43910eb --- /dev/null +++ b/src/it/tests/t0038_subsurface_parent_state.rs @@ -0,0 +1,38 @@ +use { + crate::{ + it::{test_error::TestResult, testrun::TestRun}, + theme::Color, + }, + std::rc::Rc, +}; + +testcase!(); + +async fn test(run: Rc) -> TestResult { + let _ds = run.create_default_setup().await?; + + let client = run.create_client().await?; + let win = client.create_window().await?; + win.set_color(255, 255, 255, 255); + win.map2().await?; + + let ss = client.comp.create_surface().await?; + let vp = client.viewporter.get_viewport(&ss)?; + vp.set_destination(100, 100)?; + let buf = client.spbm.create_buffer(Color::SOLID_BLACK)?; + ss.attach(buf.id)?; + ss.commit()?; + + let ss = client.sub.get_subsurface(ss.id, win.surface.id).await?; + ss.set_position(0, 0)?; + win.surface.commit()?; + + client.compare_screenshot("1", false).await?; + + ss.set_position(100, 100)?; + win.surface.commit()?; + + client.compare_screenshot("2", false).await?; + + Ok(()) +} diff --git a/src/it/tests/t0038_subsurface_parent_state/screenshot_1.qoi b/src/it/tests/t0038_subsurface_parent_state/screenshot_1.qoi new file mode 100644 index 0000000000000000000000000000000000000000..988bc7671d6457cc816bad517a2afbcf0c715f82 GIT binary patch literal 8139 zcmeI%F%Cd56vpv~Ls(5>);LNNCy