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

Support extern Context implementations #6330

Open
sagudev opened this issue Sep 26, 2024 · 3 comments
Open

Support extern Context implementations #6330

sagudev opened this issue Sep 26, 2024 · 3 comments

Comments

@sagudev
Copy link
Contributor

sagudev commented Sep 26, 2024

In servo we want to use vello for 2d canvas and we want it in content process (where it shouldn't access GPU), so we cannot run wgpu in that process. Our initial plan was to rewrite part of vello that uses wgpu to reuse webgpu implementation already present in content process (mostly just wrappers around wgpu's IDs and that sends IPC messages to wgpu thread) or abstract vello's wgpu usage behind something we can implement with our webgpu implementation. Ironically, wgpu is already an abstraction, so instead of another level of abstracting we could just implement our own wgpu::context::Context using servo's webgpu types and provide custom instance (that has our own context inside) to vello and let it do its 🪄 .

I already have dummy impl of how that would look like altogether: https://github.com/sagudev/wgpuidex/tree/dummy-vello and changes needed in wgpu to support external context impl: v22...sagudev:wgpu:extern-impl-experiment.

Describe alternatives you've considered
Alternative is that we would have ipc based backend in tree and consumers would provide ipc sender on instance creation (and we could also provide impl needed for receivers), that could be useful for those who wants to do process isolation. I wonder if it would be possible for servo/firefox to use such impl on content process, or is wgpu abstraction to much high level 🤔.

Or simply do nothing as this is really obscure request.

Additional context
This will fight with #6324

We have chat about servo using vello on zulip: https://servo.zulipchat.com/#narrow/stream/263398-general/topic/Using.20Vello.20in.20Servo

@Wumpf
Copy link
Member

Wumpf commented Sep 26, 2024

This will fight with #6324

Very true unfortunately so far! @cwfitzgerald and me were hoping to simplify the wgpu forwarding a lot by no longer assuming arbitrary context trait impls. The motivation is to make it much easier to follow the path of a wgpu call to wgpu-core internals (hard & annoying even for active maintainers) and allow the indirection to be optimized out completely for native and webgl/webgpu-only cases.

Or simply do nothing as this is really obscure request.

still pondering this. I don't think it's unreasonable to be able to redirect the api. Usually the requests we get about this are for wgpu-hal which is harder still.
It's also super interesting how little changes you needed right now for your dummy impl on wgpu.

Maybe we can extend the #6324 proposal to make space for a custom context next to wgpucore & webgpu. The fact that we do already have slight api differences for native & wasm doesn't make this easier, but as I understand you'd be only interested in native.

@sagudev
Copy link
Contributor Author

sagudev commented Sep 26, 2024

The motivation is to make it much easier to follow the path of a wgpu call to wgpu-core internals (hard & annoying even for active maintainers) and allow the indirection to be optimized out completely for native and webgl/webgpu-only cases.

I agree with motivation, the indirection really makes it hard.

still pondering this. I don't think it's unreasonable to be able to redirect the api. Usually the requests we get about this are for wgpu-hal which is harder still.

IIRC somebody did try to do something like this to be able to trace wgpu or smth like that, but I do not recall at which level (but probably hal).

Maybe we can extend the #6324 proposal to make space for a custom context

That would be ideal.

@cwfitzgerald
Copy link
Member

We could have a variant hold a Box<dyn Trait>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants