surface: attach sync sub-surface commits to parent commits

src/compositor.rs

@@ -221,6 +221,7 @@ fn start_compositor2(
         double_click_interval_usec: Cell::new(400 * 1000),
         double_click_distance: Cell::new(5),
         create_default_seat: Cell::new(true),
+        subsurface_ids: Default::default(),
     });
     state.tracker.register(ClientId::from_raw(0));
     create_dummy_output(&state);

src/ifs/wl_surface.rs

@@ -26,7 +26,7 @@ use {
             },
             wl_surface::{
                 cursor::CursorSurface,
-                wl_subsurface::{PendingSubsurfaceData, WlSubsurface},
+                wl_subsurface::{PendingSubsurfaceData, SubsurfaceId, WlSubsurface},
                 wp_fractional_scale_v1::WpFractionalScaleV1,
                 wp_tearing_control_v1::WpTearingControlV1,
                 wp_viewport::WpViewport,
@@ -67,6 +67,7 @@ use {
     jay_config::video::Transform,
     std::{
         cell::{Cell, RefCell},
+        collections::hash_map::{Entry, OccupiedEntry},
         fmt::{Debug, Formatter},
         mem,
         ops::{Deref, DerefMut},
@@ -188,12 +189,6 @@ struct BufferPoints {
     y2: f32,
 }
 
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-enum CommitContext {
-    RootCommit,
-    ChildCommit,
-}
-
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 enum CommitAction {
     ContinueCommit,
@@ -201,8 +196,7 @@ enum CommitAction {
 }
 
 trait SurfaceExt {
-    fn prepare_commit(&self, ctx: CommitContext, pending: &mut PendingState) -> CommitAction {
+    fn commit_requested(self: Rc<Self>, pending: &mut PendingState) -> CommitAction {
-        let _ = ctx;
         let _ = pending;
         CommitAction::ContinueCommit
     }
@@ -258,6 +252,17 @@ trait SurfaceExt {
     fn into_xsurface(self: Rc<Self>) -> Option<Rc<XSurface>> {
         None
     }
+
+    fn consume_pending_child(
+        &self,
+        surface: &WlSurface,
+        child: SubsurfaceId,
+        consume: &mut dyn FnMut(
+            OccupiedEntry<SubsurfaceId, CommittedSubsurface>,
+        ) -> Result<(), WlSurfaceError>,
+    ) -> Result<(), WlSurfaceError> {
+        surface.pending.borrow_mut().consume_child(child, consume)
+    }
 }
 
 pub struct NoneSurfaceExt;
@@ -287,6 +292,98 @@ struct PendingState {
     subsurface: Option<Box<PendingSubsurfaceData>>,
     xdg_surface: Option<Box<PendingXdgSurfaceData>>,
     layer_surface: Option<Box<PendingLayerSurfaceData>>,
+    subsurfaces: AHashMap<SubsurfaceId, CommittedSubsurface>,
+}
+
+struct CommittedSubsurface {
+    subsurface: Rc<WlSubsurface>,
+    state: PendingState,
+}
+
+impl PendingState {
+    fn merge(&mut self, next: &mut Self, client: &Rc<Client>) {
+        // discard state
+
+        if next.buffer.is_some() {
+            if let Some(Some(prev)) = self.buffer.take() {
+                if !prev.destroyed() {
+                    prev.send_release();
+                }
+            }
+        }
+        for fb in self.presentation_feedback.drain(..) {
+            fb.send_discarded();
+            let _ = client.remove_obj(&*fb);
+        }
+
+        // overwrite state
+
+        macro_rules! opt {
+            ($name:ident) => {
+                if let Some(n) = next.$name.take() {
+                    self.$name = Some(n);
+                }
+            };
+        }
+        opt!(buffer);
+        opt!(opaque_region);
+        opt!(input_region);
+        opt!(src_rect);
+        opt!(dst_size);
+        opt!(scale);
+        opt!(transform);
+        opt!(xwayland_serial);
+        opt!(tearing);
+        opt!(content_type);
+        {
+            let (dx1, dy1) = self.offset;
+            let (dx2, dy2) = mem::take(&mut next.offset);
+            self.offset = (dx1 + dx2, dy1 + dy2);
+        }
+        self.frame_request.append(&mut next.frame_request);
+        self.damage |= mem::take(&mut next.damage);
+        mem::swap(
+            &mut self.presentation_feedback,
+            &mut next.presentation_feedback,
+        );
+        macro_rules! merge_ext {
+            ($name:ident) => {
+                if let Some(e) = &mut self.$name {
+                    if let Some(n) = &mut next.$name {
+                        e.merge(n);
+                    }
+                } else {
+                    self.$name = next.$name.take();
+                }
+            };
+        }
+        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);
+                }
+                Entry::Vacant(v) => {
+                    v.insert(state);
+                }
+            }
+        }
+    }
+
+    fn consume_child(
+        &mut self,
+        child: SubsurfaceId,
+        consume: impl FnOnce(
+            OccupiedEntry<SubsurfaceId, CommittedSubsurface>,
+        ) -> Result<(), WlSurfaceError>,
+    ) -> Result<(), WlSurfaceError> {
+        match self.subsurfaces.entry(child) {
+            Entry::Occupied(oe) => consume(oe),
+            _ => Ok(()),
+        }
+    }
 }
 
 #[derive(Default)]
@@ -660,21 +757,14 @@ impl WlSurface {
         Ok(())
     }
 
-    fn do_commit(self: &Rc<Self>, ctx: CommitContext) -> Result<(), WlSurfaceError> {
+    fn apply_state(self: &Rc<Self>, pending: &mut PendingState) -> Result<(), WlSurfaceError> {
-        let ext = self.ext.get();
+        for (_, mut subsurface) in pending.subsurfaces.drain() {
-        let pending = &mut *self.pending.borrow_mut();
+            subsurface
-        if ext.prepare_commit(ctx, pending) == CommitAction::AbortCommit {
+                .subsurface
-            return Ok(());
+                .surface
-        }
+                .apply_state(&mut subsurface.state)?;
-        ext.clone().before_apply_commit(pending)?;
-        {
-            let children = self.children.borrow();
-            if let Some(children) = children.deref() {
-                for child in children.subsurfaces.values() {
-                    child.surface.do_commit(CommitContext::ChildCommit)?;
-                }
-            }
         }
+        self.ext.get().before_apply_commit(pending)?;
         let mut scale_changed = false;
         if let Some(scale) = pending.scale.take() {
             scale_changed = true;
@@ -869,14 +959,18 @@ impl WlSurface {
                 cursor.update_hardware_cursor();
             }
         }
-        ext.after_apply_commit(pending);
+        self.ext.get().after_apply_commit(pending);
         self.client.state.damage();
         Ok(())
     }
 
     fn commit(self: &Rc<Self>, parser: MsgParser<'_, '_>) -> Result<(), WlSurfaceError> {
         let _req: Commit = self.parse(parser)?;
-        self.do_commit(CommitContext::RootCommit)?;
+        let ext = self.ext.get();
+        let pending = &mut *self.pending.borrow_mut();
+        if ext.commit_requested(pending) == CommitAction::ContinueCommit {
+            self.apply_state(pending)?;
+        }
         Ok(())
     }
 
@@ -1051,6 +1145,18 @@ impl WlSurface {
             consumer.send_feedback(fb);
         }
     }
+
+    fn consume_pending_child(
+        &self,
+        child: SubsurfaceId,
+        mut consume: impl FnMut(
+            OccupiedEntry<SubsurfaceId, CommittedSubsurface>,
+        ) -> Result<(), WlSurfaceError>,
+    ) -> Result<(), WlSurfaceError> {
+        self.ext
+            .get()
+            .consume_pending_child(self, child, &mut consume)
+    }
 }
 
 object_base! {

src/ifs/wl_surface/wl_subsurface.rs

@@ -2,7 +2,7 @@ use {
     crate::{
         client::ClientError,
         ifs::wl_surface::{
-            CommitAction, CommitContext, PendingState, StackElement, SurfaceExt, SurfaceRole,
+            CommitAction, CommittedSubsurface, PendingState, StackElement, SurfaceExt, SurfaceRole,
             WlSurface, WlSurfaceError, WlSurfaceId,
         },
         leaks::Tracker,
@@ -10,7 +10,8 @@ use {
         rect::Rect,
         utils::{
             buffd::{MsgParser, MsgParserError},
-            linkedlist::LinkedNode,
+            clonecell::CloneCell,
+            linkedlist::{LinkedNode, NodeRef},
             numcell::NumCell,
             option_ext::OptionExt,
         },
@@ -18,6 +19,8 @@ use {
     },
     std::{
         cell::{Cell, RefCell, RefMut},
+        collections::hash_map::{Entry, OccupiedEntry},
+        mem,
         ops::Deref,
         rc::Rc,
     },
@@ -29,14 +32,18 @@ const BAD_SURFACE: u32 = 0;
 
 const MAX_SUBSURFACE_DEPTH: u32 = 100;
 
+linear_ids!(SubsurfaceIds, SubsurfaceId, u64);
+
 pub struct WlSubsurface {
     id: WlSubsurfaceId,
+    unique_id: SubsurfaceId,
     pub surface: Rc<WlSurface>,
     pub(super) parent: Rc<WlSurface>,
     pub position: Cell<Rect>,
     sync_requested: Cell<bool>,
     sync_ancestor: Cell<bool>,
     node: RefCell<Option<LinkedNode<StackElement>>>,
+    latest_node: CloneCell<Option<NodeRef<StackElement>>>,
     depth: NumCell<u32>,
     pub tracker: Tracker<Self>,
 }
@@ -47,17 +54,17 @@ pub struct PendingSubsurfaceData {
     position: Option<(i32, i32)>,
 }
 
-fn update_children_sync(surface: &WlSubsurface, sync: bool) {
+impl PendingSubsurfaceData {
-    let children = surface.surface.children.borrow();
+    pub fn merge(&mut self, next: &mut Self) {
-    if let Some(children) = &*children {
+        macro_rules! opt {
-        for child in children.subsurfaces.values() {
+            ($name:ident) => {
-            let was_sync = child.sync();
+                if let Some(n) = next.$name.take() {
-            child.sync_ancestor.set(sync);
+                    self.$name = Some(n);
-            let is_sync = child.sync();
+                }
-            if was_sync != is_sync {
+            };
-                update_children_sync(child, sync);
-            }
         }
+        opt!(node);
+        opt!(position);
     }
 }
 
@@ -85,12 +92,14 @@ impl WlSubsurface {
     pub fn new(id: WlSubsurfaceId, surface: &Rc<WlSurface>, parent: &Rc<WlSurface>) -> Self {
         Self {
             id,
+            unique_id: surface.client.state.subsurface_ids.next(),
             surface: surface.clone(),
             parent: parent.clone(),
             position: Cell::new(Default::default()),
             sync_requested: Cell::new(false),
             sync_ancestor: Cell::new(false),
             node: RefCell::new(None),
+            latest_node: Default::default(),
             depth: NumCell::new(0),
             tracker: Default::default(),
         }
@@ -133,6 +142,7 @@ impl WlSubsurface {
                 sub_surface: self.clone(),
             })
         };
+        self.latest_node.set(Some(node.to_ref()));
         self.pending().node = Some(node);
         self.surface.set_toplevel(self.parent.toplevel.get());
         self.sync_ancestor.set(sync_ancestor);
@@ -145,8 +155,12 @@ impl WlSubsurface {
     fn destroy(&self, parser: MsgParser<'_, '_>) -> Result<(), WlSubsurfaceError> {
         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)
+        })?;
         self.surface.pending.borrow_mut().subsurface.take();
         *self.node.borrow_mut() = None;
+        self.latest_node.take();
         {
             let mut children = self.parent.children.borrow_mut();
             if let Some(children) = &mut *children {
@@ -193,18 +207,16 @@ impl WlSubsurface {
                     Some(s) => s,
                     _ => return Err(WlSubsurfaceError::NotASibling(sibling, self.surface.id)),
                 };
-                let node = match &sibling.pending().node {
+                let sibling_node = match sibling.latest_node.get() {
-                    Some(n) => n.to_ref(),
+                    Some(n) => n,
-                    _ => match sibling.node.borrow().deref() {
+                    _ => return Ok(()),
-                        Some(n) => n.to_ref(),
-                        _ => return Ok(()),
-                    },
                 };
                 match above {
-                    true => node.append(element),
+                    true => sibling_node.append(element),
-                    _ => node.prepend(element),
+                    _ => sibling_node.prepend(element),
                 }
             };
+            self.latest_node.set(Some(node.to_ref()));
             self.pending().node.replace(node);
         }
         Ok(())
@@ -226,24 +238,56 @@ impl WlSubsurface {
         self.sync_requested.get() || self.sync_ancestor.get()
     }
 
-    fn update_sync(&self, sync: bool) {
+    fn update_sync(&self, sync: bool) -> Result<(), WlSurfaceError> {
         let was_sync = self.sync();
         self.sync_requested.set(sync);
         let is_sync = self.sync();
         if was_sync != is_sync {
-            update_children_sync(self, is_sync);
+            self.handle_sync_change(is_sync)?;
         }
+        Ok(())
+    }
+
+    fn handle_sync_change(&self, is_sync: bool) -> Result<(), WlSurfaceError> {
+        if !is_sync {
+            self.on_desync()?;
+        }
+        let children = self.surface.children.borrow();
+        if let Some(children) = &*children {
+            for child in children.subsurfaces.values() {
+                let was_sync = child.sync();
+                child.sync_ancestor.set(is_sync);
+                let is_sync = child.sync();
+                if was_sync != is_sync {
+                    child.handle_sync_change(is_sync)?;
+                }
+            }
+        }
+        Ok(())
+    }
+
+    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)?;
+        }
+        Ok(())
     }
 
     fn set_sync(&self, parser: MsgParser<'_, '_>) -> Result<(), WlSubsurfaceError> {
         let _req: SetSync = self.surface.client.parse(self, parser)?;
-        self.update_sync(true);
+        self.update_sync(true)?;
         Ok(())
     }
 
     fn set_desync(&self, parser: MsgParser<'_, '_>) -> Result<(), WlSubsurfaceError> {
         let _req: SetDesync = self.surface.client.parse(self, parser)?;
-        self.update_sync(false);
+        self.update_sync(false)?;
         Ok(())
     }
 }
@@ -262,15 +306,27 @@ object_base! {
 impl Object for WlSubsurface {
     fn break_loops(&self) {
         *self.node.borrow_mut() = None;
+        self.latest_node.take();
     }
 }
 
 simple_add_obj!(WlSubsurface);
 
 impl SurfaceExt for WlSubsurface {
-    fn prepare_commit(&self, ctx: CommitContext, _pending: &mut PendingState) -> CommitAction {
+    fn commit_requested(self: Rc<Self>, pending: &mut PendingState) -> CommitAction {
-        if ctx == CommitContext::RootCommit && self.sync() {
+        if self.sync() {
-            log::debug!("Aborting commit due to 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),
+                    });
+                }
+            }
             return CommitAction::AbortCommit;
         }
         CommitAction::ContinueCommit
@@ -301,6 +357,21 @@ impl SurfaceExt for WlSubsurface {
     fn into_subsurface(self: Rc<Self>) -> Option<Rc<WlSubsurface>> {
         Some(self)
     }
+
+    fn consume_pending_child(
+        &self,
+        surface: &WlSurface,
+        child: SubsurfaceId,
+        consume: &mut dyn FnMut(
+            OccupiedEntry<SubsurfaceId, CommittedSubsurface>,
+        ) -> Result<(), WlSurfaceError>,
+    ) -> Result<(), WlSurfaceError> {
+        self.parent
+            .consume_pending_child(self.unique_id, |mut oe| {
+                oe.get_mut().state.consume_child(child, &mut *consume)
+            })?;
+        surface.pending.borrow_mut().consume_child(child, consume)
+    }
 }
 
 #[derive(Debug, Error)]

src/ifs/wl_surface/xdg_surface.rs

@@ -79,6 +79,19 @@ pub struct PendingXdgSurfaceData {
     geometry: Option<Rect>,
 }
 
+impl PendingXdgSurfaceData {
+    pub fn merge(&mut self, next: &mut Self) {
+        macro_rules! opt {
+            ($name:ident) => {
+                if let Some(n) = next.$name.take() {
+                    self.$name = Some(n);
+                }
+            };
+        }
+        opt!(geometry);
+    }
+}
+
 pub trait XdgSurfaceExt: Debug {
     fn initial_configure(self: Rc<Self>) -> Result<(), XdgSurfaceError> {
         Ok(())

src/ifs/wl_surface/zwlr_layer_surface_v1.rs

@@ -76,6 +76,25 @@ pub struct PendingLayerSurfaceData {
     any: bool,
 }
 
+impl PendingLayerSurfaceData {
+    pub fn merge(&mut self, next: &mut Self) {
+        macro_rules! opt {
+            ($name:ident) => {
+                if let Some(n) = next.$name.take() {
+                    self.$name = Some(n);
+                }
+            };
+        }
+        opt!(size);
+        opt!(anchor);
+        opt!(exclusive_zone);
+        opt!(margin);
+        opt!(keyboard_interactivity);
+        opt!(layer);
+        self.any |= mem::take(&mut next.any);
+    }
+}
+
 impl ZwlrLayerSurfaceV1 {
     pub fn new(
         id: ZwlrLayerSurfaceV1Id,

src/it/tests/t0007_subsurface.rs

@@ -37,6 +37,7 @@ async fn test(run: Rc<TestRun>) -> Result<(), TestError> {
     buffer.fill(Color::from_rgba_straight(255, 255, 255, 255));
 
     child.attach(buffer.id)?;
+    child.commit()?;
 
     parent.map().await?;
 
@@ -45,10 +46,12 @@ async fn test(run: Rc<TestRun>) -> Result<(), TestError> {
     client.compare_screenshot("1").await?;
 
     sub.place_below(parent.surface.id)?;
+    child.commit()?;
     parent.map().await?;
     client.compare_screenshot("2").await?;
 
     sub.place_above(parent.surface.id)?;
+    child.commit()?;
     parent.map().await?;
     client.compare_screenshot("1").await?;
 

src/it/tests/t0012_subsurface_focus.rs

@@ -29,6 +29,7 @@ async fn test(run: Rc<TestRun>) -> TestResult {
     nss.set_position(100, 100)?;
     let buffer = client.shm.create_buffer(100, 100)?;
     ns.attach(buffer.id)?;
+    ns.commit()?;
 
     run.cfg.set_fullscreen(ds.seat.id(), true)?;
     client.sync().await;

src/state.rs

@@ -30,6 +30,7 @@ use {
             wl_output::{OutputId, PersistentOutputState},
             wl_seat::{SeatIds, WlSeatGlobal},
             wl_surface::{
+                wl_subsurface::SubsurfaceIds,
                 zwp_idle_inhibitor_v1::{IdleInhibitorId, IdleInhibitorIds, ZwpIdleInhibitorV1},
                 NoneSurfaceExt, WlSurface,
             },
@@ -157,6 +158,7 @@ pub struct State {
     pub double_click_interval_usec: Cell<u64>,
     pub double_click_distance: Cell<i32>,
     pub create_default_seat: Cell<bool>,
+    pub subsurface_ids: SubsurfaceIds,
 }
 
 // impl Drop for State {