From 4c18b4bbd94320f3c962c2db4e8c0b22e682598e Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Wed, 7 Jun 2023 09:23:13 +0200 Subject: [PATCH 1/3] Cleanup: Use winit's HTML Canvas accessor instead of our own --- internal/backends/winit/lib.rs | 3 --- internal/backends/winit/renderer/femtovg.rs | 5 ----- internal/backends/winit/winitwindowadapter.rs | 9 ++++++--- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/internal/backends/winit/lib.rs b/internal/backends/winit/lib.rs index 93b676a7b4b..75d1ae84462 100644 --- a/internal/backends/winit/lib.rs +++ b/internal/backends/winit/lib.rs @@ -43,9 +43,6 @@ mod renderer { fn as_core_renderer(&self) -> &dyn i_slint_core::renderer::Renderer; fn resize_event(&self, size: PhysicalSize) -> Result<(), PlatformError>; - - #[cfg(target_arch = "wasm32")] - fn html_canvas_element(&self) -> web_sys::HtmlCanvasElement; } #[cfg(feature = "renderer-winit-femtovg")] diff --git a/internal/backends/winit/renderer/femtovg.rs b/internal/backends/winit/renderer/femtovg.rs index 7cd83059149..8fecddf1eca 100644 --- a/internal/backends/winit/renderer/femtovg.rs +++ b/internal/backends/winit/renderer/femtovg.rs @@ -50,9 +50,4 @@ impl super::WinitCompatibleRenderer for GlutinFemtoVGRenderer { fn resize_event(&self, size: PhysicalWindowSize) -> Result<(), PlatformError> { self.renderer.resize(size) } - - #[cfg(target_arch = "wasm32")] - fn html_canvas_element(&self) -> web_sys::HtmlCanvasElement { - self.renderer.html_canvas_element() - } } diff --git a/internal/backends/winit/winitwindowadapter.rs b/internal/backends/winit/winitwindowadapter.rs index a6397ddbf2f..be2cf06b07f 100644 --- a/internal/backends/winit/winitwindowadapter.rs +++ b/internal/backends/winit/winitwindowadapter.rs @@ -13,6 +13,9 @@ use std::rc::Rc; #[cfg(target_arch = "wasm32")] use std::rc::Weak; +#[cfg(target_arch = "wasm32")] +use winit::platform::web::WindowExtWebSys; + use crate::renderer::WinitCompatibleRenderer; use const_field_offset::FieldOffsets; @@ -415,7 +418,7 @@ impl WindowAdapterSealed for WinitWindowAdapter { // Auto-resize to the preferred size if users (SlintPad) requests it #[cfg(target_arch = "wasm32")] { - let canvas = self.renderer().html_canvas_element(); + let canvas = winit_window.canvas(); if canvas .dataset() @@ -461,7 +464,7 @@ impl WindowAdapterSealed for WinitWindowAdapter { #[cfg(target_arch = "wasm32")] { - let html_canvas = self.renderer().html_canvas_element(); + let html_canvas = winit_window.canvas(); let existing_canvas_size = winit::dpi::LogicalSize::new( html_canvas.client_width() as f32, html_canvas.client_height() as f32, @@ -587,7 +590,7 @@ impl WindowAdapterSealed for WinitWindowAdapter { corelib::window::InputMethodRequest::Enable { .. } => { let mut vkh = self.virtual_keyboard_helper.borrow_mut(); let h = vkh.get_or_insert_with(|| { - let canvas = self.renderer().html_canvas_element(); + let canvas = self.winit_window().canvas(); super::wasm_input_helper::WasmInputHelper::new(self.self_weak.clone(), canvas) }); h.show(); From 322ce96b91aa647a613551b044e83607c2eeb84d Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Wed, 7 Jun 2023 09:34:07 +0200 Subject: [PATCH 2/3] winit: Remove the html canvas element from the GL context It's not needed anymore. We can pass the canvas element to the renderer once and then we don't need it anymore. --- internal/backends/winit/lib.rs | 1 - internal/backends/winit/renderer/femtovg.rs | 17 ++++++------ .../winit/renderer/femtovg/glcontext.rs | 25 +----------------- internal/backends/winit/winitwindowadapter.rs | 8 +----- internal/renderers/femtovg/lib.rs | 26 ++++++++----------- 5 files changed, 22 insertions(+), 55 deletions(-) diff --git a/internal/backends/winit/lib.rs b/internal/backends/winit/lib.rs index 75d1ae84462..a8970b9b6c6 100644 --- a/internal/backends/winit/lib.rs +++ b/internal/backends/winit/lib.rs @@ -30,7 +30,6 @@ mod renderer { pub(crate) trait WinitCompatibleRenderer { fn new( window_builder: winit::window::WindowBuilder, - #[cfg(target_arch = "wasm32")] canvas_id: &str, ) -> Result<(Self, winit::window::Window), PlatformError> where Self: Sized; diff --git a/internal/backends/winit/renderer/femtovg.rs b/internal/backends/winit/renderer/femtovg.rs index 8fecddf1eca..7319b1b4054 100644 --- a/internal/backends/winit/renderer/femtovg.rs +++ b/internal/backends/winit/renderer/femtovg.rs @@ -6,6 +6,9 @@ use i_slint_core::platform::PlatformError; use i_slint_core::renderer::Renderer; use i_slint_renderer_femtovg::FemtoVGRenderer; +#[cfg(target_arch = "wasm32")] +use winit::platform::web::WindowExtWebSys; + mod glcontext; pub struct GlutinFemtoVGRenderer { @@ -15,18 +18,16 @@ pub struct GlutinFemtoVGRenderer { impl super::WinitCompatibleRenderer for GlutinFemtoVGRenderer { fn new( window_builder: winit::window::WindowBuilder, - #[cfg(target_arch = "wasm32")] canvas_id: &str, ) -> Result<(Self, winit::window::Window), PlatformError> { let (winit_window, opengl_context) = crate::event_loop::with_window_target(|event_loop| { - glcontext::OpenGLContext::new_context( - window_builder, - event_loop.event_loop_target(), - #[cfg(target_arch = "wasm32")] - canvas_id, - ) + glcontext::OpenGLContext::new_context(window_builder, event_loop.event_loop_target()) })?; - let renderer = FemtoVGRenderer::new(opengl_context)?; + let renderer = FemtoVGRenderer::new( + opengl_context, + #[cfg(target_arch = "wasm32")] + winit_window.canvas(), + )?; Ok((Self { renderer }, winit_window)) } diff --git a/internal/backends/winit/renderer/femtovg/glcontext.rs b/internal/backends/winit/renderer/femtovg/glcontext.rs index 8f4b2ae8840..483788d76f2 100644 --- a/internal/backends/winit/renderer/femtovg/glcontext.rs +++ b/internal/backends/winit/renderer/femtovg/glcontext.rs @@ -17,15 +17,9 @@ pub struct OpenGLContext { context: glutin::context::PossiblyCurrentContext, #[cfg(not(target_arch = "wasm32"))] surface: glutin::surface::Surface, - #[cfg(target_arch = "wasm32")] - canvas: web_sys::HtmlCanvasElement, } unsafe impl i_slint_renderer_femtovg::OpenGLContextWrapper for OpenGLContext { - #[cfg(target_arch = "wasm32")] - fn html_canvas_element(&self) -> web_sys::HtmlCanvasElement { - self.canvas.clone() - } fn ensure_current(&self) -> Result<(), PlatformError> { #[cfg(not(target_arch = "wasm32"))] if !self.context.is_current() { @@ -192,7 +186,6 @@ impl OpenGLContext { pub fn new_context( window_builder: winit::window::WindowBuilder, window_target: &winit::event_loop::EventLoopWindowTarget, - canvas_id: &str, ) -> Result<(winit::window::Window, Self), PlatformError> { let window = window_builder.build(window_target).map_err(|winit_os_err| { format!( @@ -200,22 +193,6 @@ impl OpenGLContext { winit_os_err ) })?; - - use wasm_bindgen::JsCast; - - let canvas = web_sys::window() - .ok_or_else(|| "FemtoVG Renderer: Could not retrieve DOM window".to_string())? - .document() - .ok_or_else(|| "FemtoVG Renderer: Could not retrieve DOM document".to_string())? - .get_element_by_id(canvas_id) - .ok_or_else(|| { - format!("FemtoVG Renderer: Could not retrieve existing HTML Canvas element '{canvas_id}'") - })? - .dyn_into::() - .map_err(|_| { - format!("FemtoVG Renderer: Specified DOM element '{canvas_id}' is not a HTML Canvas") - })?; - - Ok((window, Self { canvas })) + Ok((window, Self {})) } } diff --git a/internal/backends/winit/winitwindowadapter.rs b/internal/backends/winit/winitwindowadapter.rs index be2cf06b07f..d7978c23731 100644 --- a/internal/backends/winit/winitwindowadapter.rs +++ b/internal/backends/winit/winitwindowadapter.rs @@ -129,13 +129,7 @@ impl WinitWindowAdapter { #[cfg(target_arch = "wasm32")] canvas_id, ) - .and_then(|builder| { - R::new( - builder, - #[cfg(target_arch = "wasm32")] - canvas_id, - ) - })?; + .and_then(|builder| R::new(builder))?; let self_rc = Rc::new_cyclic(|self_weak| Self { window: OnceCell::with_value(corelib::api::Window::new(self_weak.clone() as _)), diff --git a/internal/renderers/femtovg/lib.rs b/internal/renderers/femtovg/lib.rs index fdcd596874e..040321a6d71 100644 --- a/internal/renderers/femtovg/lib.rs +++ b/internal/renderers/femtovg/lib.rs @@ -47,8 +47,6 @@ pub unsafe trait OpenGLContextWrapper { fn resize(&self, size: PhysicalWindowSize) -> Result<(), PlatformError>; #[cfg(not(target_arch = "wasm32"))] fn get_proc_address(&self, name: &std::ffi::CStr) -> *const std::ffi::c_void; - #[cfg(target_arch = "wasm32")] - fn html_canvas_element(&self) -> web_sys::HtmlCanvasElement; } /// Use the FemtoVG renderer when implementing a custom Slint platform where you deliver events to @@ -61,6 +59,8 @@ pub struct FemtoVGRenderer { rendering_metrics_collector: RefCell>>, // Last field, so that it's dropped last and context exists and is current when destroying the FemtoVG canvas opengl_context: Box, + #[cfg(target_arch = "wasm32")] + canvas_id: String, } impl FemtoVGRenderer { @@ -68,7 +68,10 @@ impl FemtoVGRenderer { /// of the OpenGLContextWrapper trait. The trait serves the purpose of giving the renderer control /// over when the make the context current, how to retrieve the address of GL functions, and when /// to swap back and front buffers. - pub fn new(opengl_context: impl OpenGLContextWrapper + 'static) -> Result { + pub fn new( + opengl_context: impl OpenGLContextWrapper + 'static, + #[cfg(target_arch = "wasm32")] html_canvas: web_sys::HtmlCanvasElement, + ) -> Result { let opengl_context = Box::new(opengl_context); #[cfg(not(target_arch = "wasm32"))] let gl_renderer = unsafe { @@ -79,16 +82,13 @@ impl FemtoVGRenderer { }; #[cfg(target_arch = "wasm32")] - let canvas = opengl_context.html_canvas_element(); - - #[cfg(target_arch = "wasm32")] - let gl_renderer = match femtovg::renderer::OpenGl::new_from_html_canvas(&canvas) { + 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 = canvas + let context_2d = html_canvas .get_context("2d") .unwrap() .unwrap() @@ -118,6 +118,8 @@ impl FemtoVGRenderer { texture_cache: Default::default(), rendering_metrics_collector: Default::default(), opengl_context, + #[cfg(target_arch = "wasm32")] + canvas_id: html_canvas.id(), }) } @@ -278,19 +280,13 @@ impl FemtoVGRenderer { ) -> Result<(), PlatformError> { use i_slint_core::api::GraphicsAPI; - let canvas_element_id = self.opengl_context.html_canvas_element().id(); let api = GraphicsAPI::WebGL { - canvas_element_id: canvas_element_id.as_str(), + canvas_element_id: self.canvas_id.as_str(), context_type: "webgl", }; callback(api); Ok(()) } - - #[cfg(target_arch = "wasm32")] - pub fn html_canvas_element(&self) -> web_sys::HtmlCanvasElement { - self.opengl_context.html_canvas_element() - } } impl Renderer for FemtoVGRenderer { From 3846b1e5a64657148bf10e9fcd11aede23311e06 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Wed, 7 Jun 2023 09:42:38 +0200 Subject: [PATCH 3/3] Simplify FemtoVG renderer API For wasm builds, just require the canvas element. --- internal/backends/winit/renderer/femtovg.rs | 13 +++++ .../winit/renderer/femtovg/glcontext.rs | 48 ++++--------------- internal/renderers/femtovg/lib.rs | 22 ++++++++- 3 files changed, 43 insertions(+), 40 deletions(-) diff --git a/internal/backends/winit/renderer/femtovg.rs b/internal/backends/winit/renderer/femtovg.rs index 7319b1b4054..2175300a080 100644 --- a/internal/backends/winit/renderer/femtovg.rs +++ b/internal/backends/winit/renderer/femtovg.rs @@ -9,6 +9,7 @@ use i_slint_renderer_femtovg::FemtoVGRenderer; #[cfg(target_arch = "wasm32")] use winit::platform::web::WindowExtWebSys; +#[cfg(not(target_arch = "wasm32"))] mod glcontext; pub struct GlutinFemtoVGRenderer { @@ -19,11 +20,23 @@ impl super::WinitCompatibleRenderer for GlutinFemtoVGRenderer { fn new( window_builder: winit::window::WindowBuilder, ) -> Result<(Self, winit::window::Window), PlatformError> { + #[cfg(not(target_arch = "wasm32"))] let (winit_window, opengl_context) = crate::event_loop::with_window_target(|event_loop| { glcontext::OpenGLContext::new_context(window_builder, event_loop.event_loop_target()) })?; + #[cfg(target_arch = "wasm32")] + let winit_window = crate::event_loop::with_window_target(|event_loop| { + window_builder.build(event_loop.event_loop_target()).map_err(|winit_os_err| { + format!( + "FemtoVG Renderer: Could not create winit window wrapper for DOM canvas: {}", + winit_os_err + ) + }) + })?; + let renderer = FemtoVGRenderer::new( + #[cfg(not(target_arch = "wasm32"))] opengl_context, #[cfg(target_arch = "wasm32")] winit_window.canvas(), diff --git a/internal/backends/winit/renderer/femtovg/glcontext.rs b/internal/backends/winit/renderer/femtovg/glcontext.rs index 483788d76f2..10f96ce3362 100644 --- a/internal/backends/winit/renderer/femtovg/glcontext.rs +++ b/internal/backends/winit/renderer/femtovg/glcontext.rs @@ -1,7 +1,6 @@ // Copyright © SixtyFPS GmbH // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-commercial -#[cfg(not(target_arch = "wasm32"))] use glutin::{ context::{ContextApi, ContextAttributesBuilder}, display::GetGlDisplay, @@ -9,19 +8,15 @@ use glutin::{ surface::{SurfaceAttributesBuilder, WindowSurface}, }; use i_slint_core::{api::PhysicalSize, platform::PlatformError}; -#[cfg(not(target_arch = "wasm32"))] use raw_window_handle::HasRawWindowHandle; pub struct OpenGLContext { - #[cfg(not(target_arch = "wasm32"))] context: glutin::context::PossiblyCurrentContext, - #[cfg(not(target_arch = "wasm32"))] surface: glutin::surface::Surface, } unsafe impl i_slint_renderer_femtovg::OpenGLContextWrapper for OpenGLContext { fn ensure_current(&self) -> Result<(), PlatformError> { - #[cfg(not(target_arch = "wasm32"))] if !self.context.is_current() { self.context.make_current(&self.surface).map_err(|glutin_error| -> PlatformError { format!("FemtoVG: Error making context current: {glutin_error}").into() @@ -30,7 +25,6 @@ unsafe impl i_slint_renderer_femtovg::OpenGLContextWrapper for OpenGLContext { Ok(()) } fn swap_buffers(&self) -> Result<(), PlatformError> { - #[cfg(not(target_arch = "wasm32"))] self.surface.swap_buffers(&self.context).map_err(|glutin_error| -> PlatformError { format!("FemtoVG: Error swapping buffers: {glutin_error}").into() })?; @@ -39,35 +33,25 @@ unsafe impl i_slint_renderer_femtovg::OpenGLContextWrapper for OpenGLContext { } fn resize(&self, _size: PhysicalSize) -> Result<(), PlatformError> { - #[cfg(not(target_arch = "wasm32"))] - { - let width = _size.width.try_into().map_err(|_| { - format!( - "Attempting to create window surface with an invalid width: {}", - _size.width - ) - })?; - let height = _size.height.try_into().map_err(|_| { - format!( - "Attempting to create window surface with an invalid height: {}", - _size.height - ) - })?; + let width = _size.width.try_into().map_err(|_| { + format!("Attempting to create window surface with an invalid width: {}", _size.width) + })?; + let height = _size.height.try_into().map_err(|_| { + format!("Attempting to create window surface with an invalid height: {}", _size.height) + })?; + + self.ensure_current()?; + self.surface.resize(&self.context, width, height); - self.ensure_current()?; - self.surface.resize(&self.context, width, height); - } Ok(()) } - #[cfg(not(target_arch = "wasm32"))] fn get_proc_address(&self, name: &std::ffi::CStr) -> *const std::ffi::c_void { self.context.display().get_proc_address(name) } } impl OpenGLContext { - #[cfg(not(target_arch = "wasm32"))] pub fn new_context( window_builder: winit::window::WindowBuilder, window_target: &winit::event_loop::EventLoopWindowTarget, @@ -181,18 +165,4 @@ impl OpenGLContext { Ok((window, Self { context, surface })) } - - #[cfg(target_arch = "wasm32")] - pub fn new_context( - window_builder: winit::window::WindowBuilder, - window_target: &winit::event_loop::EventLoopWindowTarget, - ) -> Result<(winit::window::Window, Self), PlatformError> { - let window = window_builder.build(window_target).map_err(|winit_os_err| { - format!( - "FemtoVG Renderer: Could not create winit window wrapper for DOM canvas: {}", - winit_os_err - ) - })?; - Ok((window, Self {})) - } } diff --git a/internal/renderers/femtovg/lib.rs b/internal/renderers/femtovg/lib.rs index 040321a6d71..26b4d33d564 100644 --- a/internal/renderers/femtovg/lib.rs +++ b/internal/renderers/femtovg/lib.rs @@ -49,6 +49,23 @@ pub unsafe trait OpenGLContextWrapper { fn get_proc_address(&self, name: &std::ffi::CStr) -> *const std::ffi::c_void; } +#[cfg(target_arch = "wasm32")] +struct WebGLNeedsNoCurrentContext; +#[cfg(target_arch = "wasm32")] +unsafe impl OpenGLContextWrapper for WebGLNeedsNoCurrentContext { + fn ensure_current(&self) -> Result<(), PlatformError> { + Ok(()) + } + + fn swap_buffers(&self) -> Result<(), PlatformError> { + Ok(()) + } + + fn resize(&self, _size: PhysicalWindowSize) -> Result<(), PlatformError> { + Ok(()) + } +} + /// Use the FemtoVG renderer when implementing a custom Slint platform where you deliver events to /// Slint and want the scene to be rendered using OpenGL and the FemtoVG renderer. pub struct FemtoVGRenderer { @@ -69,9 +86,12 @@ impl FemtoVGRenderer { /// over when the make the context current, how to retrieve the address of GL functions, and when /// to swap back and front buffers. pub fn new( - opengl_context: impl OpenGLContextWrapper + 'static, + #[cfg(not(target_arch = "wasm32"))] opengl_context: impl OpenGLContextWrapper + 'static, #[cfg(target_arch = "wasm32")] html_canvas: web_sys::HtmlCanvasElement, ) -> Result { + #[cfg(target_arch = "wasm32")] + let opengl_context = WebGLNeedsNoCurrentContext {}; + let opengl_context = Box::new(opengl_context); #[cfg(not(target_arch = "wasm32"))] let gl_renderer = unsafe {