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 for OffscreenCanvas in Web Backend #1837

Closed
anlumo opened this issue Aug 21, 2021 · 3 comments · Fixed by #1932
Closed

Support for OffscreenCanvas in Web Backend #1837

anlumo opened this issue Aug 21, 2021 · 3 comments · Fixed by #1932
Labels
area: api Issues related to API surface type: question Further information is requested

Comments

@anlumo
Copy link
Contributor

anlumo commented Aug 21, 2021

Is your feature request related to a problem? Please describe.

I've looked into the way the web backend creates the rendering context:

fn instance_create_surface(
&self,
handle: &impl raw_window_handle::HasRawWindowHandle,
) -> Self::SurfaceId {
let canvas_attribute = match handle.raw_window_handle() {
raw_window_handle::RawWindowHandle::Web(web_handle) => web_handle.id,
_ => panic!("expected valid handle for canvas"),
};
let canvas_node: wasm_bindgen::JsValue = web_sys::window()
.and_then(|win| win.document())
.and_then(|doc| {
doc.query_selector_all(&format!("[data-raw-handle=\"{}\"]", canvas_attribute))
.ok()
})
.and_then(|nodes| nodes.get(0))
.expect("expected to find single canvas")
.into();
let canvas_element: web_sys::HtmlCanvasElement = canvas_node.into();
let context: wasm_bindgen::JsValue = match canvas_element.get_context("gpupresent") {
Ok(Some(ctx)) => ctx.into(),
_ => panic!("expected to get context from canvas"),
};
Sendable(context.into())
}

This seems like a hack, since I can't pass in a canvas directly, I have to create it elsewhere, insert it into the DOM, assign a random id and then pass in the id.

This is fine for demo applications, but when you have an existing web application you want to integrate into, this doesn't seem like a great idea, since the developer has to be careful not to re-use the same id multiple times if there are multiple canvases in the DOM.

Also, eventually it'll be interesting to use wgpu-rs to render in a Web Worker using OffscreenCanvas. Right now, OffscreenCanvas doesn't support webgpu anyways, but as far as my research has unearthed, this will change at some point.

Describe the solution you'd like

Not passing in a raw_handle, but a HtmlCanvasElement or OffscreenCanvas directly. Both supply the get_context API needed for the backend. Alternatively, the backend could also receive the context directly, but that would require the application to set it up with the right parameters, which might not be trivial (though it looks like it is right now).

Describe alternatives you've considered

I've looked into whether I can skip the function shown above and create this context directly, but all of the types needed aren't pub.

Additional context

This is more relevant for the WebGL2 Backend #1686 right now, where it would really work.

@kvark kvark added area: api Issues related to API surface type: question Further information is requested labels Aug 21, 2021
@grovesNL
Copy link
Collaborator

This is fine for demo applications, but when you have an existing web application you want to integrate into, this doesn't seem like a great idea, since the developer has to be careful not to re-use the same id multiple times if there are multiple canvases in the DOM.

This is how raw-window-handle works on the web in general. See https://docs.rs/raw-window-handle/0.3.3/raw_window_handle/web/struct.WebHandle.html#structfield.id It's really useful in general because it allows native and web applications to run in the same way (e.g. using winit+wgpu to run anywhere).

We should support offscreen canvas or canvases that haven't been added to the DOM somehow though. We could expose a wasm-only extension that accepts those web-sys types directly instead like you mentioned. The downsides are:

  • this can only be implemented on the web, so it would be either compiled out on native or accept canvas as an empty type/panic
  • we would have a dependency on a specific version of web-sys in the public API, so we'd probably have to re-export these types, and it may cause problems when multiple web-sys versions are being used (usually unintentionally)

Maybe there are some other options?

@anlumo
Copy link
Contributor Author

anlumo commented Aug 21, 2021

Your solution sounds good! It could be optional behind a feature flag. web-sys doesn't have a lot of major version updates, since it's just an autogenerated API wrapper.

I can see how this workaround is a good solution when your main platform is native and you just want to get it to work on the web somehow, but when the web is the main target, it causes problems, especially when you want to integrate it with existing web frameworks.

@Herschel
Copy link
Contributor

Herschel commented Sep 8, 2021

I've been hitting this too, needing to do the dance of adding the canvas to the DOM with the proper ID. My case is a further complicated because I am using the Shadow DOM, which query_selector_all can't penetrate, so I have to add the canvas to the main document, create the surface, then swap it back to where I actually want it.

I think adding web-only methods that are compiled out on native is fine. Ideally the surface creation methods can directly take a web_sys::HtmlCanvasElement or web_sys::OffscreenCanvas. Unfortunately, I'm not sure if the dependency on web_sys could really be avoided -- it can indeed be problematic because often wasm-bindgen (and thus web_sys) have to be pinned to a specific version anyways, because wasm-bindgen-cli version must match exactly.

Herschel added a commit to Herschel/wgpu that referenced this issue Sep 9, 2021
Add specialized methods on web for creating a wgpu Surface directly
from an web_sys::HTMLCanvasElement and OffscreenCanvas.

 * Instance::create_surface_from_canvas
 * Instance::create_surface_from_offscreen_canvas

Fixes gfx-rs#1837.
kvark pushed a commit that referenced this issue Sep 10, 2021
Add specialized methods on web for creating a wgpu Surface directly
from an web_sys::HTMLCanvasElement and OffscreenCanvas.

 * Instance::create_surface_from_canvas
 * Instance::create_surface_from_offscreen_canvas

Fixes #1837.
@anlumo anlumo mentioned this issue Jan 20, 2022
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api Issues related to API surface type: question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants