rust-windowing / raw-window-handle Goto Github PK

Issue #5 proposed to add OpenGL support which is, strictly speaking, outside the current scope of raw-window-handle where you only can grab the raw C handles from a windowing system, and that's it.

If there is a consensus to expand the project by adding OpenGL interop functionality, I think a change in scope is in order. Changing the name of the crate to something like gfx-wsi-interop would allow us to implement any functionality (including Vulkan, Metal) that makes interop between the systems both backend-agnostic and simple.

Windows are apparently effectively !Send on Windows and macOS, and the lifetime issues that Android's window handles have make marking window handles Send perhaps a bit suspect.

I believe we should treat RawWindowHandles somewhat like pointers, in that they can be constructed by safe code, but can't be used directly without unsafe. Thus, we should make the handles Send, but provide enough information in order for user code to tell if they're using the handle on the correct thread. Additionally, any API creating ready-made RawWindowHandles must be unsafe, as is already the case with HasRawWindowHandle. This all assumes we can find a workable solution to #84.

It's been almost a year since the last release and I've been looking forward to using the version of raw-window-handle that doesn't depend on libc. Is there any chance of a new release soon?

I'm only really familiar with macOS and iOS, but on those platforms it would make sense to split the window and the view handles in two.

That way, users could make some sort of interface in the native toolkit, and just provide the NSView handle to wgpu to render into. With the current design, we're assuming that the whole window's content is what should be rendered into.

I think this is roughly how it already works on web, e.g. you can lay out the page in a certain way, and then just create a canvas that you pass on to the renderer to do its thing. (Though arguably winit shouldn't be creating a canvas, perhaps instead it should provide a div that the graphics library itself can insert the canvas into?)

Would this sort of thing work on other platforms?

The use case is to save extra imports, like

fn raw_display_handle() -> RawDisplayHandle {
    let mut handle = WaylandDisplayHandle::empty();
    handle.wayland_display = display;
    handle.into()
}

currently the extra impls from the alloc feature aren't in the docs at all. Before the next release we should fix this so that people can be aware of the extra feature and the extra impls it provides.

We got a bit sidetracked by this in #70, so I'm opening this issue to track it separately.

To summarize the problem briefly:
Letting users use windows with crates like wgpu without requiring the user to write any unsafe code is desirable.
TrustedWindowHandle can't really be used safely after it has been around for a bit. Unfortunately, there's no guarantee that the underlying window handle won't become invalid while a TrustedWindowHandle is alive.
gfx-rs/wgpu#1463 has a more in-depth outline of the issue at hand, and suggests that raw-window-handle should add the following impls of HasRawWindowHandle:

impl<'a, T: HasRawWindowHandle> HasRawWindowHandle for &'a T { /* .. */ }
impl<T: HasRawWindowHandle> HasRawWindowHandle for Rc<T> { /* .. */ }
impl<T: HasRawWindowHandle> HasRawWindowHandle for Arc<T> { /* .. */ }

The addition of these impls of HasRawWindowHandle makes TrustedWindowHandle seem kind of useless (for lack of a less biting word).

When user wants to deal with platform specific code they most of the time need raw display, like e.g. wl_display. However RawWindowHandle provides with both of them.

I'd suggest to logically split them into RawDisplayHandle and RawWindowHandle, where RawDisplayHandle would have only reference to display and RawWindowHandle could have only window, like wl_surface or be the way it is right now?

The issue I'm having is that I want Display on every platform, but I don't want surface, since the surface isn't yet created.

For example winit could implement raw_display_handle on event loop, so I can pass to e.g. glutin.

A more conventional Rust ecosystem license like MIT or Apache 2 has significant upsides, such as allowing Google employees to contribute.

It looks like RawWindowHandle, and the individual platform handles, are marked !Send by default due to the pointers they contain. I couldn't find any documentation one way or another on whether this is explicitly intended to be a part of the safety contract for using a RawWindowHandle or not. I assume that there are some use cases where moving a handle between threads is perfectly safe, and others where it is not. Do existing libraries rely on RawWindowHandle not implementing Send for safety? Or would it be fine for it to impl Send?

Windows in X11 are defined as "XIDs", which are unique 32-bit IDs. However, there are separate types in this crate for Xlib and XCB window handles, despite the fact that, fundamentally, they refer to the same thing.

pub struct XlibWindowHandle {
    /// An Xlib `Window`.
    pub window: c_ulong,
    /// An Xlib visual ID, or 0 if unknown.
    pub visual_id: c_ulong,
}

pub struct XcbWindowHandle {
    /// An X11 `xcb_window_t`.
    pub window: u32, // Based on xproto.h
    /// An X11 `xcb_visualid_t`, or 0 if unknown.
    pub visual_id: u32,
}

Xlib uses c_ulong because of an extremely characteristic poor design decision, XCB gets this right. At the next breaking change, it would be nice to combine them into one type to remove any ambiguity.

According to this, it looks like one needs a WlShm object in order to display data on a window without the use of hardware acceleration. When looking through other code, such as minifb's, this assertion seems to be supported. It also appears to be the case that it is not possible to access this object with with what is exposed right now (just the surface and display). Is there some way to access this that I am not seeing? This is entirely possible as I'm not that familiar with the Wayland API.

Recently, I played a bit with Android in combination with winit/Vulkan/wgpu and there are some topics which should be addressed imo:

1. raw-window-handle docs:

The exact handles returned by raw_window_handle must remain consistent between multiple calls to raw_window_handle, and must be valid for at least the lifetime of the HasRawWindowHandle implementer.

This constraint is in particular hard to uphold for winit as the window handle is only available between certain events in the event loop. As far as I know the ANativeWindow may change ov ertime (Suspend-Resumed cycles).

2. Change the type of a_native_window from *mut _ to Option<NonNull<_>> or it should be explictly annotated that it may be null. Right now this is not clear, some graphic libraries assume it's always a valid handle. On the other hand, winit currently panicks when requesting the RawWindowHandle as it shouldn't return a nullptr, which basically shifts the issue to the client to uphold the requirement of only requesting the handle if it safe to do so. This makes it actually quite hard to write portable implementations for anything Vulkan based (extensions query and surface support via RawWindowHandle). Using the Option<NonNull<_>> would make it safe for libraries consuming the handle as well as window libraries to say that a handle might not be available right now.

In some cases it can be beneficial to find out what platform the window is going to be running on before the actual window is created. An example when doing Vulkan and Qt interop:

The Qt application starts running, no window generated.
Query for which platform is running.
The Vulkan instance is generated using surface extensions dependant on the platform running.
The Vulkan code needs only load the surface function pointers for the platform in question
The window is generated using the Vulkan instance
Vulkan grabs the normal RawWindowHandle from the generated window.

This essentially means the Vulkan instance will only have to request the exact surface instance extensions upon creation, instead of safe-guarding by loading all the Linux surface extensions.

Qt requires a VkInstance to exist already to create QVulkanWindow, so this functionality would be nice.

All c_void are in some sense similar since you can always just cast between them, however, since core offers a c_void type we should at least use that common point of communication as much as possible.

Right now platform-specific modules and variants are only on platforms that are known to support them. We probably should expose them more broadly, probably everywhere.

Upsides:

1. Allows much wider range of support. Concretely:
   • This allows code to support custom/json targets, so long as the windowing code can provide the right handle, and other libraries can consume it.
   • This allows support for various unixes that can support e.g. X11 but are not in the set of linux | dragonfly | freebsd | netbsd | openbsd (the current cfg). For some examples, Solaris is left out by the current cfg, or even macOS, which can use X11 (and xlib — in College I even got XCFE running under it) if the user has XQuartz installed.
   • Mac Catalyst would be able to provide either RawWindowHandle::MacOS or RawWindowHandle::IOS, which reflects the fact that it can use either AppKit or UIKit (despite being target_os = "ios").
   • I don't really know for certain, but I'd guess that there are some weird windows configurations that would benefit from this too. Certainly stuff like XBox, but that's probably covered by "json target".
2. Downstream code doesn't have to ensure that the cfgs they use match (or are a subset of) the ones raw-window-handle uses.

Downsides:

1. It means that removing a handle type is a breaking change even if it's only usable on an OS nobody uses (or that isn't supported by Rust).

   • This is the biggest downside, but in practice the set of windowing handle types is pretty small, and we can just deprecate them instead if it's an issue. It's also not like a single struct definition really hurts anything.

2. It's less clear to code that's consuming raw handles which combinations of handle variants and OSes are possible.

   • That said, this lack of clarity reflects reality, where you can do dumb things like run XFCE on macOS under XQuartz (I don't know if this is still possible, and would assume it's not, although I did compile and run Xlib-using C code for macOS a few months ago, so that much definitely is still possible).
   • I don't think we consider the set of cfgs we use to be part of the stable API anyway, so this is likely a moot point.
   • To a certain extent, I think it's still pretty clear (you can get RawWindowHandle::Xlib anywhere someone can get xlib running) this can be addressed by documentation.

3. It forces us to continue using fully opaque types in all supported backends.

   • TBH I think this is actually a good thing, as it keeps things reusable (using objc types directly would be a problem for code that uses fruity for example).

4. Slightly more code sent to rustc == slightly slower compiles.

   • This crate is teeny and this adds no deps though, so it doesn't matter.

I find the current API a bit strange, since it basically offers an enum that just has a single member on most platforms.

Is there any reason why the RawWindowHandle isn't just a platform-specific type that is decided at compile-time? It's not like people are going to be matching against the platform's implementation at runtime, especially since this is impossible because the enum has only the members available for the current platform.

Both wayland-rs and x11rb have pure-Rust backends that don't require C libraries for X or Wayland. But to use raw-window-handle it's necessary to use xcb/xlib/libwayland to have a pointer to put in the display and window handle.

Are there any good ways to support this? It could be supported with feature flags that pull in x11rb and wayland-rs as dependencies, but that is a bit problematic in terms of API stability.

This would generally be limited to software rendering (such as with softbuffer) since the EGL and Vulkan WSI implementations are written in C and use these C libraries. So its usefulness would be limited, but it would be nice to have.

I'm writing a GUI framework that may use breadx, which is an X implementation not based on either Xlib or XCB, but written by hand. I'd like this GUI framework to be able to return RawWindowHandle for use in e.g. wgpu.

However, I would be unable to return a RawWindowHandle from my GUI instance if it is using breadx. I could adapt this by returning an Option<RawWindowHandle> instead of a RawWindowHandle. However, this would mean that I couldn't implement HasRawWindowHandle for this system, unless I want the implementation for that to panic when breadx is used.

I was thinking that this crate could add a more generic window handle, like this:

struct GenericHandle {
    display: Box<dyn Any>,
    window: usize,
}

Although I imagine there's a better way of doing this.

Given that rust 1.64 got released we can get rid of it.

Since #70 was merged, I was wondering if there is anything left to do before releasing version 0.4 on crates.io, or if the release can go forward.

The current implementation treats X11 as a monolith. Per Vulkan convention, the X11 case should probably be renamed Xlib, and an Xcb case added.

In #105, I said that we should decide what we want to do about higher level frameworks, such as GTK, Qt, or others. However, it is difficult to know what level of abstraction to operate at. For instance, when it comes to GTK, do you provide:

• A pointer to the GtkWidget, the highest level abstraction,
• A pointer to the GdkSurface, since that's the windowing system used underneath.
• Since the underlying system window is usually exposed in some way by the framework, say that the higher level framework should just fill out the RawWindowHandle with that corresponding raw window.

Thoughts? I'm leaning more towards option #3 the more I think about it.

so I got sent here by a error ......

thread 'main' panicked at 'Could not retrieve any SDL2 window info, and Osspial wrote the trait to be infallible despite that clearly not always being the case, so now you got this panic. htt
ps://github.com/rust-windowing/raw-window-handle/issues/new', /home/vivax/.cargo/registry/src/github.com-1ecc6299db9ec823/beryllium-0.3.2/src/window/raw_window.rs:107:7
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Related to gfx-rs/gfx#3545

Currently, RWH has the following information for a wayland window:
- surface: A pointer to a wl_surface.
- display: A pointer to a wl_display.

If one needs to create an EGL context for drawing on this window, they have to go through wl_egl_create_surface, which gets the surface as well as the window size, and returns a wl_egl_window pointer.

Problem is: RWH doesn't hold any information about the window size. So we are in a situation that on one side of the API we have a real window, and it has the size. And on the other side we have EGL, and it wants the window handle and the size. But in the middle, RWH loses the information about the size. It sounds like RWH isn't accomplishing its goal of abstracting the window in this case?

Note that this isn't a problem for Xlib. I suspect that's because an Xlib window knows about its size (like probably any other window in the RWH enumeration). So the Xlib window is closer to wl_egl_window than it is to wl_surface.

I want to stress that I have no experience with Wayland (or X11), and I'm just raising a concern and searching for more input about the solution here.

At first, I thought that maybe wl_egl_window field could be added to the Wayland variant of RWH. However, I realized that this isn't going to help, since winit doesn't necessarily want to create an EGL window. So if we want RWH to work, it needs to communicate the window size, at least for the Wayland variant.

It's traditional for core Rust projects to be MIT OR Apache-2.0. Personally I also like to see the Zlib license as an option as well.

Could we get additional license options for Apache-2.0 at least, and possibly Zlib as well?

This would need a sign off from all those who have added code (I'll list them here without pings and we can ping them if people generally agree with this issue at all).

• Osspial (lead)
• LeshaInc marked stuff as "derive Hash" but I don't think that's significant.
• aloucks similarly put the initial derives on the enum
• dxenonb added readme stuff but not code
• msiglreith renamed a field and added some docs lines
• repi helped fix the last license thing actually, but no code changes.
• ryanisaacg added the web handle
• Didgy74 added XCB and Android.

The types and traits in raw-window-handle act as a standard for many other libraries to interact with each other. Changes that break backwards-compatibility require many different projects to be updated. This means that such changes should not be made without a good reason.

In my opinion, there is not a good reason for the change from *mut c_void to Option<NonNull<c_void>>. The two types encode the exact same set of values and there is not really a difference in semantic meaning between the two. This change will require code churn in every single crate that depends on raw-window-handle for essentially no gain. In contrast, the removal of the libc dependency and the addition of TrustedWindowHandle are backwards-compatible changes that crates can benefit from solely by bumping the raw-window-handle version number.

For more explanation as to why I think this is a misuse of NonNull: As explained in the docs, NonNull<T> has two differences from *mut T. First, it is guaranteed not to hold a zero bit pattern, which enables the null pointer optimization. Second, it is covariant over T, whereas *mut T is invariant (irrelevant in raw-window-handle's case). Nothing about NonNull<T> guarantees that it holds a valid pointer to a T (in fact, it has a dangling constructor). This is why Option<NonNull<c_void>> provides no real semantic difference in the case of raw-window-handle.

The impl HasRawWindowHandle for RawWindowHandle added in #29 is a soundness hole, since it allows safe code to instantiate an invalid handle and pass it to a function that relies on HasRawWindowHandle being implemented correctly:

fn use_raw_window_handle<H: HasRawWindowHandle>(h: H) {/*impl*/}

use_raw_window_handle(RawWindowHandle::Windows(WindowsHandle {
    hwnd: 41 as *mut c_void,
    hinstance: 43 as *mut c_void,
    ..WindowsHandle::empty()
}));

Removing that implementation is trivial, but there are two unresolved questions:

• Should we expose some sort of pub RawWindowHandleActive(RawWindowHandle) struct that can only be initialized with an unsafe function?
• Does removing the impl require a bump to 0.4.0? The official Rust semver guidelines say that soundness fixes don't count as breaking changes, so I'm leaning towards no.

Exposing an enum variant for Android with an ANativeWindow would be great!

I'm unable to compile wgpu-rs, and it looks like the problem is is in raw-window-handle.
wgpu is hard-coded to use a u64 for window, but raw-window-handle defines window as a c_ulong:

pub struct XlibHandle {
/// An Xlib Window.
pub window: c_ulong,
...

It seems like c_ulong should really c_ulonglong.

The wgpu error:

error[E0308]: mismatched types
--> wgpu-native/src/device.rs:85:9
|
85 | window,
| ^^^^^^ expected u32, found u64
|
help: you can convert an u64 to u32 and panic if the converted value wouldn't fit
|
85 | window: window.try_into().unwrap(),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Apparently, this is what one uses to render DX12 in UWP: https://docs.microsoft.com/en-us/uwp/api/windows.ui.core.corewindow?view=winrt-19041
See also - https://github.com/msiglreith/rostkatze/wiki/RKZ-extensions
cc @msiglreith

It seems to me that the requirements of TrustedHandle::new being correct are exactly that the handle matches the invariants on HasRawWindowHandle, i.e.

impl TrustedWindowHandle {
    fn from_handle<T: HasRawWindowHandle>(has_handle: &T){
        // Safe: `impl HasRawWindowHandle` requires that `raw_window_handle`
        // returns a valid handle
        unsafe { Self::new(has_handle.raw_window_handle()) };
    }
}

would be valid (and useful)

Original comment from (#52 (comment))

Would it be possible to have a safe impl of new or equivalent for any T: HasRawWindowHandle?

Specifically, an implementation which would replace https://github.com/bevyengine/bevy/blob/a68c78cef5fb2112617de095efd4e64cba1587eb/crates/bevy_winit/src/winit_windows.rs#L117-L119

(I need this for bevyengine/bevy#1279, which is currently assuming that the impl could be correct, so I would like to know if that assumption is wrong anyway. That PR is otherwise blocked on #58 anyway)

Should there be a type-system relationship between these?

I think it would make sense to make the normal handles subtraits of the raw handles, i.e. trait HasWindowHandle: HasRawWindowHandle { ... }?

Or maybe we should do a blanket impl<T: HasWindowHandle> for HasRawWindowHandle {}, or vice-versa?

Window handles on Android can be invalidated between a pair of "suspend" and "resume" events. This makes it difficult to safely use window handles without asking end users to wire up the missing event handling. Thus, an API naïvely accepting a T: HasRawWindowHandle is inherently unsafe on Android.

After looking into issue 45 in more detail
ref: #45

I believe the bug is indeed in raw-window-handle.
I also believe that the best place to fix this is in raw-window-handle.

Here's the flow:
raw-window-handle defines Window as u64. Note that c_ulong is mapped to a u64 via:
crate libc-0.2.66/src/unix/linux_like/linux/gnu/b64/x86_64/not_x32.rs

So wgpu can't use the correct 'usize' - raw-window-handle forces u64 if wgpu wants to successfully compile on 64-bit ubuntu.
But of course if wgpu uses u64 to compile successfully on 64-bit ubuntu, it will fail to compile on 32-bit armv7l.

The root cause of this problem seems to be the libc crate's definition of u_long maps to u64 - in a perfect world it would map to usize.

However, it seems that the libc folks new better but wanted this behavior anyway. They've been very careful about usize/isize issues. From reading libc crate issues around usize I'm now more sure that this is the desired/intended behavior. I don't believe we can use c_ulong for the xlib window.

Would the raw-window-handle folks be open to a PR changing:
pub struct XlibHandle {
/// An Xlib Window.
pub window: c_ulong,

to this?

pub struct XlibHandle {
/// An Xlib Window.
pub window: usize,

Thanks!

Say I create the iOS struct, pass in pointers to UIView alone, without the rest - because technically they would be unnecessary and unused.

How would gfx-hal know wether or not they need to setup things for Metal vs the UView containing a CAMetalLayer and etc?

To clarify, there is no indication mentioned whether or not the related graphics stuff has been setup before hand (e.g using SDL2 to set everything up) vs expecting gfx-hal to create the necessary contexts and CAMetalLayer?

In #110 I added safer versions of the window handles. However, we might need to make sure they work with all platforms, present and future, before we release them.

Platform specific issues that I'm aware of:

• The Android application can be randomly invalidated, making it undefined behavior to use the handles. However, we use Active to work around that by only allowing usage of the handles while the application is active.
• Windows in X11 (and Wayland, I think?) can be deleted at any point using 100% safe code. I don't think this is an issue, since I don't think any undefined behavior can result from this; worst comes to worse you get a BadWindow error. However, I'm not sure if there are any implications on the GPU end that we should be aware of.

Vulkan requires for surface creation at least a HWND and HINSTANCE handle.

As discussed in rust-gamedev/wg#26, this can improve usability by allowing for statically-checked totality and rendering additional traits unnceessary, and does not incur any additional inconvenience for users on platforms which do not support a given windowing system, as inapplicable enum cases can still be conditionally compiled out.

This isn't a problem, but some people have this repo on watch, and this is the easiest way to get all your attention.

The crate is now being maintained by me. I'll try to work on the issues and PRs, but I don't have high knowledge of all the windowing systems so I'll be counting on the rest of yall's continuing support.

Just a note here that OpenGL requires additional info than just the window pointer for proper context creation, so we'll have to be sure to add that as a cargo feature (off by default).

Currently, there is only one way to go from winit::Window to RawWindowHandle. Once you get a raw window handle, everything at the lower level has to work with it directly. This is an unfortunate limitation.

For example, gfx-backend-vulkan::Instance::create_surface expects HasRawWindowHandle. That means we can't get RawWindowHandle at any layer above it, but sometimes we really want that, e.g. when we construct RawWindowHandle manually on the client side.

For libraries that have C APIs, like wgpu-native or gfx-portability, it would be great if we could also use raw window handle stuff from C. That would allow us to drop all the platform-specific WSI functions from the public API and leave only one that could be universally used by GLFW, SDL, Winit, and other clients. Currently, we can use a single API point from Rust, but not from C.

The conditional compilation stuff is an unnecessary complication to the system.

Basically, every library that consumes a HasRawWindowHandle value already needs to have a default, catch-all case where they get some unknown handle type and then error out. We even want the enum to be tagged as non-exhaustive so that the library can be "future proof" and force people to have a default cause so that we don't break semver when new window systems are eventually added.

So since your consuming code's match statement already needs to say

  unknown => return Err(format!("unknown window handle type: {:?}", unknown)),

at the end of it, then having the IOS handle conditionally compiled to not exist on windows doesn't actually add anything. Either way you can't have exhaustive matching.

And since we're just giving all of our handle definitions in terms of libc types and core::ffi::c_void and so forth there's not really a problem with pulling in all sorts of platform dependencies if we just have every variant compiled in on all platforms.

So I think that never doing any conditional compilation is probably better in the long term. Makes things simpler to reason about.

Proposal

Expose following enum to only provide information about the window handle type, not the actual handle and a corresponding interface in the RawWindowHandle trait:

enum RawWindowHandleTy {
      AndroidHandle,
      X11Handle,
      WaylandHandle,
      WindowsHandle,
      ...
}

Motivation

On Android, it's not possible to gather the RawWindowHandle immediately after creation. At least for winit it can only be queried once certain events have been received in the event loop. This imposes restrictions for client apis, which relay on the information provided by the RawWindowHandle early on. I'm particular case it's related to deriving the required surface extensions for Vulkan WSI. Ideally, this could be soley derived from the introduced RawWindowHandleTy above as some platforms support multiple different WSI approaches.

Cons

• Breaking API changes
• Feels a bit like a bandaid but I'm currently lacking a better idea /:

Some more details and discussion over at winit: rust-windowing/winit#1624

This would be a very small break, so it has to be in the next breaking release.

What do we need to fix before the next major release?

There are requests for us doing the server trick again, which I'll be up for (trying to do), so mostly opening this issue to keep track of that.

It's redundant. Anybody who wants this can just get it via

let ns_window = handle.ns_window as *mut Object;
let ns_view: *mut Object = msg_send![ns_window, contentView];

or similar. Same goes for ns_window_controller (via the windowController message) and ns_view_controller (via the contentViewController message), which don't currently exist, but are mentioned in comments as potential future extensions.

Following from rust-windowing/winit#2259, the WebWindowHandle.id is a flaky way to share canvas references between crates. As discussed in that thread, synchronizing the IDs is challenging (leading to duplicates) and it isn't possible to query canvas elements in a Shadow DOM or offscreen canvases at all.

What consumers need is a way to pass the canvas reference through HasRawWindowHandle. AFAICT, that can be done with either an enum with variants for web_sys::HtmlCanvasElement and web_sys::OffscreenCanvas, or with wasm_bindgen::JsValue. It looks like another option is the FromWasmAbi and IntoWasmAbi traits provided by wasm_bindgen if the reference needs to cross the WASM ABI boundary for some reason.

I think the best way forward in the short-term is a feature to enable the use of one of these reference types in addition to the id: u32. It is preferable to replace the id entirely but may not be realistic without a deprecation period. The feature also mitigates a potential downside that it adds a dependency on web_sys and/or wasm_bindgen.

Open to discussion and other alternatives, but the current design appears to be a significant shortcoming in web environments.