Added Fn's to work with any kind of WebGl2RenderingContext source.
#101
Conversation
src/web.rs
Outdated
|
|
||
| pub fn get_webgl2_context_from_callback<V, CB>( | ||
| viewport_size_pixels: V, | ||
| cb: CB |
There was a problem hiding this comment.
Is there any reason to pass a callback here, rather than just passing the web_sys::WebGl2RenderingContext directly?
E.g. the signature could be:
pub fn use_existing_webgl2_context(
viewport_size_pixels: impl Into<UVec2>,
context: web_sys::WebGl2RenderingContext
) -> Result<GLRenderer, BacktraceError<GLRendererCreationError>>
{ ... }There was a problem hiding this comment.
No particular reason. Will change it.
src/lib.rs
Outdated
| ) -> Result<Self, BacktraceError<GLRendererCreationError>> | ||
| where | ||
| V: Into<UVec2>, | ||
| S: FnOnce() -> WebGl2RenderingContext |
There was a problem hiding this comment.
This creates a public dependency on the web_sys crate. Would it be possible to create a new feature for this in Cargo.toml please?
There was a problem hiding this comment.
Why public? i see that on wasm32 target arch this dep is included with WebGl2RenderingContext feature . So it is working only on wasm32.
There was a problem hiding this comment.
Sorry, I was unclear. I meant that this exposes WebGl2RenderingContext in the public Speedy2D API, which forces us to continue depending on web-sys in the future (to avoid breaking the code of existing Speedy2D users). If we ever want to replace web-sys with another library, we'd have to release a breaking v3.0.0 update.
I'm okay with this, as long as the use_existing_webgl2_context API is gated behind a feature. For example:
[features]
default = ["windowing", "image-loading"]
windowing = ["glutin"]
image-loading = ["image"]
web-sys-features = []#[cfg(any(feature = "web-sys-features", doc, doctest))]
pub fn use_existing_webgl2_context(
...Then the use_existing_webgl2_context function will only be enabled if web-sys-features is set.
|
Thank you for this! I've left a couple of comments. Also the tests and docs don't seem to build at the moment. |
| /// The parameter `viewport_size_pixels` should be set to | ||
| /// the initial canvas size, however this can be changed later using | ||
| /// [GLRenderer:: set_viewport_size_pixels()]. | ||
| #[cfg(target_arch = "wasm32")] |
There was a problem hiding this comment.
I hope this is Ok. I am not sure why this does not compile with doc and doctest also, bc web-sys dep has this options in cargo.toml.
There was a problem hiding this comment.
I think the issue was this:
#[cfg(target_arch = "wasm32")]
use web_sys::WebGl2RenderingContext;The WebGl2RenderingContext was imported for wasm32 but not doc/doctest. If you make the cfg next to use match the cfg on the function, it should hopefully work.
There was a problem hiding this comment.
(or, just use the full namespace web_sys::WebGl2RenderingContext directly as you've done now, and re-add doc and doctest to the function cfg)
2 participants
No description provided.