Remove `gpu-descriptor` dependency by inlining the crate by a1phyr · Pull Request #9078 · gfx-rs/wgpu

a1phyr · 2026-02-18T17:35:26Z

Description

Stop depending on gpu-descriptor and inline the part of their code that we need directly.

gpu-descriptor is very passively maintained and has outdated dependencies. Inlining the crate into wgpu-hal prevents this and simplifies our code at a small cost (~300 LoC). Additionally, this should reduce compile times a bit.

Each commit can be reviewed independently. The first commit bring parts of gpu-descriptor and gpu-descriptor-types in wgpu-hal/src/vulkan/descriptor.rs and does minor import changes to accommodate for this. Other are refactoring that take advantage of the inlining.

Testing
Ran tests

Squash or Rebase?

Rebase

Checklist

Run cargo fmt.
Run taplo format.
Run cargo clippy --tests. If applicable, add:
- --target wasm32-unknown-unknown
Run cargo xtask test to run tests.
If this contains user-facing changes, add a CHANGELOG.md entry.

inner-daemons

So I'm no lawyer but if this is a line-for-line copy, that seems slightly legally dubious. More specifically, we will almost certainly need to bundle the copyright notice and license text, which it doesn't look like you did here. Overall, I'm not sure why we need this but I'll let another Mozillan handle it, since they have a whole system for these things.

Also, I realize this is an early review, and you might be in the middle of fixing the copyright fun business.

Also move the implementation to `descriptor.rs`.

This function is only ever used with a single item, so we can simplify it using this knowledge.

a1phyr · 2026-02-19T11:26:28Z

Well, from the MIT license:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

Which I did in wgpu-hal/src/vulkan/descriptor.rs. Also, wgpu is also licensed under MIT/APACHE, so the license is bundled. Am I missing something?

cwfitzgerald · 2026-02-19T16:11:03Z

On licensing, yeah this is fine, the one note would be to make the LICENSE field of wgpu-hal "(MIT OR Apache-2.0) AND MIT" to be fully formally correct.

a1phyr · 2026-02-20T12:40:22Z

Well gpu-descriptor is also "MIT OR Apache-2.0" and Apache-2.0 is pretty much the same as MIT about redistribution (keep attribution and license) so I think that it is fine as is?

~~On another note, do you know why some Metal tests are failing? I didn't touch this backend at all.~~

inner-daemons

Preliminary review.

I'm curious, what's necessarily the goal here? Is there a reason we need to remove the dependency? And if we do remove the dependency, why not just rewrite the logic as needed? It seems like copying the code over left us with some unnecessary artifacts.

Anyway, thanks for the work put in here. I'm gonna probably just gonna defer to the next reviewer for most stuff.

inner-daemons · 2026-02-27T05:14:36Z

+        };
+
+        if pool_size == Default::default() {
+            pool_size.sampler = 1;


What's this for?

I don't know. Maybe to prevent zero-sized pools?

inner-daemons · 2026-02-27T05:20:21Z

+            .max(minimal_set_count) // at least enough for allocation
+            .max(self.total.min(MAX_SETS)) // at least as much as was allocated so far capped to MAX_SETS
+            .checked_next_power_of_two() // rounded up to nearest 2^N
+            .unwrap_or(i32::MAX as u32);


This seems sketchy, plus shouldn't it be i32::MAX as u32 + 1 either way?

Yes, probably (this branch was not reachable anyway). Replaced this code by a much more readable clamp. Not sure about what the next_power_of_two() is about though.

inner-daemons · 2026-02-27T05:21:17Z

+            .checked_next_power_of_two() // rounded up to nearest 2^N
+            .unwrap_or(i32::MAX as u32);
+
+        max_sets = (u32::MAX / self.size.sampler.max(1)).min(max_sets);


This definitely needs a comment explainign what its doing dividing by the descriptor count.

Well probably, but I can't really explain it.

It's to avoid overflowing count * max_sets calculations below.

a1phyr · 2026-02-27T13:16:02Z

I'm curious, what's necessarily the goal here? Is there a reason we need to remove the dependency?

Well, mainly the fact that it's not really maintained. Also this enables us to tailor fit this code.

And if we do remove the dependency, why not just rewrite the logic as needed? It seems like copying the code over left us with some unnecessary artifacts.

This is what I started with some low-hanging fruits in commits after the first, but looks that I missed some.

a1phyr · 2026-03-02T11:50:34Z

CI failure is a DX12 timeout, so probably unrelated.

teoxoy

I found some more improvements/simplifications and decided to implement the allocator from scratch (#9161). I think it ended up better suited to our Vulkan backend (using knowledge of BGL creation/destruction). A new implementation also avoids the question of how do we make sure we comply with the license brought up at the maintainers meeting last week (we talked about it but didn't come to a resolution).

teoxoy · 2026-03-03T15:31:05Z

-            }
-        }
+        unsafe { self.raw.create_descriptor_pool(&vk_info, None) }
+            .map_err(super::map_host_device_oom_err)


This won't handle vk::Result::ERROR_FRAGMENTATION which is a valid error type for this cmd to return. We can probably just map it to an OOM error for now.

teoxoy · 2026-03-03T16:20:41Z

-                Err(DeviceAllocationError::Unexpected)
-            }
        }
+        .map_err(super::map_host_device_oom_err)


This one needs to additionally handle ERROR_FRAGMENTED_POOL (note that this one is different from ERROR_FRAGMENTATION) & ERROR_OUT_OF_POOL_MEMORY. Mapping them to an OOM error is probably fine.

teoxoy · 2026-03-03T16:31:55Z

    ///
    /// Pool must be created from this device.
-    unsafe fn alloc_descriptor_sets<'a>(
+    unsafe fn alloc_descriptor_sets(


Should now be alloc_descriptor_set and can return a single vk::DescriptorSet. The docs also need updating.

teoxoy · 2026-03-03T17:30:35Z

-fn handle_unexpected(err: vk::Result) -> ! {
+pub(super) fn handle_unexpected(err: vk::Result) -> ! {
    panic!("Unexpected Vulkan error: `{err}`")
 }


We shouldn't need to use this function anymore.

teoxoy · 2026-03-04T14:50:05Z

+            .checked_next_power_of_two() // rounded up to nearest 2^N
+            .unwrap_or(i32::MAX as u32);
+
+        max_sets = (u32::MAX / self.size.sampler.max(1)).min(max_sets);


It's to avoid overflowing count * max_sets calculations below.

teoxoy · 2026-03-04T21:06:56Z

+            .ok()
+            .is_some_and(|count| count <= bucket.total));
+
+        unsafe { bucket.free(device, self.raw_sets_cache.drain(..), set.pool_id) };


raw_sets_cache seems unnecessary.

teoxoy · 2026-03-04T21:09:43Z

+                }
+
+                if let Some(last_id) = last {
+                    unsafe { bucket.free(device, self.raw_sets_cache.drain(..), last_id) };


We only ever allocate one set at a time, this looks like dead code.

Together with gfx-rs/wgpu#9078, this should deduplicate hashbrown for wgpu and eventually for Ruffle. Co-authored-by: Tomáš Procházka <SuchAFuriousDeath@users.noreply.github.com>

inner-daemons · 2026-03-14T02:50:10Z

@a1phyr Just FYI, this is likely to be replaced by #9161 unless something goes horribly wrong.

a1phyr · 2026-03-17T16:33:39Z

Yeah, this is what I figured out, this is why I didn't answer to the last reviews.

inner-daemons reviewed Feb 18, 2026

View reviewed changes

a1phyr added 7 commits February 19, 2026 12:22

Inline gpu-descriptor crate

db10add

Remove generics from old gpu_descriptor types

e39b1da

Inline destroy_descriptor_pool

a1d1d05

Remove DescriptorDevice trait

bbc7414

Also move the implementation to `descriptor.rs`.

Remove Allocation type

4a1e80f

Improve error handling

36b6dd1

Simplify DescriptorAllocator::free

3ee67e4

This function is only ever used with a single item, so we can simplify it using this knowledge.

a1phyr force-pushed the remove-gpu-descriptor branch from ad76441 to 3ee67e4 Compare February 19, 2026 11:23

cwfitzgerald assigned teoxoy and inner-daemons Feb 25, 2026

inner-daemons self-requested a review February 25, 2026 16:25

SuchAFuriousDeath mentioned this pull request Feb 25, 2026

chore: Bump hashbrown to ^0.16 petgraph/petgraph#967

Merged

inner-daemons suggested changes Feb 27, 2026

View reviewed changes

Take into account that we always allocate a single descriptor

0dc3976

a1phyr force-pushed the remove-gpu-descriptor branch from 1af5b40 to 7a6ac42 Compare February 27, 2026 13:08

Remove useless gpu-descriptor types

bc46bf9

a1phyr force-pushed the remove-gpu-descriptor branch from 7a6ac42 to 93a0882 Compare February 27, 2026 13:14

Remove useless fields from DescriptorTotalCount

d5d937a

a1phyr force-pushed the remove-gpu-descriptor branch from 93a0882 to 1a787ab Compare February 27, 2026 13:17

Remove pop + push logic

4b83123

a1phyr force-pushed the remove-gpu-descriptor branch from 1a787ab to 4b83123 Compare February 27, 2026 14:06

teoxoy mentioned this pull request Mar 4, 2026

[vk] implement our own descriptor allocator #9161

Merged

teoxoy reviewed Mar 4, 2026

View reviewed changes

cwfitzgerald closed this in #9161 Apr 29, 2026

Uh oh!

Conversation

a1phyr commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

inner-daemons left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a1phyr commented Feb 19, 2026

Uh oh!

cwfitzgerald commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

a1phyr commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

inner-daemons left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a1phyr Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

a1phyr commented Feb 27, 2026

Uh oh!

a1phyr commented Mar 2, 2026

Uh oh!

teoxoy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inner-daemons commented Mar 14, 2026

Uh oh!

a1phyr commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

a1phyr commented Feb 18, 2026 •

edited

Loading

inner-daemons left a comment •

edited

Loading

cwfitzgerald commented Feb 19, 2026 •

edited

Loading

a1phyr commented Feb 20, 2026 •

edited

Loading

a1phyr Feb 27, 2026 •

edited

Loading