wayland.compositor: Correct frame callback handling

anvil/src/glium_drawer.rs

@@ -24,6 +24,7 @@ use smithay::{
         data_device::DnDIconRole,
         seat::CursorImageRole,
         shm::with_buffer_contents as shm_buffer_contents,
+        SERIAL_COUNTER as SCOUNTER,
     },
 };
 
@@ -380,6 +381,11 @@ impl<F: GLGraphicsBackend + 'static> GliumDrawer<F> {
                             },
                         );
                     }
+
+                    // send a frame event to the surface if applicable
+                    if let Some(callback) = data.borrow_mut().frame_callback.take() {
+                        callback.done(SCOUNTER.next_serial());
+                    }
                 }
             },
             |_, _, _, _| true,

anvil/src/shell.rs

@@ -11,7 +11,7 @@ use smithay::{
     reexports::{
         wayland_protocols::xdg_shell::server::xdg_toplevel,
         wayland_server::{
-            protocol::{wl_buffer, wl_pointer::ButtonState, wl_shell_surface, wl_surface},
+            protocol::{wl_buffer, wl_callback, wl_pointer::ButtonState, wl_shell_surface, wl_surface},
             Display,
         },
     },
@@ -29,6 +29,7 @@ use smithay::{
                 XdgSurfacePendingState, XdgSurfaceRole,
             },
         },
+        SERIAL_COUNTER as SCOUNTER,
     },
 };
 
@@ -305,10 +306,6 @@ pub fn init_shell(
                 let window_map = window_map.as_ref().unwrap();
                 surface_commit(&surface, ctoken, &buffer_utils, &*window_map)
             }
-            SurfaceEvent::Frame { callback } => {
-                callback.quick_assign(|_, _, _| unreachable!());
-                callback.done(0)
-            }
         },
         log.clone(),
     );
@@ -659,6 +656,7 @@ pub struct SurfaceData {
     ///
     /// `0` means unlimited.
     pub max_size: (i32, i32),
+    pub frame_callback: Option<wl_callback::WlCallback>,
 }
 
 impl SurfaceData {
@@ -754,6 +752,15 @@ fn surface_commit(
             None => {}
         }
 
+        // process the frame callback if any
+        if let Some(callback) = attributes.frame_callback.take() {
+            if let Some(old_callback) = data.frame_callback.take() {
+                // fire the old unfired callback to clean it up
+                old_callback.done(SCOUNTER.next_serial());
+            }
+            data.frame_callback = Some(callback);
+        }
+
         window_map.borrow().find(surface)
     });
 

src/wayland/compositor/handlers.rs

@@ -82,9 +82,9 @@ where
                 });
             }
             wl_surface::Request::Frame { callback } => {
-                let mut user_impl = self.implem.borrow_mut();
+                SurfaceData::<R>::with_data(&surface, move |d| {
-                trace!(self.log, "Calling user implementation for wl_surface.frame");
+                    d.frame_callback = Some((*callback).clone());
-                (&mut *user_impl)(SurfaceEvent::Frame { callback }, surface, CompositorToken::make());
+                });
             }
             wl_surface::Request::SetOpaqueRegion { region } => {
                 let attributes = region.map(|r| {

src/wayland/compositor/mod.rs

@@ -84,7 +84,7 @@ use wayland_server::{
     protocol::{
         wl_buffer, wl_callback, wl_compositor, wl_output, wl_region, wl_subcompositor, wl_surface::WlSurface,
     },
-    Display, Filter, Global, Main, UserDataMap,
+    Display, Filter, Global, UserDataMap,
 };
 
 /// Description of which part of a surface
@@ -156,6 +156,14 @@ pub struct SurfaceAttributes {
     /// Hint provided by the client to suggest that only this part
     /// of the surface was changed and needs to be redrawn
     pub damage: Damage,
+    /// The frame callback associated with this surface for the commit
+    ///
+    /// The be triggered to notify the client about when it would be a
+    /// good time to start drawing its next frame.
+    ///
+    /// An example possibility would be to trigger it once the frame
+    /// associated with this commit has been displayed on the screen.
+    pub frame_callback: Option<wl_callback::WlCallback>,
     /// User-controlled data
     ///
     /// This is your field to host whatever you need.
@@ -171,6 +179,7 @@ impl Default for SurfaceAttributes {
             opaque_region: None,
             input_region: None,
             damage: Damage::Full,
+            frame_callback: None,
             user_data: UserDataMap::new(),
         }
     }
@@ -511,22 +520,8 @@ pub enum SurfaceEvent {
     /// The double-buffered state has been validated by the client
     ///
     /// At this point, the pending state that has been accumulated in the [`SurfaceAttributes`] associated
-    /// to this surface should be integrated into the current state of the surface.
+    /// to this surface should be atomically integrated into the current state of the surface.
-    ///
-    /// See [`wayland_server::protocol::wl_surface::Implementation::commit`](https://docs.rs/wayland-server/0.10.1/wayland_server/protocol/wl_surface/struct.Implementation.html#structfield.commit)
-    /// for more details
     Commit,
-    /// The client asks to be notified when would be a good time to update the contents of this surface
-    ///
-    /// You must keep the provided [`WlCallback`](wayland_server::protocol::wl_callback::WlCallback)
-    /// and trigger it at the appropriate time by calling its `done()` method.
-    ///
-    /// See [`wayland_server::protocol::wl_surface::Implementation::frame`](https://docs.rs/wayland-server/0.10.1/wayland_server/protocol/wl_surface/struct.Implementation.html#structfield.frame)
-    /// for more details
-    Frame {
-        /// The created `WlCallback`
-        callback: Main<wl_callback::WlCallback>,
-    },
 }
 
 #[cfg(test)]