Skip to content

Avoid using WasmAbi functions on WebGPU backend#3657

Merged
cwfitzgerald merged 5 commits intogfx-rs:trunkfrom
grovesNL:dynamic-dispatch-workaround
Apr 10, 2023
Merged

Avoid using WasmAbi functions on WebGPU backend#3657
cwfitzgerald merged 5 commits intogfx-rs:trunkfrom
grovesNL:dynamic-dispatch-workaround

Conversation

@grovesNL
Copy link
Copy Markdown
Collaborator

@grovesNL grovesNL commented Apr 8, 2023 •
edited
Loading

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
Fixes #3430

Description
IntoWasmAbi and FromWasmAbi can be problematic for our use case because IntoWasmAbi takes ownership of our type. We could workaround this in different ways, but the most straightforward path right now seems to be to avoid using these entirely and instead not tracking IDs for the WebGPU backend.

We could look at doing this in a few different ways once we internally make some changes to better support multi-threading (e.g., many types will be Arc internally so we could consider some other approaches).

Testing
Tested the WebGPU examples that are known to work (hello-triangle, boids, msaa-line, etc.) in Canary.

@grovesNL grovesNL mentioned this pull request Apr 8, 2023
Closed
@niklaskorz
Copy link
Copy Markdown
Contributor

niklaskorz commented Apr 8, 2023 •
edited
Loading

Tried it on https://github.com/niklaskorz/linon/tree/dynamic-dispatch-workaround and it works (deployed on https://niklaskorz.github.io/linon/):

grafik

A note though (that might be unrelated to this PR, not sure): using depth textures without a stencil part in the format does not seem to work in Chrome, i.e. using Depth32Float instead of Depth24PlusStencil8 gives this error, even though no stencil ops are actually set in the render pass:

stencilLoadOp is (LoadOp::Load) and stencilStoreOp is (StoreOp::Store) when stencilReadOnly (0) or the attachment ([TextureView of Texture "depth_texture"]) has no stencil aspect.
 - While validating depthStencilAttachment.
 - While encoding [CommandEncoder "encoder"].BeginRenderPass([RenderPassDescriptor "rpass"]).

[Invalid CommandBuffer "encoder"] is invalid.
    at ValidateObject (..<URL>)
    at ValidateSubmit (..<URL>)

localhost/:1 WebGPU: too many warnings, no more warnings will be reported to the console for this GPUDevice.

Edit: ok this is indeed unrelated to this PR but it's an existing bug in wgpu, I'll try to fix it :)

@niklaskorz niklaskorz mentioned this pull request Apr 8, 2023
Merged
3 tasks
maxammann added a commit to maxammann/maplibre-rs that referenced this pull request Apr 10, 2023
@maxammann
Upgrade to wgpu from this PR: gfx-rs/wgpu#3657
c5c8618
cwfitzgerald
cwfitzgerald approved these changes Apr 10, 2023
View reviewed changes
Copy link
Copy Markdown
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.

All seems reasonable, thank you for taking the time to fix this!

SludgePhD
SludgePhD reviewed Apr 10, 2023
View reviewed changes
@cwfitzgerald cwfitzgerald merged commit cfc038a into gfx-rs:trunk Apr 10, 2023
maxammann added a commit to maplibre/maplibre-rs that referenced this pull request Apr 10, 2023
@maxammann
Upgrade to wgpu from this PR: gfx-rs/wgpu#3657 (#272)
4945edc
@grovesNL grovesNL deleted the dynamic-dispatch-workaround branch April 11, 2023 11:38
@philpax philpax mentioned this pull request Apr 11, 2023
Closed
@mockersf mockersf mentioned this pull request Apr 12, 2023
Merged
3 tasks
@philpax philpax mentioned this pull request Apr 14, 2023
Merged
@eddyb eddyb mentioned this pull request Apr 15, 2023
Closed
This was referenced Apr 21, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cwfitzgerald cwfitzgerald cwfitzgerald approved these changes

+1 more reviewer

@SludgePhD SludgePhD SludgePhD left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dynamic dispatch layer breaks on web

4 participants

@grovesNL @niklaskorz @cwfitzgerald @SludgePhD