Don't clear canvas by resetting width and height

[jwmerrill]

This issue is part of trying to make animations run more smoothly in Firefox.

Currently, on each draw of a collage, the following is executed:

https://github.com/elm-lang/core/blob/9687580e6f7b012cdee712c2f14471b272c85488/src/Native/Graphics/Collage.js#L324

function nextContext(transforms) {
    while (i < kids.length) {
        var node = kids[i];
        if (node.getContext) {
            node.width = w;
            node.height = h;
            node.style.width = w + 'px';
            node.style.height = h + 'px';
            ++i;
            return transform(transforms, node.getContext('2d'));
        }
        div.removeChild(node);
    }
    var canvas = makeCanvas(w,h);
    div.appendChild(canvas);
    // we have added a new node, so we must step our position
    ++i;
    return transform(transforms, canvas.getContext('2d'));
}

Resetting the canvas node's height and width serves two purposes:

1. Clearing the canvas
2. Resetting any transforms that were applied to the canvas on the previous frame.

Unfortunately, it appears that resetting the canvas this way also produces a canvas-sized amount of garbage on every frame in Firefox.

Clearing the canvas can also be accomplished by calling ctx.clearRect(0, 0, w, h). However, this does not reset the transforms. Transforms can be reset through disciplined use of ctx.save() and ctx.restore(), but there are currently saves that are never restored:

https://github.com/elm-lang/core/blob/9687580e6f7b012cdee712c2f14471b272c85488/src/Native/Graphics/Collage.js#L313

function transform(transforms, ctx) {
    ctx.translate(w/2, h/2);
    ctx.scale(1,-1);
    var len = transforms.length;
    for (var i = 0; i < len; ++i) {
        var m = transforms[i];
        ctx.save();
        ctx.transform(m[0], m[3], m[1], m[4], m[2], m[5]);
    }
    return ctx;
}

There are two problems here:

1. No one ever saves before or restores after the translate and scale in the first two lines of this function
2. When there are additional transforms passed, it appears that the save inside the for loop is not matched anywhere by a corresponding restore.

I think there is a potentially large win to be had by using transforms in a more disciplined way so that it is possible to avoid resetting the canvas width and height on every draw if they have not changed.

[evancz]

That is a great find! I can imagine that some saves were placed accidentally, or lived through a refactor that they should not have. Are you interested in taking a swing at putting them in the right places? Based on what I recall about the architecture of the rendering code, there should be distinct transitions between levels:

• step into a group
• step out of a group
• step out of everything to embed HTML
• step back into everything to pick back up with another canvas

I imagine all saves should be on the stepping in and all restores should be on the stepping out. Overall, this sounds like a very plausible improvement!

[jwmerrill]

This might be a little premature. I'm doing some experiments with different ways of clearing the canvas, and they don't totally bear out what I asserted above. I'm going to close this until I collect a little more evidence.

[jwmerrill]

Here's my inconclusive test: http://jsbin.com/vicawitefi/2/edit?js,output

In this version, you switch between strategies by commenting/uncommenting. I'm not seeing a huge difference between them.