1923636 - Slightly modified Codepen demo is 10x slower with GPU-canvas/D2d-canvas compared to Chrome. (Time spent in PathOps and memcpy)

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Mayank Bansal]

Open attached testcase (Original at https://codepen.io/soulwire/pen/nBNpQG)

Chrome: https://share.firefox.dev/3NgGJsa (3.4s)
Skia-canvas: https://share.firefox.dev/3ZSMBPZ (3.4s)
Nightly gpu-canvas: https://share.firefox.dev/3zS80ht (37s)
d2d-canvas: https://share.firefox.dev/4gRCLUB (34s)

Regressoin rnge
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1634cff61cb0e270b9660ebdc234cfcf3fed2c68&tochange=b94f3d418f54f964368486a65cd5d15e828867db
Suspect: bug 1839470

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[Mayank Bansal]

Attachment: Standalone_testcase.html

Comment 3

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1839470

:jrmuizel, since you are the author of the regressor, bug 1839470, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Erich Gubler [:ErichDonGubler] (he/him)]

Tentatively triaging as S2, but escalating to gfx-triage for confirmation.

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: Bug 1923636 - Avoid copying paths when transforming. r?aosmond

TransformedCopyToBuilder is called any time in Canvas2D the transform
is modified and then a path op is used. On cases that repeatedly change
the transform while building paths, this leads to fairly significant
expense in repeatedly copying the path contents as well as allocating
and deallocating paths.

To work around this, a new primitive TransformedMoveToBuilder is provided
that allows in-place transforming of the path contents. Because paths that
are "finished" and referenced externally can't be safely modified, extra
static methods (i.e. ToBuilder) are added to guarantee that there are no
other refs to the path before modifying its contents.

One snag is that ArcParams could previously not handle transforms, so it
is modified to allow a transform to be supplied to an Arc op, which is
only flattened out when it is streamed to a sink.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb3d8312e42e
Avoid copying paths when transforming. r=aosmond

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/cb3d8312e42e

Comment 8

[Mayank Bansal]

Profile with latest Nightly:

AC2D: https://share.firefox.dev/4eWZjSc (6.5s)
skia-canvas: https://share.firefox.dev/4f7J6JB (1.7s)
Chrome: https://share.firefox.dev/4dJdQjj (3.6s)

We improved massively, but are 2x slower than Chrome and 4x slower than Skia-canvas here. Worth filing a new bug?

Comment 9

[Lee Salzman [:lsalzman]]

(In reply to Mayank Bansal from comment #8)

Profile with latest Nightly:

AC2D: https://share.firefox.dev/4eWZjSc (6.5s)
skia-canvas: https://share.firefox.dev/4f7J6JB (1.7s)
Chrome: https://share.firefox.dev/4dJdQjj (3.6s)

We improved massively, but are 2x slower than Chrome and 4x slower than Skia-canvas here. Worth filing a new bug?

I am okay with where we are at currently now that I optimized this, so I don't imagine I will be doing much further work on this unless this turns out to be an obnoxious bottleneck in the wild outside of demos. The way we track transforms in Canvas2D has some pros and cons vs Chrome, and there aren't too many ways to optimize this more than I have, I think.