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

Add wgpu::Adapter API to users #340

Merged
merged 7 commits into from
Mar 8, 2023
Merged

Add wgpu::Adapter API to users #340

merged 7 commits into from
Mar 8, 2023

Conversation

tseli0s
Copy link
Contributor

@tseli0s tseli0s commented Feb 26, 2023

"Closes" #339

There's also the alternative of strictly exposing only wgpu::AdapterInfo. Which may be desired if we don't want to give any access to the adapter itself for whatever reason (Keep in mind with the commits as they are currently we give at most a mutable ref to the adapter, which may allow someone to modify things they shouldn't). I avoided this because why hide a simple adapter when the crate exposes other parts of wgpu? And at worst, let users decide if they want to get access to the adapter.

If the function names aren't good (eg. wgpu shouldn't be there) or we need to wrap around wgpu::Adapter then lmk, I'll fix it.
Verified to work under GNU/Linux as expected.

Copy link
Owner

@parasyte parasyte left a comment

Choose a reason for hiding this comment

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

Hi @AndroGR, thanks for the PR.

I was hesitant on this one following the conversation in #339 (reply in thread) but I think what you have here is the right thing to do for now. I have just one change to request, and that's removing the mut variant of the method. I believe it is not needed.

I would also rename the method to adapter or get_adapter. (My current preference is adapter, and I am planning to rename get_frame to frame, et al. in another PR for consistency.)

src/lib.rs Outdated Show resolved Hide resolved
@tseli0s
Copy link
Contributor Author

tseli0s commented Mar 7, 2023

I would also rename the method to adapter or get_adapter. (My current preference is adapter, and I am planning to rename get_frame to frame, et al. in another PR for consistency.)

Well, I am going to name it adapter and I'll leave it up to you to decide for the style you prefer. Although since we are returning only a reference to it I imagine adapter makes more sense here (Like some popular crates do it)

Copy link
Owner

@parasyte parasyte left a comment

Choose a reason for hiding this comment

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

Thank you!

@parasyte parasyte merged commit bbfd49d into parasyte:main Mar 8, 2023
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.

2 participants