Support extern context impl by sagudev · Pull Request #6658 · gfx-rs/wgpu

sagudev · 2024-12-04T19:50:13Z

Connections
Based on #6619, fixes #6330

Description
This PR enables users of wgpu to provide custom Instance impl (and other wgpu types as all other types are derived from Instance->Adapter->Device), by adding Custom variant and custom backend that has dyn wrapper for all types: struct DynDevice(Arc<dyn DeviceInterface>). I'm intermixing Custom with Dyn because I cannot really decide on name (CustomDevice is weird, but we already have Dyn* in wgpu-hal).
There are few rough sports:

RenderBundleEncoderInterface::finish (other backends hacks around by not using dyn dispatch, but we can't:

wgpu/wgpu/src/api/render_bundle_encoder.rs

Lines 54 to 60 in b876b8c

    
               pub fn finish(self, desc: &RenderBundleDescriptor<'_>) -> RenderBundle { 
        
                   let bundle = match self.inner { 
        
                       #[cfg(wgpu_core)] 
        
                       dispatch::DispatchRenderBundleEncoder::Core(b) => b.finish(desc), 
        
                       #[cfg(webgpu)] 
        
                       dispatch::DispatchRenderBundleEncoder::WebGPU(b) => b.finish(desc), 
        
                   };

Testing
There is wgpu-custom crate that showcases how one can create Custom wgpu types.

Checklist

Run cargo fmt.
Run taplo format.
Run cargo clippy. If applicable, add:
- --target wasm32-unknown-unknown
- --target wasm32-unknown-emscripten
Run cargo xtask test to run tests.
Add change to CHANGELOG.md. See simple instructions inside file.

sagudev · 2024-12-05T05:52:33Z

TBH, I have no idea what's wrong with WASM.

sagudev · 2024-12-06T08:47:36Z

wgpu/src/api/instance.rs

+    #[doc(hidden)]
+    /// Creates Instance from custom context implementation
+    pub fn from_custom_instance(instance: std::sync::Arc<dyn InstanceInterface>) -> Self {
+        Self {
+            inner: dispatch::DispatchInstance::Custom(backend::custom::DynContext::new(instance)),
+        }
+    }


This is triggering visibility problems for WASM, but I do not know why.

cwfitzgerald · 2024-12-11T07:27:20Z

Ping me if this needs looking at or you need help before it is out of draft, going to un-assign myself for bookeeeping

sagudev · 2025-01-08T17:57:56Z

In 79ecded I removed InterfaceTypes trait as it's not really needed anymore (this makes writing Custom/Dyn wrappers easier as they do not need to impl traits directly, avoiding a lot of boilerplate), all bounds are actually enforced in dispatch_types_inner anyway. The only useful application was making writing macros easier, but I replaced this with mod interface_types that has aliases for interface types (there is a lot of this pattern in servo as it was used before associate trait types), but we could also just write more stuff in macro and remove the completely. Does approach like this sounds reasonable @cwfitzgerald? I would be willing to split this into separate PR if it's ok.

cwfitzgerald · 2025-01-08T19:44:59Z

Hmm... I don't love it as it makes things less structured, but can't see a strong argument against removing the trait. Will fully consider it when I review this. You can leave it as part of this PR for now.

sagudev · 2025-01-09T06:47:15Z

I have alternative approach in sagudev@ce58280 which keeps the trait but instead of using interfaces in bounds it uses Dispatch<Target = dyn Interface> (clone of Deref trait, I avoid Deref to avoid compiler magic as it makes bounds less explicit esp between Dispatch and DispatchMut). One thing to consider here is that implementations of custom context will need to provide all types, even those that are not needed (ex. if one does not need/use compute pass). Should I switch to this approach @cwfitzgerald?

cwfitzgerald · 2025-01-09T15:49:48Z

I have alternative approach in sagudev@ce58280 which keeps the trait but instead of using interfaces in bounds it uses Dispatch<Target = dyn Interface> (clone of Deref trait, I avoid Deref to avoid compiler magic as it makes bounds less explicit esp between Dispatch and DispatchMut).

I'm going to leave this up to your own judgement which is cleaner and more parsable, my initial impression is that the mod { type } approach is less infrastructure and less confusing than adding the Dispatch one

sagudev · 2025-01-29T21:21:20Z

@cwfitzgerald I think this is ready for concept review, it still needs more docs and maybe some polish 💅🏾.

cwfitzgerald · 2025-01-29T21:39:02Z

Nice! I'm going to take this out of draft for bookkeeping

cwfitzgerald

Don't love the whole new type alias thing, but don't really hate it enough to say no

wgpu-custom/Cargo.toml

wgpu/src/dispatch.rs

sagudev · 2025-02-13T14:38:32Z

Don't love the whole new type alias thing, but don't really hate it enough to say no

I think we could actually inline all types uses into macro invocation (all 3 of them). Would that be better?

cwfitzgerald · 2025-02-15T00:04:24Z

I think we could actually inline all types uses into macro invocation

Hmm, yeah I think that's fine - the whole appeal to me is to make sure things have uniform naming conventions and are all filled out, so having them all in one place seems to do that trick fine.

sagudev · 2025-02-15T07:03:19Z

I think we could actually inline all types uses into macro invocation

Hmm, yeah I think that's fine - the whole appeal to me is to make sure things have uniform naming conventions and are all filled out, so having them all in one place seems to do that trick fine.

I am not sure I understand, should I inline all types uses into macro or keep the mods?

cwfitzgerald · 2025-02-15T07:21:38Z

Oh sorry was unclear, inline into the macro.

cwfitzgerald

Some small things, but I think this is good to go after that!

examples/custom-backend/Cargo.toml

wgpu/src/backend/custom.rs

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

cwfitzgerald

I have some futzing I want to do to the examples, but I'll do that post land.

Thanks!

DJMcNab · 2025-04-17T15:48:26Z

Has this been released? I'm trying to check the changelog for when this got released

sagudev · 2025-04-17T16:33:38Z

Has this been released? I'm trying to check the changelog for when this got released

Yes it was, but there are still some rough edges: #7533

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

sagudev requested a review from a team as a code owner December 4, 2024 19:50

sagudev marked this pull request as draft December 4, 2024 19:50

sagudev changed the title ~~Init CustomContext and Dyn_~~ WIP: SUpport extern context impl Dec 4, 2024

sagudev force-pushed the custom-ctx branch from 3d9022f to eb0adc0 Compare December 4, 2024 20:49

sagudev changed the title ~~WIP: SUpport extern context impl~~ WIP: Support extern context impl Dec 4, 2024

cwfitzgerald self-assigned this Dec 5, 2024

sagudev force-pushed the custom-ctx branch from f74b3c7 to 1d94171 Compare December 5, 2024 05:51

sagudev mentioned this pull request Dec 5, 2024

Implement Clone on Api Types and Arc Dispatcher #6665

Merged

6 tasks

sagudev commented Dec 6, 2024

View reviewed changes

sagudev force-pushed the custom-ctx branch 2 times, most recently from 9090201 to 088532e Compare December 6, 2024 09:07

cwfitzgerald removed their assignment Dec 11, 2024

sagudev force-pushed the custom-ctx branch from 7414a46 to f52082a Compare January 3, 2025 08:04

sagudev mentioned this pull request Jan 3, 2025

Move Arc inside dispatch enum #6848

Closed

sagudev force-pushed the custom-ctx branch from ce9fc9f to 79ecded Compare January 8, 2025 17:48

sagudev force-pushed the custom-ctx branch from cf26dcd to 97ad87e Compare January 10, 2025 07:31

sagudev force-pushed the custom-ctx branch 2 times, most recently from cf170b3 to d693c1b Compare January 29, 2025 18:58

sagudev changed the title ~~WIP: Support extern context impl~~ Support extern context impl Jan 29, 2025

cwfitzgerald marked this pull request as ready for review January 29, 2025 21:39

sagudev mentioned this pull request Jan 30, 2025

Add Vello as alternative 2d canvas backend servo/servo#35230

Closed

5 tasks

cwfitzgerald self-assigned this Feb 5, 2025

cwfitzgerald requested changes Feb 13, 2025

View reviewed changes

wgpu-custom/Cargo.toml Outdated Show resolved Hide resolved

wgpu/src/dispatch.rs Outdated Show resolved Hide resolved

sagudev force-pushed the custom-ctx branch from 891c2e0 to ef343eb Compare February 15, 2025 07:44

cwfitzgerald requested changes Mar 3, 2025

View reviewed changes

examples/custom-backend/Cargo.toml Show resolved Hide resolved

wgpu/src/backend/custom.rs Show resolved Hide resolved

sagudev force-pushed the custom-ctx branch from c89ef8d to 0d16f81 Compare March 4, 2025 10:32

Support extern context impl

2f925d8

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

sagudev force-pushed the custom-ctx branch from 0d16f81 to 2f925d8 Compare March 4, 2025 10:39

DJMcNab mentioned this pull request Mar 4, 2025

Better API for already GPU-resident images linebender/vello#664

Closed

cwfitzgerald approved these changes Mar 4, 2025

View reviewed changes

cwfitzgerald merged commit 7e66495 into gfx-rs:trunk Mar 4, 2025
34 checks passed

cwfitzgerald mentioned this pull request Mar 4, 2025

A few adjustments to custom_backend example #7265

Merged

6 tasks

sharmajai pushed a commit to sharmajai/wgpu that referenced this pull request Oct 12, 2025

Support extern context impl (gfx-rs#6658)

52185cf

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

	pub fn finish(self, desc: &RenderBundleDescriptor<'_>) -> RenderBundle {
	let bundle = match self.inner {
	#[cfg(wgpu_core)]
	dispatch::DispatchRenderBundleEncoder::Core(b) => b.finish(desc),
	#[cfg(webgpu)]
	dispatch::DispatchRenderBundleEncoder::WebGPU(b) => b.finish(desc),
	};

Conversation

sagudev commented Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sagudev commented Dec 5, 2024

Uh oh!

sagudev Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

cwfitzgerald commented Dec 11, 2024

Uh oh!

sagudev commented Jan 8, 2025

Uh oh!

cwfitzgerald commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sagudev commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwfitzgerald commented Jan 9, 2025

Uh oh!

sagudev commented Jan 29, 2025

Uh oh!

cwfitzgerald commented Jan 29, 2025

Uh oh!

cwfitzgerald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sagudev commented Feb 13, 2025

Uh oh!

cwfitzgerald commented Feb 15, 2025

Uh oh!

sagudev commented Feb 15, 2025

Uh oh!

cwfitzgerald commented Feb 15, 2025

Uh oh!

cwfitzgerald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cwfitzgerald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DJMcNab commented Apr 17, 2025

Uh oh!

sagudev commented Apr 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sagudev commented Dec 4, 2024 •

edited

Loading

cwfitzgerald commented Jan 8, 2025 •

edited

Loading

sagudev commented Jan 9, 2025 •

edited

Loading