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

Proof of concept druid running on web #30

Closed
wants to merge 6 commits into from
Closed

Conversation

tedsta
Copy link

@tedsta tedsta commented Mar 6, 2019

Hey Raph long time no chat!

I've been wanting to do some hobby web tools and since somebody already did the heavy lifting getting piet working on wasm I figured I'd give this a shot.

Is web something you are interested in targeting with Druid?

image

@raphlinus
Copy link
Contributor

Hi @tedsta! Indeed, it's been a while.

Yes, targeting web is definitely something I'm interested in. I wanted to respond quickly before doing a full review. There is some foundational work in druid remaining that will affect ports, so it's a project management question whether we should limit the number of ports while that's churning, or bring all ports along. There's similarly a Linux port (using a gtk dependency) where we need to make some similar decisions.

I hope to review this in more detail soon; this week is challenging for finding chunks of time.

@tedsta
Copy link
Author

tedsta commented Mar 7, 2019

Take your time - I should emphasize the proof-of-concept-ness of this PR a bit more. Also note this is the first time I've even looked at wasm and web_sys stuff.

I switched over to the calculator example and while it renders ok it doesn't react to button presses properly, so I'll be looking at that next.

I think it could be helpful to have the web port in the churn so that assumptions aren't locked in that aren't valid on web. But I understand if it's too much - I could go play with the linux port instead.

Current state (questions are just things I'm asking myself):

  • Everything except rendering and mouse events is stubbed out
  • Relies on the DOM already having a canvas element named "canvas". Should we just create a canvas in the body? Assume the body is empty at start?
  • There is no main loop, just set up the callbacks. The rust entry point actually exits. This makes the "idle queue" a bit problematic... my code assumes that idle callbacks will only be queued after some kind of UI event, but I that isn't valid when external things like network messages can update the GUI.

I need to do some learning here - I don't know when callbacks I register with web_sys are called. I assumed they are called on the main thread, and so if I hog the thread with a main loop, they would never be called? (haven't actually tested) Or maybe the callbacks will preempt my main loop?

  • std::time doesn't work on wasm32-unknown-unknown, so I made a little Timer object with an implementation for wasm32 and another for everyone else.

  • How do you exit the web app? Close the tab? Do web APIs give a way to do that?

@@ -166,7 +167,7 @@ fn setup_web_callbacks(window_state: &Rc<WindowState>) {

let event = MouseEvent {
x: event.offset_x() as i32,
y: event.offset_x() as i32,
Copy link
Author

@tedsta tedsta Mar 8, 2019

Choose a reason for hiding this comment

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

.... figured out why half of the calc example buttons didn't work... 🙄

@msand
Copy link

msand commented Mar 8, 2019

Hello! This seems like an interesting project. I'm thinking you probably want to use request_animation_frame for the main loop: https://github.com/rustwasm/wasm-bindgen/blob/fcee4656923a1128d0a2faf781242ee2d7835db4/examples/request-animation-frame/src/lib.rs#L10-L14

https://rustwasm.github.io/wasm-bindgen/api/web_sys/struct.Window.html#method.request_animation_frame
This should allow the js event loop to function performantly and not hog the main ui thread.

@tedsta
Copy link
Author

tedsta commented Mar 10, 2019

Thanks for the tip @msand, idle callbacks now use request_animation_frame(_)

@msand
Copy link

msand commented Mar 10, 2019

@tedsta You're welcome 👍Thanks for working on this! I just realized, if it's actually idle you want rather than high fps render loop, then you might want to change to requestIdleCallback instead: https://rustwasm.github.io/wasm-bindgen/api/web_sys/struct.Window.html#method.request_idle_callback

@raphlinus raphlinus mentioned this pull request Aug 24, 2019
@jjl
Copy link
Contributor

jjl commented Aug 24, 2019

This rebases cleanly against master and works (more or less).

I've pushed some changes to my web-hidpi branch which is a continuation of your web branch:

  • The DPI calculation caused 1/3 of the content in each dimension to be clipped, this is rectified
  • Mouse positions are now translated into physical positions instead of css positions
  • Added window resizing

@jjl
Copy link
Contributor

jjl commented Aug 25, 2019

I have been thinking about next steps.

At the minute the event loop is handled through the window. I would like to move this to the run loop but it's not immediately clear what the correct way of doing this is, particularly with regard to supporting other platforms - where the cross platform api stops and the per-platform begins is not entirely clear to me at present.I am thinking to copy what is done in windows for now, but will probably wait to see what happens for multiple window support on other platforms before looking for an alternative solution.

Speaking of windows... We should decide what constitutes a window on the web backend. My current thoughts are that yes, it should be a canvas but no, it should not necessarily be full screen. Perhaps full screen can be a mode opted into in the same way one might for other platforms.

This leaves open a few questions:

  • Should we map windows to canvas elements such that when multiple windows are supported on other platforms, it maps multiple canvas elements rendered in one document? (my view: yes)
  • Should these be optionally individually resizable? (i think so, because we'd want that on other platforms)
  • Should creating and closing a window map to creating and destroying a canvas? (seems logical)
  • What are we to do about ownership of windows and the run loop? (???)

@jjl
Copy link
Contributor

jjl commented Aug 25, 2019

Seems I may have spoken too soon, my master was out of date and the muggle merge has broken it. Perhaps going through it will answer some of my other questions

@jjl
Copy link
Contributor

jjl commented Sep 4, 2019

I seem to have everything except Text working in my local copy - a shame we need text everywhere for the calculator.

It's a bit awkward trying to infer what the web structure should be from the windows version, but I've gotten most of it down, I think. I'm currently just working out if i can get a mutable canvas context ref to the WinCtxOwner without requesting the context afresh.

elrnv added a commit to elrnv/druid that referenced this pull request Mar 26, 2020
A basic wasm backend is added as a continuation of linebender#30.
Since std::time isn't available on wasm, an additional dependency is
added to abstract std::time::Instant to work with wasm.
elrnv added a commit to elrnv/druid that referenced this pull request Mar 26, 2020
A basic wasm backend is added as a continuation of linebender#30.
Since std::time isn't available on wasm, an additional dependency is
added to abstract std::time::Instant to work with wasm.
@elrnv elrnv mentioned this pull request Mar 26, 2020
elrnv added a commit to elrnv/druid that referenced this pull request Apr 3, 2020
A basic wasm backend is added as a continuation of linebender#30.
Since std::time isn't available on wasm, an additional dependency is
added to abstract std::time::Instant to work with wasm.
elrnv added a commit to elrnv/druid that referenced this pull request Apr 10, 2020
A basic wasm backend is added as a continuation of linebender#30.
Since std::time isn't available on wasm, an additional dependency is
added to abstract std::time::Instant to work with wasm.
elrnv added a commit to elrnv/druid that referenced this pull request Apr 12, 2020
A basic wasm backend is added as a continuation of linebender#30.
Since std::time isn't available on wasm, an additional dependency is
added to abstract std::time::Instant to work with wasm.
luleyleo added a commit that referenced this pull request Apr 15, 2020
* Add wasm backend + fix std::time dep

A basic wasm backend is added as a continuation of #30.
Since std::time isn't available on wasm, an additional dependency is
added to abstract std::time::Instant to work with wasm.

* Disable simple logger on wasm

This commit ensures that all examples run with minimal changes.
More specifically we don't need to remove the .use_simple_logger() call
on all Apps.

* Fix resizing in wasm backends

The resize callback should be registered to the window, since resizing
the window doesn't call the resize callback for canvases even if those
do get resized.

* Added scrolling support + fixed hidpi handling

The scroll example now works as expected on hi and low dpi screens.

* Use explicit lifetime in StrOrChar

This makes passing it easier to pass owned strings when creating a new
KeyEvent, which is necessary when interfacing with WASM.

* Prevent the browser from going back on backspace

This should be the default behavior to make text widgets usable.
At a later iteration it may be better to only disable the browser from
going back when a widget is selected that is expecting keyboard input
where backspace is intended for something else.

I expect this functionality would require some way of accessing the
web_sys event from within a druid callback.

* Fix key text + require console_log + adjust style

This commit addresses comments in the code review:

Key text:
  - web_sys returns the name of each pressed key from which we can
    generate a viable printable string. This mechanism is revised to
    include all non-printable keys as listed in MDN.
  - notably, the tab character and numpad characters have gained
    printable text in this commit.

console_log:
  - console_log is not required in wasm builds.
  - integrated console_log with the use_simple_logger api. `console_log`
    is initialized with log level `trace` as is done in simple logger by
    default.

Style:
  - non std use statements are now before third party use statements.
  - key_to_text function signature refactor
  - specify the log module when calling log::{warn, error} explicitly.

* Fix up console_log dependency

Moved console_log dependency from druid-shell to druid, since this is
where it is initialized. This fixes the wasm build.

* Fix the build script for wasm examples

This addresses the CI failures caused by the weird build of the wasm
examples.

To elaborate:
In order to include each of the examples in the wasm example, a
symlink to the examples directory is created in build.rs along with the
corresponding examples.rs module which declares examples known to work
with wasm. To satisfy CI, the examples.rs committed to the repo must be
empty since rustfmt does not call "build.rs".

* Add .gitignore to wasm examples + fix clippy bugs

The included .gitignore ignores the generated examples module. The
examples.rs should remain committed, but it need not be ever changed.

* Fix x11 keycodes StrOrChar conversion

The newly introduced lifetime parameter must be specified exmplicitly.


* Add --no-run to `cargo test` for wasm targets

wasm targets cannot be run in the normal way.

* Do not build the unit test module for wasm32

The tests cannot be run in the normal way anyways. Leaving this for a
future PR to flush out.

* Install necessary deps in wasm CI for macos/ubuntu

* Add warnings for unimplemented file ops in web backend

This commit exposes the error of the missing implementation for file dialogs in the web backend using log::warn. This is a temporary solution until the `open_file_sync` and `save_as_sync` functions propagate the error downstream using Result instead of Option.

* Rework the generated examples in the wasm example

This commit attempts a different approach at including the automatically
generated examples from the parent directory.
This method satisfies both rustfmt and cargo test as before, but it also
doesn't modify any files in the source tree, keeping the diff clean
after a build.

* Rename switch example js entry point for wasm build

This commit fixes the generated html file for the switch example to load
"switch_demo" instead of "switch" for the switch example. The word
switch conflicts with JavaScript's switch statement, which is the reason
for this awkwardness.

Co-authored-by: Leopold Luley <git@leopoldluley.de>
@xStrom
Copy link
Member

xStrom commented Apr 15, 2020

Now that #759 has landed, is there still a reason to keep this PR open?

@raphlinus
Copy link
Contributor

Nope, this was the basis of that, so safe to close. Thanks for the heads-up!

@raphlinus raphlinus closed this Apr 15, 2020
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.

None yet

5 participants