cleanup popup implementation.

- bunch of other misc cleanups

Author: james-lawrence

druid-shell/src/backend/wayland/application.rs

@@ -181,13 +181,7 @@ impl Application {
         let (outputsaddedtx, outputsaddedrx) = calloop::channel::channel::<Output>();
 
         let globals = wl::GlobalManager::new_with_cb(&attached_server, {
-            move |event, registry, data| {
-                tracing::debug!(
-                    "global manager event received {:?}\n{:?}\n{:?}",
-                    event,
-                    registry,
-                    data
-                );
+            move |event, registry, _ctx| {
                 match event {
                     wl::GlobalEvent::New {
                         id,
@@ -270,15 +264,6 @@ impl Application {
             .sync_roundtrip(&mut (), |_, _, _| unreachable!())
             .map_err(Error::fatal)?;
 
-        let mut globals_list = globals.list();
-        globals_list.sort_by(|(_, name1, version1), (_, name2, version2)| {
-            name1.cmp(name2).then(version1.cmp(version2))
-        });
-
-        for (id, name, version) in globals_list.into_iter() {
-            tracing::debug!("{:?}@{:?} - {:?}", name, version, id);
-        }
-
         let xdg_base = globals
             .instantiate_exact::<XdgWmBase>(2)
             .map_err(|e| Error::global("xdg_wm_base", 2, e))?;
@@ -370,7 +355,7 @@ impl Application {
                         if capabilities.contains(wl_seat::Capability::Keyboard)
                             && seat.keyboard.is_none()
                         {
-                            seat.keyboard = Some(app_data.keyboard.attach(id, seat.wl_seat.clone()))
+                            seat.keyboard = Some(app_data.keyboard.attach(id, seat.wl_seat.clone()));
                         }
                         if capabilities.contains(wl_seat::Capability::Pointer)
                             && seat.pointer.is_none()
@@ -403,7 +388,7 @@ impl Application {
     }
 
     pub fn run(self, _handler: Option<Box<dyn AppHandler>>) {
-        tracing::info!("run initiated");
+        tracing::info!("wayland event loop initiated");
         // NOTE if we want to call this function more than once, we will need to put the timer
         // source back.
         let timer_source = self.data.timer_source.borrow_mut().take().unwrap();
@@ -431,6 +416,7 @@ impl Application {
                 }
             })
             .unwrap();
+
         handle
             .insert_source(
                 self.data.outputs_removed.borrow_mut().take().unwrap(),
@@ -456,20 +442,30 @@ impl Application {
         let signal = eventloop.get_signal();
         let handle = handle.clone();
 
-        eventloop
-            .run(
-                Duration::from_millis(20),
-                &mut self.data.clone(),
-                move |appdata| {
-                    if appdata.shutdown.get() {
-                        tracing::debug!("shutting down");
-                        signal.stop();
-                    }
+        let res = eventloop.run(
+            Duration::from_millis(20),
+            &mut self.data.clone(),
+            move |appdata| {
+                if appdata.shutdown.get() {
+                    tracing::debug!("shutting down, requested");
+                    signal.stop();
+                    return;
+                }
 
-                    ApplicationData::idle_repaint(handle.clone());
-                },
-            )
-            .unwrap();
+                if appdata.handles.borrow().len() == 0 {
+                    tracing::debug!("shutting down, no window remaining");
+                    signal.stop();
+                    return;
+                }
+
+                ApplicationData::idle_repaint(handle.clone());
+            },
+        );
+
+        match res {
+            Ok(_) => tracing::info!("wayland event loop completed"),
+            Err(cause) => tracing::error!("wayland event loop failed {:?}", cause),
+        }
     }
 
     pub fn quit(&self) {
@@ -576,18 +572,8 @@ impl ApplicationData {
         }
     }
 
-    pub(super) fn popup<'a>(&self, surface: &'a surfaces::popup::Surface) -> Result<(), Error> {
-        match self.acquire_current_window() {
-            None => return Err(Error::string("parent window does not exist")),
-            Some(winhandle) => winhandle.popup(surface),
-        }
-    }
-
     fn handle_timer_event(&self, _token: TimerToken) {
-        // Shouldn't be necessary.
-        self.timer_handle.cancel_all_timeouts();
         // Don't borrow the timers in case the callbacks want to add more.
-        // TODO make this in the stack (smallvec)
         let mut expired_timers = Vec::with_capacity(1);
         let mut timers = self.timers.borrow_mut();
         let now = Instant::now();
@@ -601,7 +587,7 @@ impl ApplicationData {
                 Some(s) => s,
                 None => {
                     // NOTE this might be expected
-                    log::warn!(
+                    tracing::warn!(
                         "received event for surface that doesn't exist any more {:?} {:?}",
                         expired,
                         expired.id()

druid-shell/src/backend/wayland/clipboard.rs

@@ -106,7 +106,10 @@ pub struct Manager {
 }
 
 impl Manager {
-    pub(super) fn new(display: &wl::Display, gm: &wl::GlobalManager) -> Result<Self, waylanderr::Error> {
+    pub(super) fn new(
+        display: &wl::Display,
+        gm: &wl::GlobalManager,
+    ) -> Result<Self, waylanderr::Error> {
         let m = gm
             .instantiate_exact::<wl_data_device_manager::WlDataDeviceManager>(3)
             .map_err(|e| waylanderr::Error::global("wl_data_device_manager", 1, e))?;
@@ -257,13 +260,15 @@ impl Clipboard {
     pub fn get_string(&self) -> Option<String> {
         vec![Clipboard::UTF8, Clipboard::TEXT, Clipboard::UTF8_STRING]
             .iter()
-            .find_map(|mimetype| match std::str::from_utf8(&self.inner.receive(*mimetype)?) {
-                Ok(s) => Some(s.to_string()),
-                Err(cause) => {
-                    tracing::error!("clipboard unable to retrieve utf8 content {:?}", cause);
-                    None
+            .find_map(
+                |mimetype| match std::str::from_utf8(&self.inner.receive(*mimetype)?) {
+                    Ok(s) => Some(s.to_string()),
+                    Err(cause) => {
+                        tracing::error!("clipboard unable to retrieve utf8 content {:?}", cause);
+                        None
+                    }
                 },
-            })
+            )
     }
 
     /// Given a list of supported clipboard types, returns the supported type which has

druid-shell/src/backend/wayland/mod.rs

@@ -16,15 +16,13 @@
 
 pub mod application;
 pub mod clipboard;
-pub mod dialog;
 pub mod error;
 mod events;
 pub mod keyboard;
 pub mod menu;
 pub mod pointers;
 pub mod screen;
 pub mod surfaces;
-pub mod util;
 pub mod window;
 
 /// Little enum to make it clearer what some return values mean.

druid-shell/src/backend/wayland/pointers.rs

@@ -107,6 +107,7 @@ pub(crate) enum PointerEvent {
 }
 
 /// An enum that we will convert into the different callbacks.
+#[derive(Debug)]
 pub(crate) enum MouseEvtKind {
     Move(mouse::MouseEvent),
     Up(mouse::MouseEvent),

druid-shell/src/backend/wayland/surfaces/layershell.rs

@@ -1,19 +1,18 @@
 use wayland_client as wlc;
 use wayland_protocols::wlr::unstable::layer_shell::v1::client as layershell;
+use wayland_protocols::xdg_shell::client::xdg_surface;
 
 use crate::kurbo;
 use crate::window;
 
 use super::super::application::Output;
 use super::super::error;
-use super::popup;
 use super::surface;
 use super::Compositor;
 use super::CompositorHandle;
 use super::Handle;
 use super::Outputs;
 use super::Popup;
-use super::PopupHandle;
 
 struct Inner {
     config: Config,
@@ -24,17 +23,32 @@ struct Inner {
     available: std::cell::RefCell<bool>,
 }
 
-impl Drop for Inner {
-    fn drop(&mut self) {
-        self.ls_surface.borrow().destroy();
+impl Inner {
+    fn popup<'a>(
+        &self,
+        surface: &'a wlc::Main<xdg_surface::XdgSurface>,
+        pos: &'a wlc::Main<wayland_protocols::xdg_shell::client::xdg_positioner::XdgPositioner>,
+    ) -> wlc::Main<wayland_protocols::xdg_shell::client::xdg_popup::XdgPopup> {
+        let popup = surface.get_popup(None, pos);
+        self.ls_surface.borrow().get_popup(&popup);
+        return popup;
     }
 }
 
 impl Popup for Inner {
-    fn popup_impl(&self, p: &popup::Surface) -> Result<(), error::Error> {
-        self.ls_surface.borrow().get_popup(&p.get_xdg_popup());
-        p.commit();
-        Ok(())
+    fn surface<'a>(
+        &self,
+        surface: &'a wlc::Main<xdg_surface::XdgSurface>,
+        pos: &'a wlc::Main<wayland_protocols::xdg_shell::client::xdg_positioner::XdgPositioner>,
+    ) -> Result<wlc::Main<wayland_protocols::xdg_shell::client::xdg_popup::XdgPopup>, error::Error>
+    {
+        return Ok(self.popup(surface, pos));
+    }
+}
+
+impl Drop for Inner {
+    fn drop(&mut self) {
+        self.ls_surface.borrow().destroy();
     }
 }
 
@@ -218,13 +232,6 @@ impl Surface {
         }
 
         tracing::info!("attempting to initialize layershell");
-        handle
-            .inner
-            .wl_surface
-            .borrow()
-            .set_popup_impl(PopupHandle {
-                inner: handle.inner.clone(),
-            });
 
         handle.inner.ls_surface.borrow().quick_assign({
             let handle = handle.clone();
@@ -319,6 +326,17 @@ impl Outputs for Surface {
     }
 }
 
+impl Popup for Surface {
+    fn surface<'a>(
+        &self,
+        popup: &'a wlc::Main<xdg_surface::XdgSurface>,
+        pos: &'a wlc::Main<wayland_protocols::xdg_shell::client::xdg_positioner::XdgPositioner>,
+    ) -> Result<wlc::Main<wayland_protocols::xdg_shell::client::xdg_popup::XdgPopup>, error::Error>
+    {
+        return Ok(self.inner.popup(popup, pos));
+    }
+}
+
 impl Handle for Surface {
     fn get_size(&self) -> kurbo::Size {
         return self.inner.wl_surface.borrow().get_size();
@@ -368,10 +386,6 @@ impl Handle for Surface {
         }
     }
 
-    fn popup(&self, popup: &popup::Surface) -> Result<(), error::Error> {
-        return self.inner.wl_surface.borrow().popup(popup);
-    }
-
     fn release(&self) {
         return self.inner.wl_surface.borrow().release();
     }
@@ -404,3 +418,9 @@ impl From<Surface> for Box<dyn Outputs> {
         Box::new(s) as Box<dyn Outputs>
     }
 }
+
+impl From<Surface> for Box<dyn Popup> {
+    fn from(s: Surface) -> Self {
+        Box::new(s) as Box<dyn Popup>
+    }
+}

druid-shell/src/backend/wayland/surfaces/mod.rs

@@ -2,6 +2,7 @@ use wayland_client::protocol::wl_shm::WlShm;
 use wayland_client::{self as wlc, protocol::wl_surface::WlSurface};
 use wayland_protocols::unstable::xdg_decoration::v1::client::zxdg_decoration_manager_v1::ZxdgDecorationManagerV1;
 use wayland_protocols::wlr::unstable::layer_shell::v1::client::zwlr_layer_shell_v1::ZwlrLayerShellV1;
+use wayland_protocols::xdg_shell::client::xdg_popup;
 use wayland_protocols::xdg_shell::client::xdg_positioner;
 use wayland_protocols::xdg_shell::client::xdg_surface;
 
@@ -43,17 +44,11 @@ impl dyn Decor {
 }
 
 pub trait Popup {
-    fn popup_impl(&self, popup: &popup::Surface) -> Result<(), error::Error>;
-}
-
-pub struct PopupHandle {
-    inner: std::sync::Arc<dyn Popup>,
-}
-
-impl PopupHandle {
-    fn popup(&self, p: &popup::Surface) -> Result<(), error::Error> {
-        self.inner.popup_impl(p)
-    }
+    fn surface<'a>(
+        &self,
+        popup: &'a wlc::Main<xdg_surface::XdgSurface>,
+        pos: &'a wlc::Main<xdg_positioner::XdgPositioner>,
+    ) -> Result<wlc::Main<xdg_popup::XdgPopup>, error::Error>;
 }
 
 pub(super) trait Outputs {
@@ -73,7 +68,6 @@ pub trait Handle {
     fn get_idle_handle(&self) -> idle::Handle;
     fn get_scale(&self) -> Scale;
     fn run_idle(&self);
-    fn popup(&self, popup: &popup::Surface) -> Result<(), error::Error>;
     fn release(&self);
     fn data(&self) -> Option<std::sync::Arc<surface::Data>>;
 }