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

Change how our raw-window-handle dependency is exposed #3075

Closed
wants to merge 1 commit into from

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Aug 31, 2023

raw-window-handle has yet to reach v1.0, and I doubt it will do so in the near future. This causes many problems to downstream users, as they must upgrade their winit version in lockstep with their wgpu (or similar) version.
In the past, we've been able to do the semver trick in raw-window-handle, but with the current API that's no longer possible.

So, until raw-window-handle becomes stable, I propose we make dependency on any version of raw-window-handle optional, and behind it's own feature flag, to make it possible for the user to select the version that they want.

This allows us to provide a much better support strategy, where instead of immediately removing support for older versions of raw-window-handle and forcing users to upgrade their rendering library, we instead slowly deprecate, perhaps over several releases if deemed necessary.

TODO:

  • Finish the implementation.
  • Add an entry to CHANGELOG.md
  • Figure out how to do examples now

@madsmtm madsmtm added the S - maintenance Repaying technical debt label Aug 31, 2023
@madsmtm madsmtm requested review from notgull and kchibisov August 31, 2023 16:10
Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea.

@Imberflur
Copy link
Contributor

This would be great! I'm currently stuck on rwh 0.4 due to using an older version of wgpu.

@madsmtm
Copy link
Member Author

madsmtm commented Sep 1, 2023

This would be great! I'm currently stuck on rwh 0.4 due to using an older version of wgpu.

Thanks for the input, an extra data point to consider that maybe we should consider re-adding v0.4 support, in a deprecated form, and at least for a little while longer.

May I ask why you're stuck on the older wgpu?

@Imberflur
Copy link
Contributor

Oh I thought this was already re-adding support but it looks like it is not!

FWIW it is not too much of a hassle for my case to add support in a fork.

May I ask why you're stuck on the older wgpu?

We currently depend on the dx11 backend to support users with older devices. Since the rewrite to use wgpu-hal, dx11 support is not fully rebuilt yet so we have been using the version of wgpu from before that. Wgpu now also has an Angle backend that can run on dx11 but we found a few things missing for our case the last time we tried it and going through Angle adds additional overhead that is not ideal on the older more resource constrained devices that we attempt to support via dx11.

Copy link
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let me know if you want me to do the Web implementation.

@madsmtm madsmtm force-pushed the raw-window-handle-no-longer-break branch from 7f0b3ef to eb8dd0b Compare September 2, 2023 13:48
@madsmtm
Copy link
Member Author

madsmtm commented Sep 2, 2023

let me know if you want me to do the Web implementation.

No thanks, I got it.

I would like some input on how we should do our examples, now that they require the rwh-0-5 feature to be enabled?

We could specify required-features = ["rwh-0-5"] for every example, but that means that users have to run with cargo run --example=my_example --features=rwh-0-5, which is a bit annoying. The other option is as I've hackily implemented, to stub out drawing to the window unless requested.

@daxpedda
Copy link
Member

daxpedda commented Sep 2, 2023

We could specify required-features = ["rwh-0-5"] for every example, but that means that users have to run with cargo run --example=my_example --features=rwh-0-5, which is a bit annoying.

[dev-dependencies]
winit = { path = "", default-features = false, features = ["rwh-0-5"] }

Is what I usually do. But that means you can't execute the examples without rwh-0-5, which might be fine?

EDIT:
Another example is to put a cfg in every example requiring either rwh-0-4 or rwh-0-5. We have done similar stuff in examples like x11_embed.

@kchibisov
Copy link
Member

You could add recent raw window handle into the default feature set, which is what it should be, the previous versions should be enabled on demand.

@madsmtm
Copy link
Member Author

madsmtm commented Sep 3, 2023

You could add recent raw window handle into the default feature set, which is what it should be, the previous versions should be enabled on demand.

The problem is that the current version could easily change many times while the version of winit stays the same.

But I guess it'd be much more pragmatic.

@kchibisov
Copy link
Member

@madsmtm feature set change is a breaking change. The default feature should be the one of the latest non-breaking release. it's not like you just specify raw-window-handle = "*".

@kchibisov
Copy link
Member

I think this is all implemented now?

@kchibisov kchibisov closed this Oct 20, 2023
@madsmtm madsmtm deleted the raw-window-handle-no-longer-break branch December 23, 2023 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S - maintenance Repaying technical debt
Development

Successfully merging this pull request may close these issues.

5 participants