Skip to content

Commit

Permalink
Remove a bunch of unwrap() (#4285)
Browse files Browse the repository at this point in the history 
The fewer unwraps, the fewer panics
  • Loading branch information
@emilk
emilk committed Apr 2, 2024
1 parent 063098e commit 1b553c9
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 29 deletions.
40 changes: 25 additions & 15 deletions crates/eframe/src/native/glow_integration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

use std::{cell::RefCell, rc::Rc, sync::Arc, time::Instant};
use std::{cell::RefCell, num::NonZeroU32, rc::Rc, sync::Arc, time::Instant};

use glutin::{
config::GlConfig,
Expand All @@ -22,9 +22,9 @@ use winit::{
};

use egui::{
epaint::ahash::HashMap, DeferredViewportUiCallback, ImmediateViewport, NumExt as _,
ViewportBuilder, ViewportClass, ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet,
ViewportInfo, ViewportOutput,
epaint::ahash::HashMap, DeferredViewportUiCallback, ImmediateViewport, ViewportBuilder,
ViewportClass, ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportInfo,
ViewportOutput,
};
#[cfg(feature = "accesskit")]
use egui_winit::accesskit_winit;
Expand Down Expand Up @@ -252,7 +252,7 @@ impl GlowWinitApp {
#[cfg(feature = "accesskit")]
{
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 {
window: Some(window),
egui_winit: Some(egui_winit),
Expand Down Expand Up @@ -544,13 +544,17 @@ impl GlowWinitRunning {
let (raw_input, viewport_ui_cb) = {
let mut glutin = self.glutin.borrow_mut();
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 {
return EventResult::Wait;
};
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 viewport_ui_cb = viewport.viewport_ui_cb.clone();

Expand Down Expand Up @@ -583,8 +587,12 @@ impl GlowWinitRunning {
..
} = &mut *glutin;
let viewport = &viewports[&viewport_id];
let window = viewport.window.as_ref().unwrap();
let gl_surface = viewport.gl_surface.as_ref().unwrap();
let Some(window) = viewport.window.as_ref() else {
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();

Expand Down Expand Up @@ -634,7 +642,9 @@ impl GlowWinitRunning {
..
} = &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
let window = viewport.window.clone().unwrap();
let gl_surface = viewport.gl_surface.as_ref().unwrap();
Expand Down Expand Up @@ -865,7 +875,7 @@ impl GlutinWindowContext {
crate::HardwareAcceleration::Off => Some(false),
};
let swap_interval = if native_options.vsync {
glutin::surface::SwapInterval::Wait(std::num::NonZeroU32::new(1).unwrap())
glutin::surface::SwapInterval::Wait(NonZeroU32::MIN)
} else {
glutin::surface::SwapInterval::DontWait
};
Expand Down Expand Up @@ -1089,8 +1099,8 @@ impl GlutinWindowContext {

// surface attributes
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 height_px = std::num::NonZeroU32::new(height_px.at_least(1)).unwrap();
let width_px = NonZeroU32::new(width_px).unwrap_or(NonZeroU32::MIN);
let height_px = NonZeroU32::new(height_px).unwrap_or(NonZeroU32::MIN);
let surface_attributes = {
use rwh_05::HasRawWindowHandle as _; // glutin stuck on old version of raw-window-handle
glutin::surface::SurfaceAttributesBuilder::<glutin::surface::WindowSurface>::new()
Expand Down Expand Up @@ -1169,8 +1179,8 @@ impl GlutinWindowContext {
}

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 height_px = std::num::NonZeroU32::new(physical_size.height.at_least(1)).unwrap();
let width_px = NonZeroU32::new(physical_size.width).unwrap_or(NonZeroU32::MIN);
let height_px = NonZeroU32::new(physical_size.height).unwrap_or(NonZeroU32::MIN);

if let Some(viewport) = self.viewports.get(&viewport_id) {
if let Some(gl_surface) = &viewport.gl_surface {
Expand Down
20 changes: 10 additions & 10 deletions crates/eframe/src/native/wgpu_integration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//! There is a bunch of improvements we could do,
//! 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 raw_window_handle::{HasDisplayHandle as _, HasWindowHandle as _};
Expand Down Expand Up @@ -435,13 +435,12 @@ impl WinitApp for WgpuWinitApp {
self.init_run_state(egui_ctx, event_loop, storage, window, builder)?
};

EventResult::RepaintNow(
running.shared.borrow().viewports[&ViewportId::ROOT]
.window
.as_ref()
.unwrap()
.id(),
)
let viewport = &running.shared.borrow().viewports[&ViewportId::ROOT];
if let Some(window) = &viewport.window {
EventResult::RepaintNow(window.id())
} else {
EventResult::Wait
}
}

winit::event::Event::Suspended => {
Expand Down Expand Up @@ -613,7 +612,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);

integration.pre_update();
Expand Down Expand Up @@ -773,7 +774,6 @@ impl WgpuWinitRunning {
// See: https://github.com/rust-windowing/winit/issues/208
// This solves an issue where the app would panic when minimizing on Windows.
if let Some(viewport_id) = viewport_id {
use std::num::NonZeroU32;
if let (Some(width), Some(height)) = (
NonZeroU32::new(physical_size.width),
NonZeroU32::new(physical_size.height),
Expand Down
9 changes: 5 additions & 4 deletions crates/egui_glow/examples/pure_glow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#![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

use std::num::NonZeroU32;

use egui_winit::winit;

/// The majority of `GlutinWindowContext` is taken from `eframe`
Expand All @@ -19,7 +21,6 @@ impl GlutinWindowContext {
// preferably add android support at the same time.
#[allow(unsafe_code)]
unsafe fn new(event_loop: &winit::event_loop::EventLoopWindowTarget<UserEvent>) -> Self {
use egui::NumExt;
use glutin::context::NotCurrentGlContext;
use glutin::display::GetGlDisplay;
use glutin::display::GlDisplay;
Expand Down Expand Up @@ -87,8 +88,8 @@ impl GlutinWindowContext {
.expect("failed to finalize glutin window")
});
let (width, height): (u32, u32) = window.inner_size().into();
let width = std::num::NonZeroU32::new(width.at_least(1)).unwrap();
let height = std::num::NonZeroU32::new(height.at_least(1)).unwrap();
let width = NonZeroU32::new(width).unwrap_or(NonZeroU32::MIN);
let height = NonZeroU32::new(height).unwrap_or(NonZeroU32::MIN);
let surface_attributes =
glutin::surface::SurfaceAttributesBuilder::<glutin::surface::WindowSurface>::new()
.build(window.raw_window_handle(), width, height);
Expand All @@ -107,7 +108,7 @@ impl GlutinWindowContext {
gl_surface
.set_swap_interval(
&gl_context,
glutin::surface::SwapInterval::Wait(std::num::NonZeroU32::new(1).unwrap()),
glutin::surface::SwapInterval::Wait(NonZeroU32::MIN),
)
.unwrap();

Expand Down

0 comments on commit 1b553c9

Please sign in to comment.