From e8d9df7bdfcdd67c9debbea5f14a22698e10b5ad Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Sun, 7 Jul 2024 20:53:37 +0200 Subject: [PATCH] API Review: Remove suspend/resume related functions from FemtoVG renderer This could've been `set_opengl_context` taking an `Option`, but it turns out that makes for an awkward interface when wanting to pass None, because that also then requires a dummy type for the OpenGL context even though none is wanted. --- internal/backends/winit/renderer/femtovg.rs | 11 +- internal/renderers/femtovg/lib.rs | 218 ++++++++++---------- 2 files changed, 117 insertions(+), 112 deletions(-) diff --git a/internal/backends/winit/renderer/femtovg.rs b/internal/backends/winit/renderer/femtovg.rs index d954e467516..e46b0edee78 100644 --- a/internal/backends/winit/renderer/femtovg.rs +++ b/internal/backends/winit/renderer/femtovg.rs @@ -6,7 +6,7 @@ use std::rc::Rc; use i_slint_core::platform::PlatformError; use i_slint_core::renderer::Renderer; -use i_slint_renderer_femtovg::FemtoVGRenderer; +use i_slint_renderer_femtovg::{FemtoVGRenderer, FemtoVGRendererExt}; #[cfg(target_arch = "wasm32")] use winit::platform::web::WindowExtWebSys; @@ -23,7 +23,10 @@ pub struct GlutinFemtoVGRenderer { impl GlutinFemtoVGRenderer { pub fn new_suspended() -> Box { - Box::new(Self { renderer: FemtoVGRenderer::new_suspended(), suspended: Cell::new(true) }) + Box::new(Self { + renderer: FemtoVGRenderer::new_without_context(), + suspended: Cell::new(true), + }) } } @@ -56,7 +59,7 @@ impl super::WinitCompatibleRenderer for GlutinFemtoVGRenderer { }) })?); - self.renderer.resume( + self.renderer.set_opengl_context( #[cfg(not(target_arch = "wasm32"))] opengl_context, #[cfg(target_arch = "wasm32")] @@ -71,7 +74,7 @@ impl super::WinitCompatibleRenderer for GlutinFemtoVGRenderer { } fn suspend(&self) -> Result<(), PlatformError> { - self.renderer.suspend() + self.renderer.clear_opengl_context() } fn is_suspended(&self) -> bool { diff --git a/internal/renderers/femtovg/lib.rs b/internal/renderers/femtovg/lib.rs index 6b9621aac7d..2b2cbfb4f19 100644 --- a/internal/renderers/femtovg/lib.rs +++ b/internal/renderers/femtovg/lib.rs @@ -140,8 +140,8 @@ impl FemtoVGRenderer { #[cfg(not(target_arch = "wasm32"))] opengl_context: impl OpenGLInterface + 'static, #[cfg(target_arch = "wasm32")] html_canvas: web_sys::HtmlCanvasElement, ) -> Result { - let this = Self::new_suspended(); - this.resume( + let this = Self::new_without_context(); + this.set_opengl_context( #[cfg(not(target_arch = "wasm32"))] opengl_context, #[cfg(target_arch = "wasm32")] @@ -150,111 +150,6 @@ impl FemtoVGRenderer { Ok(this) } - /// Creates a new renderer in suspended state. Any attempts at rendering, etc. will produce an error, - /// until [`Self::resume()`] was called successfully. - pub fn new_suspended() -> Self { - let opengl_context = Box::new(SuspendedRenderer {}); - - Self { - maybe_window_adapter: Default::default(), - rendering_notifier: Default::default(), - canvas: RefCell::new(None), - graphics_cache: Default::default(), - texture_cache: Default::default(), - rendering_metrics_collector: Default::default(), - rendering_first_time: Cell::new(true), - opengl_context: RefCell::new(opengl_context), - #[cfg(target_arch = "wasm32")] - canvas_id: Default::default(), - } - } - - /// Suspends the renderer by deleting all OpenGL sources moving into a suspended state. - /// All subsequence attempt at rendering will produce an error, until [`Self::resume()`] - /// is called successfully. - /// - /// Use this to transfer a renderer between different window surfaces. - pub fn suspend(&self) -> Result<(), i_slint_core::platform::PlatformError> { - // Ensure the context is current before the renderer is destroyed - if self.opengl_context.borrow().ensure_current().is_ok() { - // If we've rendered a frame before, then we need to invoke the RenderingTearDown notifier. - if !self.rendering_first_time.get() { - if let Some(callback) = self.rendering_notifier.borrow_mut().as_mut() { - self.with_graphics_api(|api| { - callback.notify(RenderingState::RenderingTeardown, &api) - }) - .ok(); - } - } - - self.graphics_cache.clear_all(); - self.texture_cache.borrow_mut().clear(); - } - - if let Some(canvas) = self.canvas.borrow_mut().take() { - if Rc::strong_count(&canvas) != 1 { - i_slint_core::debug_log!("internal warning: there are canvas references left when destroying the window. OpenGL resources will be leaked.") - } - } - - *self.opengl_context.borrow_mut() = Box::new(SuspendedRenderer {}); - Ok(()) - } - - /// Resumes a previously suspended renderer. - pub fn resume( - &self, - #[cfg(not(target_arch = "wasm32"))] opengl_context: impl OpenGLInterface + 'static, - #[cfg(target_arch = "wasm32")] html_canvas: web_sys::HtmlCanvasElement, - ) -> Result<(), i_slint_core::platform::PlatformError> { - #[cfg(target_arch = "wasm32")] - let opengl_context = WebGLNeedsNoCurrentContext {}; - - let opengl_context = Box::new(opengl_context); - #[cfg(not(target_arch = "wasm32"))] - let gl_renderer = unsafe { - femtovg::renderer::OpenGl::new_from_function_cstr(|name| { - opengl_context.get_proc_address(name) - }) - .unwrap() - }; - - #[cfg(target_arch = "wasm32")] - let gl_renderer = match femtovg::renderer::OpenGl::new_from_html_canvas(&html_canvas) { - Ok(gl_renderer) => gl_renderer, - Err(_) => { - use wasm_bindgen::JsCast; - - // I don't believe that there's a way of disabling the 2D canvas. - let context_2d = html_canvas - .get_context("2d") - .unwrap() - .unwrap() - .dyn_into::() - .unwrap(); - context_2d.set_font("20px serif"); - // We don't know if we're rendering on dark or white background, so choose a "color" in the middle for the text. - context_2d.set_fill_style(&wasm_bindgen::JsValue::from_str("red")); - context_2d - .fill_text("Slint requires WebGL to be enabled in your browser", 0., 30.) - .unwrap(); - panic!("Cannot proceed without WebGL - aborting") - } - }; - - let femtovg_canvas = femtovg::Canvas::new_with_text_context( - gl_renderer, - self::fonts::FONT_CACHE.with(|cache| cache.borrow().text_context.clone()), - ) - .unwrap(); - let canvas = Rc::new(RefCell::new(femtovg_canvas)); - - *self.canvas.borrow_mut() = canvas.into(); - *self.opengl_context.borrow_mut() = opengl_context; - self.rendering_first_time.set(true); - Ok(()) - } - /// Render the scene using OpenGL. pub fn render(&self) -> Result<(), i_slint_core::platform::PlatformError> { self.internal_render_with_post_callback( @@ -655,12 +550,19 @@ impl RendererSealed for FemtoVGRenderer { impl Drop for FemtoVGRenderer { fn drop(&mut self) { - self.suspend().ok(); + self.clear_opengl_context().ok(); } } #[doc(hidden)] pub trait FemtoVGRendererExt { + fn new_without_context() -> Self; + fn set_opengl_context( + &self, + #[cfg(not(target_arch = "wasm32"))] opengl_context: impl OpenGLInterface + 'static, + #[cfg(target_arch = "wasm32")] html_canvas: web_sys::HtmlCanvasElement, + ) -> Result<(), i_slint_core::platform::PlatformError>; + fn clear_opengl_context(&self) -> Result<(), i_slint_core::platform::PlatformError>; fn render_transformed_with_post_callback( &self, rotation_angle_degrees: f32, @@ -672,6 +574,106 @@ pub trait FemtoVGRendererExt { #[doc(hidden)] impl FemtoVGRendererExt for FemtoVGRenderer { + /// Creates a new renderer in suspended state without OpenGL. Any attempts at rendering, etc. will produce an error, + /// until [`Self::set_opengl_context()`] was called successfully. + fn new_without_context() -> Self { + let opengl_context = Box::new(SuspendedRenderer {}); + + Self { + maybe_window_adapter: Default::default(), + rendering_notifier: Default::default(), + canvas: RefCell::new(None), + graphics_cache: Default::default(), + texture_cache: Default::default(), + rendering_metrics_collector: Default::default(), + rendering_first_time: Cell::new(true), + opengl_context: RefCell::new(opengl_context), + #[cfg(target_arch = "wasm32")] + canvas_id: Default::default(), + } + } + + fn clear_opengl_context(&self) -> Result<(), i_slint_core::platform::PlatformError> { + // Ensure the context is current before the renderer is destroyed + if self.opengl_context.borrow().ensure_current().is_ok() { + // If we've rendered a frame before, then we need to invoke the RenderingTearDown notifier. + if !self.rendering_first_time.get() { + if let Some(callback) = self.rendering_notifier.borrow_mut().as_mut() { + self.with_graphics_api(|api| { + callback.notify(RenderingState::RenderingTeardown, &api) + }) + .ok(); + } + } + + self.graphics_cache.clear_all(); + self.texture_cache.borrow_mut().clear(); + } + + if let Some(canvas) = self.canvas.borrow_mut().take() { + if Rc::strong_count(&canvas) != 1 { + i_slint_core::debug_log!("internal warning: there are canvas references left when destroying the window. OpenGL resources will be leaked.") + } + } + + *self.opengl_context.borrow_mut() = Box::new(SuspendedRenderer {}); + + Ok(()) + } + + fn set_opengl_context( + &self, + #[cfg(not(target_arch = "wasm32"))] opengl_context: impl OpenGLInterface + 'static, + #[cfg(target_arch = "wasm32")] html_canvas: web_sys::HtmlCanvasElement, + ) -> Result<(), i_slint_core::platform::PlatformError> { + #[cfg(target_arch = "wasm32")] + let opengl_context = WebGLNeedsNoCurrentContext {}; + + let opengl_context = Box::new(opengl_context); + #[cfg(not(target_arch = "wasm32"))] + let gl_renderer = unsafe { + femtovg::renderer::OpenGl::new_from_function_cstr(|name| { + opengl_context.get_proc_address(name) + }) + .unwrap() + }; + + #[cfg(target_arch = "wasm32")] + let gl_renderer = match femtovg::renderer::OpenGl::new_from_html_canvas(&html_canvas) { + Ok(gl_renderer) => gl_renderer, + Err(_) => { + use wasm_bindgen::JsCast; + + // I don't believe that there's a way of disabling the 2D canvas. + let context_2d = html_canvas + .get_context("2d") + .unwrap() + .unwrap() + .dyn_into::() + .unwrap(); + context_2d.set_font("20px serif"); + // We don't know if we're rendering on dark or white background, so choose a "color" in the middle for the text. + context_2d.set_fill_style(&wasm_bindgen::JsValue::from_str("red")); + context_2d + .fill_text("Slint requires WebGL to be enabled in your browser", 0., 30.) + .unwrap(); + panic!("Cannot proceed without WebGL - aborting") + } + }; + + let femtovg_canvas = femtovg::Canvas::new_with_text_context( + gl_renderer, + self::fonts::FONT_CACHE.with(|cache| cache.borrow().text_context.clone()), + ) + .unwrap(); + let canvas = Rc::new(RefCell::new(femtovg_canvas)); + + *self.canvas.borrow_mut() = canvas.into(); + *self.opengl_context.borrow_mut() = opengl_context; + self.rendering_first_time.set(true); + Ok(()) + } + fn render_transformed_with_post_callback( &self, rotation_angle_degrees: f32,