From b335fba4079252361df957079c67af2d4b1aa354 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 12:27:49 +0200 Subject: [PATCH 01/18] Move egui state structs to dedicated module --- crates/fj-viewer/src/graphics/renderer.rs | 23 +---------------------- crates/fj-viewer/src/gui.rs | 21 +++++++++++++++++++++ crates/fj-viewer/src/lib.rs | 2 ++ 3 files changed, 24 insertions(+), 22 deletions(-) create mode 100644 crates/fj-viewer/src/gui.rs diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index c896005d4..1f67fd39a 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -10,6 +10,7 @@ use wgpu_glyph::ab_glyph::InvalidFont; use crate::{ camera::Camera, + gui::EguiState, screen::{Screen, Size}, }; @@ -19,28 +20,6 @@ use super::{ vertices::Vertices, DEPTH_FORMAT, }; -#[derive(Default)] -struct EguiOptionsState { - show_trace: bool, - show_layout_debug_on_hover: bool, - show_debug_text_example: bool, - show_settings_ui: bool, - show_inspection_ui: bool, -} - -pub struct EguiState { - pub winit_state: egui_winit::State, - pub context: egui::Context, - rpass: egui_wgpu::renderer::RenderPass, - options: EguiOptionsState, -} - -impl std::fmt::Debug for EguiState { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("EguiState {}") - } -} - /// Graphics rendering state and target abstraction #[derive(Debug)] pub struct Renderer { diff --git a/crates/fj-viewer/src/gui.rs b/crates/fj-viewer/src/gui.rs new file mode 100644 index 000000000..e1d23e955 --- /dev/null +++ b/crates/fj-viewer/src/gui.rs @@ -0,0 +1,21 @@ +#[derive(Default)] +pub struct EguiOptionsState { + pub show_trace: bool, + pub show_layout_debug_on_hover: bool, + pub show_debug_text_example: bool, + pub show_settings_ui: bool, + pub show_inspection_ui: bool, +} + +pub struct EguiState { + pub winit_state: egui_winit::State, + pub context: egui::Context, + pub rpass: egui_wgpu::renderer::RenderPass, + pub options: EguiOptionsState, +} + +impl std::fmt::Debug for EguiState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("EguiState {}") + } +} diff --git a/crates/fj-viewer/src/lib.rs b/crates/fj-viewer/src/lib.rs index b6b4be702..4428e99ff 100644 --- a/crates/fj-viewer/src/lib.rs +++ b/crates/fj-viewer/src/lib.rs @@ -18,3 +18,5 @@ pub mod camera; pub mod graphics; pub mod input; pub mod screen; + +mod gui; From 7d24332f856f6a9a33cd8c16e203d9cb4d246ad2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 12:39:20 +0200 Subject: [PATCH 02/18] Make struct field name more explicit --- crates/fj-viewer/src/graphics/renderer.rs | 10 +++++----- crates/fj-viewer/src/gui.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index 1f67fd39a..18b8f12f6 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -244,7 +244,7 @@ impl Renderer { egui: EguiState { context: egui_context, winit_state: egui_winit_state, - rpass: egui_rpass, + render_pass: egui_rpass, options: Default::default(), }, }) @@ -716,7 +716,7 @@ impl Renderer { }; for (id, image_delta) in &textures_delta.set { - self.egui.rpass.update_texture( + self.egui.render_pass.update_texture( &self.device, &self.queue, *id, @@ -724,10 +724,10 @@ impl Renderer { ); } for id in &textures_delta.free { - self.egui.rpass.free_texture(id); + self.egui.render_pass.free_texture(id); } - self.egui.rpass.update_buffers( + self.egui.render_pass.update_buffers( &self.device, &self.queue, clipped_primitives, @@ -758,7 +758,7 @@ impl Renderer { }; // Record all render passes. - self.egui.rpass.execute( + self.egui.render_pass.execute( encoder, output_view, clipped_primitives, diff --git a/crates/fj-viewer/src/gui.rs b/crates/fj-viewer/src/gui.rs index e1d23e955..832080f9f 100644 --- a/crates/fj-viewer/src/gui.rs +++ b/crates/fj-viewer/src/gui.rs @@ -10,7 +10,7 @@ pub struct EguiOptionsState { pub struct EguiState { pub winit_state: egui_winit::State, pub context: egui::Context, - pub rpass: egui_wgpu::renderer::RenderPass, + pub render_pass: egui_wgpu::renderer::RenderPass, pub options: EguiOptionsState, } From 0628d01d2c556378de8f891ffa24cd43bc57b661 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 12:49:38 +0200 Subject: [PATCH 03/18] Clean up comment --- crates/fj-window/src/run.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index cdebae724..b133dc59d 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -96,22 +96,14 @@ pub fn run( .. } = &event { + // In theory we could/should check if `egui` wants "exclusive" use + // of this event here. But with the current integration with Fornjot + // we're kinda blurring the lines between "app" and "platform", so + // for the moment we pass every event to both `egui` & Fornjot. // - // Note: In theory we could/should check if `egui` wants "exclusive" use - // of this event here. - // - // But with the current integration with Fornjot we're kinda blurring - // the lines between "app" and "platform", so for the moment we pass - // every event to both `egui` & Fornjot. - // - // The primary visible impact of this currently is that if you drag - // a title bar that overlaps the model then both the model & window - // get moved. - // - // TODO: Revisit this. - // - // TODO: Encapsulate the egui state/context access better. - // + // The primary visible impact of this currently is that if you drag + // a title bar that overlaps the model then both the model & window + // get moved. renderer .egui .winit_state From c9ae2761900f50b5d74965d3af658d7b0b20cd29 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 12:52:02 +0200 Subject: [PATCH 04/18] Move use of `egui_winit` state out of `fj-viewer` --- crates/fj-viewer/src/graphics/renderer.rs | 2 +- crates/fj-window/src/run.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index 18b8f12f6..babc6a408 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -282,6 +282,7 @@ impl Renderer { config: &mut DrawConfig, window: &egui_winit::winit::window::Window, status: &mut StatusReport, + egui_input: egui::RawInput, ) -> Result<(), DrawError> { let aspect_ratio = self.surface_config.width as f64 / self.surface_config.height as f64; @@ -387,7 +388,6 @@ impl Renderer { // It's still not the *latest* `egui` version though. // - let egui_input = self.egui.winit_state.take_egui_input(window); self.egui.context.begin_frame(egui_input); fn get_bbox_size_text(aabb: &Aabb<3>) -> String { diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index b133dc59d..5677135b3 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -174,11 +174,15 @@ pub fn run( camera.update_planes(&shape.aabb); } + let egui_input = + renderer.egui.winit_state.take_egui_input(window.window()); + if let Err(err) = renderer.draw( &camera, &mut draw_config, window.window(), &mut status, + egui_input, ) { warn!("Draw error: {}", err); } From 006996a3d532b3ee8799d08c597887423e6b1763 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 12:55:01 +0200 Subject: [PATCH 05/18] Add dependency on `egui-winit` to `fj-window` --- Cargo.lock | 1 + crates/fj-window/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index daca4b3f6..fe33fea7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1130,6 +1130,7 @@ dependencies = [ name = "fj-window" version = "0.19.0" dependencies = [ + "egui-winit", "fj-host", "fj-interop", "fj-operations", diff --git a/crates/fj-window/Cargo.toml b/crates/fj-window/Cargo.toml index 005b26b0e..ada8da742 100644 --- a/crates/fj-window/Cargo.toml +++ b/crates/fj-window/Cargo.toml @@ -15,6 +15,7 @@ fj-host.workspace = true fj-operations.workspace = true fj-viewer.workspace = true fj-interop.workspace = true +egui-winit = "0.19.0" futures = "0.3.24" thiserror = "1.0.35" tracing = "0.1.37" From b7aa5cfc581dab4c09cc1658c10961dc81e32348 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 12:55:43 +0200 Subject: [PATCH 06/18] Move `egui_winit` state out of `fj-viewer` --- crates/fj-viewer/src/graphics/renderer.rs | 4 +--- crates/fj-viewer/src/gui.rs | 1 - crates/fj-window/src/run.rs | 8 +++----- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index babc6a408..149bbcf85 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -45,7 +45,7 @@ impl Renderer { /// Returns a new `Renderer`. pub async fn new( screen: &impl Screen, - event_loop: &EventLoop<()>, + _: &EventLoop<()>, ) -> Result { let instance = wgpu::Instance::new(wgpu::Backends::PRIMARY); @@ -99,7 +99,6 @@ impl Renderer { // Don't ask me how I know. // - let egui_winit_state = egui_winit::State::new(event_loop); let egui_context = egui::Context::default(); // This is sound, as `window` is an object to create a surface upon. @@ -243,7 +242,6 @@ impl Renderer { egui: EguiState { context: egui_context, - winit_state: egui_winit_state, render_pass: egui_rpass, options: Default::default(), }, diff --git a/crates/fj-viewer/src/gui.rs b/crates/fj-viewer/src/gui.rs index 832080f9f..b0fb68d98 100644 --- a/crates/fj-viewer/src/gui.rs +++ b/crates/fj-viewer/src/gui.rs @@ -8,7 +8,6 @@ pub struct EguiOptionsState { } pub struct EguiState { - pub winit_state: egui_winit::State, pub context: egui::Context, pub render_pass: egui_wgpu::renderer::RenderPass, pub options: EguiOptionsState, diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 5677135b3..2bf7fbbcc 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -43,6 +43,7 @@ pub fn run( let mut input_handler = input::Handler::default(); let mut renderer = block_on(Renderer::new(&window, &event_loop))?; + let mut egui_winit_state = egui_winit::State::new(&event_loop); let mut draw_config = DrawConfig::default(); @@ -104,10 +105,7 @@ pub fn run( // The primary visible impact of this currently is that if you drag // a title bar that overlaps the model then both the model & window // get moved. - renderer - .egui - .winit_state - .on_event(&renderer.egui.context, window_event); + egui_winit_state.on_event(&renderer.egui.context, window_event); } // fj-window events @@ -175,7 +173,7 @@ pub fn run( } let egui_input = - renderer.egui.winit_state.take_egui_input(window.window()); + egui_winit_state.take_egui_input(window.window()); if let Err(err) = renderer.draw( &camera, From 8c63e080ba304730070d559783cce74057931985 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 12:56:31 +0200 Subject: [PATCH 07/18] Remove unused argument --- crates/fj-viewer/src/graphics/renderer.rs | 2 -- crates/fj-window/src/run.rs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index 149bbcf85..67ebbc512 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -1,6 +1,5 @@ use std::{io, mem::size_of}; -use egui_winit::winit::event_loop::EventLoop; use fj_interop::status_report::StatusReport; use fj_math::{Aabb, Point}; use thiserror::Error; @@ -45,7 +44,6 @@ impl Renderer { /// Returns a new `Renderer`. pub async fn new( screen: &impl Screen, - _: &EventLoop<()>, ) -> Result { let instance = wgpu::Instance::new(wgpu::Backends::PRIMARY); diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 2bf7fbbcc..915fcf791 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -42,7 +42,7 @@ pub fn run( let mut focus_point = None; let mut input_handler = input::Handler::default(); - let mut renderer = block_on(Renderer::new(&window, &event_loop))?; + let mut renderer = block_on(Renderer::new(&window))?; let mut egui_winit_state = egui_winit::State::new(&event_loop); let mut draw_config = DrawConfig::default(); From 481222dbcb916d30d596b8b479116b5e51df9db5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 12:58:03 +0200 Subject: [PATCH 08/18] Simplify constructor argument --- crates/fj-viewer/src/graphics/renderer.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index 67ebbc512..bc13abeb8 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -42,9 +42,7 @@ pub struct Renderer { impl Renderer { /// Returns a new `Renderer`. - pub async fn new( - screen: &impl Screen, - ) -> Result { + pub async fn new(screen: &impl Screen) -> Result { let instance = wgpu::Instance::new(wgpu::Backends::PRIMARY); // From 679f1ff22666e8383f42d683fafc33ccc918c111 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:00:47 +0200 Subject: [PATCH 09/18] Simplify method argument --- crates/fj-viewer/src/graphics/renderer.rs | 4 ++-- crates/fj-window/src/run.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index bc13abeb8..7558bcb38 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -274,7 +274,7 @@ impl Renderer { &mut self, camera: &Camera, config: &mut DrawConfig, - window: &egui_winit::winit::window::Window, + scale_factor: f32, status: &mut StatusReport, egui_input: egui::RawInput, ) -> Result<(), DrawError> { @@ -555,7 +555,7 @@ impl Renderer { // Note: `scale_factor` can be overridden via `WINIT_X11_SCALE_FACTOR` environment variable, // see: // - window.scale_factor() as f32, + scale_factor, egui::Rgba::TRANSPARENT, &egui_paint_jobs, &egui_output.textures_delta, diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 915fcf791..2c01cfac1 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -178,7 +178,7 @@ pub fn run( if let Err(err) = renderer.draw( &camera, &mut draw_config, - window.window(), + window.window().scale_factor() as f32, &mut status, egui_input, ) { From af7b9be9cf8128e73d348c4c46b8a2378b3a8913 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:01:59 +0200 Subject: [PATCH 10/18] Remove unused dependency --- Cargo.lock | 1 - crates/fj-viewer/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe33fea7d..92c45bb12 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1116,7 +1116,6 @@ dependencies = [ "bytemuck", "egui", "egui-wgpu", - "egui-winit", "fj-interop", "fj-math", "raw-window-handle 0.4.3", diff --git a/crates/fj-viewer/Cargo.toml b/crates/fj-viewer/Cargo.toml index a048b6a21..ce606abf0 100644 --- a/crates/fj-viewer/Cargo.toml +++ b/crates/fj-viewer/Cargo.toml @@ -13,7 +13,6 @@ categories.workspace = true [dependencies] bytemuck = "1.12.1" egui = "0.19.0" -egui-winit = "0.19.0" fj-interop.workspace = true fj-math.workspace = true raw-window-handle = "0.4.3" From 18fd0533fb365dee51bf07f1994efa4c1a73beb9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:02:30 +0200 Subject: [PATCH 11/18] Remove unused feature declaration --- Cargo.lock | 8 -------- crates/fj-viewer/Cargo.toml | 5 +---- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 92c45bb12..ba5f04829 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -853,11 +853,9 @@ checksum = "f12a8d8f92a429e2e9399003bc01d48f54aefa6fec126cbd3462e313c0ee0e91" dependencies = [ "bytemuck", "egui", - "pollster", "tracing", "type-map", "wgpu", - "winit", ] [[package]] @@ -2603,12 +2601,6 @@ dependencies = [ "miniz_oxide", ] -[[package]] -name = "pollster" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5da3b0203fd7ee5720aa0b5e790b591aa5d3f41c3ed2c34a3a393382198af2f7" - [[package]] name = "ppv-lite86" version = "0.2.16" diff --git a/crates/fj-viewer/Cargo.toml b/crates/fj-viewer/Cargo.toml index ce606abf0..078d53844 100644 --- a/crates/fj-viewer/Cargo.toml +++ b/crates/fj-viewer/Cargo.toml @@ -13,6 +13,7 @@ categories.workspace = true [dependencies] bytemuck = "1.12.1" egui = "0.19.0" +egui-wgpu = "0.19.0" fj-interop.workspace = true fj-math.workspace = true raw-window-handle = "0.4.3" @@ -20,7 +21,3 @@ thiserror = "1.0.35" tracing = "0.1.37" wgpu = "0.13.1" wgpu_glyph = "0.17.0" - -[dependencies.egui-wgpu] -version = "0.19.0" -features = ["winit"] From 5eee10e92d6486bedfa17900f0db4d51f6fab966 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:08:41 +0200 Subject: [PATCH 12/18] Clean up comment --- crates/fj-viewer/src/graphics/renderer.rs | 38 ++++++++--------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index 7558bcb38..514304914 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -45,35 +45,23 @@ impl Renderer { pub async fn new(screen: &impl Screen) -> Result { let instance = wgpu::Instance::new(wgpu::Backends::PRIMARY); + // The implementation of the integration with `egui` is likely to need + // to change "significantly" depending on what architecture approach is + // chosen going forward. // - // NOTE: The implementation of the integration with `egui` is - // likely to need to change "significantly"[0] depending - // on what architecture approach is chosen going - // forward. + // The current implementation is somewhat complicated by virtue of + // "sitting somewhere in the middle" in relation to being neither a + // standalone integration nor fully using `egui` as a framework. // - // The current implementation is somewhat complicated by - // virtue of "sitting somewhere in the middle" in - // relation to being neither a standalone integration - // nor fully using `egui` as a framework. + // This is a result of a combination of the current integration being + // "proof of concept" level, and using `egui-winit` & `egui-wgpu`, which + // are both relatively new additions to the core `egui` ecosystem. // - // This is a result of a combination of the current - // integration being "proof of concept" level; and, using - // `egui-winit` & `egui-wgpu` which are both relatively - // new additions to the core `egui` ecosystem. - // - // It is recommended to read the following for additional - // helpful context for choosing an architecture: - // - // * - // - // * - // - // [0] By way of specific example, the recent addition - // of Android support lead to considerable API - // change related to `wgpu` & `winit`, see: - // - // * + // It is recommended to read the following for additional helpful + // context for choosing an architecture: // + // - https://github.com/emilk/egui/blob/eeae485629fca24a81a7251739460b671e1420f7/README.md#what-is-the-difference-between-egui-and-eframe + // - https://github.com/emilk/egui/blob/eeae485629fca24a81a7251739460b671e1420f7/README.md#how-do-i-render-3d-stuff-in-an-egui-area // // NOTE: If at some point you use `Painter` or similar and you From 2592008c2b3b0c28462de22db815495ea8a9b964 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:10:31 +0200 Subject: [PATCH 13/18] Clean up and move comment --- crates/fj-viewer/src/graphics/renderer.rs | 20 -------------------- crates/fj-viewer/src/gui.rs | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index 514304914..e1579ae7a 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -63,26 +63,6 @@ impl Renderer { // - https://github.com/emilk/egui/blob/eeae485629fca24a81a7251739460b671e1420f7/README.md#what-is-the-difference-between-egui-and-eframe // - https://github.com/emilk/egui/blob/eeae485629fca24a81a7251739460b671e1420f7/README.md#how-do-i-render-3d-stuff-in-an-egui-area - // - // NOTE: If at some point you use `Painter` or similar and you - // get this error: - // - // `VK_ERROR_NATIVE_WINDOW_IN_USE_KHR` - // - // and/or: - // - // `wgpu_core::device: surface configuration failed: Native window is in use` - // - // it's *probably(?)* because the swapchain has already - // been created for the window (e.g. by an integration) - // and *not* because of a regression of this issue - // (probably): - // - // - // - // Don't ask me how I know. - // - let egui_context = egui::Context::default(); // This is sound, as `window` is an object to create a surface upon. diff --git a/crates/fj-viewer/src/gui.rs b/crates/fj-viewer/src/gui.rs index b0fb68d98..f5392fd5f 100644 --- a/crates/fj-viewer/src/gui.rs +++ b/crates/fj-viewer/src/gui.rs @@ -1,3 +1,19 @@ +//! GUI-related code +//! +//! If at some point you use `Painter` or similar and you get this error: +//! +//! `VK_ERROR_NATIVE_WINDOW_IN_USE_KHR` +//! +//! and/or: +//! +//! `wgpu_core::device: surface configuration failed: Native window is in use` +//! +//! it's *probably(?)* because the swap chain has already been created for the +//! window (e.g. by an integration) and *not* because of a regression of this +//! issue (probably): +//! +//! + #[derive(Default)] pub struct EguiOptionsState { pub show_trace: bool, From f5375d0617d4c68428c8ea8a3cf607fde5917fd7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:18:06 +0200 Subject: [PATCH 14/18] Clean up comment --- crates/fj-viewer/src/graphics/renderer.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index e1579ae7a..d206a4ffe 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -171,18 +171,17 @@ impl Renderer { let pipelines = Pipelines::new(&device, &bind_group_layout, color_format); + // We need to hold on to this, otherwise it might cause the egui font + // texture to get dropped after drawing one frame. // - // Note: We need to hold on to this otherwise (from my memory) - // it causes the egui font texture to get dropped after - // drawing one frame. + // This then results in an `egui_wgpu_backend` error of + // `BackendError::Internal` with message: // - // This then results in an `egui_wgpu_backend` error of - // `BackendError::Internal` with message: - // - // "Texture 0 used but not live" - // - // See also: + // ``` + // Texture 0 used but not live + // ``` // + // See also: let egui_rpass = egui_wgpu::renderer::RenderPass::new( &device, surface_config.format, From 9e4d697b60eba1a95b342d083a63f8df023869e8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:19:38 +0200 Subject: [PATCH 15/18] Refactor --- crates/fj-viewer/src/graphics/renderer.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index d206a4ffe..780eb6ba7 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -188,6 +188,12 @@ impl Renderer { 1, ); + let egui = EguiState { + context: egui_context, + render_pass: egui_rpass, + options: Default::default(), + }; + Ok(Self { surface, features, @@ -203,11 +209,7 @@ impl Renderer { geometries, pipelines, - egui: EguiState { - context: egui_context, - render_pass: egui_rpass, - options: Default::default(), - }, + egui, }) } From 5f02afc14d979d032eb748dd0dfc397c1b2ee84d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:21:22 +0200 Subject: [PATCH 16/18] Consolidate egui initialization in constructor --- crates/fj-viewer/src/graphics/renderer.rs | 43 +-------------------- crates/fj-viewer/src/gui.rs | 47 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 42 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index 780eb6ba7..da13fb72d 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -45,26 +45,6 @@ impl Renderer { pub async fn new(screen: &impl Screen) -> Result { let instance = wgpu::Instance::new(wgpu::Backends::PRIMARY); - // The implementation of the integration with `egui` is likely to need - // to change "significantly" depending on what architecture approach is - // chosen going forward. - // - // The current implementation is somewhat complicated by virtue of - // "sitting somewhere in the middle" in relation to being neither a - // standalone integration nor fully using `egui` as a framework. - // - // This is a result of a combination of the current integration being - // "proof of concept" level, and using `egui-winit` & `egui-wgpu`, which - // are both relatively new additions to the core `egui` ecosystem. - // - // It is recommended to read the following for additional helpful - // context for choosing an architecture: - // - // - https://github.com/emilk/egui/blob/eeae485629fca24a81a7251739460b671e1420f7/README.md#what-is-the-difference-between-egui-and-eframe - // - https://github.com/emilk/egui/blob/eeae485629fca24a81a7251739460b671e1420f7/README.md#how-do-i-render-3d-stuff-in-an-egui-area - - let egui_context = egui::Context::default(); - // This is sound, as `window` is an object to create a surface upon. let surface = unsafe { instance.create_surface(screen.window()) }; @@ -171,28 +151,7 @@ impl Renderer { let pipelines = Pipelines::new(&device, &bind_group_layout, color_format); - // We need to hold on to this, otherwise it might cause the egui font - // texture to get dropped after drawing one frame. - // - // This then results in an `egui_wgpu_backend` error of - // `BackendError::Internal` with message: - // - // ``` - // Texture 0 used but not live - // ``` - // - // See also: - let egui_rpass = egui_wgpu::renderer::RenderPass::new( - &device, - surface_config.format, - 1, - ); - - let egui = EguiState { - context: egui_context, - render_pass: egui_rpass, - options: Default::default(), - }; + let egui = EguiState::new(&device, surface_config.format); Ok(Self { surface, diff --git a/crates/fj-viewer/src/gui.rs b/crates/fj-viewer/src/gui.rs index f5392fd5f..102fbab82 100644 --- a/crates/fj-viewer/src/gui.rs +++ b/crates/fj-viewer/src/gui.rs @@ -29,6 +29,53 @@ pub struct EguiState { pub options: EguiOptionsState, } +impl EguiState { + pub fn new( + device: &wgpu::Device, + texture_format: wgpu::TextureFormat, + ) -> Self { + // The implementation of the integration with `egui` is likely to need + // to change "significantly" depending on what architecture approach is + // chosen going forward. + // + // The current implementation is somewhat complicated by virtue of + // "sitting somewhere in the middle" in relation to being neither a + // standalone integration nor fully using `egui` as a framework. + // + // This is a result of a combination of the current integration being + // "proof of concept" level, and using `egui-winit` & `egui-wgpu`, which + // are both relatively new additions to the core `egui` ecosystem. + // + // It is recommended to read the following for additional helpful + // context for choosing an architecture: + // + // - https://github.com/emilk/egui/blob/eeae485629fca24a81a7251739460b671e1420f7/README.md#what-is-the-difference-between-egui-and-eframe + // - https://github.com/emilk/egui/blob/eeae485629fca24a81a7251739460b671e1420f7/README.md#how-do-i-render-3d-stuff-in-an-egui-area + + let context = egui::Context::default(); + + // We need to hold on to this, otherwise it might cause the egui font + // texture to get dropped after drawing one frame. + // + // This then results in an `egui_wgpu_backend` error of + // `BackendError::Internal` with message: + // + // ``` + // Texture 0 used but not live + // ``` + // + // See also: + let render_pass = + egui_wgpu::renderer::RenderPass::new(device, texture_format, 1); + + Self { + context, + render_pass, + options: Default::default(), + } + } +} + impl std::fmt::Debug for EguiState { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str("EguiState {}") From 427678910a87671d7261bfb3fd9c941183760a6e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:22:30 +0200 Subject: [PATCH 17/18] Fix comment --- crates/fj-viewer/src/graphics/renderer.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index da13fb72d..b7dbda831 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -62,10 +62,10 @@ impl Renderer { let available_features = adapter.features(); // By requesting the intersection of desired and available features, - // we ensure two things: + // we prevent two things: // - // 1. That requesting the device doesn't panic, which would happen - // if we requested unavailable features. + // 1. That requesting the device panics, which would happen if we + // requested unavailable features. // 2. That a developer ends up accidentally using features that // happen to be available on their machine, but that aren't // necessarily available for all the users. From a1a7b1d6557e18229c926f78da1d097358540fb7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 12 Oct 2022 13:23:01 +0200 Subject: [PATCH 18/18] Remove outdated comment --- crates/fj-viewer/src/graphics/renderer.rs | 35 ----------------------- 1 file changed, 35 deletions(-) diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index b7dbda831..0b78bbe5e 100644 --- a/crates/fj-viewer/src/graphics/renderer.rs +++ b/crates/fj-viewer/src/graphics/renderer.rs @@ -275,41 +275,6 @@ impl Renderer { } } - // - // NOTE: The following comment was written for the original - // proof-of-concept which targeted older versions of - // Fornjot & `egui`, so some details may be outdated & - // not entirely apply to this updated implementation. - // - // It's included here in case it still provides some - // useful context. - // - // - // This integration is basically the result of locating the - // `.present()` call in the `egui` example, here: - // - // - // - // and then the equivalent call in `renderer.rs`, here: - // - // - // - // Then working backwards from there to merge the functionality. - // - // In addition, the following examples were also referenced: - // - // * "Make the example more like an actual use case #17" - // - // This removes some non-essential code from the example - // which helps clarify what's *actually* necessary. - // - // * "Update to 0.17, use official winit backend #18" - // - // This uses a more up-to-date `egui` version which - // included some API changes. - // It's still not the *latest* `egui` version though. - // - self.egui.context.begin_frame(egui_input); fn get_bbox_size_text(aabb: &Aabb<3>) -> String {