1896738 - Inconsistent performances of putImageData function

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Calixte Denizet (:calixte)]

Attachment: plop.html

STR:

• open the attachment
• run it several times with the profiler

Here are several profiles:

• ~380ms to render the image in the main thread: https://share.firefox.dev/3wCKKTe
• ~2.2s: https://share.firefox.dev/3UG7Zno
• ~611ms: https://share.firefox.dev/4bnnjfF

Out of curiosity, I tested with gfx.canvas.willreadfrequently.enabled set to trueand in passing willReadFrequently: true when getting the context, and the results are almost the same.

Comment 1

[Lee Salzman [:lsalzman]]

I'd need to look deeper, but it seems like it is spending a really large amount of time clearing the DT for some reason. I'll see if I can take a look behind the curtain and figure out a reason why, and if that can be optimized somehow.

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: Bug 1896738 - Avoid unnecessary clears when using PutImageData on a new canvas. r?aosmond

This fixes two issues. The first is that the dirty rect passed to EnsureTarget was being
transformed, whereas PutImageData is supposed to bypass transforms. The second is that
PutImageData actually initializes the alpha channel properly, such that Skia DTs do not
need the alpha channel of pixels that are modified by PutImageData to be cleared first,
so that PutImageData can effectively function as the clear itself.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a97df11ef6f
Avoid unnecessary clears when using PutImageData on a new canvas. r=aosmond

Comment 4

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/8a97df11ef6f

Comment 5

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:lsalzman, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox127 to wontfix.

For more information, please visit BugBot documentation.

Comment 6

[Andrej (:aglavic)]

(In reply to Pulsebot from comment #3)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a97df11ef6f
Avoid unnecessary clears when using PutImageData on a new canvas. r=aosmond

Perfherder has detected a talos performance change from push 8a97df11ef6fe5685d068f4e80ba94864c5abf0b.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
6%	pdfpaint issue6741.pdf	windows10-64-shippable-qr	e10s fission stylo webrender-sw	4,442.41 -> 4,160.24
3%	pdfpaint issue10491.pdf	windows10-64-shippable-qr	e10s fission stylo webrender-sw	465.43 -> 449.38
3%	pdfpaint issue10491.pdf	windows10-64-shippable-qr	e10s fission stylo webrender-sw	463.99 -> 448.46
3%	pdfpaint issue8076.pdf	windows10-64-shippable-qr	e10s fission stylo webrender	5,887.67 -> 5,738.45

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 444

For more information on performance sheriffing please see our FAQ.