Context, Surface should not be generic over raw-window-handle types

[dhardy]

Since the next winit version makes Window a trait it is no longer possible to write the type of a Surface or Context without some shenanigans.

For example, I was able to make the following work:

/// Any handle supporting both [`HasDisplayHandle`] and [`HasWindowHandle`]
pub trait HasDisplayAndWindowHandle: HasDisplayHandle + HasWindowHandle + 'static {}
impl<W: HasDisplayHandle + HasWindowHandle + 'static> HasDisplayAndWindowHandle for W {}

type Surface = softbuffer::Surface<Arc<dyn HasDisplayAndWindowHandle>, Arc<dyn HasDisplayAndWindowHandle>>;

[madsmtm]

You should be able to do Surface<OwnedDisplayHandle, Arc<dyn Window>>? That's at least a bit simpler.

---

Anyhow, I've been wanting this too, but I don't think it's possible?

On a lot of platforms, window (surface) and display server handles are reference-counted, and on these we can make Surface hold a reference to the window / display server internally.

But on Wayland (and maybe X11, I can't remember), the protocol only exposes "create" and "destroy" actions for handles, there is no "Arc::clone" in the ABI for the native handle type.

The wayland-client crate gets around this by adding an Arc wrapper on top of the handle, but we can't rely on that, since if we did, we'd be limited to a single wayland-client version (or at least to handles created with wayland-client, whereas users might want to use Wayland handles passed over FFI).

EDIT: See also related discussion in rust-windowing/raw-window-handle#120 and rust-windowing/raw-window-handle#158.

[dhardy]

You should be able to do Surface<OwnedDisplayHandle, Arc<dyn Window>>? That's at least a bit simpler.

Not in this case, though I can use softbuffer::Surface<Arc<dyn HasDisplayHandle + 'a>, Arc<dyn HasWindowHandle + 'a>>.

I'm using an intermediate API which does not require winit:

    fn new_surface<'window>(
        &mut self,
        window: std::sync::Arc<dyn HasDisplayAndWindowHandle + Send + Sync>,
        _: bool,
    ) -> std::result::Result<Self::Surface<'window>, RunError>
    where
        Self: Sized,
    {
        let display = window.clone() as Arc<dyn HasDisplayHandle>;
        let window = window as Arc<dyn HasWindowHandle>;
        let context =
            softbuffer::Context::new(display).map_err(|err| RunError::Graphics(Box::new(err)))?;
        let surface = softbuffer::Surface::new(&context, window)
            .map_err(|err| RunError::Graphics(Box::new(err)))?;

        Ok(Surface {
            size: Size::ZERO,
            surface,
            draw: Draw::default(),
        })
    }

Complication 1: it must support wgpu which needs a SurfaceTarget (i.e. both display + window handle in a single object which gets boxed).

Complication 2: it must (now) support constructing a softbuffer::Surface, and though it might be possible to parameterise over W: HasDisplayHandle + HasWindowHandle that would be a huge headache.

Now, I need to use Arc for clone() since winit implements both interfaces on a single object but softbuffer wants two objects, but I can't pass those parameters separately since wgpu requires both interfaces supported by a single parameter, and since Rust doesn't (yet?) support dyn HasDisplayHandle + HasWindowHandle as a type I need a trait supporting both.

It's a mess, and the only improvement I can see would be to move trait HasDisplayAndWindowHandle into raw-window-handle. Well, unless either RWH merges the two handles or winit (and WGPU) unmerge them (i.e. winit::Window does not impl HasDisplayHandle).

[dhardy]

The wayland-client crate gets around this by adding an Arc wrapper on top of the handle

So my window handle uses Arc twice with softbuffer while wgpu uses a Box and a lifetime parameter too!

Personally I think I would just require winit::Window: Clone + Send + Sync + 'static but I recognise that there are trade-offs and other people have already put a lot of thought into this.

[MarijnS95]

Complication 1: it must support wgpu which needs a SurfaceTarget (i.e. both display + window handle in a single object which gets boxed).

Specifically that WindowHandle trait is problematic; raw-window-handle defines them as two dependent things for a reason as recombining them seems to break a bunch of patterns at least. I thought I was pulling that apart in support of gfx-rs/wgpu#8012 but I must have misremembered because it's not happening in that PR.

EDIT: I remember now, I mentioned this when integrating that WGPU PR in bevy (where most/all issue-reporters came from) and only complained about it in code thus far:

https://github.com/bevyengine/bevy/pull/20357/files#diff-856082d916c7309cbe6c8b2a231f703c8ab0769db951591018a5f217e6c42823R385-R405

Let's make sure that's swiftly dealt with, as you already recognize WGPU needs to split these out again.

Personally I think I would just require winit::Window: Clone + Send + Sync + 'static but I recognise that there are trade-offs and other people have already put a lot of thought into this.

You mean softbuffer should require winit::Window? Keep in mind that winit is intentionally not a dependency so that softbuffer can be used with any windowing backend or low-level implementer, with raw-window-handle as the common "envelope".

[dhardy]

You mean softbuffer should require winit::Window?

No... ah, I see what you mean; consumers would still need to specify Arc<dyn ...> or Box<dyn ...> if not using the winit::Window type or a reference.

Conclusion:

1. The correct way to use these traits (where a more specific parameterisation cannot be used) is as in softbuffer::Surface<Arc<dyn HasDisplayHandle>, Arc<dyn HasWindowHandle>>. (Perhaps there should be a typedef for this?)
2. wgpu should consume the display + window handles in the same way, not Box<dyn WindowHandle>.

[madsmtm]

The correct way to use these traits (where a more specific parameterisation cannot be used) is as in softbuffer::Surface<Arc<dyn HasDisplayHandle>, Arc<dyn HasWindowHandle>>. (Perhaps there should be a typedef for this?)

We could add it as the default type parameters?

[MarijnS95]

Would Arc<dyn HasDisplayHandle> work for winit::event_loop::OwnedDisplayHandle?

[dhardy]

We could add it as the default type parameters?

This would be useful (partly just to document the recommended usage).