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

Initial WebGL2 support. #218

Merged
merged 19 commits into from
Nov 16, 2021
Merged

Initial WebGL2 support. #218

merged 19 commits into from
Nov 16, 2021

Conversation

parasyte
Copy link
Owner

@parasyte
Copy link
Owner Author

There are some things I don't really like about this example. Most annoying is that winit does not support resize events on web targets, so the canvas doesn't scale to fit available space. Also I'm unhappy with the boilerplate main() function that has two different implementations depending on the target. But what are you gunna do?

@parasyte
Copy link
Owner Author

The WASM CI task does take a few more minutes because it needs to install wasm-bindgen-cli and just. Well, just is technically optional, but it could also be installed by downloading a release build instead of compiling it.

@JMS55
Copy link
Contributor

JMS55 commented Oct 29, 2021

I had a similar experience when porting Sandbox to wasm. I'm starting to think it might be a good idea to offer a winit feature for pixels that would integrate things more - wasm setup, simplified input handling, etc. What are your thoughts?

@parasyte
Copy link
Owner Author

Yep, thought about that very briefly. I don't want winit as a direct dependency, but we can absolutely wrap up the boilerplate into its own crate. As long as it can take care of things like resize, I believe that would be a huge improvement over rolling your own WASM container.

@MarkAnthonyM
Copy link
Contributor

I think I got a version of this example going that has some resize capability, but it's kinda janky. It doesn't smoothly resize, but jumps to bigger and bigger sizes based on the current browser window size.
like so: https://media.giphy.com/media/5FPB18f3h6oQRt7pWE/giphy.gif

I honestly wasn't too sure what I was doing, just used other code I found as a template and sort of fiddled with it to get it going. So I'm not sure if I can figure how to get it resizing smoothly, but I'll still give it a shot. If I get it working, is there a way to include the changes in this specific pull request?

@parasyte
Copy link
Owner Author

@MarkAnthonyM You can create a PR against the webgl2-support branch while this PR is open. I'll gladly accept any improvements here.

FWIW, I looked briefly at how the egui demo handles resize for web builds. It just does its own DOM access to determine the browser's client size. I'm not sure if this is the right way to do this, since winit appears to expect it has exclusive control over the canvas size according to this comment.

It may or may not be useful, but it's an extra datapoint and some prior art in the area.

@JMS55
Copy link
Contributor

JMS55 commented Oct 30, 2021

Resize support would be great! Is the non-smooth resizing not just pixels only snapping to integer multiples? I.e., pixels won't scale your game by 1.343 or anything, only 1x, 2x, 4x, etc...

@parasyte
Copy link
Owner Author

The linked gif looks correct, to me. 🤷

@MarkAnthonyM
Copy link
Contributor

@JMS55 Oh ok, I didn't know about that

I was under the impression that the height and width of pixel's rendering area was suppose to grow at the same height and width values that the browser client does. Like following closely to the edges of the browser window, leaving no black borders. Now that I think about it a little bit, I guess that wouldn't really make sense from pixel's perspective.

In that case, if this is the expected behavior, I'll just clean up the code and send off a PR.

@parasyte
Copy link
Owner Author

I was under the impression that the height and width of pixel's rendering area was suppose to grow at the same height and width values that the browser client does. Like following closely to the edges of the browser window, leaving no black borders.

The expected behavior can be accomplished on the caller-side. For instance, the minimal-imgui example resizes the pixel buffer to match the window size (other examples do not):

// Resize the window
if let Some(size) = input.window_resized() {
if size.width > 0 && size.height > 0 {
// Resize the surface texture
pixels.resize_surface(size.width, size.height);
// Resize the world
let LogicalSize { width, height } = size.to_logical(scale_factor);
world.resize(width, height);
pixels.resize_buffer(width, height);
}
}

In that case, if this is the expected behavior, I'll just clean up the code and send off a PR.

That sounds good to me! Thank you.

@MarkAnthonyM
Copy link
Contributor

With this extension trait, the WASM-specific part of run becomes just a call to window.init_web(). I would really like to encapsulate the bifurcated main, though. The best I've come up with for that is just moving it to a bare function in whatever crate this boilerplate goes into:

fn main() {
    pixels_web::app(run());
}

For general purpose, the init_web method should panic if a canvas was already added to the body. Prevent calling it twice and doubling the canvas element and resize listener.

I'm open to bikeshedding the method names and the design. Any ideas for improving main?

I think your idea for abstracting away the fluff currently in main makes sense. I can't think of anything else other then decorating stuff with macros, but that seems a little overkill. I did a quick dry run, as I wasn't sure what the signature of pixels_web::app() should look like, and it worked out fine. But I started wondering about where pixels_web would be located. Would it be top level with pixels? Something like?

root
|------pixels
|       |------examples
|       |------internals
|       |------src
|------pixels_web
        |------src

@parasyte
Copy link
Owner Author

parasyte commented Nov 3, 2021

I did a quick dry run, as I wasn't sure what the signature of pixels_web::app() should look like, and it worked out fine.

Me too. I came up with:

pub fn app(future: impl Future<Output = ()> + 'static)

But I started wondering about where pixels_web would be located. Would it be top level with pixels?

After I put my thoughts into usable code, I recognized that it doesn't have any relation to pixels at all. It's just an extension trait for winit and an abstraction for polling a future in WASM and native environments. I would almost want to call the crate winit-web because of that!

It doesn't seem to matter where it lives. Either here (a top-level crate as you've shown) or in another repo by another owner entirely... I am hesitant to reorganize this repo just for a new top-level crate that only indirectly has anything to do with the main library.

You can create a new repo for it if you like! Or if you would rather have someone else maintain it, I can.

- Set the background color to black (matches the border color used by `pixels`)
- Remove the margin and scroll bars
- Copy index.html to the target directory
- Do not produce typescript output
- Add a recipe to clean the target directory
- The `PixelsBuilder::build_async` recommendation was accidentally added
  to the wrong constructor.
- Examples should reference `wgpu` through the reexport.
- Minor simplifications like `use` and removing unnecessary `let`
  bindings.
- Normalize all hidden `SurfaceTexture`s in examples to use the same size
  as the pixel buffer. This is just my OCD showing. Nothing to see here.
@MarkAnthonyM
Copy link
Contributor

Yea, I'll throw the crate together as a separate repo and maintain it. At least until winit figures out how exactly it wants to handle the situation related to the resizing boilerplate. That leaves me wondering about main again though...

Anyway, I'll send another PR to update the example when I get everything going

@parasyte
Copy link
Owner Author

parasyte commented Nov 4, 2021

I read in one of the winit issues that it will also need a similar workaround to receive mouse and touch events. pixels has a few methods for converting between window coordinates and pixel buffer coordinates, which is intended to be used with pointer position events. The Conway example uses it for demonstration purposes. That would be a nice bonus for the crate to support.

And I’m sure it could grow into something of a stop-gap for several web events that winit doesn’t support yet.

- Enables all `wgpu` backends on WASM targets by default
@parasyte
Copy link
Owner Author

I think this is pretty much done from the Pixels API standpoint. The rest of the work here is making the minimal-web example smaller by abstracting some of the special sauce into a new crate, as discussed above. But I don't think that should block us. This PR will enable pixels on the web and is plenty enough reason for a new release ASAP.

Any thoughts from collaborators?

- This one looks quite a bit different with the CSS and resize working!
@parasyte
Copy link
Owner Author

Since no one has responded over the weekend, I'll take that as good-to-go. 🙂

@parasyte parasyte merged commit b185ec3 into master Nov 16, 2021
@parasyte parasyte deleted the webgl2-support branch November 16, 2021 19:37
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

Successfully merging this pull request may close these issues.

WASM support
3 participants