chore: Support multiple render backends #2359
Conversation
fredizzimo
force-pushed
the
fsundvik/support-render-backend
branch
3 times, most recently
from
31deb86
to
038b995
Compare
fredizzimo
force-pushed
the
fsundvik/support-render-backend
branch
2 times, most recently
from
70fc54a
to
110ea99
Compare
| use crate::profiling::{opengl::create_opengl_gpu_context, GpuCtx}; | ||
|
|
||
| pub struct OpenGLSkiaRenderer { | ||
| // NOTE: The destruction order is important, so don't re-arrange |
There was a problem hiding this comment.
Maybe a SAFETY instead of NOTE could fit better in order to convey that a critical invariant is upheld here.
There was a problem hiding this comment.
I'll keep the NOTE: the syntax highlighter highligts it, but not SAFETY:
| NonZeroU32::new(size.width).unwrap(), | ||
| NonZeroU32::new(size.height).unwrap(), |
There was a problem hiding this comment.
Those unwraps might very well blow up if starting in e.g. an initially minimized state, need to try that out.
There was a problem hiding this comment.
The code is just moved, so it has been the same before. clamp_render_buffer_size should ensure that it's not zero.
|
|
||
| let interface = skia_safe::gpu::gl::Interface::new_load_with(|name| { | ||
| if name == "eglGetCurrentDisplay" { | ||
| return std::ptr::null(); |
There was a problem hiding this comment.
Why is eglGetCurrentDisplay filtered out? Can't we let get_proc_address also find eglGetCurrentDisplay? (possibly this is a peculiarity of EGL, I've never used EGL)
There was a problem hiding this comment.
I have no idea, it was introduced here
After some discussion here
- OpenGL Renderer #491 (comment)
- [Investigation] Loading eglGetCurrentDisplay function pointer causes segfault #661
I found no issues in the glutin repository.
src/renderer/vsync/mod.rs
Outdated
| #[cfg(target_os = "linux")] | ||
| use std::env; |
There was a problem hiding this comment.
Fwiw due to the moved detection logic, this import is actually unused and yields a warning
warning: unused import: `std::env`
--> src/renderer/vsync/mod.rs:17:5
|
17 | use std::env;
| ^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by defaultThere was a problem hiding this comment.
That is now fixed. I was doing most of my testing on Windows, and that was part of one late change that never got properly tested on Linux.
|
Test results:
|
|
After retrying a few times between main and this branch on Xwayland, I can't seem to reproduce what I found initially, and found no meaningful differences. Ignore that then. |
|
I'm not sure about the XWayland freeze, I can't think of any changes that could cause it, except this one 110ea99, but that should not affect x11 or xwayland at all. The rest of the code should have stayed functionally the same. |
fredizzimo
force-pushed
the
fsundvik/support-render-backend
branch
from
110ea99
to
8c2e80c
Compare
fredizzimo
force-pushed
the
fsundvik/support-render-backend
branch
from
8c2e80c
to
dda3c83
Compare
But currently only OpenGL is supported.
It does not work with Windows swap chains, and might cause problems on Wayland as well.
fredizzimo
force-pushed
the
fsundvik/support-render-backend
branch
from
dda3c83
to
4dabee7
Compare
What kind of change does this PR introduce?
This is mostly a refactoring in order to support multiple render backends, like D3D, and makes the actual PR that implements the D3D backend much smaller.
SkiaRenderer, which can have different implementations depending on the platformopenglimplementationVSyncWinhas been renamed toVSyncWinDwmto allow a swapchain implementation for D3D.Did this PR introduce a breaking change?