Closed Bug 624505 Opened 9 years ago Closed 9 years ago

Unnecessary uploads of canvas

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b10

People

(Reporter: jrmuizel, Assigned: vlad)

References

Details

Attachments

(1 file)

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.
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.
Assignee: nobody → vladimir
Attachment #502628 - Flags: superreview?(roc)
Attachment #502628 - Flags: review?
Attachment #502628 - Flags: review? → review?(jmuizelaar)
Attachment #502628 - Flags: superreview?(roc) → superreview+
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?
(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.
Blocks: 593412
Attachment #502628 - Flags: review?(jmuizelaar) → review+
(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.
Attachment #502628 - Flags: approval2.0?
Comment on attachment 502628 [details] [diff] [review]
don't update unnecessarily

Approved as long as try results are good
Attachment #502628 - Flags: approval2.0? → approval2.0+
I checked the try results before pushing.

http://hg.mozilla.org/mozilla-central/rev/a31c9bed05c6
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
You need to log in before you can comment on or make changes to this bug.