Avoid unnecessary Snapshot() copies by clearing the pattern in gfxSurfaceDrawable

RESOLVED FIXED in Firefox 28

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

unspecified
mozilla29
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox28 fixed, firefox29 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
gfxSurfaceDrawable::Draw() sets a gfxPattern(Snapshot()) to the gfxContext's pattern. If the gfxSurfaceDrawable is created by CreateSamplingRestrictedDrawable it will be destroyed before the Snapshot(). This causes the Snapshot to be copied unnecessarily.

If we set the pattern to something else we'll destroy the snapshot before the gfxSurfaceDrawable and save a copy.
(Assignee)

Comment 1

5 years ago
Created attachment 8348812 [details] [diff] [review]
Avoid unnecessary Snapshot() copies by clearing the pattern in gfxSurfaceDrawable
Attachment #8348812 - Flags: review?(bas)
What backends does this actually copy for? I thought we often just transferred ownership of the underlying data/object when a DT gets destroyed while a snapshot still exists.

It seems like this is quite an easy trap to fall into, making it fast seems worthwhile.
(Assignee)

Comment 3

5 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #2)
> What backends does this actually copy for? I thought we often just
> transferred ownership of the underlying data/object when a DT gets destroyed
> while a snapshot still exists.

Cairo and CG.

> It seems like this is quite an easy trap to fall into, making it fast seems
> worthwhile.

I agree.
https://hg.mozilla.org/mozilla-central/rev/767fa5332415
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
(Assignee)

Comment 5

5 years ago
This gave a 4-7% improvement on tscroll on OS X
Duplicate of this bug: 951214
(Assignee)

Updated

4 years ago
Attachment #8348812 - Flags: review?(bas)
(Assignee)

Comment 7

4 years ago
Comment on attachment 8348812 [details] [diff] [review]
Avoid unnecessary Snapshot() copies by clearing the pattern in gfxSurfaceDrawable

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 896489
User impact if declined: Lower performance when drawing scaled images
Testing completed (on m-c, etc.): Has been on m-c for a little while now
Risk to taking this patch (and alternatives if risky): Very low risk.
Attachment #8348812 - Flags: approval-mozilla-aurora?
Attachment #8348812 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/54709f91b972
status-firefox28: --- → fixed
status-firefox29: --- → fixed
Does this have or need tests?
Flags: needinfo?(jmuizelaar)
Flags: in-testsuite?
(Assignee)

Comment 10

4 years ago
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #9)
> Does this have or need tests?

The existing reftests should be sufficient.
Flags: needinfo?(jmuizelaar)
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.