rust-windowing / raw-window-handle

A common windowing interoperability library for Rust
Apache License 2.0
314 stars 49 forks source link

Take `self` instead of `&self` in raw window handles? #145

Closed madsmtm closed 6 months ago

madsmtm commented 1 year ago

We should consider taking self instead of &self, and instead suggest that people implement the trait for &TheirType.

This would allow implementers that want other semantics than shared reference semantics, to have those - but at the same time, it might come at a cost to usability, unsure!

notgull commented 1 year ago

This would make it impossible to have a Box<dyn HasWindowHandle>, which I currently rely on for my wgpu fork with the new changes. Are there any explicit benefits to this approach?

Lokathor commented 1 year ago

Given that we recently removed Copy from RawWindowHandle stuff I would not suggest this change.

madsmtm commented 1 year ago

Ah, taking self makes the trait no longer object safe.

Main benefit I see is if a library wanted to restrict rendering to &mut references, e.g. to prevent two libraries from trying to render at the same time (which is definitely possible with the current design).

Lokathor commented 1 year ago

I suspect that whatever rendering system would fail to double initialize even if you have the handle in two places.

madsmtm commented 1 year ago

I asked the wgpu developers about this on Matrix, seems like it is handled by the rendering system, but not really guaranteed.

I guess I'll leave this one open at the very least until we've confirmed that this is indeed a non-issue.

notgull commented 1 year ago

Similar to how users of AsFd should be sure to know that other users may access the underlying resource in parallel, although they can be sure that the underlying resource isn't closed, HasWindowHandle users should be mindful that others may be using the window as well. I think the underlying APIs are prepared for it.

Lokathor commented 1 year ago

At minimum, the OS always also knows about your window handle, and will laugh you out of the room if you tell it that it's not allowed to do anything

kchibisov commented 1 year ago

You can initialize multiple times with egl just fine given that you're getting a reference to global singleton (as of now, there will be extensions to avoid that, but it'll fail). I'm sure you can do something like that with X11 just fine as well.

But yeah, making it self after removing Copy is not gonna fly.

madsmtm commented 6 months ago

I agree with the points raised here, and am in favour of Has[Raw]WindowHandle taking &self.