From 0c528fb862d070baa232409bdddfc1c6ee1cddec Mon Sep 17 00:00:00 2001
From: pm100 <paulmoore100@hotmail.com>
Date: Sun, 25 Aug 2024 23:55:34 -0700
Subject: [PATCH] Fix crash when changing viewport settings (#4862)

* Fixes #3959

There are two bugs racing each other here, which is why it sometimes
crashes and sometimes the app just silently exists

Bug 1
When the window is recreated a Destroyed event arrives (due to the Drop
of the old window). The code that receives this event does not look to
see if its the main viewport or a secondary one and unconditionally
closes the app. The code path for other platforms is slightly different
and does check.

I have moved the code that handles the destroy to be in the same place
and have the same behavior as the other platforms.

Bug 2

At recreate time the window and winit entries of the viewport are set to
None (forcin g them to be recreated). But the surface is still bound to
the old window, this causes the next context switch to fail. So I simply
added a viewport.gl_surface = None too,


This is my first egui PR so I hope I have not broken anything. If
nothing else I understand a little better how egui works.
---
 crates/eframe/src/native/epi_integration.rs  |  4 ---
 crates/eframe/src/native/glow_integration.rs | 13 ++++++++++
 crates/eframe/src/native/wgpu_integration.rs | 26 +++++++++++++++-----
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs
index 541dcdd34b7..11029b9e5bc 100644
--- a/crates/eframe/src/native/epi_integration.rs
+++ b/crates/eframe/src/native/epi_integration.rs
@@ -236,10 +236,6 @@ impl EpiIntegration {
         use winit::event::{ElementState, MouseButton, WindowEvent};
 
         match event {
-            WindowEvent::Destroyed => {
-                log::debug!("Received WindowEvent::Destroyed");
-                self.close = true;
-            }
             WindowEvent::MouseInput {
                 button: MouseButton::Left,
                 state: ElementState::Pressed,
diff --git a/crates/eframe/src/native/glow_integration.rs b/crates/eframe/src/native/glow_integration.rs
index 63e631532b1..87116b71227 100644
--- a/crates/eframe/src/native/glow_integration.rs
+++ b/crates/eframe/src/native/glow_integration.rs
@@ -803,6 +803,18 @@ impl GlowWinitRunning {
                 }
             }
 
+            winit::event::WindowEvent::Destroyed => {
+                log::debug!(
+                    "Received WindowEvent::Destroyed for viewport {:?}",
+                    viewport_id
+                );
+                if viewport_id == Some(ViewportId::ROOT) {
+                    return EventResult::Exit;
+                } else {
+                    return EventResult::Wait;
+                }
+            }
+
             _ => {}
         }
 
@@ -1360,6 +1372,7 @@ fn initialize_or_update_viewport(
                 );
                 viewport.window = None;
                 viewport.egui_winit = None;
+                viewport.gl_surface = None;
             }
 
             viewport.deferred_commands.append(&mut delta_commands);
diff --git a/crates/eframe/src/native/wgpu_integration.rs b/crates/eframe/src/native/wgpu_integration.rs
index 4228c52bb5c..0bcce42c638 100644
--- a/crates/eframe/src/native/wgpu_integration.rs
+++ b/crates/eframe/src/native/wgpu_integration.rs
@@ -158,6 +158,7 @@ impl WgpuWinitApp {
             ViewportClass::Root,
             self.native_options.viewport.clone(),
             None,
+            painter,
         )
         .initialize_window(event_loop, egui_ctx, viewport_from_window, painter);
     }
@@ -927,8 +928,14 @@ fn render_immediate_viewport(
             ..
         } = &mut *shared.borrow_mut();
 
-        let viewport =
-            initialize_or_update_viewport(viewports, ids, ViewportClass::Immediate, builder, None);
+        let viewport = initialize_or_update_viewport(
+            viewports,
+            ids,
+            ViewportClass::Immediate,
+            builder,
+            None,
+            painter,
+        );
         if viewport.window.is_none() {
             event_loop_context::with_current_event_loop(|event_loop| {
                 viewport.initialize_window(event_loop, egui_ctx, viewport_from_window, painter);
@@ -1051,7 +1058,7 @@ fn handle_viewport_output(
         let ids = ViewportIdPair::from_self_and_parent(viewport_id, parent);
 
         let viewport =
-            initialize_or_update_viewport(viewports, ids, class, builder, viewport_ui_cb);
+            initialize_or_update_viewport(viewports, ids, class, builder, viewport_ui_cb, painter);
 
         if let Some(window) = viewport.window.as_ref() {
             let old_inner_size = window.inner_size();
@@ -1084,13 +1091,14 @@ fn handle_viewport_output(
     remove_viewports_not_in(viewports, painter, viewport_from_window, viewport_output);
 }
 
-fn initialize_or_update_viewport(
-    viewports: &mut Viewports,
+fn initialize_or_update_viewport<'a>(
+    viewports: &'a mut Viewports,
     ids: ViewportIdPair,
     class: ViewportClass,
     mut builder: ViewportBuilder,
     viewport_ui_cb: Option<Arc<dyn Fn(&egui::Context) + Send + Sync>>,
-) -> &mut Viewport {
+    painter: &mut egui_wgpu::winit::Painter,
+) -> &'a mut Viewport {
     crate::profile_function!();
 
     if builder.icon.is_none() {
@@ -1135,6 +1143,12 @@ fn initialize_or_update_viewport(
                 );
                 viewport.window = None;
                 viewport.egui_winit = None;
+                if let Err(err) = pollster::block_on(painter.set_window(viewport.ids.this, None)) {
+                    log::error!(
+                        "when rendering viewport_id={:?}, set_window Error {err}",
+                        viewport.ids.this
+                    );
+                }
             }
 
             viewport.deferred_commands.append(&mut delta_commands);