Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clean up the Skia renderer API #2839

Merged
merged 1 commit into from
Jun 7, 2023
Merged

Clean up the Skia renderer API #2839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 15 additions & 14 deletions api/cpp/platform.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,18 +213,6 @@ type SkiaRendererOpaque = *const c_void;
type SkiaRenderer = i_slint_renderer_skia::SkiaRenderer;

struct CppRawHandle(RawWindowHandle, RawDisplayHandle);
// Safety: the C++ code should ensure that the handle is valid
unsafe impl raw_window_handle::HasRawWindowHandle for CppRawHandle {
fn raw_window_handle(&self) -> RawWindowHandle {
self.0
}
}
// Safety: the C++ code should ensure that the handle is valid
unsafe impl raw_window_handle::HasRawDisplayHandle for CppRawHandle {
fn raw_display_handle(&self) -> RawDisplayHandle {
self.1
}
}

// the raw handle type are #[non_exhaustive], so they can't be initialize with the convenient syntax. Work that around.
macro_rules! init_raw {
Expand Down Expand Up @@ -302,10 +290,23 @@ pub unsafe extern "C" fn slint_skia_renderer_new(
handle_opaque: CppRawHandleOpaque,
size: IntSize,
) -> SkiaRendererOpaque {
let handle = &*(handle_opaque as *const CppRawHandle);

// Safety: This is safe because the handle remains valid; the next rwh release provides `new()` without unsafe.
let active_handle = unsafe { raw_window_handle::ActiveHandle::new_unchecked() };

// Safety: the C++ code should ensure that the handle is valid
let (window_handle, display_handle) = unsafe {
(
raw_window_handle::WindowHandle::borrow_raw(handle.0, active_handle),
raw_window_handle::DisplayHandle::borrow_raw(handle.1),
)
};

let boxed_renderer: Box<SkiaRenderer> = Box::new(
SkiaRenderer::new(
&*(handle_opaque as *const CppRawHandle),
&*(handle_opaque as *const CppRawHandle),
window_handle,
display_handle,
PhysicalSize { width: size.width, height: size.height },
)
.unwrap(),
Expand Down
24 changes: 22 additions & 2 deletions internal/backends/winit/renderer/skia.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
use i_slint_core::api::PhysicalSize as PhysicalWindowSize;
use i_slint_core::platform::PlatformError;

use raw_window_handle::HasRawDisplayHandle;
use raw_window_handle::HasRawWindowHandle;

pub struct SkiaRenderer {
renderer: i_slint_renderer_skia::SkiaRenderer,
}
Expand Down Expand Up @@ -33,9 +36,26 @@ impl super::WinitCompatibleRenderer for SkiaRenderer {
)
})?;

// Safety: This is safe because the handle remains valid; the next rwh release provides `new()` without unsafe.
let active_handle = unsafe { raw_window_handle::ActiveHandle::new_unchecked() };

// Safety: API wise we can't guarantee that the window/display handles remain valid, so we
// use unsafe here. However the winit window adapter keeps the winit window alive as long as
// the renderer.
// TODO: remove once winit implements HasWindowHandle/HasDisplayHandle
let (window_handle, display_handle) = unsafe {
(
raw_window_handle::WindowHandle::borrow_raw(
winit_window.raw_window_handle(),
active_handle,
),
raw_window_handle::DisplayHandle::borrow_raw(winit_window.raw_display_handle()),
)
};

let renderer = i_slint_renderer_skia::SkiaRenderer::new(
&winit_window,
&winit_window,
window_handle,
display_handle,
PhysicalWindowSize::new(width, height),
)?;

Expand Down
20 changes: 14 additions & 6 deletions internal/renderers/skia/d3d_surface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use i_slint_core::api::PhysicalSize as PhysicalWindowSize;
use i_slint_core::platform::PlatformError;
use std::cell::RefCell;

use raw_window_handle::HasRawWindowHandle;

use winapi::{
shared::{dxgi, dxgi1_2, dxgi1_3, dxgi1_4, dxgiformat},
shared::{
Expand Down Expand Up @@ -72,7 +74,7 @@ impl SwapChain {
command_queue: ComPtr<d3d12::ID3D12CommandQueue>,
device: &ComPtr<d3d12::ID3D12Device>,
mut gr_context: skia_safe::gpu::DirectContext,
window: &dyn raw_window_handle::HasRawWindowHandle,
window_handle: raw_window_handle::WindowHandle<'_>,
size: PhysicalWindowSize,
dxgi_factory: &ComPtr<dxgi1_4::IDXGIFactory4>,
) -> Result<Self, PlatformError> {
Expand All @@ -87,7 +89,7 @@ impl SwapChain {
..Default::default()
};

let hwnd = match window.raw_window_handle() {
let hwnd = match window_handle.raw_window_handle() {
raw_window_handle::RawWindowHandle::Win32(raw_window_handle::Win32WindowHandle {
hwnd,
..
Expand Down Expand Up @@ -289,8 +291,8 @@ impl super::Surface for D3DSurface {
const SUPPORTS_GRAPHICS_API: bool = false;

fn new(
window: &dyn raw_window_handle::HasRawWindowHandle,
_display: &dyn raw_window_handle::HasRawDisplayHandle,
window_handle: raw_window_handle::WindowHandle<'_>,
_display_handle: raw_window_handle::DisplayHandle<'_>,
size: PhysicalWindowSize,
) -> Result<Self, i_slint_core::platform::PlatformError> {
let factory_flags = 0;
Expand Down Expand Up @@ -412,8 +414,14 @@ impl super::Surface for D3DSurface {
let gr_context = unsafe { skia_safe::gpu::DirectContext::new_d3d(&backend_context, None) }
.ok_or_else(|| format!("unable to create Skia D3D DirectContext"))?;

let swap_chain =
RefCell::new(SwapChain::new(queue, &device, gr_context, &window, size, &dxgi_factory)?);
let swap_chain = RefCell::new(SwapChain::new(
queue,
&device,
gr_context,
window_handle,
size,
&dxgi_factory,
)?);

Ok(Self { swap_chain })
}
Expand Down
10 changes: 5 additions & 5 deletions internal/renderers/skia/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ pub struct SkiaRenderer {
impl SkiaRenderer {
/// Creates a new renderer is associated with the provided window adapter.
pub fn new(
native_window: &impl raw_window_handle::HasRawWindowHandle,
native_display: &impl raw_window_handle::HasRawDisplayHandle,
window_handle: raw_window_handle::WindowHandle<'_>,
display_handle: raw_window_handle::DisplayHandle<'_>,
size: PhysicalWindowSize,
) -> Result<Self, PlatformError> {
let surface = DefaultSurface::new(&native_window, &native_display, size)?;
let surface = DefaultSurface::new(window_handle, display_handle, size)?;

Ok(Self {
rendering_notifier: Default::default(),
Expand Down Expand Up @@ -350,8 +350,8 @@ impl i_slint_core::renderer::Renderer for SkiaRenderer {
trait Surface {
const SUPPORTS_GRAPHICS_API: bool;
fn new(
window: &dyn raw_window_handle::HasRawWindowHandle,
display: &dyn raw_window_handle::HasRawDisplayHandle,
window_handle: raw_window_handle::WindowHandle<'_>,
display_handle: raw_window_handle::DisplayHandle<'_>,
size: PhysicalWindowSize,
) -> Result<Self, PlatformError>
where
Expand Down
7 changes: 4 additions & 3 deletions internal/renderers/skia/metal_surface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use i_slint_core::api::PhysicalSize as PhysicalWindowSize;
use metal::MTLPixelFormat;
use objc::{rc::autoreleasepool, runtime::YES};

use raw_window_handle::HasRawWindowHandle;
use skia_safe::gpu::mtl;

use std::cell::RefCell;
Expand All @@ -22,8 +23,8 @@ impl super::Surface for MetalSurface {
const SUPPORTS_GRAPHICS_API: bool = false;

fn new(
window: &dyn raw_window_handle::HasRawWindowHandle,
_display: &dyn raw_window_handle::HasRawDisplayHandle,
window_handle: raw_window_handle::WindowHandle<'_>,
_display_handle: raw_window_handle::DisplayHandle<'_>,
size: PhysicalWindowSize,
) -> Result<Self, i_slint_core::platform::PlatformError> {
let device = metal::Device::system_default()
Expand All @@ -38,7 +39,7 @@ impl super::Surface for MetalSurface {
layer.set_drawable_size(CGSize::new(size.width as f64, size.height as f64));

unsafe {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detail: the unsafe could probably have a smaller scope (feel free to ignore)

let view = match window.raw_window_handle() {
let view = match window_handle.raw_window_handle() {
raw_window_handle::RawWindowHandle::AppKit(
raw_window_handle::AppKitWindowHandle { ns_view, .. },
) => ns_view,
Expand Down
27 changes: 14 additions & 13 deletions internal/renderers/skia/opengl_surface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use glutin::{
};
use i_slint_core::api::PhysicalSize as PhysicalWindowSize;
use i_slint_core::{api::GraphicsAPI, platform::PlatformError};
use raw_window_handle::{HasRawDisplayHandle, HasRawWindowHandle};

pub struct OpenGLSurface {
fb_info: skia_safe::gpu::gl::FramebufferInfo,
Expand All @@ -25,8 +26,8 @@ impl super::Surface for OpenGLSurface {
const SUPPORTS_GRAPHICS_API: bool = true;

fn new(
window: &dyn raw_window_handle::HasRawWindowHandle,
display: &dyn raw_window_handle::HasRawDisplayHandle,
window_handle: raw_window_handle::WindowHandle<'_>,
display_handle: raw_window_handle::DisplayHandle<'_>,
size: PhysicalWindowSize,
) -> Result<Self, PlatformError> {
let width: std::num::NonZeroU32 = size.width.try_into().map_err(|_| {
Expand All @@ -37,7 +38,7 @@ impl super::Surface for OpenGLSurface {
})?;

let (current_glutin_context, glutin_surface) =
Self::init_glutin(window, display, width, height)?;
Self::init_glutin(window_handle, display_handle, width, height)?;

let fb_info = {
use glow::HasContext;
Expand Down Expand Up @@ -188,8 +189,8 @@ impl super::Surface for OpenGLSurface {

impl OpenGLSurface {
fn init_glutin(
_window: &dyn raw_window_handle::HasRawWindowHandle,
_display: &dyn raw_window_handle::HasRawDisplayHandle,
_window_handle: raw_window_handle::WindowHandle<'_>,
_display_handle: raw_window_handle::DisplayHandle<'_>,
width: NonZeroU32,
height: NonZeroU32,
) -> Result<
Expand All @@ -207,15 +208,15 @@ impl OpenGLSurface {
} else if #[cfg(not(target_family = "windows"))] {
let prefs = [glutin::display::DisplayApiPreference::Egl];
} else {
let prefs = [glutin::display::DisplayApiPreference::EglThenWgl(Some(_window.raw_window_handle()))];
let prefs = [glutin::display::DisplayApiPreference::EglThenWgl(Some(_window_handle))];
}
}

let try_create_surface =
|display_api_preference| -> Result<(_, _), Box<dyn std::error::Error>> {
let gl_display = unsafe {
glutin::display::Display::new(
_display.raw_display_handle(),
_display_handle.raw_display_handle(),
display_api_preference,
)?
};
Expand All @@ -233,8 +234,8 @@ impl OpenGLSurface {

// Upstream advises to use this only on Windows.
#[cfg(target_family = "windows")]
let config_template_builder = config_template_builder
.compatible_with_native_window(_window.raw_window_handle());
let config_template_builder =
config_template_builder.compatible_with_native_window(_window_handle);

let config_template = config_template_builder.build();

Expand All @@ -260,10 +261,10 @@ impl OpenGLSurface {
major: 2,
minor: 0,
})))
.build(Some(_window.raw_window_handle()));
.build(Some(_window_handle.raw_window_handle()));

let fallback_context_attributes =
ContextAttributesBuilder::new().build(Some(_window.raw_window_handle()));
ContextAttributesBuilder::new().build(Some(_window_handle.raw_window_handle()));

let not_current_gl_context = unsafe {
gl_display.create_context(&config, &gles_context_attributes).or_else(|_| {
Expand All @@ -272,7 +273,7 @@ impl OpenGLSurface {
};

let attrs = SurfaceAttributesBuilder::<WindowSurface>::new().build(
_window.raw_window_handle(),
_window_handle.raw_window_handle(),
width,
height,
);
Expand Down Expand Up @@ -307,7 +308,7 @@ impl OpenGLSurface {
if let raw_window_handle::RawWindowHandle::AppKit(raw_window_handle::AppKitWindowHandle {
ns_view,
..
}) = _window.raw_window_handle()
}) = _window_handle.raw_window_handle()
{
use cocoa::appkit::NSView;
let view_id: cocoa::base::id = ns_view as *const _ as *mut _;
Expand Down
15 changes: 9 additions & 6 deletions internal/renderers/skia/vulkan_surface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ use vulkano::swapchain::{
use vulkano::sync::{FlushError, GpuFuture};
use vulkano::{sync, Handle, VulkanLibrary, VulkanObject};

use raw_window_handle::HasRawDisplayHandle;
use raw_window_handle::HasRawWindowHandle;

pub struct VulkanSurface {
gr_context: RefCell<skia_safe::gpu::DirectContext>,
recreate_swapchain: Cell<bool>,
Expand All @@ -34,8 +37,8 @@ impl super::Surface for VulkanSurface {
const SUPPORTS_GRAPHICS_API: bool = false;

fn new(
window: &dyn raw_window_handle::HasRawWindowHandle,
display: &dyn raw_window_handle::HasRawDisplayHandle,
window_handle: raw_window_handle::WindowHandle<'_>,
display_handle: raw_window_handle::DisplayHandle<'_>,
size: PhysicalWindowSize,
) -> Result<Self, i_slint_core::platform::PlatformError> {
let library = VulkanLibrary::new()
Expand Down Expand Up @@ -65,7 +68,7 @@ impl super::Surface for VulkanSurface {
)
.map_err(|instance_err| format!("Error creating Vulkan instance: {instance_err}"))?;

let surface = create_surface(&instance, window, display)
let surface = create_surface(&instance, window_handle, display_handle)
.map_err(|surface_err| format!("Error creating Vulkan surface: {surface_err}"))?;

let device_extensions =
Expand Down Expand Up @@ -361,10 +364,10 @@ impl super::Surface for VulkanSurface {

fn create_surface(
instance: &Arc<Instance>,
window: &dyn raw_window_handle::HasRawWindowHandle,
display: &dyn raw_window_handle::HasRawDisplayHandle,
window_handle: raw_window_handle::WindowHandle<'_>,
display_handle: raw_window_handle::DisplayHandle<'_>,
) -> Result<Arc<Surface>, vulkano::swapchain::SurfaceCreationError> {
match (window.raw_window_handle(), display.raw_display_handle()) {
match (window_handle.raw_window_handle(), display_handle.raw_display_handle()) {
#[cfg(target_os = "macos")]
(
raw_window_handle::RawWindowHandle::AppKit(raw_window_handle::AppKitWindowHandle {
Expand Down