diff --git a/crates/eframe/src/native/glow_integration.rs b/crates/eframe/src/native/glow_integration.rs index cb53609ee..451aa0d82 100644 --- a/crates/eframe/src/native/glow_integration.rs +++ b/crates/eframe/src/native/glow_integration.rs @@ -31,16 +31,17 @@ use egui::{ }; #[cfg(feature = "accesskit")] use egui_winit::accesskit_winit; - -use crate::{ - App, AppCreator, CreationContext, NativeOptions, Result, Storage, - native::{epi_integration::EpiIntegration, winit_integration::is_invisible_or_minimized}, -}; +use log::warn; use super::{ epi_integration, event_loop_context, winit_integration::{EventResult, UserEvent, WinitApp, create_egui_context}, }; +use crate::epaint::textures::TexturesDelta; +use crate::{ + App, AppCreator, CreationContext, NativeOptions, Result, Storage, + native::{epi_integration::EpiIntegration, winit_integration::is_invisible_or_minimized}, +}; // ---------------------------------------------------------------------------- // Types: @@ -73,6 +74,16 @@ struct GlowWinitRunning<'app> { // NOTE: one painter shared by all viewports. painter: Rc>, + + /// Any not yet applied deltas for this app. + pending_deltas: TexturesDelta, +} + +impl Drop for GlowWinitRunning<'_> { + fn drop(&mut self) { + // Avoid debug panic when dropping unapplied deltas on teardown + self.pending_deltas.clear(); + } } /// This struct will contain both persistent and temporary glutin state. @@ -114,6 +125,9 @@ struct Viewport { info: ViewportInfo, actions_requested: Vec, + /// Any not yet applied deltas for this viewport. + pending_delta: TexturesDelta, + /// The user-callback that shows the ui. /// None for immediate viewports. viewport_ui_cb: Option>, @@ -125,6 +139,13 @@ struct Viewport { egui_winit: Option, } +impl Drop for Viewport { + fn drop(&mut self) { + // Avoid debug panic when dropping unapplied deltas on teardown + self.pending_delta.clear(); + } +} + // ---------------------------------------------------------------------------- impl<'app> GlowWinitApp<'app> { @@ -353,6 +374,7 @@ impl<'app> GlowWinitApp<'app> { app, glutin, painter, + pending_deltas: Default::default(), })) } } @@ -653,6 +675,7 @@ impl GlowWinitRunning<'_> { app, glutin, painter, + pending_deltas, .. } = self; @@ -666,6 +689,7 @@ impl GlowWinitRunning<'_> { pixels_per_point, viewport_output, } = full_output; + pending_deltas.append(textures_delta); glutin.remove_viewports_not_in(&viewport_output); @@ -687,30 +711,28 @@ impl GlowWinitRunning<'_> { egui_winit.handle_platform_output_with_event_loop(&window, event_loop, platform_output); - // Upload textures even when not visible: the atlas dirty region is already - // consumed, so dropping the delta would desync the font texture. - let has_texture_updates = !textures_delta.set.is_empty() || !textures_delta.free.is_empty(); - if is_visible || has_texture_updates { - // We may need to switch contexts again, because of immediate viewports: - frame_timer.pause(); - change_gl_context(current_gl_context, not_current_gl_context, gl_surface); - frame_timer.resume(); - } - - for (id, image_delta) in &textures_delta.set { - painter.set_texture(*id, image_delta); - } - if is_visible { let clipped_primitives = integration.egui_ctx.tessellate(shapes, pixels_per_point); + { + // We may need to switch contexts again, because of immediate viewports: + frame_timer.pause(); + change_gl_context(current_gl_context, not_current_gl_context, gl_surface); + frame_timer.resume(); + } + let screen_size_in_pixels: [u32; 2] = window.inner_size().into(); if !clear_before_update { painter.clear(screen_size_in_pixels, clear_color); } - painter.paint_primitives(screen_size_in_pixels, pixels_per_point, &clipped_primitives); + painter.paint_and_update_textures( + screen_size_in_pixels, + pixels_per_point, + &clipped_primitives, + pending_deltas, + ); { for action in viewport.actions_requested.drain(..) { @@ -772,11 +794,6 @@ impl GlowWinitRunning<'_> { } } - // Free textures *after* painting, since they may still be used in the frame we just drew. - for id in &textures_delta.free { - painter.free_texture(*id); - } - glutin.handle_viewport_output(event_loop, &integration.egui_ctx, &viewport_output); integration.report_frame_time(frame_timer.total_time_sec()); // don't count auto-save time as part of regular frame time @@ -1120,6 +1137,7 @@ impl GlutinWindowContext { deferred_commands: vec![], info: viewport_info, actions_requested: Default::default(), + pending_delta: Default::default(), viewport_ui_cb: None, gl_surface: None, window: window.map(Arc::new), @@ -1436,6 +1454,7 @@ fn initialize_or_update_viewport( deferred_commands: vec![], info: Default::default(), actions_requested: Default::default(), + pending_delta: Default::default(), viewport_ui_cb, window: None, egui_winit: None, @@ -1584,8 +1603,10 @@ fn render_immediate_viewport( } = &mut *glutin; let Some(viewport) = viewports.get_mut(&viewport_id) else { + warn!("Viewport disappeared unexpectedly!"); return; }; + viewport.pending_delta.append(textures_delta); viewport.info.events.clear(); // they should have been processed @@ -1621,7 +1642,7 @@ fn render_immediate_viewport( screen_size_in_pixels, pixels_per_point, &clipped_primitives, - &textures_delta, + &mut viewport.pending_delta, ); { diff --git a/crates/eframe/src/native/wgpu_integration.rs b/crates/eframe/src/native/wgpu_integration.rs index 918b8419f..e01b4d9a3 100644 --- a/crates/eframe/src/native/wgpu_integration.rs +++ b/crates/eframe/src/native/wgpu_integration.rs @@ -17,12 +17,13 @@ use winit::{ use ahash::HashMap; use egui::{ - DeferredViewportUiCallback, FullOutput, ImmediateViewport, OrderedViewportIdMap, + DeferredViewportUiCallback, FullOutput, ImmediateViewport, OrderedViewportIdMap, TexturesDelta, ViewportBuilder, ViewportClass, ViewportId, ViewportIdPair, ViewportIdSet, ViewportInfo, ViewportOutput, }; #[cfg(feature = "accesskit")] use egui_winit::accesskit_winit; +use log::warn; use winit_integration::UserEvent; use crate::{ @@ -65,6 +66,15 @@ struct WgpuWinitRunning<'app> { /// Wrapped in an `Rc>` so it can be re-entrantly shared via a weak-pointer. shared: Rc>, + + pending_deltas: TexturesDelta, +} + +impl Drop for WgpuWinitRunning<'_> { + fn drop(&mut self) { + // Avoid debug panic when dropping unapplied deltas on teardown + self.pending_deltas.clear(); + } } /// Everything needed by the immediate viewport renderer.\ @@ -91,6 +101,9 @@ pub struct Viewport { info: ViewportInfo, actions_requested: Vec, + /// Any not yet applied deltas for this viewport. + pending_delta: TexturesDelta, + /// `None` for sync viewports. viewport_ui_cb: Option>, @@ -102,6 +115,13 @@ pub struct Viewport { egui_winit: Option, } +impl Drop for Viewport { + fn drop(&mut self) { + // Avoid debug panic when dropping unapplied deltas on teardown + self.pending_delta.clear(); + } +} + // ---------------------------------------------------------------------------- impl<'app> WgpuWinitApp<'app> { @@ -328,6 +348,7 @@ impl<'app> WgpuWinitApp<'app> { viewport_ui_cb: None, window: Some(window), egui_winit: Some(egui_winit), + pending_delta: Default::default(), }, ); @@ -358,6 +379,7 @@ impl<'app> WgpuWinitApp<'app> { integration, app, shared, + pending_deltas: Default::default(), })) } } @@ -596,6 +618,7 @@ impl WgpuWinitRunning<'_> { app, integration, shared, + pending_deltas, } = self; let mut frame_timer = crate::stopwatch::Stopwatch::new(); @@ -699,6 +722,8 @@ impl WgpuWinitRunning<'_> { viewport_output, } = full_output; + pending_deltas.append(textures_delta); + remove_viewports_not_in(viewports, painter, viewport_from_window, &viewport_output); let Some(viewport) = viewports.get_mut(&viewport_id) else { @@ -735,7 +760,7 @@ impl WgpuWinitRunning<'_> { pixels_per_point, app.clear_color(&egui_ctx.global_style().visuals), &clipped_primitives, - &textures_delta, + pending_deltas, screenshot_commands, window, ); @@ -1125,8 +1150,11 @@ fn render_immediate_viewport( } = &mut *shared_mut; let Some(viewport) = viewports.get_mut(&ids.this) else { + warn!("Viewport disappeared unexpectedly!"); return; }; + viewport.pending_delta.append(textures_delta); + viewport.info.events.clear(); // they should have been processed let (Some(egui_winit), Some(window)) = (&mut viewport.egui_winit, &viewport.window) else { return; @@ -1149,7 +1177,7 @@ fn render_immediate_viewport( pixels_per_point, [0.0, 0.0, 0.0, 0.0], &clipped_primitives, - &textures_delta, + &mut viewport.pending_delta, vec![], window, ); @@ -1268,6 +1296,7 @@ fn initialize_or_update_viewport<'a>( viewport_ui_cb, window: None, egui_winit: None, + pending_delta: Default::default(), }) } diff --git a/crates/eframe/src/web/app_runner.rs b/crates/eframe/src/web/app_runner.rs index e259e4019..f913e9b6d 100644 --- a/crates/eframe/src/web/app_runner.rs +++ b/crates/eframe/src/web/app_runner.rs @@ -324,7 +324,6 @@ impl AppRunner { /// Paint the results of the last call to [`Self::logic`]. pub fn paint(&mut self) { - let textures_delta = std::mem::take(&mut self.textures_delta); let clipped_primitives = std::mem::take(&mut self.clipped_primitives); if let Some(clipped_primitives) = clipped_primitives { @@ -347,7 +346,7 @@ impl AppRunner { self.app.clear_color(&self.egui_ctx.global_style().visuals), &clipped_primitives, self.egui_ctx.pixels_per_point(), - &textures_delta, + &mut self.textures_delta, screenshot_commands, ) { log::error!("Failed to paint: {}", super::string_from_js_value(&err)); diff --git a/crates/eframe/src/web/web_painter.rs b/crates/eframe/src/web/web_painter.rs index fe751bf16..69f60e271 100644 --- a/crates/eframe/src/web/web_painter.rs +++ b/crates/eframe/src/web/web_painter.rs @@ -24,7 +24,7 @@ pub(crate) trait WebPainter { clear_color: [f32; 4], clipped_primitives: &[egui::ClippedPrimitive], pixels_per_point: f32, - textures_delta: &egui::TexturesDelta, + textures_delta: &mut egui::TexturesDelta, capture: Vec, ) -> Result<(), JsValue>; diff --git a/crates/eframe/src/web/web_painter_glow.rs b/crates/eframe/src/web/web_painter_glow.rs index c9b846d50..04d48210e 100644 --- a/crates/eframe/src/web/web_painter_glow.rs +++ b/crates/eframe/src/web/web_painter_glow.rs @@ -61,13 +61,16 @@ impl WebPainter for WebPainterGlow { clear_color: [f32; 4], clipped_primitives: &[egui::ClippedPrimitive], pixels_per_point: f32, - textures_delta: &egui::TexturesDelta, + textures_delta: &mut egui::TexturesDelta, capture: Vec, ) -> Result<(), JsValue> { let canvas_dimension = [self.canvas.width(), self.canvas.height()]; - for (id, image_delta) in &textures_delta.set { - self.painter.set_texture(*id, image_delta); + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for (id, image_deltas) in textures_delta.set.drain() { + for image_delta in image_deltas { + self.painter.set_texture(id, &image_delta); + } } egui_glow::painter::clear(self.painter.gl(), canvas_dimension, clear_color); @@ -79,7 +82,8 @@ impl WebPainter for WebPainterGlow { self.screenshots.push((image, capture)); } - for &id in &textures_delta.free { + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for id in textures_delta.free.drain() { self.painter.free_texture(id); } diff --git a/crates/eframe/src/web/web_painter_wgpu.rs b/crates/eframe/src/web/web_painter_wgpu.rs index 0665d99f9..273675475 100644 --- a/crates/eframe/src/web/web_painter_wgpu.rs +++ b/crates/eframe/src/web/web_painter_wgpu.rs @@ -164,7 +164,7 @@ impl WebPainter for WebPainterWgpu { clear_color: [f32; 4], clipped_primitives: &[egui::ClippedPrimitive], pixels_per_point: f32, - textures_delta: &egui::TexturesDelta, + textures_delta: &mut egui::TexturesDelta, capture_data: Vec, ) -> Result<(), JsValue> { let capture = !capture_data.is_empty(); @@ -210,13 +210,16 @@ impl WebPainter for WebPainterWgpu { let user_cmd_bufs = { let mut renderer = render_state.renderer.write(); - for (id, image_delta) in &textures_delta.set { - renderer.update_texture( - &render_state.device, - &render_state.queue, - *id, - image_delta, - ); + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for (id, image_deltas) in textures_delta.set.drain() { + for image_delta in image_deltas { + renderer.update_texture( + &render_state.device, + &render_state.queue, + id, + &image_delta, + ); + } } renderer.update_buffers( @@ -388,8 +391,9 @@ impl WebPainter for WebPainterWgpu { // However, once we called `wgpu::Queue::submit`, it is up for wgpu to determine how long the underlying gpu resource has to live. { let mut renderer = render_state.renderer.write(); - for id in &textures_delta.free { - renderer.free_texture(id); + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for id in textures_delta.free.drain() { + renderer.free_texture(&id); } } diff --git a/crates/egui-wgpu/src/winit.rs b/crates/egui-wgpu/src/winit.rs index 1abf2d2fa..62b5d9197 100644 --- a/crates/egui-wgpu/src/winit.rs +++ b/crates/egui-wgpu/src/winit.rs @@ -478,7 +478,7 @@ impl Painter { pixels_per_point: f32, clear_color: [f32; 4], clipped_primitives: &[epaint::ClippedPrimitive], - textures_delta: &epaint::textures::TexturesDelta, + textures_delta: &mut epaint::textures::TexturesDelta, capture_data: Vec, window: &Arc, ) -> f32 { @@ -545,21 +545,6 @@ impl Painter { commands_submitted: false, }; - { - // Upload textures before the surface-dependent early-returns below: - // uploads only need the device + queue, and the atlas dirty region is - // already consumed, so dropping the delta would desync the font texture. - let mut renderer = render_state.renderer.write(); - for (id, image_delta) in &textures_delta.set { - renderer.update_texture( - &render_state.device, - &render_state.queue, - *id, - image_delta, - ); - } - } - let Some(surface_state) = self.surfaces.get_mut(&viewport_id) else { return vsync_sec; }; @@ -579,6 +564,18 @@ impl Painter { let user_cmd_bufs = { let mut renderer = render_state.renderer.write(); + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for (id, image_deltas) in textures_delta.set.drain() { + for image_delta in image_deltas { + renderer.update_texture( + &render_state.device, + &render_state.queue, + id, + &image_delta, + ); + } + } + renderer.update_buffers( &render_state.device, &render_state.queue, @@ -742,8 +739,9 @@ impl Painter { // However, once we called `wgpu::Queue::submit`, it is up for wgpu to determine how long the underlying gpu resource has to live. { let mut renderer = render_state.renderer.write(); - for id in &textures_delta.free { - renderer.free_texture(id); + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for id in textures_delta.free.drain() { + renderer.free_texture(&id); } } diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index bfd6410bd..78f6ead11 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -775,6 +775,7 @@ impl Context { /// ui.label("Hello egui!"); /// }); /// // handle full_output + /// # full_output.drop_without_applying_deltas(); /// ``` #[must_use] pub fn run_ui(&self, new_input: RawInput, mut run_ui: impl FnMut(&mut Ui)) -> FullOutput { @@ -892,6 +893,7 @@ impl Context { /// /// let full_output = ctx.end_pass(); /// // handle full_output + /// # full_output.drop_without_applying_deltas(); /// ``` pub fn begin_pass(&self, mut new_input: RawInput) { profiling::function_scope!(); @@ -4296,6 +4298,7 @@ mod test { assert_eq!(num_calls, 1); assert_eq!(output.platform_output.num_completed_passes, 1); assert!(!output.platform_output.requested_discard()); + output.drop_without_applying_deltas(); } // A single call, with a denied request to discard: @@ -4321,6 +4324,7 @@ mod test { .reason, "test" ); + output.drop_without_applying_deltas(); } } @@ -4341,6 +4345,7 @@ mod test { assert_eq!(num_calls, 1); assert_eq!(output.platform_output.num_completed_passes, 1); assert!(!output.platform_output.requested_discard()); + output.drop_without_applying_deltas(); } // Request discard once: @@ -4363,6 +4368,7 @@ mod test { !output.platform_output.requested_discard(), "The request should have been cleared when fulfilled" ); + output.drop_without_applying_deltas(); } // Request discard twice: @@ -4387,6 +4393,7 @@ mod test { output.platform_output.requested_discard(), "The unfulfilled request should be reported" ); + output.drop_without_applying_deltas(); } } @@ -4415,6 +4422,7 @@ mod test { !output.platform_output.requested_discard(), "The request should have been cleared when fulfilled" ); + output.drop_without_applying_deltas(); } } } diff --git a/crates/egui/src/data/output.rs b/crates/egui/src/data/output.rs index 5808ccabc..dc55d4712 100644 --- a/crates/egui/src/data/output.rs +++ b/crates/egui/src/data/output.rs @@ -16,7 +16,7 @@ pub struct FullOutput { /// Texture changes since last frame (including the font texture). /// - /// The backend needs to apply [`crate::TexturesDelta::set`] _before_ painting, + /// The backend needs to apply [`crate::TexturesDelta::push`] _before_ painting, /// and free any texture in [`crate::TexturesDelta::free`] _after_ painting. /// /// It is assumed that all egui viewports share the same painter and texture namespace. @@ -68,6 +68,12 @@ impl FullOutput { } } } + + /// [`epaint::textures::TexturesDelta`] will panic when dropped with still unapplied deltas, + /// this is a helper to clear the deltas. + pub fn drop_without_applying_deltas(mut self) { + self.textures_delta.clear(); + } } /// Information about text being edited. diff --git a/crates/egui/src/lib.rs b/crates/egui/src/lib.rs index f0d4698e3..88de74b49 100644 --- a/crates/egui/src/lib.rs +++ b/crates/egui/src/lib.rs @@ -673,18 +673,20 @@ pub enum WidgetType { pub fn __run_test_ctx(mut run_ui: impl FnMut(&Context)) { let ctx = Context::default(); ctx.set_fonts(FontDefinitions::empty()); // prevent fonts from being loaded (save CPU time) - let _ = ctx.run_ui(Default::default(), |ui| { + let output = ctx.run_ui(Default::default(), |ui| { run_ui(ui.ctx()); }); + output.drop_without_applying_deltas(); } /// For use in tests; especially doctests. pub fn __run_test_ui(mut add_contents: impl FnMut(&mut Ui)) { let ctx = Context::default(); ctx.set_fonts(FontDefinitions::empty()); // prevent fonts from being loaded (save CPU time) - let _ = ctx.run_ui(Default::default(), |ui| { + let output = ctx.run_ui(Default::default(), |ui| { add_contents(ui); }); + output.drop_without_applying_deltas(); } pub fn accesskit_root_id() -> Id { diff --git a/crates/egui/src/text_selection/label_text_selection.rs b/crates/egui/src/text_selection/label_text_selection.rs index a49579eee..80cc90c8a 100644 --- a/crates/egui/src/text_selection/label_text_selection.rs +++ b/crates/egui/src/text_selection/label_text_selection.rs @@ -773,7 +773,7 @@ mod tests { .or_default() .selection = Some(test_selection()); - let _ = ctx.run_ui(RawInput::default(), |_| {}); + let output = ctx.run_ui(RawInput::default(), |_| {}); assert!( plugin .lock() @@ -782,11 +782,13 @@ mod tests { .is_some_and(ViewportLabelSelectionState::has_selection), "a pass in another viewport must not clear the child viewport selection" ); + output.drop_without_applying_deltas(); - let _ = ctx.run_ui(child_viewport_input(child_viewport_id), |_| {}); + let output = ctx.run_ui(child_viewport_input(child_viewport_id), |_| {}); assert!( !plugin.lock().has_selection(), "the selection must be cleared when its labels disappear from the same viewport" ); + output.drop_without_applying_deltas(); } } diff --git a/crates/egui/src/viewport.rs b/crates/egui/src/viewport.rs index 1b1e64fe1..962b065a3 100644 --- a/crates/egui/src/viewport.rs +++ b/crates/egui/src/viewport.rs @@ -71,9 +71,8 @@ use std::sync::Arc; -use epaint::{Pos2, Vec2}; - use crate::{AsId, Context, Id, Ui}; +use epaint::{Pos2, Vec2}; // ---------------------------------------------------------------------------- diff --git a/crates/egui_demo_lib/benches/benchmark.rs b/crates/egui_demo_lib/benches/benchmark.rs index d9186b80a..36accead3 100644 --- a/crates/egui_demo_lib/benches/benchmark.rs +++ b/crates/egui_demo_lib/benches/benchmark.rs @@ -28,18 +28,21 @@ pub fn criterion_benchmark(c: &mut Criterion) { // The most end-to-end benchmark. c.bench_function("demo_with_tessellate__realistic", |b| { b.iter(|| { - let full_output = ctx.run_ui(RawInput::default(), |ui| { + let mut full_output = ctx.run_ui(RawInput::default(), |ui| { demo_windows.ui(ui); }); - ctx.tessellate(full_output.shapes, full_output.pixels_per_point) + ctx.tessellate(full_output.shapes, full_output.pixels_per_point); + + full_output.textures_delta.clear(); // Don't panic on drop with unapplied deltas }); }); c.bench_function("demo_no_tessellate", |b| { b.iter(|| { - ctx.run_ui(RawInput::default(), |ui| { + let output = ctx.run_ui(RawInput::default(), |ui| { demo_windows.ui(ui); - }) + }); + output.drop_without_applying_deltas(); }); }); @@ -49,6 +52,7 @@ pub fn criterion_benchmark(c: &mut Criterion) { c.bench_function("demo_only_tessellate", |b| { b.iter(|| ctx.tessellate(full_output.shapes.clone(), full_output.pixels_per_point)); }); + full_output.drop_without_applying_deltas(); } if false { @@ -66,7 +70,7 @@ pub fn criterion_benchmark(c: &mut Criterion) { { let ctx = egui::Context::default(); - let _ = ctx.run_ui(RawInput::default(), |ui| { + let output = ctx.run_ui(RawInput::default(), |ui| { c.bench_function("label &str", |b| { b.iter_batched_ref( || create_benchmark_ui(ui), @@ -86,11 +90,12 @@ pub fn criterion_benchmark(c: &mut Criterion) { ); }); }); + output.drop_without_applying_deltas(); } { let ctx = egui::Context::default(); - let _ = ctx.run_ui(RawInput::default(), |ui| { + let output = ctx.run_ui(RawInput::default(), |ui| { let mut group = c.benchmark_group("button"); // To ensure we have a valid image, let's use the font texture. The size @@ -134,6 +139,8 @@ pub fn criterion_benchmark(c: &mut Criterion) { ); }); }); + + output.drop_without_applying_deltas(); } { diff --git a/crates/egui_demo_lib/src/lib.rs b/crates/egui_demo_lib/src/lib.rs index 3d715f3c0..45f4640bd 100644 --- a/crates/egui_demo_lib/src/lib.rs +++ b/crates/egui_demo_lib/src/lib.rs @@ -72,11 +72,12 @@ fn test_egui_e2e() { const NUM_FRAMES: usize = 5; for _ in 0..NUM_FRAMES { - let full_output = ctx.run_ui(raw_input.clone(), |ui| { + let mut full_output = ctx.run_ui(raw_input.clone(), |ui| { demo_windows.ui(ui); }); let clipped_primitives = ctx.tessellate(full_output.shapes, full_output.pixels_per_point); assert!(!clipped_primitives.is_empty()); + full_output.textures_delta.clear(); // Don't panic on drop with unapplied deltas } } @@ -91,7 +92,7 @@ fn test_egui_zero_window_size() { const NUM_FRAMES: usize = 5; for _ in 0..NUM_FRAMES { - let full_output = ctx.run_ui(raw_input.clone(), |ui| { + let mut full_output = ctx.run_ui(raw_input.clone(), |ui| { demo_windows.ui(ui); }); let clipped_primitives = ctx.tessellate(full_output.shapes, full_output.pixels_per_point); @@ -100,6 +101,7 @@ fn test_egui_zero_window_size() { "There should be nothing to show, has at least one primitive with clip_rect: {:?}", clipped_primitives[0].clip_rect ); + full_output.textures_delta.clear(); // Don't panic on drop with unapplied deltas } } diff --git a/crates/egui_glow/src/painter.rs b/crates/egui_glow/src/painter.rs index 97588ede8..004f5d58c 100644 --- a/crates/egui_glow/src/painter.rs +++ b/crates/egui_glow/src/painter.rs @@ -358,17 +358,21 @@ impl Painter { screen_size_px: [u32; 2], pixels_per_point: f32, clipped_primitives: &[egui::ClippedPrimitive], - textures_delta: &egui::TexturesDelta, + textures_delta: &mut egui::TexturesDelta, ) { profiling::function_scope!(); - for (id, image_delta) in &textures_delta.set { - self.set_texture(*id, image_delta); + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for (id, image_deltas) in textures_delta.set.drain() { + for image_delta in image_deltas { + self.set_texture(id, &image_delta); + } } self.paint_primitives(screen_size_px, pixels_per_point, clipped_primitives); - for &id in &textures_delta.free { + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for id in textures_delta.free.drain() { self.free_texture(id); } } diff --git a/crates/egui_glow/src/winit.rs b/crates/egui_glow/src/winit.rs index 6ed67ae8e..9d2b71310 100644 --- a/crates/egui_glow/src/winit.rs +++ b/crates/egui_glow/src/winit.rs @@ -107,8 +107,11 @@ impl EguiGlow { let shapes = std::mem::take(&mut self.shapes); let mut textures_delta = std::mem::take(&mut self.textures_delta); - for (id, image_delta) in textures_delta.set { - self.painter.set_texture(id, &image_delta); + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for (id, image_deltas) in textures_delta.set.drain() { + for image_delta in image_deltas { + self.painter.set_texture(id, &image_delta); + } } let pixels_per_point = self.pixels_per_point; @@ -117,7 +120,8 @@ impl EguiGlow { self.painter .paint_primitives(dimensions, pixels_per_point, &clipped_primitives); - for id in textures_delta.free.drain(..) { + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for id in textures_delta.free.drain() { self.painter.free_texture(id); } } diff --git a/crates/egui_kittest/src/lib.rs b/crates/egui_kittest/src/lib.rs index 6873be579..2a8575ef1 100644 --- a/crates/egui_kittest/src/lib.rs +++ b/crates/egui_kittest/src/lib.rs @@ -147,7 +147,7 @@ impl<'a, State> Harness<'a, State> { response = app.run(ui, &mut state, false); }); - renderer.handle_delta(&output.textures_delta); + renderer.handle_delta(&mut output.textures_delta); let mut harness = Self { app, @@ -269,7 +269,7 @@ impl<'a, State> Harness<'a, State> { .take() .expect("AccessKit was disabled"), ); - self.renderer.handle_delta(&output.textures_delta); + self.renderer.handle_delta(&mut output.textures_delta); self.output = output; self.handle_viewport_commands(); diff --git a/crates/egui_kittest/src/renderer.rs b/crates/egui_kittest/src/renderer.rs index 0806c4ead..3bffb3844 100644 --- a/crates/egui_kittest/src/renderer.rs +++ b/crates/egui_kittest/src/renderer.rs @@ -1,4 +1,5 @@ use egui::TexturesDelta; +use std::mem; pub trait TestRenderer { /// We use this to pass the glow / wgpu render state to [`eframe::Frame`]. @@ -6,7 +7,7 @@ pub trait TestRenderer { fn setup_eframe(&self, _cc: &mut eframe::CreationContext<'_>, _frame: &mut eframe::Frame) {} /// Handle a [`TexturesDelta`] by updating the renderer's textures. - fn handle_delta(&mut self, delta: &TexturesDelta); + fn handle_delta(&mut self, delta: &mut TexturesDelta); /// Render the [`crate::Harness`] and return the resulting image. /// @@ -25,7 +26,7 @@ pub trait TestRenderer { /// By default, this will create a wgpu renderer if the wgpu feature is enabled. pub enum LazyRenderer { Uninitialized { - texture_ops: Vec, + textures_delta: TexturesDelta, builder: Option Box>>, }, Initialized { @@ -39,7 +40,7 @@ impl Default for LazyRenderer { return Self::new(crate::wgpu::WgpuTestRenderer::new); #[cfg(not(feature = "wgpu"))] return Self::Uninitialized { - texture_ops: Vec::new(), + textures_delta: Vec::new(), builder: None, }; } @@ -48,16 +49,19 @@ impl Default for LazyRenderer { impl LazyRenderer { pub fn new(create_renderer: impl FnOnce() -> T + 'static) -> Self { Self::Uninitialized { - texture_ops: Vec::new(), + textures_delta: Default::default(), builder: Some(Box::new(move || Box::new(create_renderer()))), } } } impl TestRenderer for LazyRenderer { - fn handle_delta(&mut self, delta: &TexturesDelta) { + fn handle_delta(&mut self, delta: &mut TexturesDelta) { match self { - Self::Uninitialized { texture_ops, .. } => texture_ops.push(delta.clone()), + Self::Uninitialized { + textures_delta: texture_ops, + .. + } => texture_ops.append(mem::take(delta)), Self::Initialized { renderer } => renderer.handle_delta(delta), } } @@ -70,15 +74,15 @@ impl TestRenderer for LazyRenderer { ) -> Result { match self { Self::Uninitialized { - texture_ops, + textures_delta, builder: build, } => { let mut renderer = build.take().ok_or({ "No default renderer available. \ Enable the wgpu feature or set one via HarnessBuilder::renderer" })?(); - for delta in texture_ops.drain(..) { - renderer.handle_delta(&delta); + if !textures_delta.is_empty() { + renderer.handle_delta(textures_delta); } let image = renderer.render(ctx, output)?; *self = Self::Initialized { renderer }; @@ -88,3 +92,14 @@ impl TestRenderer for LazyRenderer { } } } + +impl Drop for LazyRenderer { + fn drop(&mut self) { + match self { + Self::Uninitialized { textures_delta, .. } => { + textures_delta.clear(); // Don't panic when dropping unapplied deltas + } + Self::Initialized { .. } => {} + } + } +} diff --git a/crates/egui_kittest/src/wgpu.rs b/crates/egui_kittest/src/wgpu.rs index 22972eabc..e5266aead 100644 --- a/crates/egui_kittest/src/wgpu.rs +++ b/crates/egui_kittest/src/wgpu.rs @@ -137,15 +137,23 @@ impl crate::TestRenderer for WgpuTestRenderer { frame.wgpu_render_state = Some(self.render_state.clone()); } - fn handle_delta(&mut self, delta: &TexturesDelta) { + fn handle_delta(&mut self, delta: &mut TexturesDelta) { let mut renderer = self.render_state.renderer.write(); - for (id, image) in &delta.set { - renderer.update_texture( - &self.render_state.device, - &self.render_state.queue, - *id, - image, - ); + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for (id, images) in delta.set.drain() { + for image in images { + renderer.update_texture( + &self.render_state.device, + &self.render_state.queue, + id, + &image, + ); + } + } + + #[expect(clippy::iter_over_hash_type)] // Order doesn't matter here + for id in delta.free.drain() { + renderer.free_texture(&id); } } diff --git a/crates/egui_kittest/tests/accesskit.rs b/crates/egui_kittest/tests/accesskit.rs index 30dcbaf29..fea8f26da 100644 --- a/crates/egui_kittest/tests/accesskit.rs +++ b/crates/egui_kittest/tests/accesskit.rs @@ -144,7 +144,8 @@ fn accesskit_output_single_egui_frame(run_ui: impl FnMut(&mut Ui)) -> TreeUpdate ctx.global_style_mut(|style| style.animation_time = 0.0); ctx.enable_accesskit(); - let output = ctx.run_ui(RawInput::default(), run_ui); + let mut output = ctx.run_ui(RawInput::default(), run_ui); + output.textures_delta.clear(); // Don't panic on drop with unapplied deltas output .platform_output diff --git a/crates/epaint/src/textures.rs b/crates/epaint/src/textures.rs index 0944a9052..1c4104a6e 100644 --- a/crates/epaint/src/textures.rs +++ b/crates/epaint/src/textures.rs @@ -1,4 +1,7 @@ use crate::{ImageData, ImageDelta, TextureId}; +use ahash::{HashMap, HashSet}; +use smallvec::{SmallVec, smallvec}; +use std::mem; // ---------------------------------------------------------------------------- @@ -40,7 +43,7 @@ impl TextureManager { options, }); - self.delta.set.push((id, ImageDelta::full(image, options))); + self.delta.push(id, ImageDelta::full(image, options)); id } @@ -58,10 +61,8 @@ impl TextureManager { // whole update meta.size = delta.image.size(); meta.bytes_per_pixel = delta.image.bytes_per_pixel(); - // since we update the whole image, we can discard all old enqueued deltas - self.delta.set.retain(|(x, _)| x != &id); } - self.delta.set.push((id, delta)); + self.delta.push(id, delta); } else { debug_assert!(false, "Tried setting texture {id:?} which is not allocated"); } @@ -74,7 +75,7 @@ impl TextureManager { meta.retain_count -= 1; if meta.retain_count == 0 { entry.remove(); - self.delta.free.push(id); + self.delta.free(id); } } else { debug_assert!(false, "Tried freeing texture {id:?} which is not allocated"); @@ -118,6 +119,12 @@ impl TextureManager { } } +impl Drop for TextureManager { + fn drop(&mut self) { + self.delta.clear(); // Prevent a debug panic on application shutdown + } +} + /// Meta-data about an allocated texture. #[derive(Clone, Debug, PartialEq, Eq)] pub struct TextureMeta { @@ -276,10 +283,10 @@ pub enum TextureWrapMode { #[must_use = "The painter must take care of this"] pub struct TexturesDelta { /// New or changed textures. Apply before painting. - pub set: Vec<(TextureId, ImageDelta)>, + pub set: HashMap>, /// Textures to free after painting. - pub free: Vec, + pub free: HashSet, } impl TexturesDelta { @@ -287,9 +294,36 @@ impl TexturesDelta { self.set.is_empty() && self.free.is_empty() } + /// Inserts a [`ImageDelta`]. + /// + /// If this [`TexturesDelta`] already contains this [`TextureId`], and this is a `whole` delta, + /// the previous deltas for this id are discarded. + pub fn push(&mut self, id: TextureId, delta: ImageDelta) { + if delta.is_whole() { + // It replaces the whole texture, fine to overwrite any previous deltas + self.set.insert(id, smallvec![delta]); + } else { + self.set.entry(id).or_default().push(delta); + } + } + + pub fn free(&mut self, id: TextureId) { + self.free.insert(id); + } + + #[expect(clippy::iter_over_hash_type)] pub fn append(&mut self, mut newer: Self) { - self.set.extend(newer.set); - self.free.append(&mut newer.free); + // Only clear previous entries on append, not on set, since within a frame a texture might + // be created and immediately removed again. + for id in &newer.free { + self.set.remove(id); + } + for (id, deltas) in newer.set.drain() { + for delta in deltas { + self.push(id, delta); + } + } + self.free.extend(mem::take(&mut newer.free)); } pub fn clear(&mut self) { @@ -298,6 +332,17 @@ impl TexturesDelta { } } +impl Drop for TexturesDelta { + fn drop(&mut self) { + debug_assert!( + self.is_empty(), + "Dropped TexturesDelta with {} unapplied deltas. Deltas need to be handled. \ + If you want to drop this intentionally call `clear` before dropping.", + self.free.len() + self.set.len() + ); + } +} + impl std::fmt::Debug for TexturesDelta { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { use std::fmt::Write as _; @@ -305,21 +350,24 @@ impl std::fmt::Debug for TexturesDelta { let mut debug_struct = f.debug_struct("TexturesDelta"); if !self.set.is_empty() { let mut string = String::new(); - for (tex_id, delta) in &self.set { - let size = delta.image.size(); - if let Some(pos) = delta.pos { - write!( - string, - "{:?} partial ([{} {}] - [{} {}]), ", - tex_id, - pos[0], - pos[1], - pos[0] + size[0], - pos[1] + size[1] - ) - .ok(); - } else { - write!(string, "{:?} full {}x{}, ", tex_id, size[0], size[1]).ok(); + #[expect(clippy::iter_over_hash_type)] + for (tex_id, deltas) in &self.set { + for delta in deltas { + let size = delta.image.size(); + if let Some(pos) = delta.pos { + write!( + string, + "{:?} partial ([{} {}] - [{} {}]), ", + tex_id, + pos[0], + pos[1], + pos[0] + size[0], + pos[1] + size[1] + ) + .ok(); + } else { + write!(string, "{:?} full {}x{}, ", tex_id, size[0], size[1]).ok(); + } } } debug_struct.field("set", &string);