diff --git a/Cargo.lock b/Cargo.lock index daca4b3f6..ba5f04829 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -853,11 +853,9 @@ checksum = "f12a8d8f92a429e2e9399003bc01d48f54aefa6fec126cbd3462e313c0ee0e91" dependencies = [ "bytemuck", "egui", - "pollster", "tracing", "type-map", "wgpu", - "winit", ] [[package]] @@ -1116,7 +1114,6 @@ dependencies = [ "bytemuck", "egui", "egui-wgpu", - "egui-winit", "fj-interop", "fj-math", "raw-window-handle 0.4.3", @@ -1130,6 +1127,7 @@ dependencies = [ name = "fj-window" version = "0.19.0" dependencies = [ + "egui-winit", "fj-host", "fj-interop", "fj-operations", @@ -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 a048b6a21..078d53844 100644 --- a/crates/fj-viewer/Cargo.toml +++ b/crates/fj-viewer/Cargo.toml @@ -13,7 +13,7 @@ categories.workspace = true [dependencies] bytemuck = "1.12.1" egui = "0.19.0" -egui-winit = "0.19.0" +egui-wgpu = "0.19.0" fj-interop.workspace = true fj-math.workspace = true raw-window-handle = "0.4.3" @@ -21,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"] diff --git a/crates/fj-viewer/src/graphics/renderer.rs b/crates/fj-viewer/src/graphics/renderer.rs index c896005d4..0b78bbe5e 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; @@ -10,6 +9,7 @@ use wgpu_glyph::ab_glyph::InvalidFont; use crate::{ camera::Camera, + gui::EguiState, screen::{Screen, Size}, }; @@ -19,28 +19,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 { @@ -64,65 +42,9 @@ pub struct Renderer { impl Renderer { /// Returns a new `Renderer`. - pub async fn new( - screen: &impl Screen, - event_loop: &EventLoop<()>, - ) -> Result { + pub async fn new(screen: &impl Screen) -> Result { let instance = wgpu::Instance::new(wgpu::Backends::PRIMARY); - // - // 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. - // - // 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: - // - // * - // - - // - // 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_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. let surface = unsafe { instance.create_surface(screen.window()) }; @@ -140,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. @@ -229,23 +151,7 @@ impl Renderer { let pipelines = Pipelines::new(&device, &bind_group_layout, color_format); - // - // 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: - // - // "Texture 0 used but not live" - // - // See also: - // - let egui_rpass = egui_wgpu::renderer::RenderPass::new( - &device, - surface_config.format, - 1, - ); + let egui = EguiState::new(&device, surface_config.format); Ok(Self { surface, @@ -262,12 +168,7 @@ impl Renderer { geometries, pipelines, - egui: EguiState { - context: egui_context, - winit_state: egui_winit_state, - rpass: egui_rpass, - options: Default::default(), - }, + egui, }) } @@ -301,8 +202,9 @@ 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> { let aspect_ratio = self.surface_config.width as f64 / self.surface_config.height as f64; @@ -373,42 +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. - // - - 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 { @@ -582,7 +448,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, @@ -737,7 +603,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, @@ -745,10 +611,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, @@ -779,7 +645,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 new file mode 100644 index 000000000..102fbab82 --- /dev/null +++ b/crates/fj-viewer/src/gui.rs @@ -0,0 +1,83 @@ +//! 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, + 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 context: egui::Context, + pub render_pass: egui_wgpu::renderer::RenderPass, + 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 {}") + } +} 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; 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" diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index cdebae724..2c01cfac1 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -42,7 +42,8 @@ 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(); @@ -96,26 +97,15 @@ 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. - // - renderer - .egui - .winit_state - .on_event(&renderer.egui.context, window_event); + // 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. + egui_winit_state.on_event(&renderer.egui.context, window_event); } // fj-window events @@ -182,11 +172,15 @@ pub fn run( camera.update_planes(&shape.aabb); } + let egui_input = + egui_winit_state.take_egui_input(window.window()); + if let Err(err) = renderer.draw( &camera, &mut draw_config, - window.window(), + window.window().scale_factor() as f32, &mut status, + egui_input, ) { warn!("Draw error: {}", err); }