Skip to content

Commit

Permalink
Fix egui-wgpu performance regression (#3580)
Browse files Browse the repository at this point in the history
Introduced in the recent multi-viewports work, we accidentally recreated
the wgpu surfaces every frame. This is now fixed.

I found this while improving the profiling of `eframe`
  • Loading branch information
emilk authored Nov 19, 2023
1 parent 960b01b commit 30ee478
Show file tree
Hide file tree
Showing 14 changed files with 374 additions and 281 deletions.
8 changes: 7 additions & 1 deletion crates/eframe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,13 @@ persistence = [
## `eframe` will call `puffin::GlobalProfiler::lock().new_frame()` for you
##
## Only enabled on native, because of the low resolution (1ms) of clocks in browsers.
puffin = ["dep:puffin", "egui/puffin", "egui_glow?/puffin", "egui-wgpu?/puffin"]
puffin = [
"dep:puffin",
"egui/puffin",
"egui_glow?/puffin",
"egui-wgpu?/puffin",
"egui-winit/puffin",
]

## Enables wayland support and fixes clipboard issue.
wayland = ["egui-winit/wayland"]
Expand Down
16 changes: 12 additions & 4 deletions crates/eframe/src/native/epi_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub fn viewport_builder<E>(

#[cfg(not(target_os = "ios"))]
if native_options.centered {
crate::profile_scope!("center");
if let Some(monitor) = event_loop.available_monitors().next() {
let monitor_size = monitor.size().to_logical::<f32>(monitor.scale_factor());
let inner_size = inner_size_points.unwrap_or(egui::Vec2 { x: 800.0, y: 600.0 });
Expand Down Expand Up @@ -76,9 +77,16 @@ pub fn apply_window_settings(
}

fn largest_monitor_point_size<E>(event_loop: &EventLoopWindowTarget<E>) -> egui::Vec2 {
crate::profile_function!();

let mut max_size = egui::Vec2::ZERO;

for monitor in event_loop.available_monitors() {
let available_monitors = {
crate::profile_scope!("available_monitors");
event_loop.available_monitors()
};

for monitor in available_monitors {
let size = monitor.size().to_logical::<f32>(monitor.scale_factor());
let size = egui::vec2(size.width, size.height);
max_size = max_size.max(size);
Expand Down Expand Up @@ -210,14 +218,14 @@ impl EpiIntegration {
self.close
}

pub fn on_event(
pub fn on_window_event(
&mut self,
app: &mut dyn epi::App,
event: &winit::event::WindowEvent<'_>,
egui_winit: &mut egui_winit::State,
viewport_id: ViewportId,
) -> EventResponse {
crate::profile_function!();
crate::profile_function!(egui_winit::short_window_event_description(event));

use winit::event::{ElementState, MouseButton, WindowEvent};

Expand Down Expand Up @@ -247,7 +255,7 @@ impl EpiIntegration {
_ => {}
}

egui_winit.on_event(&self.egui_ctx, event, viewport_id)
egui_winit.on_window_event(&self.egui_ctx, event, viewport_id)
}

pub fn pre_update(&mut self) {
Expand Down
82 changes: 46 additions & 36 deletions crates/eframe/src/native/glow_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ impl WinitApp for GlowWinitApp {
event_loop: &EventLoopWindowTarget<UserEvent>,
event: &winit::event::Event<'_, UserEvent>,
) -> Result<EventResult> {
crate::profile_function!();
crate::profile_function!(winit_integration::short_event_description(event));

Ok(match event {
winit::event::Event::Resumed => {
Expand Down Expand Up @@ -468,19 +468,20 @@ impl WinitApp for GlowWinitApp {

impl GlowWinitRunning {
fn run_ui_and_paint(&mut self, window_id: WindowId) -> EventResult {
crate::profile_function!();

let Some(viewport_id) = self
.glutin
.borrow()
.viewport_from_window
.get(&window_id)
.copied()
else {
return EventResult::Wait;
};
.glutin
.borrow()
.viewport_from_window
.get(&window_id)
.copied()
else {
return EventResult::Wait;
};

#[cfg(feature = "puffin")]
puffin::GlobalProfiler::lock().new_frame();
crate::profile_scope!("frame");

{
let glutin = self.glutin.borrow();
Expand Down Expand Up @@ -565,15 +566,22 @@ impl GlowWinitRunning {

let clipped_primitives = integration.egui_ctx.tessellate(shapes, pixels_per_point);

*current_gl_context = Some(
current_gl_context
.take()
.unwrap()
.make_not_current()
.unwrap()
.make_current(gl_surface)
.unwrap(),
);
{
// TODO: only do this if we actually have multiple viewports
crate::profile_scope!("change_gl_context");

let not_current = {
crate::profile_scope!("make_not_current");
current_gl_context
.take()
.unwrap()
.make_not_current()
.unwrap()
};

crate::profile_scope!("make_current");
*current_gl_context = Some(not_current.make_current(gl_surface).unwrap());
}

let screen_size_in_pixels: [u32; 2] = window.inner_size().into();

Expand Down Expand Up @@ -646,6 +654,8 @@ impl GlowWinitRunning {
window_id: WindowId,
event: &winit::event::WindowEvent<'_>,
) -> EventResult {
crate::profile_function!(egui_winit::short_window_event_description(event));

let viewport_id = self
.glutin
.borrow()
Expand Down Expand Up @@ -710,7 +720,7 @@ impl GlowWinitRunning {
if let Some(viewport_id) = viewport_id {
let mut glutin = self.glutin.borrow_mut();
if let Some(viewport) = glutin.viewports.get_mut(&viewport_id) {
break 'res self.integration.on_event(
break 'res self.integration.on_window_event(
self.app.as_mut(),
event,
viewport.egui_winit.as_mut().unwrap(),
Expand Down Expand Up @@ -1127,11 +1137,11 @@ impl Viewport {
/// Update the stored `ViewportInfo`.
fn update_viewport_info(&mut self) {
let Some(window) = &self.window else {
return;
};
return;
};
let Some(egui_winit) = &self.egui_winit else {
return;
};
return;
};
egui_winit.update_viewport_info(&mut self.info, window);
}
}
Expand Down Expand Up @@ -1245,15 +1255,15 @@ fn render_immediate_viewport(
let mut glutin = glutin.borrow_mut();

let Some(viewport) = glutin.viewports.get_mut(&ids.this) else {
return;
};
return;
};
viewport.update_viewport_info();
let Some(winit_state) = &mut viewport.egui_winit else {
return;
};
return;
};
let Some(window) = &viewport.window else {
return;
};
return;
};

let mut raw_input = winit_state.take_egui_input(window, ids);
raw_input.viewports = glutin
Expand Down Expand Up @@ -1290,15 +1300,15 @@ fn render_immediate_viewport(
} = &mut *glutin;

let Some(viewport) = viewports.get_mut(&ids.this) else {
return;
};
return;
};

let Some(winit_state) = &mut viewport.egui_winit else {
return;
};
return;
};
let (Some(window), Some(gl_surface)) = (&viewport.window, &viewport.gl_surface) else {
return;
};
return;
};

let screen_size_in_pixels: [u32; 2] = window.inner_size().into();

Expand Down
72 changes: 5 additions & 67 deletions crates/eframe/src/native/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ use winit::event_loop::{EventLoop, EventLoopBuilder};

use egui::epaint::ahash::HashMap;

use crate::{epi, native::winit_integration::EventResult, Result};
use crate::{
epi,
native::winit_integration::{short_event_description, EventResult},
Result,
};

use super::winit_integration::{UserEvent, WinitApp};

Expand Down Expand Up @@ -397,69 +401,3 @@ pub fn run_wgpu(
let wgpu_eframe = WgpuWinitApp::new(&event_loop, app_name, native_options, app_creator);
run_and_exit(event_loop, wgpu_eframe);
}

// ----------------------------------------------------------------------------

// For the puffin profiler!
#[allow(dead_code)] // Only used for profiling
fn short_event_description(event: &winit::event::Event<'_, UserEvent>) -> &'static str {
use winit::event::{DeviceEvent, Event, StartCause, WindowEvent};

match event {
Event::Suspended => "Event::Suspended",
Event::Resumed => "Event::Resumed",
Event::MainEventsCleared => "Event::MainEventsCleared",
Event::RedrawRequested(_) => "Event::RedrawRequested",
Event::RedrawEventsCleared => "Event::RedrawEventsCleared",
Event::LoopDestroyed => "Event::LoopDestroyed",
Event::UserEvent(user_event) => match user_event {
UserEvent::RequestRepaint { .. } => "UserEvent::RequestRepaint",
#[cfg(feature = "accesskit")]
UserEvent::AccessKitActionRequest(_) => "UserEvent::AccessKitActionRequest",
},
Event::DeviceEvent { event, .. } => match event {
DeviceEvent::Added { .. } => "DeviceEvent::Added",
DeviceEvent::Removed { .. } => "DeviceEvent::Removed",
DeviceEvent::MouseMotion { .. } => "DeviceEvent::MouseMotion",
DeviceEvent::MouseWheel { .. } => "DeviceEvent::MouseWheel",
DeviceEvent::Motion { .. } => "DeviceEvent::Motion",
DeviceEvent::Button { .. } => "DeviceEvent::Button",
DeviceEvent::Key { .. } => "DeviceEvent::Key",
DeviceEvent::Text { .. } => "DeviceEvent::Text",
},
Event::NewEvents(start_cause) => match start_cause {
StartCause::ResumeTimeReached { .. } => "NewEvents::ResumeTimeReached",
StartCause::WaitCancelled { .. } => "NewEvents::WaitCancelled",
StartCause::Poll => "NewEvents::Poll",
StartCause::Init => "NewEvents::Init",
},
Event::WindowEvent { event, .. } => match event {
WindowEvent::Resized { .. } => "WindowEvent::Resized",
WindowEvent::Moved { .. } => "WindowEvent::Moved",
WindowEvent::CloseRequested { .. } => "WindowEvent::CloseRequested",
WindowEvent::Destroyed { .. } => "WindowEvent::Destroyed",
WindowEvent::DroppedFile { .. } => "WindowEvent::DroppedFile",
WindowEvent::HoveredFile { .. } => "WindowEvent::HoveredFile",
WindowEvent::HoveredFileCancelled { .. } => "WindowEvent::HoveredFileCancelled",
WindowEvent::ReceivedCharacter { .. } => "WindowEvent::ReceivedCharacter",
WindowEvent::Focused { .. } => "WindowEvent::Focused",
WindowEvent::KeyboardInput { .. } => "WindowEvent::KeyboardInput",
WindowEvent::ModifiersChanged { .. } => "WindowEvent::ModifiersChanged",
WindowEvent::Ime { .. } => "WindowEvent::Ime",
WindowEvent::CursorMoved { .. } => "WindowEvent::CursorMoved",
WindowEvent::CursorEntered { .. } => "WindowEvent::CursorEntered",
WindowEvent::CursorLeft { .. } => "WindowEvent::CursorLeft",
WindowEvent::MouseWheel { .. } => "WindowEvent::MouseWheel",
WindowEvent::MouseInput { .. } => "WindowEvent::MouseInput",
WindowEvent::TouchpadMagnify { .. } => "WindowEvent::TouchpadMagnify",
WindowEvent::SmartMagnify { .. } => "WindowEvent::SmartMagnify",
WindowEvent::TouchpadRotate { .. } => "WindowEvent::TouchpadRotate",
WindowEvent::TouchpadPressure { .. } => "WindowEvent::TouchpadPressure",
WindowEvent::AxisMotion { .. } => "WindowEvent::AxisMotion",
WindowEvent::Touch { .. } => "WindowEvent::Touch",
WindowEvent::ScaleFactorChanged { .. } => "WindowEvent::ScaleFactorChanged",
WindowEvent::ThemeChanged { .. } => "WindowEvent::ThemeChanged",
WindowEvent::Occluded { .. } => "WindowEvent::Occluded",
},
}
}
Loading

0 comments on commit 30ee478

Please sign in to comment.