Skip to content

Commit

Permalink
* Fix crash when spamming a viewport with other viewports
Browse files Browse the repository at this point in the history
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
  • Loading branch information
konkitoman committed Sep 11, 2023
1 parent fc90a7e commit 1cbf333
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 29 deletions.
53 changes: 26 additions & 27 deletions crates/egui/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
Expand All @@ -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)
});
Expand All @@ -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| {
Expand All @@ -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()
Expand Down
7 changes: 5 additions & 2 deletions crates/egui/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 1cbf333

Please sign in to comment.