Skip to content

WIP - wasm_bindgen backend for eventloop-2.0#845

Closed
spennydl wants to merge 14 commits intorust-windowing:masterfrom
spennydl:web-sys
Closed

WIP - wasm_bindgen backend for eventloop-2.0#845
spennydl wants to merge 14 commits intorust-windowing:masterfrom
spennydl:web-sys

Conversation

@spennydl
Copy link
Copy Markdown

@spennydl spennydl commented Apr 23, 2019 •
edited
Loading

wasm_bindgen support

This PR adds a wasm backend using wasm_bindgen and web-sys. This backend will be used when target_arch=wasm32 and the websys feature is enabled.

Implemenation status and todo

  • selectable backend
  • basic event loop and window creation
  • wrap a canvas
  • control flow
    • ControllFlow::Poll
    • ControlFlow::Wait
    • ControlFlow::WaitUntil
    • ControlFlow::Exit
  • install handlers for mouse and keyboard events - In Progress
  • install handlers for resize events - In Progress
  • milestone 1 event loop feature-complete
  • pointer capture/manipulation - rethinking this. i'm not sure it makes sense to support?
  • canvas placement/resize through window's getters and setters
  • be responsive - play nice with media queries etc that resize the canvas
  • Tested on all platforms changed - I'm testing using the wasm-pack-template and wasm-app project templates
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented
  • and more i'm sure! i'll be updating this list closer to milestone 1 being completed

Closes #395

@spennydl spennydl mentioned this pull request Apr 23, 2019
Closed
goddessfreya
goddessfreya suggested changes Apr 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@goddessfreya goddessfreya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: A lot of your files are missing a new line at the end.

@goddessfreya
Copy link
Copy Markdown
Contributor

goddessfreya commented Apr 25, 2019

Question: Do you intend to update the emscripten backend in an other PR, or are you not interested?

goddessfreya
goddessfreya reviewed Apr 25, 2019
View reviewed changes
src/platform_impl/mod.rs
@@ -18,9 +18,12 @@ mod platform;
#[cfg(target_os = "emscripten")]
Copy link
Copy Markdown
Contributor

@goddessfreya goddessfreya Apr 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Will emscripten conflict with websys with some targets?

Copy link
Copy Markdown
Author

@spennydl spennydl Apr 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't believe so, the target triple used for websys is wasm32-unknown-unknown. i would assume that if someone is building for wasm32-unknown-emscripten they wouldn't also have the websys feature set. i could add something like target_os = "unknown" to the cfg condition below (line21) to be safe

goddessfreya
goddessfreya reviewed Apr 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@goddessfreya goddessfreya left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question.

@goddessfreya goddessfreya added the DS - web Affects the Web backend (WebAssembly/WASM) label Apr 25, 2019
@elinorbgr elinorbgr mentioned this pull request May 17, 2019
Closed
@iceiix iceiix mentioned this pull request May 19, 2019
Open
@iceiix
Copy link
Copy Markdown

iceiix commented May 19, 2019

Is this PR missing some files or expected to not compile yet? Getting this error when trying to build, the CI systems are also failing with the same error:

error[E0463]: can't find crate for `wasm_bindgen`
 --> src/platform/websys.rs:1:1
  |
1 | extern crate wasm_bindgen;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

@iceiix iceiix mentioned this pull request May 19, 2019
Closed
@spennydl
Copy link
Copy Markdown
Author

spennydl commented May 19, 2019

Is this PR missing some files or expected to not compile yet? Getting this error when trying to build, the CI systems are also failing with the same error:

error[E0463]: can't find crate for `wasm_bindgen`
 --> src/platform/websys.rs:1:1
  |
1 | extern crate wasm_bindgen;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

This backend needs to be built for the wasm32-unkown-unkown target, and it looks like none of the jobs are currently building for that. The build command we need is cargo build --target wasm32-unkown-unknown --features="websys".

I'll be pushing some changes tomorrow sometime, i can update the circleci config to include it.

@ryanisaacg ryanisaacg mentioned this pull request Jun 6, 2019
Closed
31 tasks
@blm768
Copy link
Copy Markdown
Contributor

blm768 commented Jun 6, 2019

So, I had no idea this PR existed, and I started hacking on the stdweb support PR (#797) to add web-sys support. So far, it seems like there will end up being a fair bit of duplication between the two implementations because they need to interface with the same Web APIs, so it might be worthwhile to consider whether all the WebAssembly efforts thus far could be merged into something more coherent.

@spennydl
Copy link
Copy Markdown
Author

spennydl commented Jun 9, 2019

So, I had no idea this PR existed, and I started hacking on the stdweb support PR (#797) to add web-sys support. So far, it seems like there will end up being a fair bit of duplication between the two implementations because they need to interface with the same Web APIs, so it might be worthwhile to consider whether all the WebAssembly efforts thus far could be merged into something more coherent.

I agree! In part I feel like that reflects the state of webassembly right now 😝 I'd be interested to hear @ryanisaacg's opinion given he's so far into the stdweb work, as well as from the project owners. Maybe open an issue?

@spennydl spennydl mentioned this pull request Jun 9, 2019
Closed
@ryanisaacg
Copy link
Copy Markdown
Contributor

ryanisaacg commented Jun 11, 2019

I think the stdweb version will be pretty much feature-complete within the month. At this point it can already run a full application, given you're not using one of the things I haven't implemented yet.

I think it probably makes the most sense to finish the stdweb version and then add web-sys alongside, possibly inline with cfg declarations to switch between them. This means that changes can be easily applied to both web backends at once. However, I don't know much about idioms in web-sys, and it may be difficult to exactly mirror the structure of the stdweb code.

@Osspial Osspial changed the base branch from eventloop-2.0 to master June 13, 2019 05:29
@goddessfreya goddessfreya mentioned this pull request Jun 17, 2019
Open
@goddessfreya
Copy link
Copy Markdown
Contributor

goddessfreya commented Jul 10, 2019

Hey, we got a new web branch for all this web stuff! Closing. If there is any features in this PR not present in the new branch, file us a new PR. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@goddessfreya goddessfreya goddessfreya requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

DS - web Affects the Web backend (WebAssembly/WASM)

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support wasm32-unknown-unknown backend?

5 participants

@spennydl @goddessfreya @iceiix @blm768 @ryanisaacg