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 device_features and adapter_features for the web backend #2986

Merged
merged 7 commits into from
Jan 16, 2023

Conversation

expenses
Copy link
Contributor

@expenses expenses commented Aug 26, 2022

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections

Description

These weren't implemented and I needed in order to be able to check for compressed texture support. I had to do an unchecked cast of the GPUSupportedFeatures class to a js_sys::Set because it doesn't seem like wasm-bindgen implements setlike and thus doesn't have a has method.

I also added some BC formats to the texture format mapping function because my code needed them. We of-course should implement all of these in time, but I didn't want to do that here.

Testing

Tested by running a program that checks for compressed texture support and renders a model with compressed textures.

@expenses
Copy link
Contributor Author

expenses commented Aug 26, 2022

I had to do an unchecked cast of the GPUSupportedFeatures class to a js_sys::Set because it doesn't seem like wasm-bindgen implements setlike and thus doesn't have a has method.

We should probably find a solution to this before merging.

@jimblandy
Copy link
Member

Just marking this as "Draft", based on comments and title.

@jimblandy jimblandy marked this pull request as draft September 2, 2022 21:29
@codecov-commenter
Copy link

codecov-commenter commented Jan 9, 2023

Codecov Report

Merging #2986 (0067020) into master (ad4dac8) will increase coverage by 17.60%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #2986       +/-   ##
===========================================
+ Coverage   47.62%   65.22%   +17.60%     
===========================================
  Files          86       86               
  Lines       42697    42725       +28     
===========================================
+ Hits        20335    27868     +7533     
+ Misses      22362    14857     -7505     
Impacted Files Coverage Δ
wgpu/src/lib.rs 51.06% <0.00%> (-0.09%) ⬇️
wgpu-core/src/device/queue.rs 70.17% <0.00%> (+0.10%) ⬆️
wgpu-core/src/device/mod.rs 66.70% <0.00%> (+0.27%) ⬆️
wgpu-core/src/instance.rs 65.67% <0.00%> (+0.81%) ⬆️
wgpu-hal/src/lib.rs 26.92% <0.00%> (+0.96%) ⬆️
wgpu-core/src/hub.rs 61.13% <0.00%> (+2.15%) ⬆️
wgpu-types/src/lib.rs 88.06% <0.00%> (+2.56%) ⬆️
wgpu-core/src/track/buffer.rs 92.14% <0.00%> (+7.27%) ⬆️
wgpu-core/src/track/texture.rs 80.97% <0.00%> (+21.51%) ⬆️
wgpu-hal/src/vulkan/instance.rs 38.37% <0.00%> (+37.35%) ⬆️
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@expenses expenses marked this pull request as ready for review January 9, 2023 13:00
@expenses
Copy link
Contributor Author

expenses commented Jan 9, 2023

Think this is about ready - I had some problems running the demos on chrome though so it'd be great if someone could check them before merging.

@expenses expenses changed the title [Draft] Implement device_features and adapter_features for the web backend Implement device_features and adapter_features for the web backend Jan 10, 2023
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.

Looks good after nit

wgpu/src/backend/web.rs Outdated Show resolved Hide resolved
@gents83
Copy link
Contributor

gents83 commented Jan 29, 2023

Trying to implement it directly in wasm-bindgen: rustwasm/wasm-bindgen#3268

@cwfitzgerald
Copy link
Member

@gents83 thank you so much for that work!

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.

5 participants