Unnecessary uploads of canvas

RESOLVED FIXED in mozilla2.0b10

Status

()

Core
Graphics
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jrmuizel, Assigned: vlad)

Tracking

unspecified
mozilla2.0b10
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
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.
Created attachment 502628 [details] [diff] [review]
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.
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.

Updated

8 years ago
Blocks: 593412
(Reporter)

Updated

8 years ago
Attachment #502628 - Flags: review?(jmuizelaar) → review+

Comment 5

8 years ago
(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.

Updated

8 years ago
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+

Comment 7

8 years ago
I checked the try results before pushing.

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