624505 - Unnecessary uploads of canvas

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

It seems like we re-upload 2d canvas's everytime we paint them.

nsCanvasRenderingContext2D::GetCanvasLayer() calls
layer->Updated(nsIntRect(0, 0, mWidth, mHeight)); which reuploads.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: don't update unnecessarily

Ouch!  Yes indeed.  This patch is untested, but I will test it shortly.

I don't actually think that MarkContextClean is needed any more, as an exposed API on HTMLCanvasElement etc -- it's there so that the Canvas frame can call it after getting the layer, but I think it can just be nuked.  It also wasn't setting InvalidateCount to 0.

This will likely have a significant perf impact on canvas, especially on non-d2d platforms.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

the patch seems to work, but I can't tell if there's any perf difference -- opening up panorama and clicking on a tab to get it to "zoom in" results in roughly the same speed with and without the patch (with GL layers under windows in my debug build), and it looks smooth regardless.  maybe whoever was having the issue in the first place can try it?

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-33bb302ee63f/

Comment 4

[Patrick Walton (:pcwalton)]

(In reply to comment #2)
> the patch seems to work, but I can't tell if there's any perf difference --
> opening up panorama and clicking on a tab to get it to "zoom in" results in
> roughly the same speed with and without the patch (with GL layers under windows
> in my debug build), and it looks smooth regardless.  maybe whoever was having
> the issue in the first place can try it?

Tested and it totally changes the profile for the better. Panorama zoom is still slow, but I have some other ideas; this may well be a prerequisite to making it fast.

Comment 5

[(no longer active)]

(In reply to comment #4)
> Tested and it totally changes the profile for the better. Panorama zoom is
> still slow, but I have some other ideas; this may well be a prerequisite to
> making it fast.

It would be awesome if you can make any other bugs that you file in the future block bug 593412, which is kind of the tracker for the panorama zooming issues.

Comment 6

[Joe Drew (not getting mail)]

Comment on attachment 502628 [details] [diff] [review]
don't update unnecessarily

Approved as long as try results are good

Comment 7

[(no longer active)]

I checked the try results before pushing.

http://hg.mozilla.org/mozilla-central/rev/a31c9bed05c6