Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Add a forwards compatible impl of HasRawWindowHandle #98

Closed
wants to merge 1 commit into from

Conversation

rib
Copy link

@rib rib commented Aug 10, 2022

Note: this is still just a WIP implementation because I'm not sure atm how to avoid a conflict with the ?Sized trait implementation that exists in this crate

This is effectively a variation of the "semver trick" that aims to
provide a blanket implementation of the v0.4 HasRawWindowHandle trait
for anything that implements the newer v0.5 traits

The is to help support compatibility between crates such as Winit 0.27
that have been updated to the latest version of raw_window_handle and
crates like Wgpu 0.13 that are dependent on raw_window_handle 0.4 (and
they can't bump the dependency without a new semver release)

This way crates like Wgpu built against raw_window_handle 0.4 will
be able to query the window handle state they need as a mapping from
the newer state provided by the v0.5 traits.

Alternative Solution

This is a potential, alternative solution to this Winit issue: rust-windowing/winit#2418

One trade off to solving this here instead of in Winit is that it's perhaps more reasonable to delete the implementation before the next release of Winit instead of maintaining it indefinitely - since this is only intended to be a stop-gap solution that makes it easier for crates to adopt Winit 0.27 while Wgpu is not yet able to update to v0.5 of this crate.

Conflicting trait:

This currently has an issue with needing to comment out the blanket
implementation of HasRawWindowHandle for ?Sized reference types since
the compiler complains that the new implementation is in conflict with
that.

I'm not currently sure how to specify the trait bounds so they can exist
together.

This is the conflicting trait impl:

unsafe impl<'a, T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for &'a T {
    fn raw_window_handle(&self) -> RawWindowHandle {
        (*self).raw_window_handle()
    }
}

This is effectively a variation of the "semver trick" that aimes to
provide a blanket implementation of the v0.4 HasRawWindowHandle trait
for anything that implements the newer v0.5 traits

The is to help support compatibility between crates such as Winit 0.27
that have been updated to the latest version of raw_window_handle and
crates like Wgpu 0.13 that are dependent on raw_window_handle 0.4 (and
they can't bump the dependency without a new semver release)

This way crates like Wgpu built against raw_window_handle 0.4 will
be able to query the window handle state they need as a mapping from
the newer state provided by the v0.5 traits.

XXX:
This currently has an issue with needing to comment out the blanket
implementation of HasRawWindowHandle for ?Sized reference types since
the compiler complains that the new implementation is in conflict with
that.

I'm not currently sure how to specify the trait bounds so they can exist
together.
@rib
Copy link
Author

rib commented Aug 10, 2022

Unfortunately it looks like the conflict with the blanket implementation for ?Sized reference types may not be solvable without Rust support for specialization: rust-lang/rust#31844

@kchibisov
Copy link
Member

In general I think that we shouldn't try to do anything to raw window handle. The particular problem we're trying to solve is that wgpu and winit has incompatible versions and wgpu can't update.

The problem could be solved by either winit or wgpu. In general I'm fine with winit solving this issue, since in any way we'd need to bump winit.

And all of this solutions are temporary, so they don't provide any value into raw-window-handle in the first place (given it'll be stable on the next release and we try to cooperate on that).

@rib
Copy link
Author

rib commented Aug 10, 2022

In general I think that we shouldn't try to do anything to raw window handle. The particular problem we're trying to solve is that wgpu and winit has incompatible versions and wgpu can't update.

The problem could be solved by either winit or wgpu. In general I'm fine with winit solving this issue, since in any way we'd need to bump winit.

And all of this solutions are temporary, so they don't provide any value into raw-window-handle in the first place (given it'll be stable on the next release and we try to cooperate on that).

yeah I tend to agree with all of this, and I'd say my personal preference at this point is to also tackle this in winit, esp since it doesn't look like it can be handled as a semver trick in 0.4.

If this had been possible it might have been nicer than solving it in winit from the pov that it didn't affect the newer version of raw_window_handle.

I'll close this PR now since anyway this approach didn't work out technically

@rib rib closed this Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants