eframe glow backend: Clear render target before calling `App::update` (#3665)

* Closes https://github.com/emilk/egui/issues/3659

Cargo.lock

@@ -975,9 +975,7 @@ name = "custom_3d_glow"
 version = "0.1.0"
 dependencies = [
  "eframe",
- "egui_glow",
  "env_logger",
- "glow 0.12.3",
 ]
 
 [[package]]
@@ -3598,6 +3596,14 @@ dependencies = [
  "winapi-util",
 ]
 
+[[package]]
+name = "test_inline_glow_paint"
+version = "0.1.0"
+dependencies = [
+ "eframe",
+ "env_logger",
+]
+
 [[package]]
 name = "test_viewports"
 version = "0.1.0"

crates/eframe/src/native/glow_integration.rs

@@ -533,6 +533,31 @@ impl GlowWinitRunning {
             (raw_input, viewport_ui_cb)
         };
 
+        {
+            // clear before we call update, so users can paint between clear-color and egui windows:
+
+            let clear_color = self
+                .app
+                .clear_color(&self.integration.egui_ctx.style().visuals);
+            let mut glutin = self.glutin.borrow_mut();
+            let GlutinWindowContext {
+                viewports,
+                current_gl_context,
+                ..
+            } = &mut *glutin;
+            let viewport = &viewports[&viewport_id];
+            let window = viewport.window.as_ref().unwrap();
+            let gl_surface = viewport.gl_surface.as_ref().unwrap();
+
+            let screen_size_in_pixels: [u32; 2] = window.inner_size().into();
+
+            change_gl_context(current_gl_context, gl_surface);
+
+            self.painter
+                .borrow()
+                .clear(screen_size_in_pixels, clear_color);
+        }
+
         // ------------------------------------------------------------
         // The update function, which could call immediate viewports,
         // so make sure we don't hold any locks here required by the immediate viewports rendeer.
@@ -579,30 +604,11 @@ impl GlowWinitRunning {
 
         let clipped_primitives = integration.egui_ctx.tessellate(shapes, pixels_per_point);
 
-        {
-            // 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());
-        }
+        // We may need to switch contexts again, because of immediate viewports:
+        change_gl_context(current_gl_context, gl_surface);
 
         let screen_size_in_pixels: [u32; 2] = window.inner_size().into();
 
-        painter.clear(
-            screen_size_in_pixels,
-            app.clear_color(&integration.egui_ctx.style().visuals),
-        );
-
         painter.paint_and_update_textures(
             screen_size_in_pixels,
             pixels_per_point,
@@ -770,6 +776,24 @@ impl GlowWinitRunning {
     }
 }
 
+fn change_gl_context(
+    current_gl_context: &mut Option<glutin::context::PossiblyCurrentContext>,
+    gl_surface: &glutin::surface::Surface<glutin::surface::WindowSurface>,
+) {
+    crate::profile_function!();
+
+    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());
+}
+
 impl GlutinWindowContext {
     #[allow(unsafe_code)]
     unsafe fn new(
@@ -1289,10 +1313,7 @@ fn render_immediate_viewport(
         let Some(viewport) = glutin.viewports.get_mut(&ids.this) else {
             return;
         };
-        let Some(egui_winit) = &mut viewport.egui_winit else {
-            return;
-        };
-        let Some(window) = &viewport.window else {
+        let (Some(egui_winit), Some(window)) = (&mut viewport.egui_winit, &viewport.window) else {
             return;
         };
         egui_winit::update_viewport_info(&mut viewport.info, egui_ctx, window);
@@ -1324,6 +1345,8 @@ fn render_immediate_viewport(
 
     // ---------------------------------------------------
 
+    let clipped_primitives = egui_ctx.tessellate(shapes, pixels_per_point);
+
     let mut glutin = glutin.borrow_mut();
 
     let GlutinWindowContext {
@@ -1335,41 +1358,49 @@ fn render_immediate_viewport(
     let Some(viewport) = viewports.get_mut(&ids.this) else {
         return;
     };
+
     viewport.info.events.clear(); // they should have been processed
 
-    let Some(winit_state) = &mut viewport.egui_winit else {
-        return;
-    };
-    let (Some(window), Some(gl_surface)) = (&viewport.window, &viewport.gl_surface) else {
+    let (Some(egui_winit), Some(window), Some(gl_surface)) = (
+        &mut viewport.egui_winit,
+        &viewport.window,
+        &viewport.gl_surface,
+    ) else {
         return;
     };
 
     let screen_size_in_pixels: [u32; 2] = window.inner_size().into();
 
-    let clipped_primitives = egui_ctx.tessellate(shapes, pixels_per_point);
-
-    let mut painter = painter.borrow_mut();
-
-    *current_gl_context = Some(
-        current_gl_context
-            .take()
-            .unwrap()
-            .make_not_current()
-            .unwrap()
-            .make_current(gl_surface)
-            .unwrap(),
-    );
+    {
+        crate::profile_function!("context-switch");
+        *current_gl_context = Some(
+            current_gl_context
+                .take()
+                .unwrap()
+                .make_not_current()
+                .unwrap()
+                .make_current(gl_surface)
+                .unwrap(),
+        );
+    }
 
     let current_gl_context = current_gl_context.as_ref().unwrap();
 
     if !gl_surface.is_current(current_gl_context) {
-        log::error!("egui::show_viewport_immediate: viewport {:?}  ({:?}) is not created in main thread, try to use wgpu!", viewport.ids.this, viewport.builder.title);
+        log::error!(
+            "egui::show_viewport_immediate: viewport {:?} ({:?}) was not created on main thread.",
+            viewport.ids.this,
+            viewport.builder.title
+        );
     }
 
-    let gl = &painter.gl().clone();
-    egui_glow::painter::clear(gl, screen_size_in_pixels, [0.0, 0.0, 0.0, 0.0]);
+    egui_glow::painter::clear(
+        painter.borrow().gl(),
+        screen_size_in_pixels,
+        [0.0, 0.0, 0.0, 0.0],
+    );
 
-    painter.paint_and_update_textures(
+    painter.borrow_mut().paint_and_update_textures(
         screen_size_in_pixels,
         pixels_per_point,
         &clipped_primitives,
@@ -1383,7 +1414,7 @@ fn render_immediate_viewport(
         }
     }
 
-    winit_state.handle_platform_output(window, egui_ctx, platform_output);
+    egui_winit.handle_platform_output(window, egui_ctx, platform_output);
 
     glutin.handle_viewport_output(egui_ctx, viewport_output);
 }

crates/eframe/src/native/wgpu_integration.rs

@@ -875,13 +875,13 @@ fn render_immediate_viewport(
             viewport.init_window(egui_ctx, viewport_from_window, painter, event_loop);
         }
 
-        let (Some(window), Some(winit_state)) = (&viewport.window, &mut viewport.egui_winit) else {
+        let (Some(window), Some(egui_winit)) = (&viewport.window, &mut viewport.egui_winit) else {
             return;
         };
         egui_winit::update_viewport_info(&mut viewport.info, egui_ctx, window);
 
-        winit_state.update_pixels_per_point(egui_ctx, window);
-        let mut input = winit_state.take_egui_input(window);
+        egui_winit.update_pixels_per_point(egui_ctx, window);
+        let mut input = egui_winit.take_egui_input(window);
         input.viewports = viewports
             .iter()
             .map(|(id, viewport)| (*id, viewport.info.clone()))
@@ -920,10 +920,7 @@ fn render_immediate_viewport(
         return;
     };
     viewport.info.events.clear(); // they should have been processed
-    let Some(winit_state) = &mut viewport.egui_winit else {
-        return;
-    };
-    let Some(window) = &viewport.window else {
+    let (Some(egui_winit), Some(window)) = (&mut viewport.egui_winit, &viewport.window) else {
         return;
     };
 
@@ -947,7 +944,7 @@ fn render_immediate_viewport(
         false,
     );
 
-    winit_state.handle_platform_output(window, &egui_ctx, platform_output);
+    egui_winit.handle_platform_output(window, &egui_ctx, platform_output);
 
     handle_viewport_output(&egui_ctx, viewport_output, viewports, *focused_viewport);
 }

examples/custom_3d_glow/Cargo.toml

@@ -12,6 +12,4 @@ publish = false
 eframe = { path = "../../crates/eframe", features = [
     "__screenshot", # __screenshot is so we can dump a screenshot using EFRAME_SCREENSHOT_TO
 ] }
-egui_glow = { path = "../../crates/egui_glow" }
 env_logger = "0.10"
-glow = "0.12"

examples/custom_3d_glow/src/main.rs

@@ -1,7 +1,7 @@
 #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] // hide console window on Windows in release
 #![allow(unsafe_code)]
 
-use eframe::egui;
+use eframe::{egui, egui_glow, glow};
 
 use egui::mutex::Mutex;
 use std::sync::Arc;

examples/test_inline_glow_paint/Cargo.toml

@@ -0,0 +1,14 @@
+[package]
+name = "test_inline_glow_paint"
+version = "0.1.0"
+authors = ["Emil Ernerfeldt <emil.ernerfeldt@gmail.com>"]
+license = "MIT OR Apache-2.0"
+edition = "2021"
+rust-version = "1.72"
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+eframe = { path = "../../crates/eframe" }
+env_logger = "0.10"

examples/test_inline_glow_paint/src/main.rs

@@ -0,0 +1,40 @@
+// Test that we can paint to the screen using glow directly.
+
+use eframe::egui;
+use eframe::glow;
+
+fn main() -> Result<(), Box<dyn std::error::Error>> {
+    env_logger::init(); // Log to stderr (if you run with `RUST_LOG=debug`).
+    let options = eframe::NativeOptions {
+        renderer: eframe::Renderer::Glow,
+        ..Default::default()
+    };
+    eframe::run_native(
+        "My test app",
+        options,
+        Box::new(|_cc| Box::<MyTestApp>::default()),
+    )?;
+    Ok(())
+}
+
+#[derive(Default)]
+struct MyTestApp {}
+
+impl eframe::App for MyTestApp {
+    fn update(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {
+        use glow::HasContext as _;
+        let gl = frame.gl().unwrap();
+
+        #[allow(unsafe_code)]
+        unsafe {
+            gl.disable(glow::SCISSOR_TEST);
+            gl.viewport(0, 0, 100, 100);
+            gl.clear_color(1.0, 0.0, 1.0, 1.0); // purple
+            gl.clear(glow::COLOR_BUFFER_BIT);
+        }
+
+        egui::Window::new("Floating Window").show(ctx, |ui| {
+            ui.label("The background should be purple.");
+        });
+    }
+}