Add Canvas API #80

Diggsey · 2018-01-14T19:46:18Z

No description provided.

koute · 2018-01-16T22:09:49Z

src/webapi/event.rs

+///
+/// MDN incorrectly documents this as a UIEvent, but in browsers it is actually
+/// just an Event.
+/// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/Events/resize)


Did you check what the specs say?

It was changed in the specs recently from a UIEvent to an Event to match expectations/browser implementations. Either way, this errors in all browsers if it's specified as a UIEvent rather than an Event.

koute · 2018-01-16T22:12:52Z

src/webapi/html_elements/canvas.rs

+        js! (
+            return @{self}.getContext(@{T::CONTEXT_TYPE});
+        ).try_into().ok().and_then(|r| T::from_reference(r))
+    }


I think this might not be flexible enough. E.g. for WebGL at some point in time in various browsers you could use one of the following names for what effectively was the same kind of a context: "webgl", "experimental-webgl", "webkit-3d", "moz-webgl", so this mechanism should support specifying a list. (Or perhaps we should just have the RenderingContext export a get_context method and just delegate calling getContext to it completely?)

OK, I will give RenderingContext a from_canvas method or something...

koute · 2018-01-16T22:13:45Z

src/webapi/html_elements/canvas.rs

+    /// cached on the disk or stored in memory at the discretion of the user agent.
+    ///
+    /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/toBlob)
+    pub fn to_blob<F: FnMut(Blob) + 'static>( &self, f: F, mime_type: Option<&str>, quality: Option<f64> ) {


FnOnce (There is a wrapper called Once which allows passing FnOnce functions to js!)

Also, semi related question - do you know why the original JS API takes a callback here instead of returning a Blob? Is it because the Blob is done asynchronously?

koute · 2018-01-16T22:20:08Z

src/webcore/macros.rs

@@ -654,6 +661,7 @@ macro_rules! reference_boilerplate {
    };
 }

+#[macro_export]


Hmm... I assume you're exporting these to allow the context to be implemented externally? Hmmm.... Could we keep those private? I would actually be fine having the RenderingContexts implementations inside stdweb for now. (They're JS APIs after all!)

Hm, OK, but I think these macros should be exposed anyway - it's not uncommon to want to wrap new types of references.

I'm not against exposing something like this macro in the future, but I'd rather keep it private for now. Unlike the js! macro this macro wasn't designed to be used outside of stdweb, so I will likely want to export something slightly different to the outside.

koute · 2018-01-16T22:20:53Z

src/webapi/html_elements/canvas.rs

+use webapi::blob::Blob;
+
+/// The HTML image element is used to manipulate the layout and presentation of
+/// `<img>` elements.


This doc comment is wrong. (:

Diggsey · 2018-01-18T00:40:45Z

@koute is this good to go?

koute · 2018-01-18T16:37:36Z

Right, sorry!

It's almost ready to go! Let me just put a few more comments.

koute · 2018-01-18T16:40:22Z

examples/canvas/src/main.rs

+}
+
+#[derive(Clone, Debug)]
+struct CanvasRenderingContext2D(Reference);


As I wrote in one of the comments, I would be fine with having the RenderingContexts implementations inside of stdweb, and I think I'd actually prefer that right now, so please move this from the example to inside the stdweb.

Also, please rename the 2D to 2d to keep with the PascalCase naming convention we've decided the other day to stick to.

Hm, OK I will move it in - I'm not planning on expanding on the set of implemented methods though.

I do think these would work better as a separate crate: have you thought about splitting up stdweb?

eg.

stdweb-core (contains just webcore)

stdweb-common (contains bindings to core js APIs, not specific to environment)

stdweb-browser (browser-specific bindings)

stdweb-node (node-specific bindings)

Adding everything into stdweb will increase compile times quite a bit, especially if you add the whole of canvas 2d, webgl and webgl 2

Yeah, it's fine not adding every method there is right now.

Yes, I did think about splitting it up in the future! The first thing I'd like to split up is webapi and webcore directories, and then we'll go from there and see if/what would make sense to further split up. However, I don't want to do that just yet.

As far as the compile times go there is probably some room for improvement too without splitting it up. (E.g. once we make the js! macro into a procedural macro)

koute · 2018-01-18T16:40:51Z

src/lib.rs

@@ -188,12 +190,16 @@ pub mod web {
            KeyupEvent,
            ClickEvent,
            DoubleClickEvent,
+            MouseDownEvent,
+            MouseUpEvent,
+            MouseMoveEvent,


These were already exported in another PR, so please get rid of them.

koute · 2018-01-18T16:42:54Z

src/webapi/html_elements/canvas.rs

+/// Trait implemented by rendering contexts which can be obtained from a canvas.
+pub trait RenderingContext {
+    /// Name which identifies this kind of rendering context.
+    fn from_canvas(canvas: &CanvasElement) -> Option<Self> where Self: Sized;


I think it would make more sense for this to return Result<Self, Self::Error>.

Diggsey · 2018-01-20T17:39:38Z

@koute updated.

koute · 2018-01-20T21:57:37Z

Looks good to me except you forgot to change 2D into 2d. (:

But to not delay merging it I'll just search & replace it myself.

Thanks a lot for the PR!

Diggsey · 2018-01-20T23:16:23Z

Oops, the comment got lost somehow. Thanks!

Boscop · 2018-03-29T14:53:05Z

It only support fill_rect right now, right?

Diggsey · 2018-03-29T15:10:55Z

@Boscop yeah, it was just laying the ground-work, no reason not to add more though!

Boscop · 2018-05-04T07:59:03Z

Can you please add clearRect and fillText, too? Then I can port my current polymer app to Rust :)
I could do clearRect myself but how to pass the string to js for fillText?
I'm interested in contributing to expand the canvas API coverage.

koute requested changes Jan 16, 2018

View reviewed changes

koute requested changes Jan 18, 2018

View reviewed changes

Diggsey added 2 commits January 20, 2018 16:45

Add Canvas API

c7bfe09

Review changes

1d6c38f

Diggsey force-pushed the canvas branch from 58c5423 to 1d6c38f Compare January 20, 2018 17:38

koute merged commit 039efb4 into koute:master Jan 20, 2018

keeslinp mentioned this pull request Mar 28, 2018

Canvas support? yewstack/yew#78

Closed

Diggsey deleted the canvas branch March 29, 2018 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Canvas API #80

Add Canvas API #80

Diggsey commented Jan 14, 2018

koute Jan 16, 2018

Diggsey Jan 16, 2018

koute Jan 16, 2018

Diggsey Jan 16, 2018

koute Jan 16, 2018

koute Jan 16, 2018

Diggsey Jan 16, 2018

koute Jan 16, 2018

Diggsey Jan 16, 2018

koute Jan 17, 2018

koute Jan 16, 2018

Diggsey commented Jan 18, 2018

koute commented Jan 18, 2018

koute Jan 18, 2018

Diggsey Jan 18, 2018

koute Jan 18, 2018

koute Jan 18, 2018

Diggsey Jan 18, 2018

koute Jan 18, 2018

Diggsey Jan 18, 2018

Diggsey commented Jan 20, 2018

koute commented Jan 20, 2018

Diggsey commented Jan 20, 2018

Boscop commented Mar 29, 2018

Diggsey commented Mar 29, 2018

Boscop commented May 4, 2018 •

edited

Add Canvas API #80

Add Canvas API #80

Conversation

Diggsey commented Jan 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Diggsey commented Jan 18, 2018

koute commented Jan 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Diggsey commented Jan 20, 2018

koute commented Jan 20, 2018

Diggsey commented Jan 20, 2018

Boscop commented Mar 29, 2018

Diggsey commented Mar 29, 2018

Boscop commented May 4, 2018 • edited

Boscop commented May 4, 2018 •

edited