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

Some bindgroups do not get deduplicated #6037

Closed
sagudev opened this issue Jul 24, 2024 · 3 comments · Fixed by #6049
Closed

Some bindgroups do not get deduplicated #6037

sagudev opened this issue Jul 24, 2024 · 3 comments · Fixed by #6049
Assignees
Labels
type: bug Something isn't working

Comments

@sagudev
Copy link
Contributor

sagudev commented Jul 24, 2024

Description
Discovered in #6012 (comment), sometimes (compatible) bindgroups with same content are not deduplicated, therefore not detected as equal (I suspect the problem originates from layouts that come from getBindGroupLayout).

Repro steps
Repro is here: https://github.com/sagudev/wgpu-problem/tree/another-empty-bind-panic-2. This is webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:default_bind_group_layouts_never_match,render_pass:pipelineType="auto0";bindingType="auto0";swap=true;empty=false;renderCommand="draw" but rewritten in rust (and to wgpu).

Expected vs observed behavior
Per CTS there should actually be no error (bind groups are the same in content): https://github.com/gpuweb/cts/blob/198d1770062c1a8aba86e7d6e001bb47bea028ee/src/webgpu/api/validation/encoding/programmable/pipeline_bind_group_compat.spec.ts#L960

When this is done it should be safe to revert 60b42f9 (ping me for CTS run on servo)

@teoxoy teoxoy added the type: bug Something isn't working label Jul 25, 2024
@teoxoy teoxoy self-assigned this Jul 25, 2024
@teoxoy
Copy link
Member

teoxoy commented Jul 25, 2024

It turns out that we are not deduplicating derived BGLs at all.

@sagudev
Copy link
Contributor Author

sagudev commented Jul 25, 2024

Why exactly BGL needs to work using deduplication, can't we just operate with duplicated bindgrops and check for compat by content, to me it seems like a big footgun.

@teoxoy
Copy link
Member

teoxoy commented Jul 25, 2024

I'm not sure that the underlying APIs allow that. I tried reducing potential misuse (forgetting to deduplicate) but it didn't work out since the other code-path that creates BGLs is quite different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants