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

Unify surface creation by introducing new SurfaceTarget enum #4984

Merged
merged 17 commits into from
Jan 12, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jan 4, 2024

Connections
Came up while talking about #2804 since all the different surface creation code paths were a bit in the way when working on this.

Description
Unifies all create_surface_X methods to just create_surface by expecting a new SurfaceTarget enum.
To be accurate, we're expecting Into<SurfaceTarget> which allows to pass anything that implements raw window handle directly.

Was a bit unsure about the create_surface_raw one. I think removing it in the way I did is correct, but I'm wonder if I blocked some usecases or made things less safe.

Future work:

  • Trickle this down to wgpu-core?
  • consolidate the values of this new enum. Right now it's just blindly ported over from existing methods

Testing
Compiles! Also ran examples on native (mac) and webgl.

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@Wumpf Wumpf requested a review from a team as a code owner January 4, 2024 15:37
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Trickle this down to wgpu-core?

Yes please!

wgpu/src/lib.rs Show resolved Hide resolved
wgpu/src/lib.rs Show resolved Hide resolved
@Wumpf
Copy link
Member Author

Wumpf commented Jan 9, 2024

Trickle this down to wgpu-core?
Yes please!

Let's leave this for another time, this PR is quite big & scary as is already

wgpu/src/lib.rs Show resolved Hide resolved
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Very good stuff!

@cwfitzgerald cwfitzgerald merged commit 4fd4a71 into gfx-rs:trunk Jan 12, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants