wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

Manage Ids correctly for equivalent BindGroupLayouts

Open kunalmohan opened this issue 5 years ago • 4 comments

Description A clear and concise description of what the bug is. While creating a BindGroupLayout, a check is done to find if any equivalent BGL already exists. If it exists, the Id for the existing BGL is returned without registering the new Id. This creates a problem for wgpu-core client since it assumes that the BGL was successfully created and continues to refer to it with the new Id while creating BindGroups and PipelineLayouts. wgpu-core then panics when trying to create bind_group or pipeline_layout that refers to such a BGL Id.

Repro steps Ideally, a runnable example we can check out. Webgpu cts is the only test/example I know of at the moment.

Expected vs observed behavior Clearly describe what you get, and how it goes across your expectations.

Extra materials Screenshots to help explain your problem. Validation logs can be attached in case there are warnings and errors. Zip-compressed API traces and GPU captures can also land here.

Platform Information about your OS, version of wgpu, your tech stack, etc. wgpu master, Should be platform-independent.

kunalmohan avatar Jul 14 '20 16:07 kunalmohan

The problem is clear to me: when we see an existing BGL matching the new one, we return the existing ID and bump the refcount. This isn't compatible with with the Web, were IDs are given to us already, and the new ID is different.

However, I don't see how we'd resolve this at wgpu level, properly... At Servo level though, you can resolve it in one of the possible ways:

  1. Synchronously block the client, wait for the ID sent by the server. This isn't an option for Gecko, since we can't stall, and we shouldn't do it for Servo either.
  2. Deduplicate the BGLs on the client side. This would let you return an existing JS object. wgpu doesn't need to know about this.

In either way, we should put some sort of a check in wgpu that we don't try deduplication if the ID is given to us.

kvark avatar Jul 14 '20 20:07 kvark

For Gecko, de-duplication on the client side is painful. It would be Rust code holding pointers to C++ wrappers of JS objects, yikes. I'd rather not rely on this hack. Instead, we need to find a solution at the wgpu-core level, which will work for all clients.

kvark avatar May 21 '21 18:05 kvark

@jimblandy is this still a problem?

cwfitzgerald avatar Jun 02 '22 16:06 cwfitzgerald

Nothing has changed on this question.

That reference to "gfx-rs#797" seems to be a broken link of some sort; I can't find what it's referring to. Maybe fallout from repo renames?

jimblandy avatar Jun 06 '22 19:06 jimblandy

I think this has been fixed by the recent bindgroup deduplication work. If this is still a problem, feel free to re-open.

jimblandy avatar Dec 15 '23 15:12 jimblandy