Bug 104304 - Parameterized GraphicsContext::save()
Summary: Parameterized GraphicsContext::save()
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Florin Malita
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-06 14:17 PST by Florin Malita
Modified: 2013-04-19 13:08 PDT (History)
10 users (show)

See Also:


Attachments
Patch (40.02 KB, patch)
2012-12-06 14:31 PST, Florin Malita
no flags Details | Formatted Diff | Diff
Patch (40.75 KB, patch)
2012-12-07 05:31 PST, Florin Malita
no flags Details | Formatted Diff | Diff
Patch (41.07 KB, patch)
2013-01-03 08:41 PST, Florin Malita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florin Malita 2012-12-06 14:17:51 PST
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.
Comment 1 Florin Malita 2012-12-06 14:31:49 PST
Created attachment 178082 [details]
Patch

First pass.
Comment 2 Build Bot 2012-12-06 15:50:44 PST
Comment on attachment 178082 [details]
Patch

Attachment 178082 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15187058
Comment 3 Florin Malita 2012-12-07 05:31:36 PST
Created attachment 178203 [details]
Patch

Mac build fix.
Comment 4 Stephen White 2012-12-07 11:49:44 PST
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.
Comment 5 Stephen Chenney 2012-12-07 12:22:25 PST
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.
Comment 6 Simon Fraser (smfr) 2012-12-09 21:26:14 PST
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.
Comment 7 Florin Malita 2013-01-03 07:50:36 PST
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?
Comment 8 Florin Malita 2013-01-03 08:41:21 PST
Created attachment 181178 [details]
Patch
Comment 9 Martin Robinson 2013-04-19 12:25:30 PDT
Skia and OpenVG have been removed now, so closing this bug.