From 4b1523ad51000a3c711cf8cb64dffaea1f8c6d77 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 30 Nov 2023 16:37:16 +0100 Subject: [PATCH] eframe glow backend: Clear render target before calling `App::update` (#3665) * Closes https://github.com/emilk/egui/issues/3659 --- Cargo.lock | 10 +- crates/eframe/src/native/glow_integration.rs | 125 ++++++++++++------- crates/eframe/src/native/wgpu_integration.rs | 13 +- examples/custom_3d_glow/Cargo.toml | 2 - examples/custom_3d_glow/src/main.rs | 2 +- examples/test_inline_glow_paint/Cargo.toml | 14 +++ examples/test_inline_glow_paint/src/main.rs | 40 ++++++ 7 files changed, 146 insertions(+), 60 deletions(-) create mode 100644 examples/test_inline_glow_paint/Cargo.toml create mode 100644 examples/test_inline_glow_paint/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 569421b9b02..7becef4e108 100644 --- a/Cargo.lock +++ b/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" diff --git a/crates/eframe/src/native/glow_integration.rs b/crates/eframe/src/native/glow_integration.rs index e1c2afe8213..6bf7336af85 100644 --- a/crates/eframe/src/native/glow_integration.rs +++ b/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, + gl_surface: &glutin::surface::Surface, +) { + 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); } diff --git a/crates/eframe/src/native/wgpu_integration.rs b/crates/eframe/src/native/wgpu_integration.rs index 6246a0d87ff..3f9cf14e983 100644 --- a/crates/eframe/src/native/wgpu_integration.rs +++ b/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); } diff --git a/examples/custom_3d_glow/Cargo.toml b/examples/custom_3d_glow/Cargo.toml index 42e1d73d4f1..849faa5c463 100644 --- a/examples/custom_3d_glow/Cargo.toml +++ b/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" diff --git a/examples/custom_3d_glow/src/main.rs b/examples/custom_3d_glow/src/main.rs index cad720b115e..93797397880 100644 --- a/examples/custom_3d_glow/src/main.rs +++ b/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; diff --git a/examples/test_inline_glow_paint/Cargo.toml b/examples/test_inline_glow_paint/Cargo.toml new file mode 100644 index 00000000000..4fabb8be269 --- /dev/null +++ b/examples/test_inline_glow_paint/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "test_inline_glow_paint" +version = "0.1.0" +authors = ["Emil Ernerfeldt "] +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" diff --git a/examples/test_inline_glow_paint/src/main.rs b/examples/test_inline_glow_paint/src/main.rs new file mode 100644 index 00000000000..1710ee154a6 --- /dev/null +++ b/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> { + 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::::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."); + }); + } +}