`wgpu::Buffer` implementing `PartialEq`, `Eq`, and `Hash`

[dfellis]

I wanted to use wgpu::Buffer in a HashSet so I can cheaply determine which buffers are used in a generated wgsl shader before binding them. But you can't create HashSet<&wgpu::Buffer> or HashSet<Rc<wgpu::Buffer>> because PartialEq, Eq, and Hash are not implemented for the type.

I personally worked around this by following the newtype pattern, which then also necessitated writing implementations for Deref and DerefMut, but it would be nice if this was just part of the wgpu package and I could drop the wrapper type.

I figure others generating shaders to execute on-the-fly may run into similar issues, which is why I'm opening the issue up for consideration. Because Buffer equality from a shader execution perspective requires that they are actually the same buffer on either side of the comparison, I simply did hashing and equality checking on wgpu::Buffer::global_id calls.

I'm willing to write the PR to do this if you agree it's useful, but I figured I shouldn't drive-by PR without getting that approval.

[kpreid]

It makes sense to me that every wgpu type that has an ID should be comparable by that ID.

[cwfitzgerald]

Yeah global ids aren't going away any time soon, so I think it's reasonable to have these

[dfellis]

So I am a bit stumped on how to test this. I have implemented the traits for Buffer but as there's no constructor for the type, I'm at a bit of a loss as to how to write unit tests.

If you have any suggestions, let me know, then I'll look for other types that have an id and add the traits for those, as well.

[Wumpf]

@dfellis have a look at the tests under wgpu/tests. They come with a test framework that makes it easier to create buffers from a device. A bit overkill on one hand but otherwise we'd need to start mocking things which gets complex (and we might just do that in that very framework if we ever figure that device creation etc. is too slow for tests that should only validate things like this)

[teoxoy]

All resources created by wgpu are unique and their global_id is also unique, they are also not cloneable, so I'm not sure what can be gained from implementing PartialEq, Eq, and Hash for them.

[dfellis]

Hi @teoxoy

To quote myself

wanted to use wgpu::Buffer in a HashSet so I can cheaply determine which buffers are used in a generated wgsl shader before binding them.

I have some code that is meta-programming a wgsl shader and then executing it. I wanted to keep track of which buffers that have been created are actually being used in a particular shader so only those are added to the bind group when I create a compute pipeline, and the easiest way to do that is to create a HashSet that I index the buffers on and then generate the bind group layout and entries just-in-time.

Not all buffers can or should be included in each of these, so I am currently wrapping it in a newtype and Rc<T> to do this. I am chugging along just fine as-is (which is also why my own work on implementing these traits and pushing up a PR is moving slowly), but I can see anyone wanting to provide a DSL for their own users, like an easy-to-use game engine, for example, could run into similar problems.

[kpreid]

what can be gained from implementing PartialEq, Eq, and Hash for them.

Besides Rc<Buffer>, &Buffer could also be used as a key (in a presumably temporary data structure) in this type of situation. In principle, the most efficient solution is comparing either of those by pointer rather than by .global_id(), or using the id itself as the data structure's key, but the convenience of an Eq and Hash implementation seems like a sufficient argument to me.

[teoxoy]

I see, those are all good points, thanks for clarifying!

[teoxoy]

Related PR: https://github.com/gfx-rs/wgpu/pull/6134. I might implement those traits in that PR or in a follow-up.