From 89c051cccc1dd575479138387f91af556c5d05b7 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Mon, 7 Feb 2022 00:55:12 +0100 Subject: [PATCH 01/22] Add error checking on X11 window creation, and fix parented X11 window creation (#113) --- src/x11/window.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/x11/window.rs b/src/x11/window.rs index 5c3b7c7..8d3ba82 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -206,7 +206,7 @@ impl Window { let window_info = WindowInfo::from_logical_size(options.size, scaling); let window_id = xcb_connection.conn.generate_id(); - xcb::create_window( + xcb::create_window_checked( &xcb_connection.conn, xcb::COPY_FROM_PARENT as u8, window_id, @@ -217,7 +217,7 @@ impl Window { window_info.physical_size().height as u16, // window height 0, // window border xcb::WINDOW_CLASS_INPUT_OUTPUT as u16, - screen.root_visual(), + if parent.is_some() { xcb::COPY_FROM_PARENT as u32 } else { screen.root_visual() }, &[( xcb::CW_EVENT_MASK, xcb::EVENT_MASK_EXPOSURE @@ -228,7 +228,7 @@ impl Window { | xcb::EVENT_MASK_KEY_RELEASE | xcb::EVENT_MASK_STRUCTURE_NOTIFY, )], - ); + ).request_check().unwrap(); xcb::map_window(&xcb_connection.conn, window_id); From a8010016fb92a4b9307da3fbe5d4d97c873feb0e Mon Sep 17 00:00:00 2001 From: George Atkinson Date: Sun, 6 Feb 2022 23:58:11 +0000 Subject: [PATCH 02/22] Bump dependencies (#100) --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d781e3c..e335061 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,14 +14,14 @@ edition = "2018" license = "MIT OR Apache-2.0" [dependencies] -keyboard-types = { version = "0.5.0", default-features = false } +keyboard-types = { version = "0.6.1", default-features = false } raw-window-handle = "0.3.3" [target.'cfg(target_os="linux")'.dependencies] xcb = { version = "0.9", features = ["thread", "xlib_xcb", "dri2"] } x11 = { version = "2.18", features = ["xlib", "xcursor"] } xcb-util = { version = "0.3", features = ["icccm"] } -nix = "0.18" +nix = "0.22.0" [target.'cfg(target_os="windows")'.dependencies] winapi = { version = "0.3.8", features = ["libloaderapi", "winuser", "windef", "minwindef", "guiddef", "combaseapi", "wingdi", "errhandlingapi"] } From 625fde7957c5bcd7f1805102abb2be26b9f01a42 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 13:13:28 +0100 Subject: [PATCH 03/22] Create X11 windows with a depth of 32-bits This should fix the inability to create OpenGL contexts with alpha channels in raw-gl-context, but it seems like that still needs a bit more work. --- src/x11/window.rs | 63 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/src/x11/window.rs b/src/x11/window.rs index 8d3ba82..2041623 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -205,10 +205,39 @@ impl Window { let window_info = WindowInfo::from_logical_size(options.size, scaling); + // The window need to have a depth of 32 so so we can create graphics contexts with alpha + // buffers + let mut depth = xcb::COPY_FROM_PARENT as u8; + let mut visual = xcb::COPY_FROM_PARENT as u32; + 'match_visual: for candidate_depth in screen.allowed_depths() { + if candidate_depth.depth() != 32 { + continue; + } + + for candidate_visual in candidate_depth.visuals() { + if candidate_visual.class() == xcb::VISUAL_CLASS_TRUE_COLOR as u8 { + depth = candidate_depth.depth(); + visual = candidate_visual.visual_id(); + break 'match_visual; + } + } + } + + // For this 32-bith depth to work, you also need to define a color map and set a border + // pixel: https://cgit.freedesktop.org/xorg/xserver/tree/dix/window.c#n818 + let colormap = xcb_connection.conn.generate_id(); + xcb::create_colormap( + &xcb_connection.conn, + xcb::COLORMAP_ALLOC_NONE as u8, + colormap, + screen.root(), + visual, + ); + let window_id = xcb_connection.conn.generate_id(); xcb::create_window_checked( &xcb_connection.conn, - xcb::COPY_FROM_PARENT as u8, + depth, window_id, parent_id, 0, // x coordinate of the new window @@ -217,18 +246,26 @@ impl Window { window_info.physical_size().height as u16, // window height 0, // window border xcb::WINDOW_CLASS_INPUT_OUTPUT as u16, - if parent.is_some() { xcb::COPY_FROM_PARENT as u32 } else { screen.root_visual() }, - &[( - xcb::CW_EVENT_MASK, - xcb::EVENT_MASK_EXPOSURE - | xcb::EVENT_MASK_POINTER_MOTION - | xcb::EVENT_MASK_BUTTON_PRESS - | xcb::EVENT_MASK_BUTTON_RELEASE - | xcb::EVENT_MASK_KEY_PRESS - | xcb::EVENT_MASK_KEY_RELEASE - | xcb::EVENT_MASK_STRUCTURE_NOTIFY, - )], - ).request_check().unwrap(); + visual, + &[ + ( + xcb::CW_EVENT_MASK, + xcb::EVENT_MASK_EXPOSURE + | xcb::EVENT_MASK_POINTER_MOTION + | xcb::EVENT_MASK_BUTTON_PRESS + | xcb::EVENT_MASK_BUTTON_RELEASE + | xcb::EVENT_MASK_KEY_PRESS + | xcb::EVENT_MASK_KEY_RELEASE + | xcb::EVENT_MASK_STRUCTURE_NOTIFY, + ), + // As mentioend above, these two values are needed to be able to create a window + // with a dpeth of 32-bits when the parent window has a different depth + (xcb::CW_COLORMAP, colormap), + (xcb::CW_BORDER_PIXEL, 0), + ], + ) + .request_check() + .unwrap(); xcb::map_window(&xcb_connection.conn, window_id); From d76b02df44bc4c3739256c7ab88f4b581e412f2c Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 17:13:51 +0100 Subject: [PATCH 04/22] Upgrade to raw-window-handle 0.4.x The main change is that all of these types are simplified, there are more different OS-specific window handle types, and they are no longer gated behind the respective targets which makes the library a bit easier to use for applications. --- Cargo.toml | 3 ++- src/macos/window.rs | 16 ++++++++-------- src/win/window.rs | 22 +++++++++++----------- src/x11/window.rs | 14 +++++++------- 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e335061..f45bdc3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,13 +9,14 @@ authors = [ "Billy Messenger ", "Anton Lazarev ", "Joakim Frostegård ", + "Robbert van der Helm ", ] edition = "2018" license = "MIT OR Apache-2.0" [dependencies] keyboard-types = { version = "0.6.1", default-features = false } -raw-window-handle = "0.3.3" +raw-window-handle = "0.4.2" [target.'cfg(target_os="linux")'.dependencies] xcb = { version = "0.9", features = ["thread", "xlib_xcb", "dri2"] } diff --git a/src/macos/window.rs b/src/macos/window.rs index 5b71b6f..bfe8316 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -16,7 +16,7 @@ use keyboard_types::KeyboardEvent; use objc::{msg_send, runtime::Object, sel, sel_impl}; -use raw_window_handle::{macos::MacOSHandle, HasRawWindowHandle, RawWindowHandle}; +use raw_window_handle::{AppKitHandle, HasRawWindowHandle, RawWindowHandle}; use crate::{ Event, EventStatus, WindowEvent, WindowHandler, WindowInfo, WindowOpenOptions, @@ -55,7 +55,7 @@ unsafe impl HasRawWindowHandle for WindowHandle { } } - RawWindowHandle::MacOS(MacOSHandle { ..MacOSHandle::empty() }) + RawWindowHandle::AppKit(AppKitHandle::empty()) } } @@ -114,7 +114,7 @@ impl Window { { let pool = unsafe { NSAutoreleasePool::new(nil) }; - let handle = if let RawWindowHandle::MacOS(handle) = parent.raw_window_handle() { + let handle = if let RawWindowHandle::AppKit(handle) = parent.raw_window_handle() { handle } else { panic!("Not a macOS window"); @@ -388,10 +388,10 @@ unsafe impl HasRawWindowHandle for Window { fn raw_window_handle(&self) -> RawWindowHandle { let ns_window = self.ns_window.unwrap_or(::std::ptr::null_mut()) as *mut c_void; - RawWindowHandle::MacOS(MacOSHandle { - ns_window, - ns_view: self.ns_view as *mut c_void, - ..MacOSHandle::empty() - }) + let mut handle = AppKitHandle::empty(); + handle.ns_window = ns_window; + handle.ns_view = self.ns_view as *mut c_void; + + RawWindowHandle::AppKit(handle) } } diff --git a/src/win/window.rs b/src/win/window.rs index f91ac17..08720f0 100644 --- a/src/win/window.rs +++ b/src/win/window.rs @@ -23,7 +23,7 @@ use std::ptr::null_mut; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; -use raw_window_handle::{windows::WindowsHandle, HasRawWindowHandle, RawWindowHandle}; +use raw_window_handle::{HasRawWindowHandle, RawWindowHandle, Win32Handle}; const BV_WINDOW_MUST_CLOSE: UINT = WM_USER + 1; @@ -80,12 +80,12 @@ impl WindowHandle { unsafe impl HasRawWindowHandle for WindowHandle { fn raw_window_handle(&self) -> RawWindowHandle { if let Some(hwnd) = self.hwnd { - RawWindowHandle::Windows(WindowsHandle { - hwnd: hwnd as *mut std::ffi::c_void, - ..WindowsHandle::empty() - }) + let mut handle = Win32Handle::empty(); + handle.hwnd = hwnd as *mut std::ffi::c_void; + + RawWindowHandle::Win32(handle) } else { - RawWindowHandle::Windows(WindowsHandle { ..WindowsHandle::empty() }) + RawWindowHandle::Win32(Win32Handle::empty()) } } } @@ -372,7 +372,7 @@ impl Window { B: Send + 'static, { let parent = match parent.raw_window_handle() { - RawWindowHandle::Windows(h) => h.hwnd as HWND, + RawWindowHandle::Win32(h) => h.hwnd as HWND, h => panic!("unsupported parent handle {:?}", h), }; @@ -560,9 +560,9 @@ impl Window { unsafe impl HasRawWindowHandle for Window { fn raw_window_handle(&self) -> RawWindowHandle { - RawWindowHandle::Windows(WindowsHandle { - hwnd: self.hwnd as *mut std::ffi::c_void, - ..WindowsHandle::empty() - }) + let mut handle = Win32Handle::empty(); + handle.hwnd = self.hwnd as *mut std::ffi::c_void; + + RawWindowHandle::Win32(handle) } } diff --git a/src/x11/window.rs b/src/x11/window.rs index 2041623..f9cff75 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use std::thread; use std::time::*; -use raw_window_handle::{unix::XlibHandle, HasRawWindowHandle, RawWindowHandle}; +use raw_window_handle::{HasRawWindowHandle, RawWindowHandle, XlibHandle}; use super::XcbConnection; use crate::{ @@ -49,7 +49,7 @@ unsafe impl HasRawWindowHandle for WindowHandle { } } - RawWindowHandle::Xlib(XlibHandle { ..raw_window_handle::unix::XlibHandle::empty() }) + RawWindowHandle::Xlib(XlibHandle::empty()) } } @@ -595,11 +595,11 @@ impl Window { unsafe impl HasRawWindowHandle for Window { fn raw_window_handle(&self) -> RawWindowHandle { - RawWindowHandle::Xlib(XlibHandle { - window: self.window_id as c_ulong, - display: self.xcb_connection.conn.get_raw_dpy() as *mut c_void, - ..raw_window_handle::unix::XlibHandle::empty() - }) + let mut handle = XlibHandle::empty(); + handle.window = self.window_id as c_ulong; + handle.display = self.xcb_connection.conn.get_raw_dpy() as *mut c_void; + + RawWindowHandle::Xlib(handle) } } From 45e29b48911abe2a8f51195ad487e98e12ff1135 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 17:18:16 +0100 Subject: [PATCH 05/22] Add the entire tree from raw-gl-handle Including @prokopyl's PR that adds more X11 error handling: https://github.com/glowcoil/raw-gl-context/pull/15 Commit: prokopyl@raw-gl-context@98cd3cf1104ee254a67e3fed30e4e6b2ae2b6821 (with `cargo fmt`) Branch base: RustAudio/baseview@b68a05b4dc02a98a7c0c0f679a8e94fa528a3517 --- src/gl/lib.rs | 100 ++++++++++++++ src/gl/macos.rs | 147 ++++++++++++++++++++ src/gl/win.rs | 312 +++++++++++++++++++++++++++++++++++++++++++ src/gl/x11.rs | 231 ++++++++++++++++++++++++++++++++ src/gl/x11/errors.rs | 131 ++++++++++++++++++ 5 files changed, 921 insertions(+) create mode 100644 src/gl/lib.rs create mode 100644 src/gl/macos.rs create mode 100644 src/gl/win.rs create mode 100644 src/gl/x11.rs create mode 100644 src/gl/x11/errors.rs diff --git a/src/gl/lib.rs b/src/gl/lib.rs new file mode 100644 index 0000000..9c43878 --- /dev/null +++ b/src/gl/lib.rs @@ -0,0 +1,100 @@ +use raw_window_handle::HasRawWindowHandle; + +use std::ffi::c_void; +use std::marker::PhantomData; + +#[cfg(target_os = "windows")] +mod win; +#[cfg(target_os = "windows")] +use win as platform; + +#[cfg(target_os = "linux")] +mod x11; +#[cfg(target_os = "linux")] +use crate::x11 as platform; + +#[cfg(target_os = "macos")] +mod macos; +#[cfg(target_os = "macos")] +use macos as platform; + +#[derive(Clone, Debug)] +pub struct GlConfig { + pub version: (u8, u8), + pub profile: Profile, + pub red_bits: u8, + pub blue_bits: u8, + pub green_bits: u8, + pub alpha_bits: u8, + pub depth_bits: u8, + pub stencil_bits: u8, + pub samples: Option, + pub srgb: bool, + pub double_buffer: bool, + pub vsync: bool, +} + +impl Default for GlConfig { + fn default() -> Self { + GlConfig { + version: (3, 2), + profile: Profile::Core, + red_bits: 8, + blue_bits: 8, + green_bits: 8, + alpha_bits: 8, + depth_bits: 24, + stencil_bits: 8, + samples: None, + srgb: true, + double_buffer: true, + vsync: false, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Profile { + Compatibility, + Core, +} + +#[derive(Debug)] +pub enum GlError { + InvalidWindowHandle, + VersionNotSupported, + CreationFailed(platform::CreationFailedError), +} + +pub struct GlContext { + context: platform::GlContext, + phantom: PhantomData<*mut ()>, +} + +impl GlContext { + pub unsafe fn create( + parent: &impl HasRawWindowHandle, + config: GlConfig, + ) -> Result { + platform::GlContext::create(parent, config).map(|context| GlContext { + context, + phantom: PhantomData, + }) + } + + pub unsafe fn make_current(&self) { + self.context.make_current(); + } + + pub unsafe fn make_not_current(&self) { + self.context.make_not_current(); + } + + pub fn get_proc_address(&self, symbol: &str) -> *const c_void { + self.context.get_proc_address(symbol) + } + + pub fn swap_buffers(&self) { + self.context.swap_buffers(); + } +} diff --git a/src/gl/macos.rs b/src/gl/macos.rs new file mode 100644 index 0000000..28f56c1 --- /dev/null +++ b/src/gl/macos.rs @@ -0,0 +1,147 @@ +use std::ffi::c_void; +use std::str::FromStr; + +use raw_window_handle::{HasRawWindowHandle, RawWindowHandle}; + +use cocoa::appkit::{ + NSOpenGLContext, NSOpenGLContextParameter, NSOpenGLPFAAccelerated, NSOpenGLPFAAlphaSize, + NSOpenGLPFAColorSize, NSOpenGLPFADepthSize, NSOpenGLPFADoubleBuffer, NSOpenGLPFAMultisample, + NSOpenGLPFAOpenGLProfile, NSOpenGLPFASampleBuffers, NSOpenGLPFASamples, NSOpenGLPFAStencilSize, + NSOpenGLPixelFormat, NSOpenGLProfileVersion3_2Core, NSOpenGLProfileVersion4_1Core, + NSOpenGLProfileVersionLegacy, NSOpenGLView, NSView, +}; +use cocoa::base::{id, nil, YES}; + +use core_foundation::base::TCFType; +use core_foundation::bundle::{CFBundleGetBundleWithIdentifier, CFBundleGetFunctionPointerForName}; +use core_foundation::string::CFString; + +use objc::{msg_send, sel, sel_impl}; + +use crate::{GlConfig, GlError, Profile}; + +pub type CreationFailedError = (); +pub struct GlContext { + view: id, + context: id, +} + +impl GlContext { + pub unsafe fn create( + parent: &impl HasRawWindowHandle, + config: GlConfig, + ) -> Result { + let handle = if let RawWindowHandle::MacOS(handle) = parent.raw_window_handle() { + handle + } else { + return Err(GlError::InvalidWindowHandle); + }; + + if handle.ns_view.is_null() { + return Err(GlError::InvalidWindowHandle); + } + + let parent_view = handle.ns_view as id; + + let version = if config.version < (3, 2) && config.profile == Profile::Compatibility { + NSOpenGLProfileVersionLegacy + } else if config.version == (3, 2) && config.profile == Profile::Core { + NSOpenGLProfileVersion3_2Core + } else if config.version > (3, 2) && config.profile == Profile::Core { + NSOpenGLProfileVersion4_1Core + } else { + return Err(GlError::VersionNotSupported); + }; + + #[rustfmt::skip] + let mut attrs = vec![ + NSOpenGLPFAOpenGLProfile as u32, version as u32, + NSOpenGLPFAColorSize as u32, (config.red_bits + config.blue_bits + config.green_bits) as u32, + NSOpenGLPFAAlphaSize as u32, config.alpha_bits as u32, + NSOpenGLPFADepthSize as u32, config.depth_bits as u32, + NSOpenGLPFAStencilSize as u32, config.stencil_bits as u32, + NSOpenGLPFAAccelerated as u32, + ]; + + if config.samples.is_some() { + #[rustfmt::skip] + attrs.extend_from_slice(&[ + NSOpenGLPFAMultisample as u32, + NSOpenGLPFASampleBuffers as u32, 1, + NSOpenGLPFASamples as u32, config.samples.unwrap() as u32, + ]); + } + + if config.double_buffer { + attrs.push(NSOpenGLPFADoubleBuffer as u32); + } + + attrs.push(0); + + let pixel_format = NSOpenGLPixelFormat::alloc(nil).initWithAttributes_(&attrs); + + if pixel_format == nil { + return Err(GlError::CreationFailed(())); + } + + let view = + NSOpenGLView::alloc(nil).initWithFrame_pixelFormat_(parent_view.frame(), pixel_format); + + if view == nil { + return Err(GlError::CreationFailed(())); + } + + view.setWantsBestResolutionOpenGLSurface_(YES); + + let () = msg_send![view, retain]; + NSOpenGLView::display_(view); + parent_view.addSubview_(view); + + let context: id = msg_send![view, openGLContext]; + let () = msg_send![context, retain]; + + context.setValues_forParameter_( + &(config.vsync as i32), + NSOpenGLContextParameter::NSOpenGLCPSwapInterval, + ); + + let () = msg_send![pixel_format, release]; + + Ok(GlContext { view, context }) + } + + pub unsafe fn make_current(&self) { + self.context.makeCurrentContext(); + } + + pub unsafe fn make_not_current(&self) { + NSOpenGLContext::clearCurrentContext(self.context); + } + + pub fn get_proc_address(&self, symbol: &str) -> *const c_void { + let symbol_name = CFString::from_str(symbol).unwrap(); + let framework_name = CFString::from_str("com.apple.opengl").unwrap(); + let framework = + unsafe { CFBundleGetBundleWithIdentifier(framework_name.as_concrete_TypeRef()) }; + let addr = unsafe { + CFBundleGetFunctionPointerForName(framework, symbol_name.as_concrete_TypeRef()) + }; + addr as *const c_void + } + + pub fn swap_buffers(&self) { + unsafe { + self.context.flushBuffer(); + let () = msg_send![self.view, setNeedsDisplay: YES]; + } + } +} + +impl Drop for GlContext { + fn drop(&mut self) { + unsafe { + let () = msg_send![self.context, release]; + let () = msg_send![self.view, release]; + } + } +} diff --git a/src/gl/win.rs b/src/gl/win.rs new file mode 100644 index 0000000..5594ae9 --- /dev/null +++ b/src/gl/win.rs @@ -0,0 +1,312 @@ +use std::ffi::{c_void, CString, OsStr}; +use std::os::windows::ffi::OsStrExt; + +use raw_window_handle::{HasRawWindowHandle, RawWindowHandle}; + +use winapi::shared::minwindef::{HINSTANCE, HMODULE}; +use winapi::shared::ntdef::WCHAR; +use winapi::shared::windef::{HDC, HGLRC, HWND}; +use winapi::um::libloaderapi::{FreeLibrary, GetProcAddress, LoadLibraryA}; +use winapi::um::wingdi::{ + wglCreateContext, wglDeleteContext, wglGetProcAddress, wglMakeCurrent, ChoosePixelFormat, + DescribePixelFormat, SetPixelFormat, SwapBuffers, PFD_DOUBLEBUFFER, PFD_DRAW_TO_WINDOW, + PFD_MAIN_PLANE, PFD_SUPPORT_OPENGL, PFD_TYPE_RGBA, PIXELFORMATDESCRIPTOR, +}; +use winapi::um::winnt::IMAGE_DOS_HEADER; +use winapi::um::winuser::{ + CreateWindowExW, DefWindowProcW, DestroyWindow, GetDC, RegisterClassW, ReleaseDC, + UnregisterClassW, CS_OWNDC, CW_USEDEFAULT, WNDCLASSW, +}; + +use crate::{GlConfig, GlError, Profile}; + +// See https://www.khronos.org/registry/OpenGL/extensions/ARB/WGL_ARB_create_context.txt + +type WglCreateContextAttribsARB = extern "system" fn(HDC, HGLRC, *const i32) -> HGLRC; + +const WGL_CONTEXT_MAJOR_VERSION_ARB: i32 = 0x2091; +const WGL_CONTEXT_MINOR_VERSION_ARB: i32 = 0x2092; +const WGL_CONTEXT_PROFILE_MASK_ARB: i32 = 0x9126; + +const WGL_CONTEXT_CORE_PROFILE_BIT_ARB: i32 = 0x00000001; +const WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB: i32 = 0x00000002; + +// See https://www.khronos.org/registry/OpenGL/extensions/ARB/WGL_ARB_pixel_format.txt + +type WglChoosePixelFormatARB = + extern "system" fn(HDC, *const i32, *const f32, u32, *mut i32, *mut u32) -> i32; + +const WGL_DRAW_TO_WINDOW_ARB: i32 = 0x2001; +const WGL_ACCELERATION_ARB: i32 = 0x2003; +const WGL_SUPPORT_OPENGL_ARB: i32 = 0x2010; +const WGL_DOUBLE_BUFFER_ARB: i32 = 0x2011; +const WGL_PIXEL_TYPE_ARB: i32 = 0x2013; +const WGL_RED_BITS_ARB: i32 = 0x2015; +const WGL_GREEN_BITS_ARB: i32 = 0x2017; +const WGL_BLUE_BITS_ARB: i32 = 0x2019; +const WGL_ALPHA_BITS_ARB: i32 = 0x201B; +const WGL_DEPTH_BITS_ARB: i32 = 0x2022; +const WGL_STENCIL_BITS_ARB: i32 = 0x2023; + +const WGL_FULL_ACCELERATION_ARB: i32 = 0x2027; +const WGL_TYPE_RGBA_ARB: i32 = 0x202B; + +// See https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_multisample.txt + +const WGL_SAMPLE_BUFFERS_ARB: i32 = 0x2041; +const WGL_SAMPLES_ARB: i32 = 0x2042; + +// See https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_framebuffer_sRGB.txt + +const WGL_FRAMEBUFFER_SRGB_CAPABLE_ARB: i32 = 0x20A9; + +// See https://www.khronos.org/registry/OpenGL/extensions/EXT/WGL_EXT_swap_control.txt + +type WglSwapIntervalEXT = extern "system" fn(i32) -> i32; + +pub type CreationFailedError = (); +pub struct GlContext { + hwnd: HWND, + hdc: HDC, + hglrc: HGLRC, + gl_library: HMODULE, +} + +extern "C" { + static __ImageBase: IMAGE_DOS_HEADER; +} + +impl GlContext { + pub unsafe fn create( + parent: &impl HasRawWindowHandle, + config: GlConfig, + ) -> Result { + let handle = if let RawWindowHandle::Windows(handle) = parent.raw_window_handle() { + handle + } else { + return Err(GlError::InvalidWindowHandle); + }; + + if handle.hwnd.is_null() { + return Err(GlError::InvalidWindowHandle); + } + + // Create temporary window and context to load function pointers + + let class_name_str = format!("raw-gl-context-window-{}", uuid::Uuid::new_v4().to_simple()); + let mut class_name: Vec = OsStr::new(&class_name_str).encode_wide().collect(); + class_name.push(0); + + let hinstance = &__ImageBase as *const IMAGE_DOS_HEADER as HINSTANCE; + + let wnd_class = WNDCLASSW { + style: CS_OWNDC, + lpfnWndProc: Some(DefWindowProcW), + hInstance: hinstance, + lpszClassName: class_name.as_ptr(), + ..std::mem::zeroed() + }; + + let class = RegisterClassW(&wnd_class); + if class == 0 { + return Err(GlError::CreationFailed(())); + } + + let hwnd_tmp = CreateWindowExW( + 0, + class as *const WCHAR, + [0].as_ptr(), + 0, + CW_USEDEFAULT, + CW_USEDEFAULT, + CW_USEDEFAULT, + CW_USEDEFAULT, + std::ptr::null_mut(), + std::ptr::null_mut(), + hinstance, + std::ptr::null_mut(), + ); + + if hwnd_tmp.is_null() { + return Err(GlError::CreationFailed(())); + } + + let hdc_tmp = GetDC(hwnd_tmp); + + let pfd_tmp = PIXELFORMATDESCRIPTOR { + nSize: std::mem::size_of::() as u16, + nVersion: 1, + dwFlags: PFD_DRAW_TO_WINDOW | PFD_SUPPORT_OPENGL | PFD_DOUBLEBUFFER, + iPixelType: PFD_TYPE_RGBA, + cColorBits: 32, + cAlphaBits: 8, + cDepthBits: 24, + cStencilBits: 8, + iLayerType: PFD_MAIN_PLANE, + ..std::mem::zeroed() + }; + + SetPixelFormat(hdc_tmp, ChoosePixelFormat(hdc_tmp, &pfd_tmp), &pfd_tmp); + + let hglrc_tmp = wglCreateContext(hdc_tmp); + if hglrc_tmp.is_null() { + ReleaseDC(hwnd_tmp, hdc_tmp); + UnregisterClassW(class as *const WCHAR, hinstance); + DestroyWindow(hwnd_tmp); + return Err(GlError::CreationFailed(())); + } + + wglMakeCurrent(hdc_tmp, hglrc_tmp); + + #[allow(non_snake_case)] + let wglCreateContextAttribsARB: Option = { + let symbol = CString::new("wglCreateContextAttribsARB").unwrap(); + let addr = wglGetProcAddress(symbol.as_ptr()); + if !addr.is_null() { + Some(std::mem::transmute(addr)) + } else { + None + } + }; + + #[allow(non_snake_case)] + let wglChoosePixelFormatARB: Option = { + let symbol = CString::new("wglChoosePixelFormatARB").unwrap(); + let addr = wglGetProcAddress(symbol.as_ptr()); + if !addr.is_null() { + Some(std::mem::transmute(addr)) + } else { + None + } + }; + + #[allow(non_snake_case)] + let wglSwapIntervalEXT: Option = { + let symbol = CString::new("wglSwapIntervalEXT").unwrap(); + let addr = wglGetProcAddress(symbol.as_ptr()); + if !addr.is_null() { + Some(std::mem::transmute(addr)) + } else { + None + } + }; + + wglMakeCurrent(hdc_tmp, std::ptr::null_mut()); + ReleaseDC(hwnd_tmp, hdc_tmp); + UnregisterClassW(class as *const WCHAR, hinstance); + DestroyWindow(hwnd_tmp); + + // Create actual context + + let hwnd = handle.hwnd as HWND; + + let hdc = GetDC(hwnd); + + #[rustfmt::skip] + let pixel_format_attribs = [ + WGL_DRAW_TO_WINDOW_ARB, 1, + WGL_ACCELERATION_ARB, WGL_FULL_ACCELERATION_ARB, + WGL_SUPPORT_OPENGL_ARB, 1, + WGL_DOUBLE_BUFFER_ARB, config.double_buffer as i32, + WGL_PIXEL_TYPE_ARB, WGL_TYPE_RGBA_ARB, + WGL_RED_BITS_ARB, config.red_bits as i32, + WGL_GREEN_BITS_ARB, config.green_bits as i32, + WGL_BLUE_BITS_ARB, config.blue_bits as i32, + WGL_ALPHA_BITS_ARB, config.alpha_bits as i32, + WGL_DEPTH_BITS_ARB, config.depth_bits as i32, + WGL_STENCIL_BITS_ARB, config.stencil_bits as i32, + WGL_SAMPLE_BUFFERS_ARB, config.samples.is_some() as i32, + WGL_SAMPLES_ARB, config.samples.unwrap_or(0) as i32, + WGL_FRAMEBUFFER_SRGB_CAPABLE_ARB, config.srgb as i32, + 0, + ]; + + let mut pixel_format = 0; + let mut num_formats = 0; + wglChoosePixelFormatARB.unwrap()( + hdc, + pixel_format_attribs.as_ptr(), + std::ptr::null(), + 1, + &mut pixel_format, + &mut num_formats, + ); + + let mut pfd: PIXELFORMATDESCRIPTOR = std::mem::zeroed(); + DescribePixelFormat( + hdc, + pixel_format, + std::mem::size_of::() as u32, + &mut pfd, + ); + SetPixelFormat(hdc, pixel_format, &pfd); + + let profile_mask = match config.profile { + Profile::Core => WGL_CONTEXT_CORE_PROFILE_BIT_ARB, + Profile::Compatibility => WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB, + }; + + #[rustfmt::skip] + let ctx_attribs = [ + WGL_CONTEXT_MAJOR_VERSION_ARB, config.version.0 as i32, + WGL_CONTEXT_MINOR_VERSION_ARB, config.version.1 as i32, + WGL_CONTEXT_PROFILE_MASK_ARB, profile_mask, + 0 + ]; + + let hglrc = + wglCreateContextAttribsARB.unwrap()(hdc, std::ptr::null_mut(), ctx_attribs.as_ptr()); + if hglrc == std::ptr::null_mut() { + return Err(GlError::CreationFailed(())); + } + + let gl_library_name = CString::new("opengl32.dll").unwrap(); + let gl_library = LoadLibraryA(gl_library_name.as_ptr()); + + wglMakeCurrent(hdc, hglrc); + wglSwapIntervalEXT.unwrap()(config.vsync as i32); + wglMakeCurrent(hdc, std::ptr::null_mut()); + + Ok(GlContext { + hwnd, + hdc, + hglrc, + gl_library, + }) + } + + pub unsafe fn make_current(&self) { + wglMakeCurrent(self.hdc, self.hglrc); + } + + pub unsafe fn make_not_current(&self) { + wglMakeCurrent(self.hdc, std::ptr::null_mut()); + } + + pub fn get_proc_address(&self, symbol: &str) -> *const c_void { + let symbol = CString::new(symbol).unwrap(); + let addr = unsafe { wglGetProcAddress(symbol.as_ptr()) as *const c_void }; + if !addr.is_null() { + addr + } else { + unsafe { GetProcAddress(self.gl_library, symbol.as_ptr()) as *const c_void } + } + } + + pub fn swap_buffers(&self) { + unsafe { + SwapBuffers(self.hdc); + } + } +} + +impl Drop for GlContext { + fn drop(&mut self) { + unsafe { + wglMakeCurrent(std::ptr::null_mut(), std::ptr::null_mut()); + wglDeleteContext(self.hglrc); + ReleaseDC(self.hwnd, self.hdc); + FreeLibrary(self.gl_library); + } + } +} diff --git a/src/gl/x11.rs b/src/gl/x11.rs new file mode 100644 index 0000000..ced977d --- /dev/null +++ b/src/gl/x11.rs @@ -0,0 +1,231 @@ +use std::ffi::{c_void, CString}; +use std::os::raw::{c_int, c_ulong}; + +use raw_window_handle::{HasRawWindowHandle, RawWindowHandle}; + +use x11::glx; +use x11::xlib; + +use crate::{GlConfig, GlError, Profile}; + +mod errors; + +#[derive(Debug)] +pub enum CreationFailedError { + InvalidFBConfig, + GetProcAddressFailed, + MakeCurrentFailed, + ContextCreationFailed, + X11Error(errors::XLibError), +} + +impl From for GlError { + fn from(e: errors::XLibError) -> Self { + GlError::CreationFailed(CreationFailedError::X11Error(e)) + } +} + +// See https://www.khronos.org/registry/OpenGL/extensions/ARB/GLX_ARB_create_context.txt + +type GlXCreateContextAttribsARB = unsafe extern "C" fn( + dpy: *mut xlib::Display, + fbc: glx::GLXFBConfig, + share_context: glx::GLXContext, + direct: xlib::Bool, + attribs: *const c_int, +) -> glx::GLXContext; + +// See https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_swap_control.txt + +type GlXSwapIntervalEXT = + unsafe extern "C" fn(dpy: *mut xlib::Display, drawable: glx::GLXDrawable, interval: i32); + +// See https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_framebuffer_sRGB.txt + +const GLX_FRAMEBUFFER_SRGB_CAPABLE_ARB: i32 = 0x20B2; + +fn get_proc_address(symbol: &str) -> *const c_void { + let symbol = CString::new(symbol).unwrap(); + unsafe { glx::glXGetProcAddress(symbol.as_ptr() as *const u8).unwrap() as *const c_void } +} + +pub struct GlContext { + window: c_ulong, + display: *mut xlib::_XDisplay, + context: glx::GLXContext, +} + +impl GlContext { + pub unsafe fn create( + parent: &impl HasRawWindowHandle, + config: GlConfig, + ) -> Result { + let handle = if let RawWindowHandle::Xlib(handle) = parent.raw_window_handle() { + handle + } else { + return Err(GlError::InvalidWindowHandle); + }; + + if handle.display.is_null() { + return Err(GlError::InvalidWindowHandle); + } + + let display = handle.display as *mut xlib::_XDisplay; + + errors::XErrorHandler::handle(display, |error_handler| { + let screen = unsafe { xlib::XDefaultScreen(display) }; + + #[rustfmt::skip] + let fb_attribs = [ + glx::GLX_X_RENDERABLE, 1, + glx::GLX_X_VISUAL_TYPE, glx::GLX_TRUE_COLOR, + glx::GLX_DRAWABLE_TYPE, glx::GLX_WINDOW_BIT, + glx::GLX_RENDER_TYPE, glx::GLX_RGBA_BIT, + glx::GLX_RED_SIZE, config.red_bits as i32, + glx::GLX_GREEN_SIZE, config.green_bits as i32, + glx::GLX_BLUE_SIZE, config.blue_bits as i32, + glx::GLX_ALPHA_SIZE, config.alpha_bits as i32, + glx::GLX_DEPTH_SIZE, config.depth_bits as i32, + glx::GLX_STENCIL_SIZE, config.stencil_bits as i32, + glx::GLX_DOUBLEBUFFER, config.double_buffer as i32, + glx::GLX_SAMPLE_BUFFERS, config.samples.is_some() as i32, + glx::GLX_SAMPLES, config.samples.unwrap_or(0) as i32, + GLX_FRAMEBUFFER_SRGB_CAPABLE_ARB, config.srgb as i32, + 0, + ]; + + let mut n_configs = 0; + let fb_config = unsafe { + glx::glXChooseFBConfig(display, screen, fb_attribs.as_ptr(), &mut n_configs) + }; + + error_handler.check()?; + + if n_configs <= 0 { + return Err(GlError::CreationFailed( + CreationFailedError::InvalidFBConfig, + )); + } + + #[allow(non_snake_case)] + let glXCreateContextAttribsARB: GlXCreateContextAttribsARB = unsafe { + let addr = get_proc_address("glXCreateContextAttribsARB"); + if addr.is_null() { + return Err(GlError::CreationFailed( + CreationFailedError::GetProcAddressFailed, + )); + } else { + std::mem::transmute(addr) + } + }; + + #[allow(non_snake_case)] + let glXSwapIntervalEXT: GlXSwapIntervalEXT = unsafe { + let addr = get_proc_address("glXSwapIntervalEXT"); + if addr.is_null() { + return Err(GlError::CreationFailed( + CreationFailedError::GetProcAddressFailed, + )); + } else { + std::mem::transmute(addr) + } + }; + + error_handler.check()?; + + let profile_mask = match config.profile { + Profile::Core => glx::arb::GLX_CONTEXT_CORE_PROFILE_BIT_ARB, + Profile::Compatibility => glx::arb::GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB, + }; + + #[rustfmt::skip] + let ctx_attribs = [ + glx::arb::GLX_CONTEXT_MAJOR_VERSION_ARB, config.version.0 as i32, + glx::arb::GLX_CONTEXT_MINOR_VERSION_ARB, config.version.1 as i32, + glx::arb::GLX_CONTEXT_PROFILE_MASK_ARB, profile_mask, + 0, + ]; + + let context = unsafe { + glXCreateContextAttribsARB( + display, + *fb_config, + std::ptr::null_mut(), + 1, + ctx_attribs.as_ptr(), + ) + }; + + error_handler.check()?; + + if context.is_null() { + return Err(GlError::CreationFailed( + CreationFailedError::ContextCreationFailed, + )); + } + + unsafe { + let res = glx::glXMakeCurrent(display, handle.window, context); + error_handler.check()?; + if res == 0 { + return Err(GlError::CreationFailed( + CreationFailedError::MakeCurrentFailed, + )); + } + + glXSwapIntervalEXT(display, handle.window, config.vsync as i32); + error_handler.check()?; + + if glx::glXMakeCurrent(display, 0, std::ptr::null_mut()) == 0 { + error_handler.check()?; + return Err(GlError::CreationFailed( + CreationFailedError::MakeCurrentFailed, + )); + } + } + + Ok(GlContext { + window: handle.window, + display, + context, + }) + }) + } + + pub unsafe fn make_current(&self) { + errors::XErrorHandler::handle(self.display, |error_handler| { + let res = glx::glXMakeCurrent(self.display, self.window, self.context); + error_handler.check().unwrap(); + if res == 0 { + panic!("make_current failed") + } + }) + } + + pub unsafe fn make_not_current(&self) { + errors::XErrorHandler::handle(self.display, |error_handler| { + let res = glx::glXMakeCurrent(self.display, 0, std::ptr::null_mut()); + error_handler.check().unwrap(); + if res == 0 { + panic!("make_not_current failed") + } + }) + } + + pub fn get_proc_address(&self, symbol: &str) -> *const c_void { + get_proc_address(symbol) + } + + pub fn swap_buffers(&self) { + errors::XErrorHandler::handle(self.display, |error_handler| { + unsafe { + glx::glXSwapBuffers(self.display, self.window); + } + error_handler.check().unwrap(); + }) + } +} + +impl Drop for GlContext { + fn drop(&mut self) {} +} diff --git a/src/gl/x11/errors.rs b/src/gl/x11/errors.rs new file mode 100644 index 0000000..ec1be08 --- /dev/null +++ b/src/gl/x11/errors.rs @@ -0,0 +1,131 @@ +use std::ffi::CStr; +use std::fmt::{Debug, Formatter}; +use x11::xlib; + +use std::panic::AssertUnwindSafe; +use std::sync::atomic::{AtomicPtr, Ordering}; +use std::sync::Mutex; + +static CURRENT_ERROR_PTR: AtomicPtr>> = + AtomicPtr::new(::std::ptr::null_mut()); + +/// A helper struct for safe X11 error handling +pub struct XErrorHandler<'a> { + display: *mut xlib::Display, + mutex: &'a Mutex>, +} + +impl<'a> XErrorHandler<'a> { + /// Syncs and checks if any previous X11 calls returned an error + pub fn check(&mut self) -> Result<(), XLibError> { + // Flush all possible previous errors + unsafe { + xlib::XSync(self.display, 0); + } + let error = self.mutex.lock().unwrap().take(); + + match error { + None => Ok(()), + Some(inner) => Err(XLibError { inner }), + } + } + + /// Sets up a temporary X11 error handler for the duration of the given closure, and allows + /// that closure to check on the latest X11 error at any time + pub fn handle T>( + display: *mut xlib::Display, + handler: F, + ) -> T { + unsafe extern "C" fn error_handler( + _dpy: *mut xlib::Display, + err: *mut xlib::XErrorEvent, + ) -> i32 { + // SAFETY: the error pointer should be safe to copy + let err = *err; + // SAFETY: the ptr can only point to a valid instance or be null + let mutex = CURRENT_ERROR_PTR.load(Ordering::SeqCst).as_ref(); + let mutex = if let Some(mutex) = mutex { + mutex + } else { + return 2; + }; + + match mutex.lock() { + Ok(mut current_error) => { + *current_error = Some(err); + 0 + } + Err(e) => { + eprintln!( + "[FATAL] raw-gl-context: Failed to lock for X11 Error Handler: {:?}", + e + ); + 1 + } + } + } + + // Flush all possible previous errors + unsafe { + xlib::XSync(display, 0); + } + + let mut mutex = Mutex::new(None); + CURRENT_ERROR_PTR.store(&mut mutex, Ordering::SeqCst); + + let old_handler = unsafe { xlib::XSetErrorHandler(Some(error_handler)) }; + let panic_result = std::panic::catch_unwind(AssertUnwindSafe(|| { + let mut h = XErrorHandler { + display, + mutex: &mutex, + }; + handler(&mut h) + })); + // Whatever happened, restore old error handler + unsafe { xlib::XSetErrorHandler(old_handler) }; + CURRENT_ERROR_PTR.store(::core::ptr::null_mut(), Ordering::SeqCst); + + match panic_result { + Ok(v) => v, + Err(e) => std::panic::resume_unwind(e), + } + } +} + +pub struct XLibError { + inner: xlib::XErrorEvent, +} + +impl XLibError { + pub fn get_display_name(&self, buf: &mut [u8]) -> &CStr { + unsafe { + xlib::XGetErrorText( + self.inner.display, + self.inner.error_code.into(), + buf.as_mut_ptr().cast(), + (buf.len() - 1) as i32, + ); + } + + *buf.last_mut().unwrap() = 0; + // SAFETY: whatever XGetErrorText did or not, we guaranteed there is a nul byte at the end of the buffer + unsafe { CStr::from_ptr(buf.as_mut_ptr().cast()) } + } +} + +impl Debug for XLibError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let mut buf = [0; 255]; + let display_name = self.get_display_name(&mut buf).to_string_lossy(); + + f.debug_struct("XLibError") + .field("error_code", &self.inner.error_code) + .field("error_message", &display_name) + .field("minor_code", &self.inner.minor_code) + .field("request_code", &self.inner.request_code) + .field("type", &self.inner.type_) + .field("resource_id", &self.inner.resourceid) + .field("serial", &self.inner.serial) + .finish() + } +} From 0f1b8353d03a47648b162f6dc0edea74bac64a46 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 17:31:19 +0100 Subject: [PATCH 06/22] Patch up the raw-gl-context modules for a feature This only needed a couple changes for the raw-window-handle migration, and for the different module paths. --- Cargo.toml | 5 +++++ src/gl/macos.rs | 7 +++---- src/gl/{lib.rs => mod.rs} | 11 ++++------- src/gl/win.rs | 14 ++++---------- src/gl/x11.rs | 35 +++++++++-------------------------- src/lib.rs | 3 +++ 6 files changed, 28 insertions(+), 47 deletions(-) rename src/gl/{lib.rs => mod.rs} (88%) diff --git a/Cargo.toml b/Cargo.toml index f45bdc3..cee5ab5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,10 @@ authors = [ edition = "2018" license = "MIT OR Apache-2.0" +[features] +default = ["opengl"] +opengl = ["uuid"] + [dependencies] keyboard-types = { version = "0.6.1", default-features = false } raw-window-handle = "0.4.2" @@ -26,6 +30,7 @@ nix = "0.22.0" [target.'cfg(target_os="windows")'.dependencies] winapi = { version = "0.3.8", features = ["libloaderapi", "winuser", "windef", "minwindef", "guiddef", "combaseapi", "wingdi", "errhandlingapi"] } +uuid = { version = "0.8", features = ["v4"], optional = true } [target.'cfg(target_os="macos")'.dependencies] cocoa = "0.24.0" diff --git a/src/gl/macos.rs b/src/gl/macos.rs index 28f56c1..0610879 100644 --- a/src/gl/macos.rs +++ b/src/gl/macos.rs @@ -18,7 +18,7 @@ use core_foundation::string::CFString; use objc::{msg_send, sel, sel_impl}; -use crate::{GlConfig, GlError, Profile}; +use super::{GlConfig, GlError, Profile}; pub type CreationFailedError = (); pub struct GlContext { @@ -28,10 +28,9 @@ pub struct GlContext { impl GlContext { pub unsafe fn create( - parent: &impl HasRawWindowHandle, - config: GlConfig, + parent: &impl HasRawWindowHandle, config: GlConfig, ) -> Result { - let handle = if let RawWindowHandle::MacOS(handle) = parent.raw_window_handle() { + let handle = if let RawWindowHandle::AppKit(handle) = parent.raw_window_handle() { handle } else { return Err(GlError::InvalidWindowHandle); diff --git a/src/gl/lib.rs b/src/gl/mod.rs similarity index 88% rename from src/gl/lib.rs rename to src/gl/mod.rs index 9c43878..5af8634 100644 --- a/src/gl/lib.rs +++ b/src/gl/mod.rs @@ -11,7 +11,7 @@ use win as platform; #[cfg(target_os = "linux")] mod x11; #[cfg(target_os = "linux")] -use crate::x11 as platform; +use self::x11 as platform; #[cfg(target_os = "macos")] mod macos; @@ -73,13 +73,10 @@ pub struct GlContext { impl GlContext { pub unsafe fn create( - parent: &impl HasRawWindowHandle, - config: GlConfig, + parent: &impl HasRawWindowHandle, config: GlConfig, ) -> Result { - platform::GlContext::create(parent, config).map(|context| GlContext { - context, - phantom: PhantomData, - }) + platform::GlContext::create(parent, config) + .map(|context| GlContext { context, phantom: PhantomData }) } pub unsafe fn make_current(&self) { diff --git a/src/gl/win.rs b/src/gl/win.rs index 5594ae9..57982c9 100644 --- a/src/gl/win.rs +++ b/src/gl/win.rs @@ -18,7 +18,7 @@ use winapi::um::winuser::{ UnregisterClassW, CS_OWNDC, CW_USEDEFAULT, WNDCLASSW, }; -use crate::{GlConfig, GlError, Profile}; +use super::{GlConfig, GlError, Profile}; // See https://www.khronos.org/registry/OpenGL/extensions/ARB/WGL_ARB_create_context.txt @@ -78,10 +78,9 @@ extern "C" { impl GlContext { pub unsafe fn create( - parent: &impl HasRawWindowHandle, - config: GlConfig, + parent: &impl HasRawWindowHandle, config: GlConfig, ) -> Result { - let handle = if let RawWindowHandle::Windows(handle) = parent.raw_window_handle() { + let handle = if let RawWindowHandle::Win32(handle) = parent.raw_window_handle() { handle } else { return Err(GlError::InvalidWindowHandle); @@ -267,12 +266,7 @@ impl GlContext { wglSwapIntervalEXT.unwrap()(config.vsync as i32); wglMakeCurrent(hdc, std::ptr::null_mut()); - Ok(GlContext { - hwnd, - hdc, - hglrc, - gl_library, - }) + Ok(GlContext { hwnd, hdc, hglrc, gl_library }) } pub unsafe fn make_current(&self) { diff --git a/src/gl/x11.rs b/src/gl/x11.rs index ced977d..b31bffc 100644 --- a/src/gl/x11.rs +++ b/src/gl/x11.rs @@ -6,7 +6,7 @@ use raw_window_handle::{HasRawWindowHandle, RawWindowHandle}; use x11::glx; use x11::xlib; -use crate::{GlConfig, GlError, Profile}; +use super::{GlConfig, GlError, Profile}; mod errors; @@ -57,8 +57,7 @@ pub struct GlContext { impl GlContext { pub unsafe fn create( - parent: &impl HasRawWindowHandle, - config: GlConfig, + parent: &impl HasRawWindowHandle, config: GlConfig, ) -> Result { let handle = if let RawWindowHandle::Xlib(handle) = parent.raw_window_handle() { handle @@ -102,18 +101,14 @@ impl GlContext { error_handler.check()?; if n_configs <= 0 { - return Err(GlError::CreationFailed( - CreationFailedError::InvalidFBConfig, - )); + return Err(GlError::CreationFailed(CreationFailedError::InvalidFBConfig)); } #[allow(non_snake_case)] let glXCreateContextAttribsARB: GlXCreateContextAttribsARB = unsafe { let addr = get_proc_address("glXCreateContextAttribsARB"); if addr.is_null() { - return Err(GlError::CreationFailed( - CreationFailedError::GetProcAddressFailed, - )); + return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); } else { std::mem::transmute(addr) } @@ -123,9 +118,7 @@ impl GlContext { let glXSwapIntervalEXT: GlXSwapIntervalEXT = unsafe { let addr = get_proc_address("glXSwapIntervalEXT"); if addr.is_null() { - return Err(GlError::CreationFailed( - CreationFailedError::GetProcAddressFailed, - )); + return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); } else { std::mem::transmute(addr) } @@ -159,18 +152,14 @@ impl GlContext { error_handler.check()?; if context.is_null() { - return Err(GlError::CreationFailed( - CreationFailedError::ContextCreationFailed, - )); + return Err(GlError::CreationFailed(CreationFailedError::ContextCreationFailed)); } unsafe { let res = glx::glXMakeCurrent(display, handle.window, context); error_handler.check()?; if res == 0 { - return Err(GlError::CreationFailed( - CreationFailedError::MakeCurrentFailed, - )); + return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); } glXSwapIntervalEXT(display, handle.window, config.vsync as i32); @@ -178,17 +167,11 @@ impl GlContext { if glx::glXMakeCurrent(display, 0, std::ptr::null_mut()) == 0 { error_handler.check()?; - return Err(GlError::CreationFailed( - CreationFailedError::MakeCurrentFailed, - )); + return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); } } - Ok(GlContext { - window: handle.window, - display, - context, - }) + Ok(GlContext { window: handle.window, display, context }) }) } diff --git a/src/lib.rs b/src/lib.rs index 5a61586..46a3695 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,9 @@ mod window; mod window_info; mod window_open_options; +#[cfg(feature = "opengl")] +pub mod gl; + pub use event::*; pub use mouse_cursor::MouseCursor; pub use window::*; From d2e3d5d1ac9a558f5518630f6e17924fcb75ced2 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 17:58:45 +0100 Subject: [PATCH 07/22] Don't enable the OpenGL feature by default --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index cee5ab5..3994885 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ edition = "2018" license = "MIT OR Apache-2.0" [features] -default = ["opengl"] +default = [] opengl = ["uuid"] [dependencies] From 6105cae01ded57dd1187e4d06302fd60e6c541a7 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 17:59:40 +0100 Subject: [PATCH 08/22] Mention OpenGL contexts in the readme --- README.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index bd6c7ea..1015ac3 100644 --- a/README.md +++ b/README.md @@ -10,13 +10,14 @@ Interested in learning more about the project? Join us on [discord](https://disc Below is a proposed list of milestones (roughly in-order) and their status. Subject to change at any time. -| Feature | Windows | Mac OS | Linux | -| ----------------------------------------------- | ------------------ | ------------------ | ------------------ | -| Spawns a window, no parent | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | -| Cross-platform API for window spawning | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | -| Can find DPI scale factor | | :heavy_check_mark: | :heavy_check_mark: | -| Basic event handling (mouse, keyboard) | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | -| Parent window support | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | +| Feature | Windows | Mac OS | Linux | +| ----------------------------------------------------- | ------------------ | ------------------ | ------------------ | +| Spawns a window, no parent | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | +| Cross-platform API for window spawning | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | +| Can find DPI scale factor | | :heavy_check_mark: | :heavy_check_mark: | +| Basic event handling (mouse, keyboard) | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | +| Parent window support | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | +| OpenGL context creation (behind the `opengl` feature) | :heavy_check_mark: | :heavy_check_mark: | :heavy_check_mark: | ## Prerequisites From 80802dfbe93bf4797c0d1758e92c2b01f8c8ed91 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 18:05:32 +0100 Subject: [PATCH 09/22] Make opengl context creation private for now --- src/gl/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gl/mod.rs b/src/gl/mod.rs index 5af8634..cb2e6c2 100644 --- a/src/gl/mod.rs +++ b/src/gl/mod.rs @@ -72,7 +72,7 @@ pub struct GlContext { } impl GlContext { - pub unsafe fn create( + pub(crate) unsafe fn create( parent: &impl HasRawWindowHandle, config: GlConfig, ) -> Result { platform::GlContext::create(parent, config) From b4a3d2bb04331d003d72c5919514183b6eee06f9 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 19:00:03 +0100 Subject: [PATCH 10/22] Add stubs for creating OpenGL contexts There are now three todo!()s when compiling with the OpenGL flag that need to be filled in, with the only nontrivial one being the X11 version. --- examples/open_window.rs | 4 ++ src/macos/window.rs | 39 ++++++++++++++++++- src/win/window.rs | 80 ++++++++++++++++++++++++-------------- src/window.rs | 7 ++++ src/window_open_options.rs | 5 +++ src/x11/window.rs | 14 +++++++ 6 files changed, 118 insertions(+), 31 deletions(-) diff --git a/examples/open_window.rs b/examples/open_window.rs index 88a9ae0..95141b9 100644 --- a/examples/open_window.rs +++ b/examples/open_window.rs @@ -36,6 +36,10 @@ fn main() { title: "baseview".into(), size: baseview::Size::new(512.0, 512.0), scale: WindowScalePolicy::SystemScaleFactor, + + // TODO: Add an example that uses the OpenGL context + #[cfg(feature = "opengl")] + gl_config: None, }; let (mut tx, rx) = RingBuffer::new(128); diff --git a/src/macos/window.rs b/src/macos/window.rs index bfe8316..fd23379 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -26,6 +26,9 @@ use crate::{ use super::keyboard::KeyboardState; use super::view::{create_view, BASEVIEW_STATE_IVAR}; +#[cfg(feature = "opengl")] +use crate::gl::{GlConfig, GlContext}; + pub struct WindowHandle { raw_window_handle: Option, close_requested: Arc, @@ -102,6 +105,9 @@ pub struct Window { /// Our subclassed NSView ns_view: id, close_requested: bool, + + #[cfg(feature = "opengl")] + gl_context: Option, } impl Window { @@ -122,7 +128,15 @@ impl Window { let ns_view = unsafe { create_view(&options) }; - let window = Window { ns_app: None, ns_window: None, ns_view, close_requested: false }; + let window = Window { + ns_app: None, + ns_window: None, + ns_view, + close_requested: false, + + #[cfg(feature = "opengl")] + gl_context: options.gl_config.map(Self::create_gl_context), + }; let window_handle = Self::init(true, window, build); @@ -146,7 +160,15 @@ impl Window { let ns_view = unsafe { create_view(&options) }; - let window = Window { ns_app: None, ns_window: None, ns_view, close_requested: false }; + let window = Window { + ns_app: None, + ns_window: None, + ns_view, + close_requested: false, + + #[cfg(feature = "opengl")] + gl_context: options.gl_config.map(Self::create_gl_context), + }; let window_handle = Self::init(true, window, build); @@ -217,6 +239,9 @@ impl Window { ns_window: Some(ns_window), ns_view, close_requested: false, + + #[cfg(feature = "opengl")] + gl_context: options.gl_config.map(Self::create_gl_context), }; let _ = Self::init(false, window, build); @@ -266,6 +291,16 @@ impl Window { pub fn close(&mut self) { self.close_requested = true; } + + #[cfg(feature = "opengl")] + pub fn gl_context(&self) -> Option<&GlContext> { + self.gl_context.as_ref() + } + + #[cfg(feature = "opengl")] + fn create_gl_context(config: GlConfig) -> GlContext { + todo!("Create the macOS OpenGL context"); + } } pub(super) struct WindowState { diff --git a/src/win/window.rs b/src/win/window.rs index 08720f0..59e0e26 100644 --- a/src/win/window.rs +++ b/src/win/window.rs @@ -34,6 +34,9 @@ use crate::{ use super::keyboard::KeyboardState; +#[cfg(feature = "opengl")] +use crate::gl::GlContext; + unsafe fn generate_guid() -> String { let mut guid: GUID = std::mem::zeroed(); CoCreateGuid(&mut guid); @@ -124,7 +127,9 @@ unsafe extern "system" fn wnd_proc( let window_state_ptr = GetWindowLongPtrW(hwnd, GWLP_USERDATA) as *mut RefCell; if !window_state_ptr.is_null() { - let mut window = Window { hwnd }; + let mut window_state = (*window_state_ptr).borrow_mut(); + + let mut window = window_state.create_window(hwnd); let mut window = crate::Window::new(&mut window); match msg { @@ -134,8 +139,6 @@ unsafe extern "system" fn wnd_proc( let physical_pos = PhyPoint { x, y }; - let mut window_state = (&*window_state_ptr).borrow_mut(); - let logical_pos = physical_pos.to_logical(&window_state.window_info); window_state.handler.on_event( @@ -149,8 +152,6 @@ unsafe extern "system" fn wnd_proc( let value = value as i32; let value = value as f32 / WHEEL_DELTA as f32; - let mut window_state = (&*window_state_ptr).borrow_mut(); - window_state.handler.on_event( &mut window, Event::Mouse(MouseEvent::WheelScrolled(ScrollDelta::Lines { @@ -162,7 +163,7 @@ unsafe extern "system" fn wnd_proc( } WM_LBUTTONDOWN | WM_LBUTTONUP | WM_MBUTTONDOWN | WM_MBUTTONUP | WM_RBUTTONDOWN | WM_RBUTTONUP | WM_XBUTTONDOWN | WM_XBUTTONUP => { - let mut mouse_button_counter = (&*window_state_ptr).borrow().mouse_button_counter; + let mut mouse_button_counter = (*window_state_ptr).borrow().mouse_button_counter; let button = match msg { WM_LBUTTONDOWN | WM_LBUTTONUP => Some(MouseButton::Left), @@ -198,44 +199,33 @@ unsafe extern "system" fn wnd_proc( } }; - (&*window_state_ptr).borrow_mut().mouse_button_counter = mouse_button_counter; + window_state.mouse_button_counter = mouse_button_counter; - (&*window_state_ptr) - .borrow_mut() - .handler - .on_event(&mut window, Event::Mouse(event)); + window_state.handler.on_event(&mut window, Event::Mouse(event)); } } WM_TIMER => { match wparam { WIN_FRAME_TIMER => { - (&*window_state_ptr).borrow_mut().handler.on_frame(&mut window); + window_state.handler.on_frame(&mut window); } _ => (), } return 0; } WM_CLOSE => { - (&*window_state_ptr) - .borrow_mut() - .handler - .on_event(&mut window, Event::Window(WindowEvent::WillClose)); + window_state.handler.on_event(&mut window, Event::Window(WindowEvent::WillClose)); // DestroyWindow(hwnd); // return 0; return DefWindowProcW(hwnd, msg, wparam, lparam); } WM_CHAR | WM_SYSCHAR | WM_KEYDOWN | WM_SYSKEYDOWN | WM_KEYUP | WM_SYSKEYUP | WM_INPUTLANGCHANGE => { - let opt_event = (&*window_state_ptr) - .borrow_mut() - .keyboard_state - .process_message(hwnd, msg, wparam, lparam); + let opt_event = + window_state.keyboard_state.process_message(hwnd, msg, wparam, lparam); if let Some(event) = opt_event { - (&*window_state_ptr) - .borrow_mut() - .handler - .on_event(&mut window, Event::Keyboard(event)); + window_state.handler.on_event(&mut window, Event::Keyboard(event)); } if msg != WM_SYSKEYDOWN { @@ -246,8 +236,6 @@ unsafe extern "system" fn wnd_proc( let width = (lparam & 0xFFFF) as u16 as u32; let height = ((lparam >> 16) & 0xFFFF) as u16 as u32; - let mut window_state = (&*window_state_ptr).borrow_mut(); - window_state.window_info = WindowInfo::from_physical_size( PhySize { width, height }, window_state.window_info.scale(), @@ -262,8 +250,6 @@ unsafe extern "system" fn wnd_proc( WM_DPICHANGED => { // To avoid weirdness with the realtime borrow checker. let new_rect = { - let mut window_state = (&*window_state_ptr).borrow_mut(); - if let WindowScalePolicy::SystemScaleFactor = window_state.scale_policy { let dpi = (wparam & 0xFFFF) as u16 as u32; let scale_factor = dpi as f64 / 96.0; @@ -319,7 +305,7 @@ unsafe extern "system" fn wnd_proc( } } - return DefWindowProcW(hwnd, msg, wparam, lparam); + DefWindowProcW(hwnd, msg, wparam, lparam) } unsafe fn register_wnd_class() -> ATOM { @@ -357,10 +343,28 @@ struct WindowState { handler: Box, scale_policy: WindowScalePolicy, dw_style: u32, + + #[cfg(feature = "opengl")] + gl_context: Arc>, +} + +impl WindowState { + #[cfg(not(feature = "opengl"))] + fn create_window(&self, hwnd: HWND) -> Window { + Window { hwnd } + } + + #[cfg(feature = "opengl")] + fn create_window(&self, hwnd: HWND) -> Window { + Window { hwnd, gl_context: self.gl_context.clone() } + } } pub struct Window { hwnd: HWND, + + #[cfg(feature = "opengl")] + gl_context: Arc>, } impl Window { @@ -478,7 +482,17 @@ impl Window { ); // todo: manage error ^ + #[cfg(feature = "opengl")] + let gl_context: Arc> = + Arc::new(todo!("Create the Windows OpenGL context")); + + #[cfg(not(feature = "opengl"))] let handler = Box::new(build(&mut crate::Window::new(&mut Window { hwnd }))); + #[cfg(feature = "opengl")] + let handler = Box::new(build(&mut crate::Window::new(&mut Window { + hwnd, + gl_context: gl_context.clone(), + }))); let (parent_handle, window_handle) = ParentHandle::new(hwnd); let parent_handle = if parented { Some(parent_handle) } else { None }; @@ -492,6 +506,9 @@ impl Window { handler, scale_policy: options.scale, dw_style: flags, + + #[cfg(feature = "opengl")] + gl_context, })); // Only works on Windows 10 unfortunately. @@ -556,6 +573,11 @@ impl Window { PostMessageW(self.hwnd, BV_WINDOW_MUST_CLOSE, 0, 0); } } + + #[cfg(feature = "opengl")] + pub fn gl_context(&self) -> Option<&GlContext> { + self.gl_context.as_ref().as_ref() + } } unsafe impl HasRawWindowHandle for Window { diff --git a/src/window.rs b/src/window.rs index 9b5dfbb..091a594 100644 --- a/src/window.rs +++ b/src/window.rs @@ -91,6 +91,13 @@ impl<'a> Window<'a> { pub fn close(&mut self) { self.window.close(); } + + /// If provided, then an OpenGL context will be created for this window. You'll be able to + /// access this context through [crate::Window::gl_context]. + #[cfg(feature = "opengl")] + pub fn gl_context(&self) -> Option<&crate::gl::GlContext> { + self.window.gl_context() + } } unsafe impl<'a> HasRawWindowHandle for Window<'a> { diff --git a/src/window_open_options.rs b/src/window_open_options.rs index c4abf74..7c5cd19 100644 --- a/src/window_open_options.rs +++ b/src/window_open_options.rs @@ -21,4 +21,9 @@ pub struct WindowOpenOptions { /// The dpi scaling policy pub scale: WindowScalePolicy, + + /// If provided, then an OpenGL context will be created for this window. You'll be able to + /// access this context through [crate::Window::gl_context]. + #[cfg(feature = "opengl")] + pub gl_config: Option, } diff --git a/src/x11/window.rs b/src/x11/window.rs index f9cff75..9bae212 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -16,6 +16,9 @@ use crate::{ use super::keyboard::{convert_key_press_event, convert_key_release_event}; +#[cfg(feature = "opengl")] +use crate::gl::GlContext; + pub struct WindowHandle { raw_window_handle: Option, close_requested: Arc, @@ -96,6 +99,9 @@ pub struct Window { new_physical_size: Option, parent_handle: Option, + + #[cfg(feature = "opengl")] + gl_context: Option, } // Hack to allow sending a RawWindowHandle between threads. Do not make public @@ -306,6 +312,9 @@ impl Window { new_physical_size: None, parent_handle, + + #[cfg(feature = "opengl")] + gl_context: todo!("Create the X11 OpenGL context"), }; let mut handler = build(&mut crate::Window::new(&mut window)); @@ -346,6 +355,11 @@ impl Window { self.close_requested = true; } + #[cfg(feature = "opengl")] + pub fn gl_context(&self) -> Option<&crate::gl::GlContext> { + self.gl_context.as_ref() + } + #[inline] fn drain_xcb_events(&mut self, handler: &mut dyn WindowHandler) { // the X server has a tendency to send spurious/extraneous configure notify events when a From 3551d5e25329ff34b8a0e350bfd33be608cfc8ab Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 20:20:20 +0100 Subject: [PATCH 11/22] Implement OpenGL contexts for X11 This should in theory work! When requesting an OpenGL context, the window visual is determined based on the matched framebuffer config. --- src/gl/mod.rs | 14 +++- src/gl/x11.rs | 182 ++++++++++++++++++++++++++++------------------ src/x11/window.rs | 94 +++++++++++++++++++----- 3 files changed, 200 insertions(+), 90 deletions(-) diff --git a/src/gl/mod.rs b/src/gl/mod.rs index cb2e6c2..f3e786f 100644 --- a/src/gl/mod.rs +++ b/src/gl/mod.rs @@ -8,10 +8,11 @@ mod win; #[cfg(target_os = "windows")] use win as platform; +// We need to use this directly within the X11 window creation to negotiate the correct visual #[cfg(target_os = "linux")] -mod x11; +pub(crate) mod x11; #[cfg(target_os = "linux")] -use self::x11 as platform; +pub(crate) use self::x11 as platform; #[cfg(target_os = "macos")] mod macos; @@ -72,6 +73,7 @@ pub struct GlContext { } impl GlContext { + #[cfg(not(target_os = "linux"))] pub(crate) unsafe fn create( parent: &impl HasRawWindowHandle, config: GlConfig, ) -> Result { @@ -79,6 +81,14 @@ impl GlContext { .map(|context| GlContext { context, phantom: PhantomData }) } + /// The X11 version needs to be set up in a different way compared to the Windows and macOS + /// versions. So the platform-specific versions should be used to construct the context within + /// baseview, and then this object can be passed to the user. + #[cfg(target_os = "linux")] + pub(crate) fn new(context: platform::GlContext) -> GlContext { + GlContext { context, phantom: PhantomData } + } + pub unsafe fn make_current(&self) { self.context.make_current(); } diff --git a/src/gl/x11.rs b/src/gl/x11.rs index b31bffc..03a5540 100644 --- a/src/gl/x11.rs +++ b/src/gl/x11.rs @@ -13,6 +13,7 @@ mod errors; #[derive(Debug)] pub enum CreationFailedError { InvalidFBConfig, + NoVisual, GetProcAddressFailed, MakeCurrentFailed, ContextCreationFailed, @@ -55,9 +56,31 @@ pub struct GlContext { context: glx::GLXContext, } +/// The frame buffer configuration along with the general OpenGL configuration to somewhat minimize +/// misuse. +pub struct FbConfig { + gl_config: GlConfig, + fb_config: *mut glx::__GLXFBConfigRec, +} + +/// The configuration a window should be created with after calling +/// [GlContext::get_fb_config_and_visual]. +pub struct WindowConfig { + pub depth: u8, + pub visual: u32, +} + impl GlContext { + /// Creating an OpenGL context under X11 works slightly different. Different OpenGL + /// configurations require different framebuffer configurations, and to be able to use that + /// context with a window the window needs to be created with a matching visual. This means that + /// you need to decide on the framebuffer config before creating the window, ask the X11 server + /// for a matching visual for that framebuffer config, crate the window with that visual, and + /// only then create the OpenGL context. + /// + /// Use [Self::get_fb_config_and_visual] to create both of these things. pub unsafe fn create( - parent: &impl HasRawWindowHandle, config: GlConfig, + parent: &impl HasRawWindowHandle, config: FbConfig, ) -> Result { let handle = if let RawWindowHandle::Xlib(handle) = parent.raw_window_handle() { handle @@ -71,6 +94,84 @@ impl GlContext { let display = handle.display as *mut xlib::_XDisplay; + errors::XErrorHandler::handle(display, |error_handler| { + #[allow(non_snake_case)] + let glXCreateContextAttribsARB: GlXCreateContextAttribsARB = unsafe { + let addr = get_proc_address("glXCreateContextAttribsARB"); + if addr.is_null() { + return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); + } else { + std::mem::transmute(addr) + } + }; + + #[allow(non_snake_case)] + let glXSwapIntervalEXT: GlXSwapIntervalEXT = unsafe { + let addr = get_proc_address("glXSwapIntervalEXT"); + if addr.is_null() { + return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); + } else { + std::mem::transmute(addr) + } + }; + + error_handler.check()?; + + let profile_mask = match config.gl_config.profile { + Profile::Core => glx::arb::GLX_CONTEXT_CORE_PROFILE_BIT_ARB, + Profile::Compatibility => glx::arb::GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB, + }; + + #[rustfmt::skip] + let ctx_attribs = [ + glx::arb::GLX_CONTEXT_MAJOR_VERSION_ARB, config.gl_config.version.0 as i32, + glx::arb::GLX_CONTEXT_MINOR_VERSION_ARB, config.gl_config.version.1 as i32, + glx::arb::GLX_CONTEXT_PROFILE_MASK_ARB, profile_mask, + 0, + ]; + + let context = unsafe { + glXCreateContextAttribsARB( + display, + config.fb_config, + std::ptr::null_mut(), + 1, + ctx_attribs.as_ptr(), + ) + }; + + error_handler.check()?; + + if context.is_null() { + return Err(GlError::CreationFailed(CreationFailedError::ContextCreationFailed)); + } + + unsafe { + let res = glx::glXMakeCurrent(display, handle.window, context); + error_handler.check()?; + if res == 0 { + return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); + } + + glXSwapIntervalEXT(display, handle.window, config.gl_config.vsync as i32); + error_handler.check()?; + + if glx::glXMakeCurrent(display, 0, std::ptr::null_mut()) == 0 { + error_handler.check()?; + return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); + } + } + + Ok(GlContext { window: handle.window, display, context }) + }) + } + + /// Find a matching framebuffer config and window visual for the given OpenGL configuration. + /// This needs to be passed to [Self::create] along with a handle to a window that was created + /// using the visual also returned from this function. + pub unsafe fn get_fb_config_and_visual( + display: *mut xlib::_XDisplay, config: GlConfig, + ) -> Result<(FbConfig, WindowConfig), GlError> { errors::XErrorHandler::handle(display, |error_handler| { let screen = unsafe { xlib::XDefaultScreen(display) }; @@ -99,79 +200,22 @@ impl GlContext { }; error_handler.check()?; - - if n_configs <= 0 { + if n_configs <= 0 || fb_config.is_null() { return Err(GlError::CreationFailed(CreationFailedError::InvalidFBConfig)); } - #[allow(non_snake_case)] - let glXCreateContextAttribsARB: GlXCreateContextAttribsARB = unsafe { - let addr = get_proc_address("glXCreateContextAttribsARB"); - if addr.is_null() { - return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); - } else { - std::mem::transmute(addr) - } - }; - - #[allow(non_snake_case)] - let glXSwapIntervalEXT: GlXSwapIntervalEXT = unsafe { - let addr = get_proc_address("glXSwapIntervalEXT"); - if addr.is_null() { - return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); - } else { - std::mem::transmute(addr) - } - }; - - error_handler.check()?; - - let profile_mask = match config.profile { - Profile::Core => glx::arb::GLX_CONTEXT_CORE_PROFILE_BIT_ARB, - Profile::Compatibility => glx::arb::GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB, - }; - - #[rustfmt::skip] - let ctx_attribs = [ - glx::arb::GLX_CONTEXT_MAJOR_VERSION_ARB, config.version.0 as i32, - glx::arb::GLX_CONTEXT_MINOR_VERSION_ARB, config.version.1 as i32, - glx::arb::GLX_CONTEXT_PROFILE_MASK_ARB, profile_mask, - 0, - ]; - - let context = unsafe { - glXCreateContextAttribsARB( - display, - *fb_config, - std::ptr::null_mut(), - 1, - ctx_attribs.as_ptr(), - ) - }; - - error_handler.check()?; - - if context.is_null() { - return Err(GlError::CreationFailed(CreationFailedError::ContextCreationFailed)); + // Now that we have a matching framebuffer config, we need to know which visual matches + // thsi config so the window is compatible with the OpenGL context we're about to create + let fb_config = *fb_config; + let visual = glx::glXGetVisualFromFBConfig(display, fb_config); + if visual.is_null() { + return Err(GlError::CreationFailed(CreationFailedError::NoVisual)); } - unsafe { - let res = glx::glXMakeCurrent(display, handle.window, context); - error_handler.check()?; - if res == 0 { - return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); - } - - glXSwapIntervalEXT(display, handle.window, config.vsync as i32); - error_handler.check()?; - - if glx::glXMakeCurrent(display, 0, std::ptr::null_mut()) == 0 { - error_handler.check()?; - return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); - } - } - - Ok(GlContext { window: handle.window, display, context }) + Ok(( + FbConfig { fb_config, gl_config: config }, + WindowConfig { depth: (*visual).depth as u8, visual: (*visual).visualid as u32 }, + )) }) } diff --git a/src/x11/window.rs b/src/x11/window.rs index 9bae212..ab56435 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -7,6 +7,8 @@ use std::thread; use std::time::*; use raw_window_handle::{HasRawWindowHandle, RawWindowHandle, XlibHandle}; +use xcb::ffi::xcb_screen_t; +use xcb::StructPtr; use super::XcbConnection; use crate::{ @@ -17,7 +19,7 @@ use crate::{ use super::keyboard::{convert_key_press_event, convert_key_release_event}; #[cfg(feature = "opengl")] -use crate::gl::GlContext; +use crate::gl::{platform, GlContext}; pub struct WindowHandle { raw_window_handle: Option, @@ -107,6 +109,12 @@ pub struct Window { // Hack to allow sending a RawWindowHandle between threads. Do not make public struct SendableRwh(RawWindowHandle); +/// Quick wrapper to satisfy [HasRawWindowHandle], because of course a raw window handle wouldn't +/// have a raw window handle, that would be silly. +struct RawWindowHandleWrapper { + handle: RawWindowHandle, +} + unsafe impl Send for SendableRwh {} type WindowOpenResult = Result; @@ -211,23 +219,33 @@ impl Window { let window_info = WindowInfo::from_logical_size(options.size, scaling); - // The window need to have a depth of 32 so so we can create graphics contexts with alpha - // buffers - let mut depth = xcb::COPY_FROM_PARENT as u8; - let mut visual = xcb::COPY_FROM_PARENT as u32; - 'match_visual: for candidate_depth in screen.allowed_depths() { - if candidate_depth.depth() != 32 { - continue; - } - - for candidate_visual in candidate_depth.visuals() { - if candidate_visual.class() == xcb::VISUAL_CLASS_TRUE_COLOR as u8 { - depth = candidate_depth.depth(); - visual = candidate_visual.visual_id(); - break 'match_visual; - } - } - } + // Now it starts becoming fun. If we're creating an OpenGL context, then we need to create + // the window with a visual that matches the framebuffer used for the OpenGL context. So the + // idea is that we first retrieve a framebuffer config that matches our wanted OpenGL + // configuration, find the visual that matches that framebuffer config, create the window + // with that visual, and then finally create an OpenGL context for the window. If we don't + // use OpenGL, then we'll just take a random visual with a 32-bit depth. + let create_default_config = || { + Self::find_visual_for_depth(&screen, 32) + .map(|visual| (32, visual)) + .unwrap_or((xcb::COPY_FROM_PARENT as u8, xcb::COPY_FROM_PARENT as u32)) + }; + #[cfg(feature = "opengl")] + let (fb_config, (depth, visual)) = match options.gl_config { + Some(gl_config) => unsafe { + platform::GlContext::get_fb_config_and_visual( + xcb_connection.conn.get_raw_dpy(), + gl_config, + ) + .map(|(fb_config, window_config)| { + (Some(fb_config), (window_config.depth, window_config.visual)) + }) + .expect("Could not fetch framebuffer config") + }, + None => (None, create_default_config()), + }; + #[cfg(not(feature = "opengl"))] + let (depth, visual) = create_default_config(); // For this 32-bith depth to work, you also need to define a color map and set a border // pixel: https://cgit.freedesktop.org/xorg/xserver/tree/dix/window.c#n818 @@ -300,6 +318,22 @@ impl Window { xcb_connection.conn.flush(); + // TODO: These APIs could use a couple tweaks now that everything is internal and there is + // no error handling anymore at this point. Everything is more or less unchanged + // compared to when raw-gl-context was a separate crate. + #[cfg(feature = "opengl")] + let gl_context = fb_config.map(|fb_config| { + let mut handle = XlibHandle::empty(); + handle.window = window_id as c_ulong; + handle.display = xcb_connection.conn.get_raw_dpy() as *mut c_void; + let handle = RawWindowHandleWrapper { handle: RawWindowHandle::Xlib(handle) }; + + // Because of the visual negotation we had to take some extra steps to create this context + let context = unsafe { platform::GlContext::create(&handle, fb_config) } + .expect("Could not create OpenGL context"); + GlContext::new(context) + }); + let mut window = Self { xcb_connection, window_id, @@ -314,7 +348,7 @@ impl Window { parent_handle, #[cfg(feature = "opengl")] - gl_context: todo!("Create the X11 OpenGL context"), + gl_context, }; let mut handler = build(&mut crate::Window::new(&mut window)); @@ -360,6 +394,22 @@ impl Window { self.gl_context.as_ref() } + fn find_visual_for_depth(screen: &StructPtr, depth: u8) -> Option { + for candidate_depth in screen.allowed_depths() { + if candidate_depth.depth() != depth { + continue; + } + + for candidate_visual in candidate_depth.visuals() { + if candidate_visual.class() == xcb::VISUAL_CLASS_TRUE_COLOR as u8 { + return Some(candidate_visual.visual_id()); + } + } + } + + None + } + #[inline] fn drain_xcb_events(&mut self, handler: &mut dyn WindowHandler) { // the X server has a tendency to send spurious/extraneous configure notify events when a @@ -617,6 +667,12 @@ unsafe impl HasRawWindowHandle for Window { } } +unsafe impl HasRawWindowHandle for RawWindowHandleWrapper { + fn raw_window_handle(&self) -> RawWindowHandle { + self.handle + } +} + fn mouse_id(id: u8) -> MouseButton { match id { 1 => MouseButton::Left, From cb714401d8a5db3001f09e0513bdd13e8fe17eb5 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 20:25:01 +0100 Subject: [PATCH 12/22] Build both with and without opengl on CI --- .github/workflows/rust.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index cb5a8cd..096c5e9 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -21,7 +21,9 @@ jobs: with: toolchain: stable override: true - - name: Build + - name: Build with default features run: cargo build --examples --workspace --verbose + - name: Build again with all features + run: cargo build --examples --workspace --all-features --verbose - name: Run tests - run: cargo test --examples --workspace --verbose + run: cargo test --examples --workspace --all-features --verbose From 690a94f9f157ddf967594d8a26526f00f39b6e7d Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 21:35:24 +0100 Subject: [PATCH 13/22] Enable the glx feature for the x11 crate --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 3994885..eb23f4e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ license = "MIT OR Apache-2.0" [features] default = [] -opengl = ["uuid"] +opengl = ["uuid", "x11/glx"] [dependencies] keyboard-types = { version = "0.6.1", default-features = false } From 2f7f177be8c039352ce9aa351cb55aa7ed624e1e Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 21:51:54 +0100 Subject: [PATCH 14/22] Move RawWindowHandleWrapper to a shared module We're going to need this for the other platforms as well. --- src/window.rs | 12 ++++++++++++ src/x11/window.rs | 13 +------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/window.rs b/src/window.rs index 091a594..a0d91f6 100644 --- a/src/window.rs +++ b/src/window.rs @@ -18,6 +18,12 @@ pub struct WindowHandle { phantom: PhantomData<*mut ()>, } +/// Quick wrapper to satisfy [HasRawWindowHandle], because of course a raw window handle wouldn't +/// have a raw window handle, that would be silly. +pub(crate) struct RawWindowHandleWrapper { + pub handle: RawWindowHandle, +} + impl WindowHandle { fn new(window_handle: platform::WindowHandle) -> Self { Self { window_handle, phantom: PhantomData::default() } @@ -105,3 +111,9 @@ unsafe impl<'a> HasRawWindowHandle for Window<'a> { self.window.raw_window_handle() } } + +unsafe impl HasRawWindowHandle for RawWindowHandleWrapper { + fn raw_window_handle(&self) -> RawWindowHandle { + self.handle + } +} diff --git a/src/x11/window.rs b/src/x11/window.rs index ab56435..c1a81ac 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -11,6 +11,7 @@ use xcb::ffi::xcb_screen_t; use xcb::StructPtr; use super::XcbConnection; +use crate::window::RawWindowHandleWrapper; use crate::{ Event, MouseButton, MouseCursor, MouseEvent, PhyPoint, PhySize, ScrollDelta, WindowEvent, WindowHandler, WindowInfo, WindowOpenOptions, WindowScalePolicy, @@ -109,12 +110,6 @@ pub struct Window { // Hack to allow sending a RawWindowHandle between threads. Do not make public struct SendableRwh(RawWindowHandle); -/// Quick wrapper to satisfy [HasRawWindowHandle], because of course a raw window handle wouldn't -/// have a raw window handle, that would be silly. -struct RawWindowHandleWrapper { - handle: RawWindowHandle, -} - unsafe impl Send for SendableRwh {} type WindowOpenResult = Result; @@ -667,12 +662,6 @@ unsafe impl HasRawWindowHandle for Window { } } -unsafe impl HasRawWindowHandle for RawWindowHandleWrapper { - fn raw_window_handle(&self) -> RawWindowHandle { - self.handle - } -} - fn mouse_id(id: u8) -> MouseButton { match id { 1 => MouseButton::Left, From fe107ab378d87ea03cd999ac03bf74778da17624 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 22:36:37 +0100 Subject: [PATCH 15/22] Implement the OpenGL context on Windows --- src/win/window.rs | 17 +++++++++++------ src/x11/window.rs | 6 ++++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/win/window.rs b/src/win/window.rs index 59e0e26..db35523 100644 --- a/src/win/window.rs +++ b/src/win/window.rs @@ -16,7 +16,7 @@ use winapi::um::winuser::{ }; use std::cell::RefCell; -use std::ffi::OsStr; +use std::ffi::{c_void, OsStr}; use std::marker::PhantomData; use std::os::windows::ffi::OsStrExt; use std::ptr::null_mut; @@ -35,7 +35,7 @@ use crate::{ use super::keyboard::KeyboardState; #[cfg(feature = "opengl")] -use crate::gl::GlContext; +use crate::{gl::GlContext, window::RawWindowHandleWrapper}; unsafe fn generate_guid() -> String { let mut guid: GUID = std::mem::zeroed(); @@ -84,7 +84,7 @@ unsafe impl HasRawWindowHandle for WindowHandle { fn raw_window_handle(&self) -> RawWindowHandle { if let Some(hwnd) = self.hwnd { let mut handle = Win32Handle::empty(); - handle.hwnd = hwnd as *mut std::ffi::c_void; + handle.hwnd = hwnd as *mut c_void; RawWindowHandle::Win32(handle) } else { @@ -483,8 +483,13 @@ impl Window { // todo: manage error ^ #[cfg(feature = "opengl")] - let gl_context: Arc> = - Arc::new(todo!("Create the Windows OpenGL context")); + let gl_context: Arc> = Arc::new(options.gl_config.map(|gl_config| { + let mut handle = Win32Handle::empty(); + handle.hwnd = hwnd as *mut c_void; + let handle = RawWindowHandleWrapper { handle: RawWindowHandle::Win32(handle) }; + + GlContext::create(&handle, gl_config).expect("Could not create OpenGL context") + })); #[cfg(not(feature = "opengl"))] let handler = Box::new(build(&mut crate::Window::new(&mut Window { hwnd }))); @@ -583,7 +588,7 @@ impl Window { unsafe impl HasRawWindowHandle for Window { fn raw_window_handle(&self) -> RawWindowHandle { let mut handle = Win32Handle::empty(); - handle.hwnd = self.hwnd as *mut std::ffi::c_void; + handle.hwnd = self.hwnd as *mut c_void; RawWindowHandle::Win32(handle) } diff --git a/src/x11/window.rs b/src/x11/window.rs index c1a81ac..8e378df 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -11,7 +11,6 @@ use xcb::ffi::xcb_screen_t; use xcb::StructPtr; use super::XcbConnection; -use crate::window::RawWindowHandleWrapper; use crate::{ Event, MouseButton, MouseCursor, MouseEvent, PhyPoint, PhySize, ScrollDelta, WindowEvent, WindowHandler, WindowInfo, WindowOpenOptions, WindowScalePolicy, @@ -20,7 +19,10 @@ use crate::{ use super::keyboard::{convert_key_press_event, convert_key_release_event}; #[cfg(feature = "opengl")] -use crate::gl::{platform, GlContext}; +use crate::{ + gl::{platform, GlContext}, + window::RawWindowHandleWrapper, +}; pub struct WindowHandle { raw_window_handle: Option, From d83517565ba8b2be1870450739e1c0dea2e6c2d2 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 22:43:27 +0100 Subject: [PATCH 16/22] Implement the OpenGL context on macOS --- src/macos/window.rs | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/macos/window.rs b/src/macos/window.rs index fd23379..a8520ed 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -1,5 +1,6 @@ use std::ffi::c_void; use std::marker::PhantomData; +use std::ptr; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -27,7 +28,10 @@ use super::keyboard::KeyboardState; use super::view::{create_view, BASEVIEW_STATE_IVAR}; #[cfg(feature = "opengl")] -use crate::gl::{GlConfig, GlContext}; +use crate::{ + gl::{GlConfig, GlContext}, + window::RawWindowHandleWrapper, +}; pub struct WindowHandle { raw_window_handle: Option, @@ -135,7 +139,9 @@ impl Window { close_requested: false, #[cfg(feature = "opengl")] - gl_context: options.gl_config.map(Self::create_gl_context), + gl_context: options + .gl_config + .map(|gl_config| Self::create_gl_context(None, ns_view, gl_config)), }; let window_handle = Self::init(true, window, build); @@ -167,7 +173,9 @@ impl Window { close_requested: false, #[cfg(feature = "opengl")] - gl_context: options.gl_config.map(Self::create_gl_context), + gl_context: options + .gl_config + .map(|gl_config| Self::create_gl_context(None, ns_view, gl_config)), }; let window_handle = Self::init(true, window, build); @@ -241,7 +249,9 @@ impl Window { close_requested: false, #[cfg(feature = "opengl")] - gl_context: options.gl_config.map(Self::create_gl_context), + gl_context: options + .gl_config + .map(|gl_config| Self::create_gl_context(Some(ns_window), ns_view, gl_config)), }; let _ = Self::init(false, window, build); @@ -298,8 +308,13 @@ impl Window { } #[cfg(feature = "opengl")] - fn create_gl_context(config: GlConfig) -> GlContext { - todo!("Create the macOS OpenGL context"); + fn create_gl_context(ns_window: Option, ns_view: id, config: GlConfig) -> GlContext { + let mut handle = AppKitHandle::empty(); + handle.ns_window = ns_window.unwrap_or(ptr::null_mut()) as *mut c_void; + handle.ns_view = ns_view as *mut c_void; + let handle = RawWindowHandleWrapper { handle: RawWindowHandle::AppKit(handle) }; + + unsafe { GlContext::create(&handle, config).expect("Could not create OpenGL context") } } } @@ -408,7 +423,7 @@ impl WindowState { // Clear ivar before triggering WindowEvent::WillClose. Otherwise, if the // handler of the event causes another call to release, this function could be // called again, leading to a double free. - ns_view_obj.set_ivar(BASEVIEW_STATE_IVAR, ::std::ptr::null() as *const c_void); + ns_view_obj.set_ivar(BASEVIEW_STATE_IVAR, ptr::null() as *const c_void); window_state.trigger_event(Event::Window(WindowEvent::WillClose)); @@ -421,7 +436,7 @@ impl WindowState { unsafe impl HasRawWindowHandle for Window { fn raw_window_handle(&self) -> RawWindowHandle { - let ns_window = self.ns_window.unwrap_or(::std::ptr::null_mut()) as *mut c_void; + let ns_window = self.ns_window.unwrap_or(ptr::null_mut()) as *mut c_void; let mut handle = AppKitHandle::empty(); handle.ns_window = ns_window; From 43860aba84a7303eed47dfafefcee6dde952afc4 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Feb 2022 23:10:53 +0100 Subject: [PATCH 17/22] Fix a whackton of warnings and clippy lints The things remaining are all of the cursor things in the X11 implementation (there's _a lot_ of it, so there's probably a reason why it's all still there but unused), and the super unsound immutable reference to mutable reference cast in the macOS implementation that Clippy automatically errors out on. --- src/gl/mod.rs | 6 +- src/gl/win.rs | 2 +- src/keyboard.rs | 1 + src/macos/window.rs | 2 +- src/win/keyboard.rs | 124 +++----------------------------------- src/win/window.rs | 11 ++-- src/x11/cursor.rs | 2 +- src/x11/window.rs | 37 ++++++------ src/x11/xcb_connection.rs | 9 ++- 9 files changed, 46 insertions(+), 148 deletions(-) diff --git a/src/gl/mod.rs b/src/gl/mod.rs index f3e786f..a7922b7 100644 --- a/src/gl/mod.rs +++ b/src/gl/mod.rs @@ -1,8 +1,10 @@ -use raw_window_handle::HasRawWindowHandle; - use std::ffi::c_void; use std::marker::PhantomData; +// On X11 creating the context is a two step process +#[cfg(not(target_os = "linux"))] +use raw_window_handle::HasRawWindowHandle; + #[cfg(target_os = "windows")] mod win; #[cfg(target_os = "windows")] diff --git a/src/gl/win.rs b/src/gl/win.rs index 57982c9..f98734f 100644 --- a/src/gl/win.rs +++ b/src/gl/win.rs @@ -255,7 +255,7 @@ impl GlContext { let hglrc = wglCreateContextAttribsARB.unwrap()(hdc, std::ptr::null_mut(), ctx_attribs.as_ptr()); - if hglrc == std::ptr::null_mut() { + if hglrc.is_null() { return Err(GlError::CreationFailed(())); } diff --git a/src/keyboard.rs b/src/keyboard.rs index c2af463..c56abd3 100644 --- a/src/keyboard.rs +++ b/src/keyboard.rs @@ -17,6 +17,7 @@ //! Keyboard types. +#[cfg(any(target_os = "linux", target_os = "macos"))] use keyboard_types::{Code, Location}; #[cfg(any(target_os = "linux", target_os = "macos"))] diff --git a/src/macos/window.rs b/src/macos/window.rs index a8520ed..0875377 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -44,7 +44,7 @@ pub struct WindowHandle { impl WindowHandle { pub fn close(&mut self) { - if let Some(_) = self.raw_window_handle.take() { + if self.raw_window_handle.take().is_some() { self.close_requested.store(true, Ordering::Relaxed); } } diff --git a/src/win/keyboard.rs b/src/win/keyboard.rs index f04d2cc..e4045d1 100644 --- a/src/win/keyboard.rs +++ b/src/win/keyboard.rs @@ -19,7 +19,6 @@ use std::cmp::Ordering; use std::collections::{HashMap, HashSet}; -use std::convert::TryInto; use std::mem; use std::ops::RangeInclusive; @@ -29,15 +28,15 @@ use winapi::shared::minwindef::{HKL, INT, LPARAM, UINT, WPARAM}; use winapi::shared::ntdef::SHORT; use winapi::shared::windef::HWND; use winapi::um::winuser::{ - GetKeyState, GetKeyboardLayout, MapVirtualKeyExW, PeekMessageW, ToUnicodeEx, VkKeyScanW, - MAPVK_VK_TO_CHAR, MAPVK_VSC_TO_VK_EX, PM_NOREMOVE, VK_ACCEPT, VK_ADD, VK_APPS, VK_ATTN, - VK_BACK, VK_BROWSER_BACK, VK_BROWSER_FAVORITES, VK_BROWSER_FORWARD, VK_BROWSER_HOME, - VK_BROWSER_REFRESH, VK_BROWSER_SEARCH, VK_BROWSER_STOP, VK_CANCEL, VK_CAPITAL, VK_CLEAR, - VK_CONTROL, VK_CONVERT, VK_CRSEL, VK_DECIMAL, VK_DELETE, VK_DIVIDE, VK_DOWN, VK_END, VK_EREOF, - VK_ESCAPE, VK_EXECUTE, VK_EXSEL, VK_F1, VK_F10, VK_F11, VK_F12, VK_F2, VK_F3, VK_F4, VK_F5, - VK_F6, VK_F7, VK_F8, VK_F9, VK_FINAL, VK_HELP, VK_HOME, VK_INSERT, VK_JUNJA, VK_KANA, VK_KANJI, - VK_LAUNCH_APP1, VK_LAUNCH_APP2, VK_LAUNCH_MAIL, VK_LAUNCH_MEDIA_SELECT, VK_LCONTROL, VK_LEFT, - VK_LMENU, VK_LSHIFT, VK_LWIN, VK_MEDIA_NEXT_TRACK, VK_MEDIA_PLAY_PAUSE, VK_MEDIA_PREV_TRACK, + GetKeyState, GetKeyboardLayout, MapVirtualKeyExW, PeekMessageW, ToUnicodeEx, MAPVK_VK_TO_CHAR, + MAPVK_VSC_TO_VK_EX, PM_NOREMOVE, VK_ACCEPT, VK_ADD, VK_APPS, VK_ATTN, VK_BACK, VK_BROWSER_BACK, + VK_BROWSER_FAVORITES, VK_BROWSER_FORWARD, VK_BROWSER_HOME, VK_BROWSER_REFRESH, + VK_BROWSER_SEARCH, VK_BROWSER_STOP, VK_CANCEL, VK_CAPITAL, VK_CLEAR, VK_CONTROL, VK_CONVERT, + VK_CRSEL, VK_DECIMAL, VK_DELETE, VK_DIVIDE, VK_DOWN, VK_END, VK_EREOF, VK_ESCAPE, VK_EXECUTE, + VK_EXSEL, VK_F1, VK_F10, VK_F11, VK_F12, VK_F2, VK_F3, VK_F4, VK_F5, VK_F6, VK_F7, VK_F8, + VK_F9, VK_FINAL, VK_HELP, VK_HOME, VK_INSERT, VK_JUNJA, VK_KANA, VK_KANJI, VK_LAUNCH_APP1, + VK_LAUNCH_APP2, VK_LAUNCH_MAIL, VK_LAUNCH_MEDIA_SELECT, VK_LCONTROL, VK_LEFT, VK_LMENU, + VK_LSHIFT, VK_LWIN, VK_MEDIA_NEXT_TRACK, VK_MEDIA_PLAY_PAUSE, VK_MEDIA_PREV_TRACK, VK_MEDIA_STOP, VK_MENU, VK_MODECHANGE, VK_MULTIPLY, VK_NEXT, VK_NONCONVERT, VK_NUMLOCK, VK_NUMPAD0, VK_NUMPAD1, VK_NUMPAD2, VK_NUMPAD3, VK_NUMPAD4, VK_NUMPAD5, VK_NUMPAD6, VK_NUMPAD7, VK_NUMPAD8, VK_NUMPAD9, VK_OEM_ATTN, VK_OEM_CLEAR, VK_PAUSE, VK_PLAY, VK_PRINT, VK_PRIOR, @@ -344,110 +343,6 @@ fn vk_to_key(vk: VkCode) -> Option { }) } -/// Convert a key to a virtual key code. -/// -/// The virtual key code is needed in various winapi interfaces, including -/// accelerators. This provides the virtual key code in the current keyboard -/// map. -/// -/// The virtual key code can have modifiers in the higher order byte when the -/// argument is a `Character` variant. See: -/// https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-vkkeyscanw -pub(crate) fn key_to_vk(key: &Key) -> Option { - Some(match key { - Key::Character(s) => { - if let Some(code_point) = s.chars().next() { - if let Ok(wchar) = (code_point as u32).try_into() { - unsafe { VkKeyScanW(wchar) as i32 } - } else { - return None; - } - } else { - return None; - } - } - Key::Cancel => VK_CANCEL, - Key::Backspace => VK_BACK, - Key::Tab => VK_TAB, - Key::Clear => VK_CLEAR, - Key::Enter => VK_RETURN, - Key::Shift => VK_SHIFT, - Key::Control => VK_CONTROL, - Key::Alt => VK_MENU, - Key::Pause => VK_PAUSE, - Key::CapsLock => VK_CAPITAL, - // TODO: disambiguate kana and hangul? same vk - Key::KanaMode => VK_KANA, - Key::JunjaMode => VK_JUNJA, - Key::FinalMode => VK_FINAL, - Key::KanjiMode => VK_KANJI, - Key::Escape => VK_ESCAPE, - Key::NonConvert => VK_NONCONVERT, - Key::Accept => VK_ACCEPT, - Key::PageUp => VK_PRIOR, - Key::PageDown => VK_NEXT, - Key::End => VK_END, - Key::Home => VK_HOME, - Key::ArrowLeft => VK_LEFT, - Key::ArrowUp => VK_UP, - Key::ArrowRight => VK_RIGHT, - Key::ArrowDown => VK_DOWN, - Key::Select => VK_SELECT, - Key::Print => VK_PRINT, - Key::Execute => VK_EXECUTE, - Key::PrintScreen => VK_SNAPSHOT, - Key::Insert => VK_INSERT, - Key::Delete => VK_DELETE, - Key::Help => VK_HELP, - Key::Meta => VK_LWIN, - Key::ContextMenu => VK_APPS, - Key::Standby => VK_SLEEP, - Key::F1 => VK_F1, - Key::F2 => VK_F2, - Key::F3 => VK_F3, - Key::F4 => VK_F4, - Key::F5 => VK_F5, - Key::F6 => VK_F6, - Key::F7 => VK_F7, - Key::F8 => VK_F8, - Key::F9 => VK_F9, - Key::F10 => VK_F10, - Key::F11 => VK_F11, - Key::F12 => VK_F12, - Key::NumLock => VK_NUMLOCK, - Key::ScrollLock => VK_SCROLL, - Key::BrowserBack => VK_BROWSER_BACK, - Key::BrowserForward => VK_BROWSER_FORWARD, - Key::BrowserRefresh => VK_BROWSER_REFRESH, - Key::BrowserStop => VK_BROWSER_STOP, - Key::BrowserSearch => VK_BROWSER_SEARCH, - Key::BrowserFavorites => VK_BROWSER_FAVORITES, - Key::BrowserHome => VK_BROWSER_HOME, - Key::AudioVolumeMute => VK_VOLUME_MUTE, - Key::AudioVolumeDown => VK_VOLUME_DOWN, - Key::AudioVolumeUp => VK_VOLUME_UP, - Key::MediaTrackNext => VK_MEDIA_NEXT_TRACK, - Key::MediaTrackPrevious => VK_MEDIA_PREV_TRACK, - Key::MediaStop => VK_MEDIA_STOP, - Key::MediaPlayPause => VK_MEDIA_PLAY_PAUSE, - Key::LaunchMail => VK_LAUNCH_MAIL, - Key::LaunchMediaPlayer => VK_LAUNCH_MEDIA_SELECT, - Key::LaunchApplication1 => VK_LAUNCH_APP1, - Key::LaunchApplication2 => VK_LAUNCH_APP2, - Key::Alphanumeric => VK_OEM_ATTN, - Key::Convert => VK_CONVERT, - Key::ModeChange => VK_MODECHANGE, - Key::Process => VK_PROCESSKEY, - Key::Attn => VK_ATTN, - Key::CrSel => VK_CRSEL, - Key::ExSel => VK_EXSEL, - Key::EraseEof => VK_EREOF, - Key::Play => VK_PLAY, - Key::ZoomToggle => VK_ZOOM, - _ => return None, - }) -} - fn code_unit_to_key(code_unit: u32) -> Key { match code_unit { 0x8 | 0x7F => Key::Backspace, @@ -692,6 +587,7 @@ impl KeyboardState { key_state[VK_LCONTROL as usize] = if has_altgr { 0x80 } else { 0 }; key_state[VK_MENU as usize] = if has_altgr { 0x80 } else { 0 }; key_state[VK_RMENU as usize] = if has_altgr { 0x80 } else { 0 }; + #[allow(clippy::iter_overeager_cloned)] for vk in PRINTABLE_VKS.iter().cloned().flatten() { let ret = ToUnicodeEx( vk as UINT, diff --git a/src/win/window.rs b/src/win/window.rs index db35523..42caafa 100644 --- a/src/win/window.rs +++ b/src/win/window.rs @@ -205,11 +205,8 @@ unsafe extern "system" fn wnd_proc( } } WM_TIMER => { - match wparam { - WIN_FRAME_TIMER => { - window_state.handler.on_frame(&mut window); - } - _ => (), + if wparam == WIN_FRAME_TIMER { + window_state.handler.on_frame(&mut window); } return 0; } @@ -414,8 +411,8 @@ impl Window { break; } - TranslateMessage(&mut msg); - DispatchMessageW(&mut msg); + TranslateMessage(&msg); + DispatchMessageW(&msg); } } } diff --git a/src/x11/cursor.rs b/src/x11/cursor.rs index 6f3851e..8e8fd88 100644 --- a/src/x11/cursor.rs +++ b/src/x11/cursor.rs @@ -101,5 +101,5 @@ pub(super) fn get_xcursor(display: *mut x11::xlib::Display, cursor: MouseCursor) MouseCursor::RowResize => loadn(&[b"split_v\0", b"v_double_arrow\0"]), }; - cursor.or(load(b"left_ptr\0")).unwrap_or(0) + cursor.or_else(|| load(b"left_ptr\0")).unwrap_or(0) } diff --git a/src/x11/window.rs b/src/x11/window.rs index 8e378df..bfe9567 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -35,7 +35,7 @@ pub struct WindowHandle { impl WindowHandle { pub fn close(&mut self) { - if let Some(_) = self.raw_window_handle.take() { + if self.raw_window_handle.take().is_some() { // FIXME: This will need to be changed from just setting an atomic to somehow // synchronizing with the window being closed (using a synchronous channel, or // by joining on the event loop thread). @@ -96,6 +96,7 @@ pub struct Window { xcb_connection: XcbConnection, window_id: u32, window_info: WindowInfo, + // FIXME: There's all this mouse cursor logic but it's never actually used, is this correct? mouse_cursor: MouseCursor, frame_interval: Duration, @@ -135,7 +136,7 @@ impl Window { let (parent_handle, mut window_handle) = ParentHandle::new(); - let thread = thread::spawn(move || { + thread::spawn(move || { Self::window_thread(Some(parent_id), options, build, tx.clone(), Some(parent_handle)); }); @@ -155,7 +156,7 @@ impl Window { let (parent_handle, mut window_handle) = ParentHandle::new(); - let thread = thread::spawn(move || { + thread::spawn(move || { Self::window_thread(None, options, build, tx.clone(), Some(parent_handle)); }); @@ -174,12 +175,14 @@ impl Window { let (tx, rx) = mpsc::sync_channel::(1); let thread = thread::spawn(move || { - Self::window_thread(None, options, build, tx.clone(), None); + Self::window_thread(None, options, build, tx, None); }); let _ = rx.recv().unwrap().unwrap(); - thread.join(); + thread.join().unwrap_or_else(|err| { + eprintln!("Window thread panicked: {:#?}", err); + }); } fn window_thread( @@ -302,16 +305,16 @@ impl Window { title.as_bytes(), ); - xcb_connection.atoms.wm_protocols.zip(xcb_connection.atoms.wm_delete_window).map( - |(wm_protocols, wm_delete_window)| { - xcb_util::icccm::set_wm_protocols( - &xcb_connection.conn, - window_id, - wm_protocols, - &[wm_delete_window], - ); - }, - ); + if let Some((wm_protocols, wm_delete_window)) = + xcb_connection.atoms.wm_protocols.zip(xcb_connection.atoms.wm_delete_window) + { + xcb_util::icccm::set_wm_protocols( + &xcb_connection.conn, + window_id, + wm_protocols, + &[wm_delete_window], + ); + } xcb_connection.conn.flush(); @@ -636,7 +639,7 @@ impl Window { handler.on_event( &mut crate::Window::new(self), - Event::Keyboard(convert_key_press_event(&event)), + Event::Keyboard(convert_key_press_event(event)), ); } @@ -645,7 +648,7 @@ impl Window { handler.on_event( &mut crate::Window::new(self), - Event::Keyboard(convert_key_release_event(&event)), + Event::Keyboard(convert_key_release_event(event)), ); } diff --git a/src/x11/xcb_connection.rs b/src/x11/xcb_connection.rs index 083249b..179e4c7 100644 --- a/src/x11/xcb_connection.rs +++ b/src/x11/xcb_connection.rs @@ -11,7 +11,6 @@ use super::cursor; pub(crate) struct Atoms { pub wm_protocols: Option, pub wm_delete_window: Option, - pub wm_normal_hints: Option, } pub struct XcbConnection { @@ -20,6 +19,7 @@ pub struct XcbConnection { pub(crate) atoms: Atoms, + // FIXME: Same here, there's a ton of unused cursor machinery in here pub(super) cursor_cache: HashMap, } @@ -46,14 +46,13 @@ impl XcbConnection { conn.set_event_queue_owner(xcb::base::EventQueueOwner::Xcb); - let (wm_protocols, wm_delete_window, wm_normal_hints) = - intern_atoms!(&conn, WM_PROTOCOLS, WM_DELETE_WINDOW, WM_NORMAL_HINTS); + let (wm_protocols, wm_delete_window) = intern_atoms!(&conn, WM_PROTOCOLS, WM_DELETE_WINDOW); Ok(Self { conn, xlib_display, - atoms: Atoms { wm_protocols, wm_delete_window, wm_normal_hints }, + atoms: Atoms { wm_protocols, wm_delete_window }, cursor_cache: HashMap::new(), }) @@ -136,7 +135,7 @@ impl XcbConnection { #[inline] pub fn get_scaling(&self) -> Option { - self.get_scaling_xft().or(self.get_scaling_screen_dimensions()) + self.get_scaling_xft().or_else(|| self.get_scaling_screen_dimensions()) } #[inline] From ba442a438236975cd6fffa98180e38a02730a864 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 10 Feb 2022 03:12:34 +0100 Subject: [PATCH 18/22] Fix overlapping cell borrows in Windows impl b4a3d2bb04331d003d72c5919514183b6eee06f9 missed some immutable borrows, but these borrows need to be local anyways because of the way the closing is implemented. --- src/win/window.rs | 45 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/src/win/window.rs b/src/win/window.rs index 42caafa..2f4f056 100644 --- a/src/win/window.rs +++ b/src/win/window.rs @@ -127,13 +127,12 @@ unsafe extern "system" fn wnd_proc( let window_state_ptr = GetWindowLongPtrW(hwnd, GWLP_USERDATA) as *mut RefCell; if !window_state_ptr.is_null() { - let mut window_state = (*window_state_ptr).borrow_mut(); - - let mut window = window_state.create_window(hwnd); - let mut window = crate::Window::new(&mut window); - match msg { WM_MOUSEMOVE => { + let mut window_state = (*window_state_ptr).borrow_mut(); + let mut window = window_state.create_window(hwnd); + let mut window = crate::Window::new(&mut window); + let x = (lparam & 0xFFFF) as i16 as i32; let y = ((lparam >> 16) & 0xFFFF) as i16 as i32; @@ -148,6 +147,10 @@ unsafe extern "system" fn wnd_proc( return 0; } WM_MOUSEWHEEL => { + let mut window_state = (*window_state_ptr).borrow_mut(); + let mut window = window_state.create_window(hwnd); + let mut window = crate::Window::new(&mut window); + let value = (wparam >> 16) as i16; let value = value as i32; let value = value as f32 / WHEEL_DELTA as f32; @@ -163,7 +166,11 @@ unsafe extern "system" fn wnd_proc( } WM_LBUTTONDOWN | WM_LBUTTONUP | WM_MBUTTONDOWN | WM_MBUTTONUP | WM_RBUTTONDOWN | WM_RBUTTONUP | WM_XBUTTONDOWN | WM_XBUTTONUP => { - let mut mouse_button_counter = (*window_state_ptr).borrow().mouse_button_counter; + let mut window_state = (*window_state_ptr).borrow_mut(); + let mut window = window_state.create_window(hwnd); + let mut window = crate::Window::new(&mut window); + + let mut mouse_button_counter = window_state.mouse_button_counter; let button = match msg { WM_LBUTTONDOWN | WM_LBUTTONUP => Some(MouseButton::Left), @@ -205,19 +212,37 @@ unsafe extern "system" fn wnd_proc( } } WM_TIMER => { + let mut window_state = (*window_state_ptr).borrow_mut(); + let mut window = window_state.create_window(hwnd); + let mut window = crate::Window::new(&mut window); + if wparam == WIN_FRAME_TIMER { window_state.handler.on_frame(&mut window); } return 0; } WM_CLOSE => { - window_state.handler.on_event(&mut window, Event::Window(WindowEvent::WillClose)); + // Make sure to release the borrow before the DefWindowProc call + { + let mut window_state = (*window_state_ptr).borrow_mut(); + let mut window = window_state.create_window(hwnd); + let mut window = crate::Window::new(&mut window); + + window_state + .handler + .on_event(&mut window, Event::Window(WindowEvent::WillClose)); + } + // DestroyWindow(hwnd); // return 0; return DefWindowProcW(hwnd, msg, wparam, lparam); } WM_CHAR | WM_SYSCHAR | WM_KEYDOWN | WM_SYSKEYDOWN | WM_KEYUP | WM_SYSKEYUP | WM_INPUTLANGCHANGE => { + let mut window_state = (*window_state_ptr).borrow_mut(); + let mut window = window_state.create_window(hwnd); + let mut window = crate::Window::new(&mut window); + let opt_event = window_state.keyboard_state.process_message(hwnd, msg, wparam, lparam); @@ -230,6 +255,10 @@ unsafe extern "system" fn wnd_proc( } } WM_SIZE => { + let mut window_state = (*window_state_ptr).borrow_mut(); + let mut window = window_state.create_window(hwnd); + let mut window = crate::Window::new(&mut window); + let width = (lparam & 0xFFFF) as u16 as u32; let height = ((lparam >> 16) & 0xFFFF) as u16 as u32; @@ -245,6 +274,8 @@ unsafe extern "system" fn wnd_proc( .on_event(&mut window, Event::Window(WindowEvent::Resized(window_info))); } WM_DPICHANGED => { + let mut window_state = (*window_state_ptr).borrow_mut(); + // To avoid weirdness with the realtime borrow checker. let new_rect = { if let WindowScalePolicy::SystemScaleFactor = window_state.scale_policy { From aa1ad823ce22fce24f0d6823ee70c452450d2bab Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Mar 2022 18:13:47 +0100 Subject: [PATCH 19/22] Use a thread local variable for the X11 error So in the situation that multiple X11 clients are handling errors from different threads they don't see and interfere with each other's errors. As mentioned in https://github.com/glowcoil/raw-gl-context/pull/15#pullrequestreview-891405163. --- src/gl/x11/errors.rs | 58 +++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/src/gl/x11/errors.rs b/src/gl/x11/errors.rs index ec1be08..965d755 100644 --- a/src/gl/x11/errors.rs +++ b/src/gl/x11/errors.rs @@ -3,11 +3,14 @@ use std::fmt::{Debug, Formatter}; use x11::xlib; use std::panic::AssertUnwindSafe; -use std::sync::atomic::{AtomicPtr, Ordering}; use std::sync::Mutex; -static CURRENT_ERROR_PTR: AtomicPtr>> = - AtomicPtr::new(::std::ptr::null_mut()); +thread_local! { + /// Used as part of [`XerrorHandler::handle()`]. When an X11 error occurs during this function, + /// the error gets copied to this mutex after which the program is allowed to resume. The error + /// can then be converted to a regular Rust Result value afterwards. + static CURRENT_X11_ERROR: Mutex> = Mutex::new(None); +} /// A helper struct for safe X11 error handling pub struct XErrorHandler<'a> { @@ -33,24 +36,15 @@ impl<'a> XErrorHandler<'a> { /// Sets up a temporary X11 error handler for the duration of the given closure, and allows /// that closure to check on the latest X11 error at any time pub fn handle T>( - display: *mut xlib::Display, - handler: F, + display: *mut xlib::Display, handler: F, ) -> T { unsafe extern "C" fn error_handler( - _dpy: *mut xlib::Display, - err: *mut xlib::XErrorEvent, + _dpy: *mut xlib::Display, err: *mut xlib::XErrorEvent, ) -> i32 { // SAFETY: the error pointer should be safe to copy let err = *err; - // SAFETY: the ptr can only point to a valid instance or be null - let mutex = CURRENT_ERROR_PTR.load(Ordering::SeqCst).as_ref(); - let mutex = if let Some(mutex) = mutex { - mutex - } else { - return 2; - }; - match mutex.lock() { + CURRENT_X11_ERROR.with(|mutex| match mutex.lock() { Ok(mut current_error) => { *current_error = Some(err); 0 @@ -62,7 +56,7 @@ impl<'a> XErrorHandler<'a> { ); 1 } - } + }) } // Flush all possible previous errors @@ -70,25 +64,23 @@ impl<'a> XErrorHandler<'a> { xlib::XSync(display, 0); } - let mut mutex = Mutex::new(None); - CURRENT_ERROR_PTR.store(&mut mutex, Ordering::SeqCst); + CURRENT_X11_ERROR.with(|mutex| { + // Make sure to clear any errors from the last call to this function + *mutex.lock().unwrap() = None; - let old_handler = unsafe { xlib::XSetErrorHandler(Some(error_handler)) }; - let panic_result = std::panic::catch_unwind(AssertUnwindSafe(|| { - let mut h = XErrorHandler { - display, - mutex: &mutex, - }; - handler(&mut h) - })); - // Whatever happened, restore old error handler - unsafe { xlib::XSetErrorHandler(old_handler) }; - CURRENT_ERROR_PTR.store(::core::ptr::null_mut(), Ordering::SeqCst); + let old_handler = unsafe { xlib::XSetErrorHandler(Some(error_handler)) }; + let panic_result = std::panic::catch_unwind(AssertUnwindSafe(|| { + let mut h = XErrorHandler { display, mutex: &mutex }; + handler(&mut h) + })); + // Whatever happened, restore old error handler + unsafe { xlib::XSetErrorHandler(old_handler) }; - match panic_result { - Ok(v) => v, - Err(e) => std::panic::resume_unwind(e), - } + match panic_result { + Ok(v) => v, + Err(e) => std::panic::resume_unwind(e), + } + }) } } From ef86e56ac7c003baf8d9d974febef719d7c3a00c Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Mar 2022 18:17:50 +0100 Subject: [PATCH 20/22] Keep the first X11 error --- src/gl/x11/errors.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/gl/x11/errors.rs b/src/gl/x11/errors.rs index 965d755..87d5287 100644 --- a/src/gl/x11/errors.rs +++ b/src/gl/x11/errors.rs @@ -45,10 +45,13 @@ impl<'a> XErrorHandler<'a> { let err = *err; CURRENT_X11_ERROR.with(|mutex| match mutex.lock() { - Ok(mut current_error) => { + // If multiple errors occur, keep the first one since that's likely going to be the + // cause of the other errors + Ok(mut current_error) if current_error.is_none() => { *current_error = Some(err); 0 } + Ok(_) => 0, Err(e) => { eprintln!( "[FATAL] raw-gl-context: Failed to lock for X11 Error Handler: {:?}", From 0aae938af0145eeff1f0156973ee5dc8b96e36d2 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Mar 2022 18:41:29 +0100 Subject: [PATCH 21/22] Remove unnecessary unsafe blocks --- src/gl/x11.rs | 49 ++++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/src/gl/x11.rs b/src/gl/x11.rs index 03a5540..f62d318 100644 --- a/src/gl/x11.rs +++ b/src/gl/x11.rs @@ -96,7 +96,7 @@ impl GlContext { errors::XErrorHandler::handle(display, |error_handler| { #[allow(non_snake_case)] - let glXCreateContextAttribsARB: GlXCreateContextAttribsARB = unsafe { + let glXCreateContextAttribsARB: GlXCreateContextAttribsARB = { let addr = get_proc_address("glXCreateContextAttribsARB"); if addr.is_null() { return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); @@ -106,7 +106,7 @@ impl GlContext { }; #[allow(non_snake_case)] - let glXSwapIntervalEXT: GlXSwapIntervalEXT = unsafe { + let glXSwapIntervalEXT: GlXSwapIntervalEXT = { let addr = get_proc_address("glXSwapIntervalEXT"); if addr.is_null() { return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); @@ -130,15 +130,13 @@ impl GlContext { 0, ]; - let context = unsafe { - glXCreateContextAttribsARB( - display, - config.fb_config, - std::ptr::null_mut(), - 1, - ctx_attribs.as_ptr(), - ) - }; + let context = glXCreateContextAttribsARB( + display, + config.fb_config, + std::ptr::null_mut(), + 1, + ctx_attribs.as_ptr(), + ); error_handler.check()?; @@ -146,20 +144,18 @@ impl GlContext { return Err(GlError::CreationFailed(CreationFailedError::ContextCreationFailed)); } - unsafe { - let res = glx::glXMakeCurrent(display, handle.window, context); - error_handler.check()?; - if res == 0 { - return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); - } + let res = glx::glXMakeCurrent(display, handle.window, context); + error_handler.check()?; + if res == 0 { + return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); + } - glXSwapIntervalEXT(display, handle.window, config.gl_config.vsync as i32); - error_handler.check()?; + glXSwapIntervalEXT(display, handle.window, config.gl_config.vsync as i32); + error_handler.check()?; - if glx::glXMakeCurrent(display, 0, std::ptr::null_mut()) == 0 { - error_handler.check()?; - return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); - } + if glx::glXMakeCurrent(display, 0, std::ptr::null_mut()) == 0 { + error_handler.check()?; + return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed)); } Ok(GlContext { window: handle.window, display, context }) @@ -173,7 +169,7 @@ impl GlContext { display: *mut xlib::_XDisplay, config: GlConfig, ) -> Result<(FbConfig, WindowConfig), GlError> { errors::XErrorHandler::handle(display, |error_handler| { - let screen = unsafe { xlib::XDefaultScreen(display) }; + let screen = xlib::XDefaultScreen(display); #[rustfmt::skip] let fb_attribs = [ @@ -195,9 +191,8 @@ impl GlContext { ]; let mut n_configs = 0; - let fb_config = unsafe { - glx::glXChooseFBConfig(display, screen, fb_attribs.as_ptr(), &mut n_configs) - }; + let fb_config = + glx::glXChooseFBConfig(display, screen, fb_attribs.as_ptr(), &mut n_configs); error_handler.check()?; if n_configs <= 0 || fb_config.is_null() { From 85b6437de827a2167d8e132525e253eb08fba2fe Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 7 Mar 2022 18:52:49 +0100 Subject: [PATCH 22/22] Swap out X11 error handling mutex for a RefCell --- src/gl/x11/errors.rs | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/gl/x11/errors.rs b/src/gl/x11/errors.rs index 87d5287..42847d9 100644 --- a/src/gl/x11/errors.rs +++ b/src/gl/x11/errors.rs @@ -2,20 +2,20 @@ use std::ffi::CStr; use std::fmt::{Debug, Formatter}; use x11::xlib; +use std::cell::RefCell; use std::panic::AssertUnwindSafe; -use std::sync::Mutex; thread_local! { /// Used as part of [`XerrorHandler::handle()`]. When an X11 error occurs during this function, - /// the error gets copied to this mutex after which the program is allowed to resume. The error - /// can then be converted to a regular Rust Result value afterwards. - static CURRENT_X11_ERROR: Mutex> = Mutex::new(None); + /// the error gets copied to this RefCell after which the program is allowed to resume. The + /// error can then be converted to a regular Rust Result value afterwards. + static CURRENT_X11_ERROR: RefCell> = RefCell::new(None); } /// A helper struct for safe X11 error handling pub struct XErrorHandler<'a> { display: *mut xlib::Display, - mutex: &'a Mutex>, + error: &'a RefCell>, } impl<'a> XErrorHandler<'a> { @@ -25,7 +25,7 @@ impl<'a> XErrorHandler<'a> { unsafe { xlib::XSync(self.display, 0); } - let error = self.mutex.lock().unwrap().take(); + let error = self.error.borrow_mut().take(); match error { None => Ok(()), @@ -44,20 +44,16 @@ impl<'a> XErrorHandler<'a> { // SAFETY: the error pointer should be safe to copy let err = *err; - CURRENT_X11_ERROR.with(|mutex| match mutex.lock() { - // If multiple errors occur, keep the first one since that's likely going to be the - // cause of the other errors - Ok(mut current_error) if current_error.is_none() => { - *current_error = Some(err); - 0 - } - Ok(_) => 0, - Err(e) => { - eprintln!( - "[FATAL] raw-gl-context: Failed to lock for X11 Error Handler: {:?}", - e - ); - 1 + CURRENT_X11_ERROR.with(|error| { + let mut error = error.borrow_mut(); + match error.as_mut() { + // If multiple errors occur, keep the first one since that's likely going to be the + // cause of the other errors + Some(_) => 1, + None => { + *error = Some(err); + 0 + } } }) } @@ -67,13 +63,13 @@ impl<'a> XErrorHandler<'a> { xlib::XSync(display, 0); } - CURRENT_X11_ERROR.with(|mutex| { + CURRENT_X11_ERROR.with(|error| { // Make sure to clear any errors from the last call to this function - *mutex.lock().unwrap() = None; + *error.borrow_mut() = None; let old_handler = unsafe { xlib::XSetErrorHandler(Some(error_handler)) }; let panic_result = std::panic::catch_unwind(AssertUnwindSafe(|| { - let mut h = XErrorHandler { display, mutex: &mutex }; + let mut h = XErrorHandler { display, error }; handler(&mut h) })); // Whatever happened, restore old error handler