Closed Bug 962667 Opened 6 years ago Closed 6 years ago

Don't optimize quartz image surfaces if there is nothing to do

Categories

(Core :: Graphics, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: mwu, Assigned: mwu)

References

Details

Attachments

(1 file, 1 obsolete file)

We don't need to do anything on OSX if the image format doesn't change. The surface is always allocated by ourselves and the image code already has it wrapped in a gfxQuartzImageSurface.
Attachment #8363778 - Flags: review?(jmuizelaar)
Blocks: 962670
(In reply to Michael Wu [:mwu] from comment #0)
> Created attachment 8363778 [details] [diff] [review]
> Avoid unnecessary creation of a new gfxQuartzImageSurface
> 
> We don't need to do anything on OSX if the image format doesn't change. The
> surface is always allocated by ourselves and the image code already has it
> wrapped in a gfxQuartzImageSurface.

OptimizeImage() is only called when mQuartzSurface is NULL which suggests that we don't still have the gfxQuartzImageSurface wrapper around. So I'm not sure how this patch helps.
Flags: needinfo?(mwu)
Huh, you're right. I missed that path.
Flags: needinfo?(mwu)
Comment on attachment 8363778 [details] [diff] [review]
Avoid unnecessary creation of a new gfxQuartzImageSurface

Clearing review while I try to understand the code a bit better.
Attachment #8363778 - Flags: review?(jmuizelaar)
This removes the code that reuses the quartz surface. This helps in cases where the image ends up being scaled. (I am assuming that scaling code is actually used. I haven't checked.) The image gets locked (via LockImageData) again when scaling, which forces a copy to a new gfxImageSurface.
Attachment #8363778 - Attachment is obsolete: true
Attachment #8364638 - Flags: review?(jmuizelaar)
Comment on attachment 8364638 [details] [diff] [review]
Avoid using mOptSurface when no optimization is necessary

Review of attachment 8364638 [details] [diff] [review]:
-----------------------------------------------------------------

This seems ok. It might be worth splitting the two pieces up into separate patches.
Attachment #8364638 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/977fb2494568
https://hg.mozilla.org/mozilla-central/rev/fc3d3be15c0c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.