Don't try to handle errors when opening platform windows

Max Brunsfeld and Nathan Sobo created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

gpui/src/app.rs                   | 136 +++++++++++++++-----------------
gpui/src/platform/mac/platform.rs |   5 
gpui/src/platform/mac/renderer.rs |  33 +++----
gpui/src/platform/mac/window.rs   |  20 +---
gpui/src/platform/mod.rs          |   3 
gpui/src/platform/test.rs         |   4 
6 files changed, 93 insertions(+), 108 deletions(-)

Detailed changes

gpui/src/app.rs 🔗

@@ -735,88 +735,82 @@ impl MutableAppContext {
     }
 
     fn open_platform_window(&mut self, window_id: usize) {
-        match self.platform.open_window(
+        let mut window = self.platform.open_window(
             window_id,
             WindowOptions {
                 bounds: RectF::new(vec2f(0., 0.), vec2f(1024., 768.)),
                 title: "Zed".into(),
             },
             self.foreground.clone(),
-        ) {
-            Err(e) => log::error!("error opening window: {}", e),
-            Ok(mut window) => {
-                let text_layout_cache = TextLayoutCache::new(self.platform.fonts());
-                let presenter = Rc::new(RefCell::new(Presenter::new(
-                    window_id,
-                    self.font_cache.clone(),
-                    text_layout_cache,
-                    self.assets.clone(),
-                    self,
-                )));
-
-                {
-                    let mut app = self.upgrade();
-                    let presenter = presenter.clone();
-                    window.on_event(Box::new(move |event| {
-                        app.update(|ctx| {
-                            if let Event::KeyDown { keystroke, .. } = &event {
-                                if ctx
-                                    .dispatch_keystroke(
-                                        window_id,
-                                        presenter.borrow().dispatch_path(ctx.as_ref()),
-                                        keystroke,
-                                    )
-                                    .unwrap()
-                                {
-                                    return;
-                                }
-                            }
+        );
+        let text_layout_cache = TextLayoutCache::new(self.platform.fonts());
+        let presenter = Rc::new(RefCell::new(Presenter::new(
+            window_id,
+            self.font_cache.clone(),
+            text_layout_cache,
+            self.assets.clone(),
+            self,
+        )));
 
-                            let actions =
-                                presenter.borrow_mut().dispatch_event(event, ctx.as_ref());
-                            for action in actions {
-                                ctx.dispatch_action_any(
-                                    window_id,
-                                    &action.path,
-                                    action.name,
-                                    action.arg.as_ref(),
-                                );
-                            }
-                        })
-                    }));
-                }
+        {
+            let mut app = self.upgrade();
+            let presenter = presenter.clone();
+            window.on_event(Box::new(move |event| {
+                app.update(|ctx| {
+                    if let Event::KeyDown { keystroke, .. } = &event {
+                        if ctx
+                            .dispatch_keystroke(
+                                window_id,
+                                presenter.borrow().dispatch_path(ctx.as_ref()),
+                                keystroke,
+                            )
+                            .unwrap()
+                        {
+                            return;
+                        }
+                    }
 
-                {
-                    let mut app = self.upgrade();
-                    let presenter = presenter.clone();
-                    window.on_resize(Box::new(move |window| {
-                        app.update(|ctx| {
-                            let scene = presenter.borrow_mut().build_scene(
-                                window.size(),
-                                window.scale_factor(),
-                                ctx,
-                            );
-                            window.present_scene(scene);
-                        })
-                    }));
-                }
+                    let actions = presenter.borrow_mut().dispatch_event(event, ctx.as_ref());
+                    for action in actions {
+                        ctx.dispatch_action_any(
+                            window_id,
+                            &action.path,
+                            action.name,
+                            action.arg.as_ref(),
+                        );
+                    }
+                })
+            }));
+        }
 
-                {
-                    let presenter = presenter.clone();
-                    self.on_window_invalidated(window_id, move |invalidation, ctx| {
-                        let mut presenter = presenter.borrow_mut();
-                        presenter.invalidate(invalidation, ctx.as_ref());
-                        let scene =
-                            presenter.build_scene(window.size(), window.scale_factor(), ctx);
-                        window.present_scene(scene);
-                    });
-                }
+        {
+            let mut app = self.upgrade();
+            let presenter = presenter.clone();
+            window.on_resize(Box::new(move |window| {
+                app.update(|ctx| {
+                    let scene = presenter.borrow_mut().build_scene(
+                        window.size(),
+                        window.scale_factor(),
+                        ctx,
+                    );
+                    window.present_scene(scene);
+                })
+            }));
+        }
 
-                self.on_debug_elements(window_id, move |ctx| {
-                    presenter.borrow().debug_elements(ctx).unwrap()
-                });
-            }
+        {
+            let presenter = presenter.clone();
+            self.on_window_invalidated(window_id, move |invalidation, ctx| {
+                let mut presenter = presenter.borrow_mut();
+                presenter.invalidate(invalidation, ctx.as_ref());
+                let scene = presenter.build_scene(window.size(), window.scale_factor(), ctx);
+                window.present_scene(scene);
+            });
         }
+
+        self.on_debug_elements(window_id, move |ctx| {
+            presenter.borrow().debug_elements(ctx).unwrap()
+        });
     }
 
     pub fn add_view<T, F>(&mut self, window_id: usize, build_view: F) -> ViewHandle<T>

gpui/src/platform/mac/platform.rs 🔗

@@ -1,6 +1,5 @@
 use super::{BoolExt as _, Dispatcher, FontSystem, Window};
 use crate::{executor, keymap::Keystroke, platform, Event, Menu, MenuItem};
-use anyhow::Result;
 use cocoa::{
     appkit::{
         NSApplication, NSApplicationActivationPolicy::NSApplicationActivationPolicyRegular,
@@ -237,8 +236,8 @@ impl platform::Platform for MacPlatform {
         id: usize,
         options: platform::WindowOptions,
         executor: Rc<executor::Foreground>,
-    ) -> Result<Box<dyn platform::Window>> {
-        Ok(Box::new(Window::open(id, options, executor, self.fonts())?))
+    ) -> Box<dyn platform::Window> {
+        Box::new(Window::open(id, options, executor, self.fonts()))
     }
 
     fn key_window_id(&self) -> Option<usize> {

gpui/src/platform/mac/renderer.rs 🔗

@@ -9,7 +9,6 @@ use crate::{
     scene::Layer,
     Scene,
 };
-use anyhow::{anyhow, Result};
 use cocoa::foundation::NSUInteger;
 use metal::{MTLPixelFormat, MTLResourceOptions, NSRange};
 use shaders::{ToFloat2 as _, ToUchar4 as _};
@@ -41,10 +40,10 @@ impl Renderer {
         device: metal::Device,
         pixel_format: metal::MTLPixelFormat,
         fonts: Arc<dyn platform::FontSystem>,
-    ) -> Result<Self> {
+    ) -> Self {
         let library = device
             .new_library_with_data(SHADERS_METALLIB)
-            .map_err(|message| anyhow!("error building metal library: {}", message))?;
+            .expect("error building metal library");
 
         let unit_vertices = [
             (0., 0.).to_float2(),
@@ -73,7 +72,7 @@ impl Renderer {
             "quad_vertex",
             "quad_fragment",
             pixel_format,
-        )?;
+        );
         let shadow_pipeline_state = build_pipeline_state(
             &device,
             &library,
@@ -81,7 +80,7 @@ impl Renderer {
             "shadow_vertex",
             "shadow_fragment",
             pixel_format,
-        )?;
+        );
         let sprite_pipeline_state = build_pipeline_state(
             &device,
             &library,
@@ -89,7 +88,7 @@ impl Renderer {
             "sprite_vertex",
             "sprite_fragment",
             pixel_format,
-        )?;
+        );
         let path_atlas_pipeline_state = build_path_atlas_pipeline_state(
             &device,
             &library,
@@ -97,8 +96,8 @@ impl Renderer {
             "path_atlas_vertex",
             "path_atlas_fragment",
             MTLPixelFormat::R8Unorm,
-        )?;
-        Ok(Self {
+        );
+        Self {
             sprite_cache,
             path_atlases,
             quad_pipeline_state,
@@ -107,7 +106,7 @@ impl Renderer {
             path_atlas_pipeline_state,
             unit_vertices,
             instances,
-        })
+        }
     }
 
     pub fn render(
@@ -713,13 +712,13 @@ fn build_pipeline_state(
     vertex_fn_name: &str,
     fragment_fn_name: &str,
     pixel_format: metal::MTLPixelFormat,
-) -> Result<metal::RenderPipelineState> {
+) -> metal::RenderPipelineState {
     let vertex_fn = library
         .get_function(vertex_fn_name, None)
-        .map_err(|message| anyhow!("error locating vertex function: {}", message))?;
+        .expect("error locating vertex function");
     let fragment_fn = library
         .get_function(fragment_fn_name, None)
-        .map_err(|message| anyhow!("error locating fragment function: {}", message))?;
+        .expect("error locating fragment function");
 
     let descriptor = metal::RenderPipelineDescriptor::new();
     descriptor.set_label(label);
@@ -737,7 +736,7 @@ fn build_pipeline_state(
 
     device
         .new_render_pipeline_state(&descriptor)
-        .map_err(|message| anyhow!("could not create render pipeline state: {}", message))
+        .expect("could not create render pipeline state")
 }
 
 fn build_path_atlas_pipeline_state(
@@ -747,13 +746,13 @@ fn build_path_atlas_pipeline_state(
     vertex_fn_name: &str,
     fragment_fn_name: &str,
     pixel_format: metal::MTLPixelFormat,
-) -> Result<metal::RenderPipelineState> {
+) -> metal::RenderPipelineState {
     let vertex_fn = library
         .get_function(vertex_fn_name, None)
-        .map_err(|message| anyhow!("error locating vertex function: {}", message))?;
+        .expect("error locating vertex function");
     let fragment_fn = library
         .get_function(fragment_fn_name, None)
-        .map_err(|message| anyhow!("error locating fragment function: {}", message))?;
+        .expect("error locating fragment function");
 
     let descriptor = metal::RenderPipelineDescriptor::new();
     descriptor.set_label(label);
@@ -771,7 +770,7 @@ fn build_path_atlas_pipeline_state(
 
     device
         .new_render_pipeline_state(&descriptor)
-        .map_err(|message| anyhow!("could not create render pipeline state: {}", message))
+        .expect("could not create render pipeline state")
 }
 
 mod shaders {

gpui/src/platform/mac/window.rs 🔗

@@ -4,7 +4,6 @@ use crate::{
     platform::{self, Event, WindowContext},
     Scene,
 };
-use anyhow::{anyhow, Result};
 use cocoa::{
     appkit::{
         NSApplication, NSBackingStoreBuffered, NSScreen, NSView, NSViewHeightSizable,
@@ -136,7 +135,7 @@ impl Window {
         options: platform::WindowOptions,
         executor: Rc<executor::Foreground>,
         fonts: Arc<dyn platform::FontSystem>,
-    ) -> Result<Self> {
+    ) -> Self {
         const PIXEL_FORMAT: metal::MTLPixelFormat = metal::MTLPixelFormat::BGRA8Unorm;
 
         unsafe {
@@ -155,13 +154,10 @@ impl Window {
                 NSBackingStoreBuffered,
                 NO,
             );
+            assert!(!native_window.is_null());
 
-            if native_window == nil {
-                return Err(anyhow!("window returned nil from initializer"));
-            }
-
-            let device = metal::Device::system_default()
-                .ok_or_else(|| anyhow!("could not find default metal device"))?;
+            let device =
+                metal::Device::system_default().expect("could not find default metal device");
 
             let layer: id = msg_send![class!(CAMetalLayer), layer];
             let _: () = msg_send![layer, setDevice: device.as_ptr()];
@@ -177,9 +173,7 @@ impl Window {
 
             let native_view: id = msg_send![VIEW_CLASS, alloc];
             let native_view = NSView::init(native_view);
-            if native_view == nil {
-                return Err(anyhow!("view return nil from initializer"));
-            }
+            assert!(!native_view.is_null());
 
             let window = Self(Rc::new(RefCell::new(WindowState {
                 id,
@@ -189,7 +183,7 @@ impl Window {
                 synthetic_drag_counter: 0,
                 executor,
                 scene_to_render: Default::default(),
-                renderer: Renderer::new(device.clone(), PIXEL_FORMAT, fonts)?,
+                renderer: Renderer::new(device.clone(), PIXEL_FORMAT, fonts),
                 command_queue: device.new_command_queue(),
                 layer,
             })));
@@ -230,7 +224,7 @@ impl Window {
 
             pool.drain();
 
-            Ok(window)
+            window
         }
     }
 

gpui/src/platform/mod.rs 🔗

@@ -17,7 +17,6 @@ use crate::{
     text_layout::Line,
     Menu, Scene,
 };
-use anyhow::Result;
 use async_task::Runnable;
 pub use event::Event;
 use std::{any::Any, ops::Range, path::PathBuf, rc::Rc, sync::Arc};
@@ -39,7 +38,7 @@ pub trait Platform {
         id: usize,
         options: WindowOptions,
         executor: Rc<executor::Foreground>,
-    ) -> Result<Box<dyn Window>>;
+    ) -> Box<dyn Window>;
     fn key_window_id(&self) -> Option<usize>;
     fn prompt_for_paths(&self, options: PathPromptOptions) -> Option<Vec<PathBuf>>;
     fn quit(&self);

gpui/src/platform/test.rs 🔗

@@ -56,8 +56,8 @@ impl super::Platform for Platform {
         _: usize,
         options: super::WindowOptions,
         _executor: Rc<super::executor::Foreground>,
-    ) -> anyhow::Result<Box<dyn super::Window>> {
-        Ok(Box::new(Window::new(options.bounds.size())))
+    ) -> Box<dyn super::Window> {
+        Box::new(Window::new(options.bounds.size()))
     }
 
     fn key_window_id(&self) -> Option<usize> {