Bug 104304 – Parameterized GraphicsContext::save()

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Layout and Rendering

[Florin Malita]

Skia (and possibly OpenVG) supports partial context saves (matrix vs. clip) and handles paint state on a separated stack in the platform code. Having call sites pass more specific information about the graphics context part that needs to be saved would enable platform-level optimizations for reducing the number of redundant save ops.

For example, code that only changes the matrix when painting its subtree could call save(SaveMatrix) instead of a full-blown save() and avoid the overhead of clip & paint save/restore.

This bug focuses on the generic plumbing needed to support such a scheme, with platform specific implementations and call site optmizations to be handled separately.

[Florin Malita]

Created attachment 178082 [details]
Patch

First pass.

[Build Bot]

Comment on attachment 178082 [details]
Patch

Attachment 178082 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15187058

[Florin Malita]

Created attachment 178203 [details]
Patch

Mac build fix.

[Stephen White]

Comment on attachment 178203 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178203&action=review

> Source/WebCore/platform/graphics/GraphicsContext.h:158
> +        SaveMatrix  = 1 << 0,
> +        SaveClip    = 1 << 1,
> +        SavePaint   = 1 << 2,

WebKit doesn't really have a notion of Paint, the way Skia does.  Since this is in platform-independent code, perhaps we should avoid introducing it.  I'm not sure what to call it, though. Maybe SaveState?  (kind of generic)  SaveOther?  SaveOtherState?  Hmm.  I'm at a loss.

[Stephen Chenney]

Comment on attachment 178203 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178203&action=review

>> Source/WebCore/platform/graphics/GraphicsContext.h:158
>> +        SavePaint   = 1 << 2,
> 
> WebKit doesn't really have a notion of Paint, the way Skia does.  Since this is in platform-independent code, perhaps we should avoid introducing it.  I'm not sure what to call it, though. Maybe SaveState?  (kind of generic)  SaveOther?  SaveOtherState?  Hmm.  I'm at a loss.

Good point. What might other platforms name it? SaveStroke | SaveFill? If it makes sense we can have more than Skia differentiates.

[Simon Fraser (smfr)]

Comment on attachment 178203 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178203&action=review

>>> Source/WebCore/platform/graphics/GraphicsContext.h:158
>>> +        SavePaint   = 1 << 2,
>> 
>> WebKit doesn't really have a notion of Paint, the way Skia does.  Since this is in platform-independent code, perhaps we should avoid introducing it.  I'm not sure what to call it, though. Maybe SaveState?  (kind of generic)  SaveOther?  SaveOtherState?  Hmm.  I'm at a loss.
> 
> Good point. What might other platforms name it? SaveStroke | SaveFill? If it makes sense we can have more than Skia differentiates.

SavePaintState sounds OK. Or, if this is everything other than matrix and clip, the name should probably reflect that.

[Florin Malita]

Comment on attachment 178203 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178203&action=review

>>>> Source/WebCore/platform/graphics/GraphicsContext.h:158
>>>> +        SavePaint   = 1 << 2,
>>> 
>>> WebKit doesn't really have a notion of Paint, the way Skia does.  Since this is in platform-independent code, perhaps we should avoid introducing it.  I'm not sure what to call it, though. Maybe SaveState?  (kind of generic)  SaveOther?  SaveOtherState?  Hmm.  I'm at a loss.
>> 
>> Good point. What might other platforms name it? SaveStroke | SaveFill? If it makes sense we can have more than Skia differentiates.
> 
> SavePaintState sounds OK. Or, if this is everything other than matrix and clip, the name should probably reflect that.

I like SavePaintState, but the latter is probably more accurate. Would SaveNonMatrixClipState stick?

[Florin Malita]

Created attachment 181178 [details]
Patch

[Martin Robinson]

Skia and OpenVG have been removed now, so closing this bug.