Fix various TODOs

Author: prokopyl

src/platform/macos/context.rs

@@ -67,7 +67,7 @@ impl WindowContext {
             return Ok(());
         }
 
-        BaseviewView::resize(view, size);
+        BaseviewView::resize(view, size, true);
 
         Ok(())
     }

src/platform/macos/view.rs

@@ -159,7 +159,7 @@ impl BaseviewView {
         }
     }
 
-    pub fn resize(this: ViewRef<Self>, size: Size) {
+    pub fn resize(this: ViewRef<Self>, size: Size, notify_host: bool) {
         let size = size.to_logical::<f64>(this.view.backing_scale_factor());
         // NOTE: macOS gives you a personal rave if you pass in fractional pixels here. Even
         // though the size is in fractional pixels.
@@ -182,7 +182,7 @@ impl BaseviewView {
             }
         }
 
-        Self::view_did_change_backing_properties(this);
+        Self::view_did_change_backing_properties(this, notify_host);
     }
 
     /// Trigger the event immediately and return the event status.
@@ -229,7 +229,7 @@ impl ViewImpl for BaseviewView {
         true
     }
 
-    fn view_did_change_backing_properties(this: ViewRef<Self>) {
+    fn view_did_change_backing_properties(this: ViewRef<Self>, notify_host: bool) {
         let current_size = this.view.size();
         let current_scale_factor = this.view.backing_scale_factor();
 
@@ -247,13 +247,17 @@ impl ViewImpl for BaseviewView {
             if let Some(Err(e)) = result {
                 warn!("Window Handler failed to resize: {}", e);
                 this.state.size.set(previous);
-                Self::resize(this, previous.into())
+
+                Self::resize(this, previous.into(), false);
+                return;
             }
 
-            // TODO: only if not coming from host
-            if let Err(e) = this.host.request_resize(new_size) {
-                warn!("Host failed to resize parent view: {}", e);
-                Self::resize(this, previous.into()) // TODO: break error loop?
+            if notify_host {
+                if let Err(e) = this.host.request_resize(new_size) {
+                    warn!("Host failed to resize parent view: {}", e);
+
+                    Self::resize(this, previous.into(), false);
+                }
             }
         }
     }

src/platform/macos/window.rs

@@ -108,7 +108,7 @@ impl WindowHandle {
         let Some(view) = self.view.load() else { return Ok(()) };
         let Some(view) = view.inner_ref() else { return Ok(()) };
 
-        BaseviewView::resize(view, size);
+        BaseviewView::resize(view, size, false);
 
         Ok(())
     }

src/platform/x11/error.rs

@@ -22,8 +22,9 @@ impl Display for FatalError {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         match self {
             FatalError::Connection(e) => e.fmt(f),
-            // TODO: better errors
-            FatalError::SendMainThread => f.write_str("SendMainThread"),
+            FatalError::SendMainThread => {
+                f.write_str("Failed to send callback from X11 thread to main thread")
+            }
         }
     }
 }
@@ -51,7 +52,7 @@ pub enum Error {
     Connect(ConnectError),
     DisplayOpenFailed(DisplayOpenFailedError),
     Handler(HandlerError),
-    Channel(RecvError),
+    MainThreadRecvResult,
     Calloop(calloop::Error),
     RequestFromMainThreadFailed(RequestFailed),
     #[cfg(feature = "opengl")]
@@ -65,7 +66,26 @@ impl Display for Error {
         match self {
             Error::Io(e) => e.fmt(f),
             Self::IdsExhausted => f.write_str("X11 IDs have been exhausted"),
-            _ => todo!(),
+            Error::CreationFailed(e) => write!(f, "Failed to create window: {e}"),
+            Error::Run(e) => write!(f, "Error in running X11 thread: {e}"),
+            Error::DylibOpen(e) => e.fmt(f),
+            Error::InitThreadsFailed(e) => e.fmt(f),
+            Error::X11(e) => write!(f, "X server replied with error: {e:?}"),
+            Error::Connection(e) => e.fmt(f),
+            Error::Parse(e) => e.fmt(f),
+            Error::GetProperty(e) => e.fmt(f),
+            Error::Connect(e) => e.fmt(f),
+            Error::DisplayOpenFailed(e) => e.fmt(f),
+            Error::Handler(e) => e.fmt(f),
+            Error::MainThreadRecvResult => {
+                f.write_str("Failed to receive Window creation response from X11 thread: channel was closed unexpectedly")
+            }
+            Error::Calloop(e) => e.fmt(f),
+            Error::RequestFromMainThreadFailed(e) => e.fmt(f),
+            #[cfg(feature = "opengl")]
+            Error::XLib(e) => e.fmt(f),
+            #[cfg(feature = "opengl")]
+            Error::Gl(e) => e.fmt(f),
         }
     }
 }
@@ -74,7 +94,6 @@ impl std::error::Error for Error {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {
             Error::Io(e) => Some(e),
-            Error::Channel(e) => Some(e),
             Error::DylibOpen(e) => Some(e),
             Error::Connect(e) => Some(e),
             Error::Handler(e) => Some(e.source()),
@@ -133,12 +152,6 @@ impl From<calloop::Error> for Error {
     }
 }
 
-impl From<RecvError> for Error {
-    fn from(value: RecvError) -> Self {
-        Self::Channel(value)
-    }
-}
-
 impl From<RequestFailed> for Error {
     fn from(value: RequestFailed) -> Self {
         Self::RequestFromMainThreadFailed(value)

src/platform/x11/event_loop.rs

@@ -134,7 +134,9 @@ impl EventLoop {
             self.window.store_size(previous);
             self.window.xcb_window.resize(previous.cast())?.check_warn();
         } else {
-            // TODO: only if this wansn't the result of a host request
+            // Host requests use resize_immediately, which stops the previous == new_size condition
+            // So if we're here, it's guaranteed not to be from a host request
+
             if let Some(host) = self.main_thread.as_mut() {
                 host.send(HostCallback::Resized {
                     new_size,

src/platform/x11/gl.rs

@@ -20,6 +20,24 @@ pub enum CreationFailedError {
     OpenError(OpenError),
 }
 
+impl Display for CreationFailedError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            CreationFailedError::NoValidFBConfig => {
+                f.write_str("Could not find a valid Framebuffer configuration")
+            }
+            CreationFailedError::NoVisual => {
+                f.write_str("Could not find a matching visual configuration")
+            }
+            CreationFailedError::GetProcAddressFailed => f.write_str("GetProcAddress failed"),
+            CreationFailedError::MakeCurrentFailed => f.write_str("MakeCurrent failed"),
+            CreationFailedError::ContextCreationFailed => f.write_str("Faile to create GL context"),
+            CreationFailedError::X11Error(e) => e.fmt(f),
+            CreationFailedError::OpenError(e) => e.fmt(f),
+        }
+    }
+}
+
 pub type GlContext = Rc<GlContextInner>;
 
 pub struct GlContextInner {

src/platform/x11/window_thread.rs

@@ -298,7 +298,8 @@ impl WindowResultSender {
 struct WindowResultReceiver(mpsc::Receiver<WindowOpenResult>);
 impl WindowResultReceiver {
     pub fn receive(self) -> Result<LoopSignal> {
-        let result = self.0.recv()?;
+        let result = self.0.recv().map_err(|_| Error::MainThreadRecvResult)?;
+
         match result {
             WindowOpenResult::Error(e) => Err(Error::CreationFailed(e)),
             WindowOpenResult::Success { loop_signal } => Ok(loop_signal),

src/platform/x11/xcb_window.rs

@@ -1,3 +1,4 @@
+use crate::platform::x11::error::CookieExt;
 use crate::platform::x11::visual_info::WindowVisualConfig;
 use crate::platform::X11Connection;
 use dpi::PhysicalSize;
@@ -111,8 +112,9 @@ impl XcbWindow {
 
 impl Drop for XcbWindow {
     fn drop(&mut self) {
-        // TODO: log error
-        let Ok(cookie) = self.connection.conn.destroy_window(self.window_id.get()) else { return };
-        let _ = cookie.check();
+        match self.connection.conn.destroy_window(self.window_id.get()) {
+            Err(e) => crate::warn!("Failed to send request to destroy X window: {}", e),
+            Ok(cookie) => cookie.check_warn(),
+        }
     }
 }

src/wrappers/appkit/view.rs

@@ -152,7 +152,7 @@ pub trait ViewImpl: Sized {
     fn become_first_responder(this: ViewRef<Self>) -> bool;
     fn resign_first_responder(this: ViewRef<Self>) -> bool;
     fn window_should_close(this: ViewRef<Self>) -> bool;
-    fn view_did_change_backing_properties(this: ViewRef<Self>);
+    fn view_did_change_backing_properties(this: ViewRef<Self>, from_host: bool);
     fn hit_test(this: ViewRef<'_, Self>, point: NSPoint) -> Option<&NSView>;
     fn view_will_move_to_window(this: ViewRef<Self>, new_window: Option<&NSWindow>);
     fn update_tracking_areas(this: ViewRef<Self>);

src/wrappers/appkit/view/implementation.rs

@@ -187,7 +187,7 @@ extern "C-unwind" fn view_did_change_backing_properties<V: ViewImpl>(
     this: &View<V>, _: Sel, _: &AnyObject,
 ) {
     let Some(inner) = this.inner_ref() else { return };
-    V::view_did_change_backing_properties(inner);
+    V::view_did_change_backing_properties(inner, true);
 }
 
 extern "C-unwind" fn hit_test<V: ViewImpl>(