Remove a bunch of `unwrap()` (#4285)

The fewer unwraps, the fewer panics
This commit is contained in:
Emil Ernerfeldt 2024-03-30 19:33:19 +01:00 committed by GitHub
parent 2ee9d30d6e
commit 5a0a1e96e0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 40 additions and 29 deletions

View File

@ -7,7 +7,7 @@
#![allow(clippy::arc_with_non_send_sync)] // glow::Context was accidentally non-Sync in glow 0.13, but that will be fixed in future releases of glow: https://github.com/grovesNL/glow/commit/c4a5f7151b9b4bbb380faa06ec27415235d1bf7e #![allow(clippy::arc_with_non_send_sync)] // glow::Context was accidentally non-Sync in glow 0.13, but that will be fixed in future releases of glow: https://github.com/grovesNL/glow/commit/c4a5f7151b9b4bbb380faa06ec27415235d1bf7e
use std::{cell::RefCell, rc::Rc, sync::Arc, time::Instant}; use std::{cell::RefCell, num::NonZeroU32, rc::Rc, sync::Arc, time::Instant};
use glutin::{ use glutin::{
config::GlConfig, config::GlConfig,
@ -22,9 +22,9 @@ use winit::{
}; };
use egui::{ use egui::{
epaint::ahash::HashMap, DeferredViewportUiCallback, ImmediateViewport, NumExt as _, epaint::ahash::HashMap, DeferredViewportUiCallback, ImmediateViewport, ViewportBuilder,
ViewportBuilder, ViewportClass, ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportClass, ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportInfo,
ViewportInfo, ViewportOutput, ViewportOutput,
}; };
#[cfg(feature = "accesskit")] #[cfg(feature = "accesskit")]
use egui_winit::accesskit_winit; use egui_winit::accesskit_winit;
@ -257,7 +257,7 @@ impl GlowWinitApp {
#[cfg(feature = "accesskit")] #[cfg(feature = "accesskit")]
{ {
let event_loop_proxy = self.repaint_proxy.lock().clone(); let event_loop_proxy = self.repaint_proxy.lock().clone();
let viewport = glutin.viewports.get_mut(&ViewportId::ROOT).unwrap(); let viewport = glutin.viewports.get_mut(&ViewportId::ROOT).unwrap(); // we always have a root
if let Viewport { if let Viewport {
window: Some(window), window: Some(window),
egui_winit: Some(egui_winit), egui_winit: Some(egui_winit),
@ -548,13 +548,17 @@ impl GlowWinitRunning {
let (raw_input, viewport_ui_cb) = { let (raw_input, viewport_ui_cb) = {
let mut glutin = self.glutin.borrow_mut(); let mut glutin = self.glutin.borrow_mut();
let egui_ctx = glutin.egui_ctx.clone(); let egui_ctx = glutin.egui_ctx.clone();
let viewport = glutin.viewports.get_mut(&viewport_id).unwrap(); let Some(viewport) = glutin.viewports.get_mut(&viewport_id) else {
return EventResult::Wait;
};
let Some(window) = viewport.window.as_ref() else { let Some(window) = viewport.window.as_ref() else {
return EventResult::Wait; return EventResult::Wait;
}; };
egui_winit::update_viewport_info(&mut viewport.info, &egui_ctx, window); egui_winit::update_viewport_info(&mut viewport.info, &egui_ctx, window);
let egui_winit = viewport.egui_winit.as_mut().unwrap(); let Some(egui_winit) = viewport.egui_winit.as_mut() else {
return EventResult::Wait;
};
let mut raw_input = egui_winit.take_egui_input(window); let mut raw_input = egui_winit.take_egui_input(window);
let viewport_ui_cb = viewport.viewport_ui_cb.clone(); let viewport_ui_cb = viewport.viewport_ui_cb.clone();
@ -587,8 +591,12 @@ impl GlowWinitRunning {
.. ..
} = &mut *glutin; } = &mut *glutin;
let viewport = &viewports[&viewport_id]; let viewport = &viewports[&viewport_id];
let window = viewport.window.as_ref().unwrap(); let Some(window) = viewport.window.as_ref() else {
let gl_surface = viewport.gl_surface.as_ref().unwrap(); return EventResult::Wait;
};
let Some(gl_surface) = viewport.gl_surface.as_ref() else {
return EventResult::Wait;
};
let screen_size_in_pixels: [u32; 2] = window.inner_size().into(); let screen_size_in_pixels: [u32; 2] = window.inner_size().into();
@ -638,7 +646,9 @@ impl GlowWinitRunning {
.. ..
} = &mut *glutin; } = &mut *glutin;
let viewport = viewports.get_mut(&viewport_id).unwrap(); let Some(viewport) = viewports.get_mut(&viewport_id) else {
return EventResult::Wait;
};
viewport.info.events.clear(); // they should have been processed viewport.info.events.clear(); // they should have been processed
let window = viewport.window.clone().unwrap(); let window = viewport.window.clone().unwrap();
let gl_surface = viewport.gl_surface.as_ref().unwrap(); let gl_surface = viewport.gl_surface.as_ref().unwrap();
@ -877,7 +887,7 @@ impl GlutinWindowContext {
crate::HardwareAcceleration::Off => Some(false), crate::HardwareAcceleration::Off => Some(false),
}; };
let swap_interval = if native_options.vsync { let swap_interval = if native_options.vsync {
glutin::surface::SwapInterval::Wait(std::num::NonZeroU32::new(1).unwrap()) glutin::surface::SwapInterval::Wait(NonZeroU32::MIN)
} else { } else {
glutin::surface::SwapInterval::DontWait glutin::surface::SwapInterval::DontWait
}; };
@ -1101,8 +1111,8 @@ impl GlutinWindowContext {
// surface attributes // surface attributes
let (width_px, height_px): (u32, u32) = window.inner_size().into(); let (width_px, height_px): (u32, u32) = window.inner_size().into();
let width_px = std::num::NonZeroU32::new(width_px.at_least(1)).unwrap(); let width_px = NonZeroU32::new(width_px).unwrap_or(NonZeroU32::MIN);
let height_px = std::num::NonZeroU32::new(height_px.at_least(1)).unwrap(); let height_px = NonZeroU32::new(height_px).unwrap_or(NonZeroU32::MIN);
let surface_attributes = { let surface_attributes = {
use rwh_05::HasRawWindowHandle as _; // glutin stuck on old version of raw-window-handle use rwh_05::HasRawWindowHandle as _; // glutin stuck on old version of raw-window-handle
glutin::surface::SurfaceAttributesBuilder::<glutin::surface::WindowSurface>::new() glutin::surface::SurfaceAttributesBuilder::<glutin::surface::WindowSurface>::new()
@ -1181,8 +1191,8 @@ impl GlutinWindowContext {
} }
fn resize(&mut self, viewport_id: ViewportId, physical_size: winit::dpi::PhysicalSize<u32>) { fn resize(&mut self, viewport_id: ViewportId, physical_size: winit::dpi::PhysicalSize<u32>) {
let width_px = std::num::NonZeroU32::new(physical_size.width.at_least(1)).unwrap(); let width_px = NonZeroU32::new(physical_size.width).unwrap_or(NonZeroU32::MIN);
let height_px = std::num::NonZeroU32::new(physical_size.height.at_least(1)).unwrap(); let height_px = NonZeroU32::new(physical_size.height).unwrap_or(NonZeroU32::MIN);
if let Some(viewport) = self.viewports.get(&viewport_id) { if let Some(viewport) = self.viewports.get(&viewport_id) {
if let Some(gl_surface) = &viewport.gl_surface { if let Some(gl_surface) = &viewport.gl_surface {

View File

@ -5,7 +5,7 @@
//! There is a bunch of improvements we could do, //! There is a bunch of improvements we could do,
//! like removing a bunch of `unwraps`. //! like removing a bunch of `unwraps`.
use std::{cell::RefCell, rc::Rc, sync::Arc, time::Instant}; use std::{cell::RefCell, num::NonZeroU32, rc::Rc, sync::Arc, time::Instant};
use parking_lot::Mutex; use parking_lot::Mutex;
use raw_window_handle::{HasDisplayHandle as _, HasWindowHandle as _}; use raw_window_handle::{HasDisplayHandle as _, HasWindowHandle as _};
@ -437,13 +437,12 @@ impl WinitApp for WgpuWinitApp {
self.init_run_state(egui_ctx, event_loop, storage, window, builder)? self.init_run_state(egui_ctx, event_loop, storage, window, builder)?
}; };
EventResult::RepaintNow( let viewport = &running.shared.borrow().viewports[&ViewportId::ROOT];
running.shared.borrow().viewports[&ViewportId::ROOT] if let Some(window) = &viewport.window {
.window EventResult::RepaintNow(window.id())
.as_ref() } else {
.unwrap() EventResult::Wait
.id(), }
)
} }
winit::event::Event::Suspended => { winit::event::Event::Suspended => {
@ -615,7 +614,9 @@ impl WgpuWinitRunning {
} }
} }
let egui_winit = egui_winit.as_mut().unwrap(); let Some(egui_winit) = egui_winit.as_mut() else {
return EventResult::Wait;
};
let mut raw_input = egui_winit.take_egui_input(window); let mut raw_input = egui_winit.take_egui_input(window);
integration.pre_update(); integration.pre_update();
@ -775,7 +776,6 @@ impl WgpuWinitRunning {
// See: https://github.com/rust-windowing/winit/issues/208 // See: https://github.com/rust-windowing/winit/issues/208
// This solves an issue where the app would panic when minimizing on Windows. // This solves an issue where the app would panic when minimizing on Windows.
if let Some(viewport_id) = viewport_id { if let Some(viewport_id) = viewport_id {
use std::num::NonZeroU32;
if let (Some(width), Some(height)) = ( if let (Some(width), Some(height)) = (
NonZeroU32::new(physical_size.width), NonZeroU32::new(physical_size.width),
NonZeroU32::new(physical_size.height), NonZeroU32::new(physical_size.height),

View File

@ -4,6 +4,8 @@
#![allow(unsafe_code)] #![allow(unsafe_code)]
#![allow(clippy::arc_with_non_send_sync)] // glow::Context was accidentally non-Sync in glow 0.13, but that will be fixed in future releases of glow: https://github.com/grovesNL/glow/commit/c4a5f7151b9b4bbb380faa06ec27415235d1bf7e #![allow(clippy::arc_with_non_send_sync)] // glow::Context was accidentally non-Sync in glow 0.13, but that will be fixed in future releases of glow: https://github.com/grovesNL/glow/commit/c4a5f7151b9b4bbb380faa06ec27415235d1bf7e
use std::num::NonZeroU32;
use egui_winit::winit; use egui_winit::winit;
/// The majority of `GlutinWindowContext` is taken from `eframe` /// The majority of `GlutinWindowContext` is taken from `eframe`
@ -19,7 +21,6 @@ impl GlutinWindowContext {
// preferably add android support at the same time. // preferably add android support at the same time.
#[allow(unsafe_code)] #[allow(unsafe_code)]
unsafe fn new(event_loop: &winit::event_loop::EventLoopWindowTarget<UserEvent>) -> Self { unsafe fn new(event_loop: &winit::event_loop::EventLoopWindowTarget<UserEvent>) -> Self {
use egui::NumExt;
use glutin::context::NotCurrentGlContext; use glutin::context::NotCurrentGlContext;
use glutin::display::GetGlDisplay; use glutin::display::GetGlDisplay;
use glutin::display::GlDisplay; use glutin::display::GlDisplay;
@ -87,8 +88,8 @@ impl GlutinWindowContext {
.expect("failed to finalize glutin window") .expect("failed to finalize glutin window")
}); });
let (width, height): (u32, u32) = window.inner_size().into(); let (width, height): (u32, u32) = window.inner_size().into();
let width = std::num::NonZeroU32::new(width.at_least(1)).unwrap(); let width = NonZeroU32::new(width).unwrap_or(NonZeroU32::MIN);
let height = std::num::NonZeroU32::new(height.at_least(1)).unwrap(); let height = NonZeroU32::new(height).unwrap_or(NonZeroU32::MIN);
let surface_attributes = let surface_attributes =
glutin::surface::SurfaceAttributesBuilder::<glutin::surface::WindowSurface>::new() glutin::surface::SurfaceAttributesBuilder::<glutin::surface::WindowSurface>::new()
.build(window.raw_window_handle(), width, height); .build(window.raw_window_handle(), width, height);
@ -107,7 +108,7 @@ impl GlutinWindowContext {
gl_surface gl_surface
.set_swap_interval( .set_swap_interval(
&gl_context, &gl_context,
glutin::surface::SwapInterval::Wait(std::num::NonZeroU32::new(1).unwrap()), glutin::surface::SwapInterval::Wait(NonZeroU32::MIN),
) )
.unwrap(); .unwrap();