Move Canvas API funtion bindings into a CanvasContext object

[JoshMarler]

@nick-thompson, I've started messing with adding more of the Canvas API. Thought I'd drop this PR over even if just as a discussion starter. Tried to encapsulate all the registering of native functions and properties into a little CanvasContext class so it's easily used from CavasView::paint().

It would be nice to add a few little drawing examples to the README maybe. The README could list each of the CanvasRenderingContext2D API functions currently supported and we could grow the list incrementally.

In this PR the following CanvasRenderingContext2D props and functions have been added:

. strokeStyle
. fillStyle
. lineWidth

. rect()
. fillRect()
. strokeRect()

. beginPath()
. lineTo()
. moveTo()
. arc()
. quadraticCurveTo()
. closePath()

. stroke()
. fill()

. rotate()
. translate()
. setTransform()

If you're happy with the shape of this I thought I could drop another PR over next which has the following:

. fillText()
. strokeText()
. font

. drawImage() - drawImage will probably just support an svg string initially.

I think with this set of API calls there is a nice starter set of functions for drawing. Perhaps nice to add gradients and stroke styles after that.

[JoshMarler]

One interesting idea around Canvas API support is augmenting the standard CanvasRenderingContext2D API with some of the additional functions in juce such as drawRoundedRect (not part of the html Canvas API).

It would be nice to offer some additional drawing functions familiar to juce users without breaking the CanvasRenderingContext2D API and potential web based UIs. There could either be a library of equivalent functions in js or function bindings in an "extras" module.

js functions might look like:

drawRoundedRect(ctx, x, y, width, height, cornerSize, lineThickness)

[JoshMarler]

Dropping comment here whilst I think about this.

Its actually probably not necessary to register methods and properties in every paint call. The CanvasContext class could simply have a pointer to a juce::Graphics instance as a data member.

Then in CanvasView::paint we just call CanvasContext::draw(&g) which reassigns the pointer used in each of the native methods and callbacks and invokes onDraw etc. That could get rid of the overhead of registering methods in every single call to paint(). I'll experiment with it later when I return to my desk.

Its probably better to make CanvasContext public inside the blueprint namespace as I'm pretty sure it can be reused in the look and feel override approach we've already discussed with the juce component wrapper idea.

EDIT: Addressed in c1526b3

[nick-thompson]

Woah, @JoshMarler you're amazing! Just seeing all these notifications now, I'll try to get through as much as I can this morning

[JoshMarler]

@nick-thompson, No problem! there's no rush. Apologies if I've spammed you a little bit! I blame the lock-down and having little else to do!

[JoshMarler]

@nick-thompson just sitting down to fixup the transform stuff and give this all a more thorough testing. I think having a transform member in CanvasContext will make the most sense based on the expected behaviour of the CanvasRenderingContext2D API. Be cool to hear your thoughts on the overall shape of this in the meantime. Still a slight work in progress. I'll aim to get this existing API calls all working flawlessly and won't add any further elements of the CanvasRenderingContext2D API until this PR goes in.

[nick-thompson]

@JoshMarler overall, love this! Inheriting the DynamicObject is such a good idea, I never thought of it. Also you covered so much of the API already 💥

I have two kind of high level thoughts– one of them is inline about the transform stuff, the other is: instead of receiving a pointer to a graphics context during paint, and a View ref at construction, what if the CanvasContext owned a juce::Image and all of the drawing routines happened on a juce::Graphics g(memberImage) pointing at that image? One on hand, that makes me feel a little better about lifetime stuff (referencing a raw pointer always makes me want to jassert everywhere and stuff), and on another I think that might leave room for some like easy draw routine caching. Then the CanvasView's paint() routine could basically be:

if (props["onDraw"] && props["onDraw"].isMethod()) {
    std::invoke(props["onDraw"].getNativeFunction(), args);
}
g.drawImageAt(canvasCtx->getImage(), 0, 0);

I think we would want to resize that image during CanvasView's resize() so that it always matches perfectly.

Besides that, I think instead of taking a View& at construction I'd rather implement a method on the CanvasContext that creates the js args for you. This way we avoid a circular dependency and the CanvasContext isn't necessarily coupled to a particular View as well. I could imagine somethign like this:

// This is CanvasView's `paint()` method
if (props["onDraw"] && props["onDraw"].isMethod()) {
    std::invoke(props["onDraw"].getNativeFunction(), canvasCtx->getNativeFunctionArgs());
}

What do you think? Besides those two ideas, I think this is basically ready to merge, and then we can tackle other paint methods in another PR as you suggested

[JoshMarler]

@JoshMarler overall, love this! Inheriting the DynamicObject is such a good idea, I never thought of it. Also you covered so much of the API already

I have two kind of high level thoughts– one of them is inline about the transform stuff, the other is: instead of receiving a pointer to a graphics context during paint, and a View ref at construction, what if the CanvasContext owned a juce::Image and all of the drawing routines happened on a juce::Graphics g(memberImage) pointing at that image? One on hand, that makes me feel a little better about lifetime stuff (referencing a raw pointer always makes me want to jassert everywhere and stuff), and on another I think that might leave room for some like easy draw routine caching. Then the CanvasView's paint() routine could basically be:

if (props["onDraw"] && props["onDraw"].isMethod()) {
    std::invoke(props["onDraw"].getNativeFunction(), args);
}
g.drawImageAt(canvasCtx->getImage(), 0, 0);

I think we would want to resize that image during CanvasView's resize() so that it always matches perfectly.

Besides that, I think instead of taking a View& at construction I'd rather implement a method on the CanvasContext that creates the js args for you. This way we avoid a circular dependency and the CanvasContext isn't necessarily coupled to a particular View as well. I could imagine somethign like this:

// This is CanvasView's `paint()` method
if (props["onDraw"] && props["onDraw"].isMethod()) {
    std::invoke(props["onDraw"].getNativeFunction(), canvasCtx->getNativeFunctionArgs());
}

What do you think? Besides those two ideas, I think this is basically ready to merge, and then we can tackle other paint methods in another PR as you suggested

@nick-thompson Ohh man yeah. I hadn't even thought about using an image. That could open up a really nice API for the look and feel override thing. Being able to cache image states/UI snapshots etc. could open up all kinds of stuff. You clearly know the juce Graphics API better than me!

I'll have a go at this now.

And yeah awesome, when your happy and this is all tidied up maybe I can implement those couple of other functions we discussed and get together another README page for the Canvas View.

[JoshMarler]

@nick-thompson. Yeah nice the transform stack does the trick.

[JoshMarler]

@nick-thompson, woop woop. Drawing to an image all working nicely. Will push up shortly. If it's cool with you I'd like to just give this whole PR a bit of a battle test. I'll try drawing various things just to make sure I haven't introduced any odd behaviour.

EDIT: Curses. It's broken transform behaviour. Back to the drawing board for a moment.

Think I have an idea on how to fix. Will deal with this in the morning.

[JoshMarler]

@nick-thompson, OK. So I have this all working with the transform stack and drawing to an Image in CanvasContext. However, resizing the image held by the CanvasContext isn't 100% straight forward.

We need to store a juce::Graphics instance in CanvasContext to preserve progressive transform state changes etc. (As per the current implementation). However, we cannot directly resize the image held by CanvasContext's Graphics instance, so instead we need to replace/reconstruct the Graphics instance when we resize.

I can push the current working state up but basically in CanvasView::resize() we call the following CanvasContext::setSize() function:

        void CanvasContext::setSize(int width, int height)
        {
            image = juce::Image(juce::Image::ARGB, width, height, true);
            graphics = std::make_unique<juce::Graphics>(image);
        }

        void CanvasView::resized() override
        {
            ctx->setSize(getWidth(), getHeight());
        }

        void CanvasView::paint (juce::Graphics& g) override
        {
            View::paint(g);

            jassert(ctx);

            if (props.contains("onDraw") && props["onDraw"].isMethod())
            {
                std::vector<juce::var> jsArgs {{ctx.get()}};
                juce::var::NativeFunctionArgs nfargs (juce::var(), jsArgs.data(), jsArgs.size());

                std::invoke(props["onDraw"].getNativeFunction(), nfargs);
            }

            g.drawImageAt(ctx->getImage(), 0, 0);
        }       

This all seems to be working fine and I thiiink this is OK but do you feel there is a more elegant way to do this? Drawing to an Image is really powerful and perfectly fits in with my look and feel requirement so I'd like to take that approach as you've suggested.

I haven't thought about any clever paint/draw routine caching as you mentioned but I think that's something you're probably better suited to address. Very interested in your ideas around that though!

If you're happy with the above then this is pretty much ready to go I think. I'll document the CanvasContext class and give it all a bit of battle testing and then squash ready for merge.

Hopefully after I can drop you a README idea for CanvasView and CanvasContext.

[nick-thompson]

@nick-thompson, OK. So I have this all working with the transform stack and drawing to an Image in CanvasContext. However, resizing the image held by the CanvasContext isn't 100% straight forward.

We need to store a juce::Graphics instance in CanvasContext to preserve progressive transform state changes etc. (As per the current implementation). However, we cannot directly resize the image held by CanvasContext's Graphics instance, so instead we need to replace/reconstruct the Graphics instance when we resize.

I can push the current working state up but basically in CanvasView::resize() we call the following CanvasContext::setSize() function:

Ahhh right right, of course. Yea, I think your idea is great. The only thing I can think of is that if we're hanging onto the Graphics instance as a member variable, we'll probably want to make sure to reset the graphics context (https://docs.juce.com/master/classGraphics.html#ab8b7cd49bf1ff738c5ff848727e3bc75) at the beginning of CanvasView::paint(), otherwise context state changes (such as transformations) will accumulate with each successive call to onDraw

Besides that, yea, happy to go ahead with this plan, then squash and merge! And I'd love your help with the README afterwards. Thanks @JoshMarler

[JoshMarler]

Ahh yeah, good shout on the reset. Not clear if resetToDefaultState clears transforms but failing that it's easy enough to just add a reset function to CanvasContext which resets the Graphics unique_ptr. Awesome. Will finish this all off first thing tomorrow

[JoshMarler]

@nick-thompson. One other question I've ignored until now before we merge this.

Juce has slightly different path behaviour to the Canvas if we take a circle drawing example:

https://www.w3schools.com/html/tryit.asp?filename=tryhtml5_canvas_tut_path2

In the above example there is no need for a call to moveTo() before the call to arc(). Juce on the other hand will stroke the path from 0,0 to the start of the arc position.

I.e the following code doesn't product the same thing as the linked browser example:

  const radius = 40;
  ctx.beginPath();
  ctx.arc(100, 50, radius, 0, 2 * Math.PI);
  ctx.stroke();

The above produces this:

To render the desired shape we need to change the drawing code to use a moveTo()

  const radius = 40;
  ctx.beginPath();
  ctx.moveTo(100 + radius, 50);
  ctx.arc(100, 50, radius, 0, 2 * Math.PI);
  ctx.stroke();

Should this be considered an issue? We could add a startNewSubPath() call to the C++ implementation of things like arc() but part of me thinks this is undesirable and users will just have to be aware of the need to call moveTo(). Calling moveTo() does not break the desired behaviour/output in a browser so the code is cross-compatible (Think I've probably answered my own question there).

            setProperty("arc", juce::var::NativeFunction {
                    [=](const juce::var::NativeFunctionArgs& args) -> juce::var {
                        jassert(args.numArguments >= 5 && args.numArguments <= 6);

                        const float x             = args.arguments[0];
                        const float y             = args.arguments[1];
                        const float radius        = args.arguments[2];
                        const float startAngle    = args.arguments[3];
                        const float endAngle      = args.arguments[4];
                        //TODO; Handle antiClockWise
                        //bool        antiClockWise = args.arguments[5];

                        const float width  = radius * 2;
                        const float height = radius * 2;

                        path.startNewSubPath(x, y);
                        path.addArc(x, y, width, height, startAngle, endAngle);

                        return juce::var();
                    }
            });

[JoshMarler]

@nick-thompson, I've squashed this all down to a single commit and added some initial comments/documentation. If you're happy with the shape of everything then I think this should be good to merge now.

[nick-thompson]

@nick-thompson, I've squashed this all down to a single commit and added some initial comments/documentation. If you're happy with the shape of everything then I think this should be good to merge now.

Awesome @JoshMarler this looks great! Two more small asks from me in there– the getProperty method and the jassertfalse becoming a warning, otherwise I'm ready to merge :)

To your point above about the moveTo, yea, that's tricky. I think you're right that it makes the most sense for users to just be aware of the need to call this moveTo here. Especially as you've put it here:

Calling moveTo() does not break the desired behaviour/output in a browser so the code is cross-compatible

I think that definitely answers the question. Maybe the takeaway here is that as we build out the documentation for CanvasView we should have a "things you should know" section or something where we can highlight little details like this

[JoshMarler]

@nick-thompson, final two points actioned. Squashed changes again so good to go.

[nick-thompson]

Merged! This is super cool, thanks for all your help Josh. As we continue building out the remaining API, I'd love to get a cool CanvasExample project going to show this feature off too :)

[JoshMarler]

@nick-thompson, Yeah a canvas example project would be awesome. Totally up for assisting with that once some more of the API is implemented and DOCS added.

I have a little TODO list compiling. The below are my next tasks but I'll probably complete these after I've finished the HotReload refactor.

. Get fillText() strokeText(), font and textAlign working.
. Get drawImage() working with string.
. Get gradients and stroke patterns (i.e. line - dash) working.