combine buffer attaches and subsurface positions into the same commit

previously, our plan was to do a subsurface set position and then wait
for the frame callback to attach the buffer for a new x11 subsurface.
This doesn't work, since compositors will only perform frame callbacks
to surfaces with attached buffers.

src/client/subsurface.rs

@@ -264,6 +264,10 @@ impl RemoteSubSurface {
             .location(loc!())?
             .as_sub_surface_mut()
             .location(loc!())?;
+
+        role.local_subsurface
+            .set_position(subsurface_state.location.x, subsurface_state.location.y);
+
         if role.sync == subsurface_state.sync {
             return Ok(());
         }

src/serialization/wayland.rs

@@ -498,15 +498,15 @@ impl PointerEvent {
 #[archive_attr(derive(bytecheck::CheckBytes, Debug))]
 pub struct SubSurfaceState {
     pub parent: WlSurfaceId,
-    pub position: Point<i32>,
+    pub location: Point<i32>,
     pub sync: bool,
 }
 
 impl SubSurfaceState {
     pub fn new(parent: &WlSurface) -> Self {
         Self {
             parent: WlSurfaceId::new(parent),
-            position: (0, 0).into(),
+            location: (0, 0).into(),
             sync: true,
         }
     }

src/server/smithay_handlers.rs

@@ -746,6 +746,11 @@ pub fn commit_impl(
         Some(Role::Cursor(_)) => {},
         Some(Role::SubSurface(subsurface_state)) => {
             subsurface_state.sync = sync;
+            subsurface_state.location = surface_data
+                .cached_state
+                .pending::<SubsurfaceCachedState>()
+                .location
+                .into();
         },
         Some(Role::XdgToplevel(toplevel_state)) => {
             set_xdg_toplevel_attributes(surface_data, toplevel_state).location(loc!())?;

src/xwayland_xdg_shell/client.rs

@@ -246,7 +246,6 @@ impl CompositorHandler for WprsState {
             let xwayland_surface = self.surfaces.get_mut(compositor_surface_id).unwrap();
             if let Some(Role::SubSurface(subsurface)) = &mut xwayland_surface.role {
                 subsurface.pending_frame_callback = false;
-                subsurface.position_initialized = true;
             }
             if let Some(x11_surface) = &xwayland_surface.x11_surface {
                 if let Some(wl_surface) = x11_surface.wl_surface() {
@@ -360,7 +359,7 @@ impl WindowHandler for WprsState {
 
         xdg_toplevel.configured = true;
 
-        xwayland_surface.try_attach_buffer(&self.client_state.qh);
+        xwayland_surface.commit_buffer(&self.client_state.qh);
     }
 }
 
@@ -418,7 +417,7 @@ impl PopupHandler for WprsState {
         //     .xdg_surface()
         //     .set_window_geometry(0, 0, geo.size.w, geo.size.h);
 
-        xwayland_surface.try_attach_buffer(&self.client_state.qh);
+        xwayland_surface.commit_buffer(&self.client_state.qh);
     }
 
     fn done(&mut self, _conn: &Connection, _qh: &QueueHandle<Self>, _popup: &Popup) {
@@ -1273,7 +1272,6 @@ pub struct XWaylandSubSurface {
     pub move_pointer_location: (f64, f64),
     pub pending_frame_callback: bool,
     pub buffer_attached: bool,
-    pub position_initialized: bool,
 }
 
 impl XWaylandSubSurface {
@@ -1334,31 +1332,38 @@ impl XWaylandSubSurface {
             move_pointer_location: (0 as f64, 0 as f64),
             pending_frame_callback: false,
             buffer_attached: false,
-            position_initialized: false,
         };
         surface.role = Some(Role::SubSurface(new_subsurface));
 
+        // the initial commit must include both the subsurface position and the buffer
+        // to prevent desyncs
         if let Some(Role::SubSurface(subsurface)) = &mut surface.role {
-            subsurface.move_(geometry.loc.x, geometry.loc.y, qh);
+            subsurface.move_without_commit(geometry.loc.x, geometry.loc.y, qh);
         }
 
         Ok(())
     }
 
-    pub(crate) fn move_(&mut self, x: i32, y: i32, qh: &QueueHandle<WprsState>) {
+    pub(crate) fn move_without_commit(&mut self, x: i32, y: i32, qh: &QueueHandle<WprsState>) {
         if !self.pending_frame_callback {
-            let local_wl_surface = self.local_subsurface.surface.wl_surface();
+            let local_wl_surface = self.wl_surface();
 
             self.local_subsurface
                 .subsurface
                 .set_position(x + self.parent_offset.x, y + self.parent_offset.y);
             local_wl_surface.frame(qh, local_wl_surface.clone());
-            local_wl_surface.commit();
             self.parent_surface.commit();
 
             self.pending_frame_callback = true;
         }
     }
+
+    pub(crate) fn move_(&mut self, x: i32, y: i32, qh: &QueueHandle<WprsState>) {
+        if !self.pending_frame_callback {
+            self.move_without_commit(x, y, qh);
+            self.wl_surface().commit();
+        }
+    }
 }
 
 impl WaylandSurface for XWaylandSubSurface {

src/xwayland_xdg_shell/compositor.rs

@@ -483,7 +483,7 @@ pub fn commit_inner(
             xwayland_surface.frame(&state.client_state.qh);
         }
 
-        xwayland_surface.try_attach_buffer(&state.client_state.qh);
+        xwayland_surface.try_attach_buffer();
     }
 
     if xwayland_surface.ready() || xwayland_surface.needs_configure() {

src/xwayland_xdg_shell/mod.rs

@@ -123,7 +123,6 @@ impl XWaylandSurface {
         match &self.role {
             Some(Role::XdgToplevel(toplevel)) if !toplevel.configured => false,
             Some(Role::XdgPopup(popup)) if !popup.configured => false,
-            Some(Role::SubSurface(subsurface)) if !subsurface.position_initialized => false,
             _ => self.x11_surface.is_some() || matches!(self.role, Some(Role::Cursor)),
         }
     }
@@ -136,22 +135,29 @@ impl XWaylandSurface {
         }
     }
 
-    fn try_attach_buffer(&mut self, qh: &QueueHandle<WprsState>) {
+    fn try_attach_buffer(&mut self) {
         if !self.buffer_attached {
             if let Some(buffer) = &self.buffer {
                 let surface = self.wl_surface();
                 // The only possible error here is AlreadyActive, which we can
                 // ignore.
                 _ = buffer.active_buffer.attach_to(surface);
                 surface.damage_buffer(0, 0, i32::MAX, i32::MAX);
-                self.frame(qh);
-                self.wl_surface().commit();
 
                 self.buffer_attached = true;
             }
         }
     }
 
+    fn commit_buffer(&mut self, qh: &QueueHandle<WprsState>) {
+        if !self.buffer_attached && self.buffer.is_some() {
+            self.try_attach_buffer();
+
+            self.frame(qh);
+            self.commit();
+        }
+    }
+
     #[instrument(skip(xdg_shell_state, qh), level = "debug")]
     fn update_x11_surface(
         &mut self,