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

Implement enumerate_adapters for rs-backend #412

Merged
merged 5 commits into from
Nov 14, 2023

Conversation

dskkato
Copy link
Contributor

@dskkato dskkato commented Nov 12, 2023

I originally used wgpu with WebGPU, but recently I came across this library. As mentioned in #409, I also needed to choose an adapter to try out compute shaders, so I implemented it on a trial basis. I'm not sure if it conforms to your coding style, but I would appreciate it if you could review it. If you are already working on something similar, please feel free to close it as appropriate.

Resolves #409

@dskkato dskkato requested a review from Korijn as a code owner November 12, 2023 14:13
@dskkato dskkato changed the title Implement enumerate adapters for rs-backend Implement enumerate_adapters for rs-backend Nov 12, 2023
Copy link
Member

@almarklein almarklein left a comment

Choose a reason for hiding this comment

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

Really nice!

The codegen sometimes makes refactorings like this a bit tricky. I added a comment for a way to work around this.

I'm not 100% sure about the API, relates to #410. But the solution you offer here - to make the user go into wgpu.backends.rs - seems like a pretty good alternative. We'd have to figure out how to document this function though.

This also relates somewhat to #408. I propose that we move this PR forward and figure out the #401 stuff later.

wgpu/backends/rs.py Outdated Show resolved Hide resolved
wgpu/backends/rs.py Outdated Show resolved Hide resolved
wgpu/backends/rs.py Show resolved Hide resolved
@dskkato
Copy link
Contributor Author

dskkato commented Nov 14, 2023

FYI: The example for wgpuInstanceEnumerateAdapters I referenced is this -> https://github.com/gfx-rs/wgpu-native/blob/trunk/examples/enumerate_adapters/main.c

Copy link
Member

@almarklein almarklein left a comment

Choose a reason for hiding this comment

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

LGTM. This is going to be interesting merging with #408 😄 I propose we merge this into main and then I'll merge main into #408.

@almarklein almarklein merged commit 5ea8956 into pygfx:main Nov 14, 2023
18 checks passed
@almarklein
Copy link
Member

Thanks for your contribution!

@almarklein almarklein mentioned this pull request Nov 14, 2023
16 tasks
@dskkato dskkato deleted the implement_enumerate_adapters branch November 14, 2023 08:24
@almarklein
Copy link
Member

For the record, when #408 is merged, it will be wgpu.backends.wgpu_native.enumerate_adapters().

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.

Add convenience function enumerate_adapters
3 participants