Skip to content

Commit

Permalink
Fix slint::Window::hide() on Wayland with winit
Browse files Browse the repository at this point in the history
On Wayland hiding a window requires destroying the surface, which
means destroying the winit window as well as the underlying graphics
surface. The latter is tricky as we have to keep the renderer around,
as our WindowAdapter trait's `renderer()` function returns a `&dyn
Renderer` and that also has to work without a window (to obtain text
metrics).

Fixes #4225

Co-Authored-By: Olivier Goffart <[email protected]>
  • Loading branch information
tronical and ogoffart committed Jul 3, 2024
1 parent 862b1e1 commit 49df131
Show file tree
Hide file tree
Showing 15 changed files with 881 additions and 307 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ members = [
'tests/driver/nodejs',
'tests/driver/rust',
'tests/screenshots',
'tests/manual/windowattributes',
'tools/compiler',
'tools/figma_import',
'tools/lsp',
Expand Down
4 changes: 3 additions & 1 deletion internal/backends/winit/accesskit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ impl NodeCollection {
fn focus_node(&self, window_adapter_weak: &Weak<WinitWindowAdapter>) -> NodeId {
window_adapter_weak
.upgrade()
.filter(|window_adapter| window_adapter.winit_window().has_focus())
.filter(|window_adapter| {
window_adapter.winit_window().map_or(false, |winit_window| winit_window.has_focus())
})
.and_then(|window_adapter| {
let window_inner = WindowInner::from_pub(window_adapter.window());
window_inner
Expand Down
27 changes: 22 additions & 5 deletions internal/backends/winit/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ pub(crate) trait EventLoopInterface {
window_attributes: winit::window::WindowAttributes,
) -> Result<winit::window::Window, winit::error::OsError>;
fn event_loop(&self) -> ActiveOrInactiveEventLoop<'_>;
fn is_wayland(&self) -> bool {
false
}
}

impl EventLoopInterface for NotRunningEventLoop {
Expand All @@ -106,6 +109,11 @@ impl EventLoopInterface for NotRunningEventLoop {
fn event_loop(&self) -> ActiveOrInactiveEventLoop<'_> {
ActiveOrInactiveEventLoop::Inactive(&self.instance)
}
#[cfg(all(unix, not(target_os = "macos"), feature = "wayland"))]
fn is_wayland(&self) -> bool {
use winit::platform::wayland::EventLoopExtWayland;
return self.instance.is_wayland();
}
}

impl<'a> EventLoopInterface for RunningEventLoop<'a> {
Expand All @@ -118,6 +126,11 @@ impl<'a> EventLoopInterface for RunningEventLoop<'a> {
fn event_loop(&self) -> ActiveOrInactiveEventLoop<'_> {
ActiveOrInactiveEventLoop::Active(self.active_event_loop)
}
#[cfg(all(unix, not(target_os = "macos"), feature = "wayland"))]
fn is_wayland(&self) -> bool {
use winit::platform::wayland::ActiveEventLoopExtWayland;
return self.active_event_loop.is_wayland();
}
}

thread_local! {
Expand Down Expand Up @@ -261,7 +274,7 @@ impl winit::application::ApplicationHandler<SlintUserEvent> for EventLoopState {
ALL_WINDOWS.with(|ws| {
for (_, window_weak) in ws.borrow().iter() {
if let Some(w) = window_weak.upgrade() {
if let Err(e) = w.renderer.resumed(w.winit_window()) {
if let Err(e) = w.ensure_window() {
self.loop_error = Some(e);
}
}
Expand All @@ -279,8 +292,12 @@ impl winit::application::ApplicationHandler<SlintUserEvent> for EventLoopState {
return;
};

#[cfg(enable_accesskit)]
window.accesskit_adapter.borrow_mut().process_event(&window.winit_window(), &event);
if let Some(winit_window) = window.winit_window() {
#[cfg(enable_accesskit)]
window.accesskit_adapter.borrow_mut().process_event(&winit_window, &event);
} else {
return;
}

let runtime_window = WindowInner::from_pub(window.window());
match event {
Expand Down Expand Up @@ -374,7 +391,7 @@ impl winit::application::ApplicationHandler<SlintUserEvent> for EventLoopState {
}
WindowEvent::CursorMoved { position, .. } => {
self.current_resize_direction = handle_cursor_move_for_resize(
window.winit_window().as_ref(),
&window.winit_window().unwrap(),
position,
self.current_resize_direction,
runtime_window
Expand Down Expand Up @@ -421,7 +438,7 @@ impl winit::application::ApplicationHandler<SlintUserEvent> for EventLoopState {
&& self.current_resize_direction.is_some()
{
handle_resize(
window.winit_window().as_ref(),
&window.winit_window().unwrap(),
self.current_resize_direction,
);
return;
Expand Down
79 changes: 40 additions & 39 deletions internal/backends/winit/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,18 @@ mod renderer {
fn render(&self, window: &i_slint_core::api::Window) -> Result<(), PlatformError>;

fn as_core_renderer(&self) -> &dyn i_slint_core::renderer::Renderer;

// Got WindowEvent::Occluded
fn occluded(&self, _: bool) {}

fn suspend(&self) -> Result<(), PlatformError>;

// Got winit::Event::Resumed
fn resumed(&self, _winit_window: Rc<winit::window::Window>) -> Result<(), PlatformError> {
Ok(())
}
fn resume(
&self,
window_attributes: winit::window::WindowAttributes,
) -> Result<Rc<winit::window::Window>, PlatformError>;

fn is_suspended(&self) -> bool;
}

#[cfg(feature = "renderer-femtovg")]
Expand All @@ -68,7 +72,8 @@ pub fn create_gl_window_with_canvas_id(
canvas_id: &str,
) -> Result<Rc<dyn WindowAdapter>, PlatformError> {
let attrs = WinitWindowAdapter::window_attributes(canvas_id)?;
let (renderer, window) = renderer::femtovg::GlutinFemtoVGRenderer::new(attrs)?;
let renderer = renderer::femtovg::GlutinFemtoVGRenderer::new_suspended();
let window = renderer.resume(attrs)?;
let adapter = WinitWindowAdapter::new(renderer, window);
Ok(adapter)
}
Expand All @@ -85,16 +90,14 @@ cfg_if::cfg_if! {
}
}

fn default_renderer_factory(
window_attributes: winit::window::WindowAttributes,
) -> Result<(Box<dyn WinitCompatibleRenderer>, Rc<winit::window::Window>), PlatformError> {
fn default_renderer_factory() -> Box<dyn WinitCompatibleRenderer> {
cfg_if::cfg_if! {
if #[cfg(enable_skia_renderer)] {
renderer::skia::WinitSkiaRenderer::new(window_attributes)
renderer::skia::WinitSkiaRenderer::new_suspended()
} else if #[cfg(feature = "renderer-femtovg")] {
renderer::femtovg::GlutinFemtoVGRenderer::new(window_attributes)
renderer::femtovg::GlutinFemtoVGRenderer::new_suspended()
} else if #[cfg(feature = "renderer-software")] {
renderer::sw::WinitSoftwareRenderer::new(window_attributes)
renderer::sw::WinitSoftwareRenderer::new_suspended()
} else {
compile_error!("Please select a feature to build with the winit backend: `renderer-femtovg`, `renderer-skia`, `renderer-skia-opengl`, `renderer-skia-vulkan` or `renderer-software`");
}
Expand All @@ -111,15 +114,16 @@ fn try_create_window_with_fallback_renderer(
feature = "renderer-skia-opengl",
feature = "renderer-skia-vulkan"
))]
renderer::skia::WinitSkiaRenderer::new,
renderer::skia::WinitSkiaRenderer::new_suspended,
#[cfg(feature = "renderer-femtovg")]
renderer::femtovg::GlutinFemtoVGRenderer::new,
renderer::femtovg::GlutinFemtoVGRenderer::new_suspended,
#[cfg(feature = "renderer-software")]
renderer::sw::WinitSoftwareRenderer::new,
renderer::sw::WinitSoftwareRenderer::new_suspended,
]
.into_iter()
.find_map(|renderer_factory| {
let (renderer, winit_window) = renderer_factory(attrs.clone()).ok()?;
let renderer = renderer_factory();
let winit_window = renderer.resume(attrs.clone()).ok()?;
Some(WinitWindowAdapter::new(
renderer,
winit_window,
Expand All @@ -146,11 +150,7 @@ pub mod native_widgets {}
/// slint::platform::set_platform(Box::new(Backend::new().unwrap()));
/// ```
pub struct Backend {
renderer_factory_fn:
fn(
window_builder: winit::window::WindowAttributes,
)
-> Result<(Box<dyn WinitCompatibleRenderer>, Rc<winit::window::Window>), PlatformError>,
renderer_factory_fn: fn() -> Box<dyn WinitCompatibleRenderer>,
event_loop_state: std::cell::RefCell<Option<crate::event_loop::EventLoopState>>,
proxy: winit::event_loop::EventLoopProxy<SlintUserEvent>,

Expand Down Expand Up @@ -195,15 +195,15 @@ impl Backend {

let renderer_factory_fn = match renderer_name {
#[cfg(feature = "renderer-femtovg")]
Some("gl") | Some("femtovg") => renderer::femtovg::GlutinFemtoVGRenderer::new,
Some("gl") | Some("femtovg") => renderer::femtovg::GlutinFemtoVGRenderer::new_suspended,
#[cfg(enable_skia_renderer)]
Some("skia") => renderer::skia::WinitSkiaRenderer::new,
Some("skia") => renderer::skia::WinitSkiaRenderer::new_suspended,
#[cfg(enable_skia_renderer)]
Some("skia-opengl") => renderer::skia::WinitSkiaRenderer::new_opengl,
Some("skia-opengl") => renderer::skia::WinitSkiaRenderer::new_opengl_suspended,
#[cfg(all(enable_skia_renderer, not(target_os = "android")))]
Some("skia-software") => renderer::skia::WinitSkiaRenderer::new_software,
Some("skia-software") => renderer::skia::WinitSkiaRenderer::new_software_suspended,
#[cfg(feature = "renderer-software")]
Some("sw") | Some("software") => renderer::sw::WinitSoftwareRenderer::new,
Some("sw") | Some("software") => renderer::sw::WinitSoftwareRenderer::new_suspended,
None => default_renderer_factory,
Some(renderer_name) => {
eprintln!(
Expand Down Expand Up @@ -266,8 +266,11 @@ impl i_slint_core::platform::Platform for Backend {
builder = hook(builder);
}

let adapter = (self.renderer_factory_fn)(builder.clone())
.map(|(renderer, window)| {
let renderer = (self.renderer_factory_fn)();

let adapter = renderer
.resume(builder.clone())
.map(|window| {
WinitWindowAdapter::new(
renderer,
window,
Expand Down Expand Up @@ -380,29 +383,27 @@ pub trait WinitWindowAccessor: private::WinitWindowAccessorSealed {

impl WinitWindowAccessor for i_slint_core::api::Window {
fn has_winit_window(&self) -> bool {
winit_window_rc_for_window(self).is_some()
i_slint_core::window::WindowInner::from_pub(self)
.window_adapter()
.internal(i_slint_core::InternalToken)
.and_then(|wa| wa.as_any().downcast_ref::<WinitWindowAdapter>())
.map_or(false, |adapter| adapter.winit_window().is_some())
}

fn with_winit_window<T>(
&self,
callback: impl FnOnce(&winit::window::Window) -> T,
) -> Option<T> {
winit_window_rc_for_window(self).as_ref().map(|w| callback(w))
i_slint_core::window::WindowInner::from_pub(self)
.window_adapter()
.internal(i_slint_core::InternalToken)
.and_then(|wa| wa.as_any().downcast_ref::<WinitWindowAdapter>())
.and_then(|adapter| adapter.winit_window().map(|w| callback(&w)))
}
}

impl private::WinitWindowAccessorSealed for i_slint_core::api::Window {}

fn winit_window_rc_for_window(
window: &i_slint_core::api::Window,
) -> Option<Rc<winit::window::Window>> {
i_slint_core::window::WindowInner::from_pub(window)
.window_adapter()
.internal(i_slint_core::InternalToken)
.and_then(|wa| wa.as_any().downcast_ref::<WinitWindowAdapter>())
.map(|adapter| adapter.winit_window())
}

#[cfg(test)]
mod testui {
slint::slint! {
Expand Down
37 changes: 27 additions & 10 deletions internal/backends/winit/renderer/femtovg.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright © SixtyFPS GmbH <[email protected]>
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0

use std::cell::Cell;
use std::rc::Rc;

use i_slint_core::platform::PlatformError;
Expand All @@ -17,12 +18,28 @@ mod glcontext;

pub struct GlutinFemtoVGRenderer {
renderer: FemtoVGRenderer,
suspended: Cell<bool>,
}

impl GlutinFemtoVGRenderer {
pub fn new(
pub fn new_suspended() -> Box<dyn WinitCompatibleRenderer> {
Box::new(Self { renderer: FemtoVGRenderer::new_suspended(), suspended: Cell::new(true) })
}
}

impl super::WinitCompatibleRenderer for GlutinFemtoVGRenderer {
fn render(&self, _window: &i_slint_core::api::Window) -> Result<(), PlatformError> {
self.renderer.render()
}

fn as_core_renderer(&self) -> &dyn Renderer {
&self.renderer
}

fn resume(
&self,
window_attributes: winit::window::WindowAttributes,
) -> Result<(Box<dyn WinitCompatibleRenderer>, Rc<winit::window::Window>), PlatformError> {
) -> Result<Rc<winit::window::Window>, PlatformError> {
#[cfg(not(target_arch = "wasm32"))]
let (winit_window, opengl_context) = crate::event_loop::with_window_target(|event_loop| {
Ok(glcontext::OpenGLContext::new_context(window_attributes, event_loop.event_loop())?)
Expand All @@ -39,7 +56,7 @@ impl GlutinFemtoVGRenderer {
})
})?);

let renderer = FemtoVGRenderer::new(
self.renderer.resume(
#[cfg(not(target_arch = "wasm32"))]
opengl_context,
#[cfg(target_arch = "wasm32")]
Expand All @@ -48,16 +65,16 @@ impl GlutinFemtoVGRenderer {
.ok_or_else(|| "FemtoVG Renderer: winit didn't return a canvas")?,
)?;

Ok((Box::new(Self { renderer }), winit_window))
self.suspended.set(false);

Ok(winit_window)
}
}

impl super::WinitCompatibleRenderer for GlutinFemtoVGRenderer {
fn render(&self, _window: &i_slint_core::api::Window) -> Result<(), PlatformError> {
self.renderer.render()
fn suspend(&self) -> Result<(), PlatformError> {
self.renderer.suspend()
}

fn as_core_renderer(&self) -> &dyn Renderer {
&self.renderer
fn is_suspended(&self) -> bool {
self.suspended.get()
}
}
Loading

0 comments on commit 49df131

Please sign in to comment.