From 1cbf3335616b1d6a5b1960281ce136a451b44458 Mon Sep 17 00:00:00 2001 From: Konkitoman Date: Mon, 11 Sep 2023 20:33:44 +0300 Subject: [PATCH] * Fix crash when spamming a viewport with other viewports The crash was causated because sync viewport, when was cleaning up inactive viewports, was cleaning his parent before he got cleaned, and his parent when tring to resume was causing the crash * Better performance because now the cleanup will be only run by the async viewports --- crates/egui/src/context.rs | 53 +++++++++++++++++++------------------- crates/egui/src/memory.rs | 7 +++-- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 40e9737da04..86e75a7bcd2 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -1540,27 +1540,9 @@ impl Context { } } - // Context Cleanup - self.write(|ctx| { - ctx.input.retain(|id, _| viewports.contains(id)); - ctx.layer_rects_prev_viewports - .retain(|id, _| viewports.contains(id)); - ctx.layer_rects_this_viewports - .retain(|id, _| viewports.contains(id)); - ctx.output.retain(|id, _| viewports.contains(id)); - ctx.frame_state.retain(|id, _| viewports.contains(id)); - ctx.graphics.retain(|id, _| viewports.contains(id)); - ctx.memory - .new_pixels_per_viewport - .retain(|id, _| viewports.contains(id)); - }); - - let repaint_after = - self.write(|ctx| ctx.repaint.end_frame(ctx.get_viewport_id(), &viewports)); let shapes = self.drain_paint_lists(); - // This is used for, - // If there are no viewport that contains the current viewpor that viewport needs to be destroyed! + // If there are no viewport that contains the current viewport that viewport needs to be destroyed! let avalibile_viewports = self.read(|ctx| { let mut avalibile_viewports = vec![ViewportId::MAIN]; for (_, id, _, _, _) in ctx.viewports.values() { @@ -1571,9 +1553,6 @@ impl Context { let viewport_id = self.get_viewport_id(); - // We should not process viewport commands when we are a sync viewport, because that will cause a deadlock or a memory race - let mut is_async = viewport_id == ViewportId::MAIN; - let mut viewports = Vec::new(); self.write(|ctx| { ctx.viewports @@ -1584,10 +1563,6 @@ impl Context { *used = false; } - if !is_async && viewport_id == *id { - is_async = render.is_some(); - } - viewports.push((*id, *parent, builder.clone(), render.clone())); (out || viewport_id != *parent) && avalibile_viewports.contains(parent) }); @@ -1598,6 +1573,7 @@ impl Context { ctx.frame_stack.pop(); ctx.frame_stack.is_empty() }); + if !is_last { let viewport_id = self.get_viewport_id(); self.write(|ctx| { @@ -1607,15 +1583,38 @@ impl Context { ctx.layer_rects_this_viewports.remove(&viewport_id).unwrap(); ctx.memory.resume_frame(viewport_id); }); + } else { + // ## Context Cleanup + self.write(|ctx| { + ctx.input.retain(|id, _| avalibile_viewports.contains(id)); + ctx.layer_rects_prev_viewports + .retain(|id, _| avalibile_viewports.contains(id)); + ctx.layer_rects_this_viewports + .retain(|id, _| avalibile_viewports.contains(id)); + ctx.output.retain(|id, _| avalibile_viewports.contains(id)); + ctx.frame_state + .retain(|id, _| avalibile_viewports.contains(id)); + ctx.graphics + .retain(|id, _| avalibile_viewports.contains(id)); + ctx.memory + .new_pixels_per_viewport + .retain(|id, _| avalibile_viewports.contains(id)); + }); } + let repaint_after = self.write(|ctx| { + ctx.repaint + .end_frame(ctx.get_viewport_id(), &avalibile_viewports) + }); + FullOutput { platform_output, repaint_after, textures_delta, shapes, viewports, - viewport_commands: if is_async { + // We should not process viewport commands when we are a sync viewport, because that will cause a deadlock for egui backend + viewport_commands: if is_last { self.write(|ctx| std::mem::take(&mut ctx.viewport_commands)) } else { Vec::new() diff --git a/crates/egui/src/memory.rs b/crates/egui/src/memory.rs index 289fd0a9b14..88d40186b96 100644 --- a/crates/egui/src/memory.rs +++ b/crates/egui/src/memory.rs @@ -564,8 +564,11 @@ impl Memory { } pub(crate) fn resume_frame(&mut self, viewport_id: ViewportId) { - self.interaction = self.interactions.remove(&viewport_id).unwrap(); - self.areas = self.viewports_areas.remove(&viewport_id).unwrap(); + self.interaction = self.interactions.remove(&viewport_id).unwrap_or_default(); + self.areas = self + .viewports_areas + .remove(&viewport_id) + .unwrap_or_default(); self.viewport_id = viewport_id; self.window_interaction = self.window_interactions.remove(&viewport_id); }