Bug 183593 – [Nicosia] Add Cairo-specific GraphicsContext operation recorder

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

[Nicosia] Add Cairo-specific GraphicsContext operation recorder

[Zan Dobersek]

Created attachment 335691 [details]
Patch

[EWS Watchlist]

Attachment 335691 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:50:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:61:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:69:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Comment on attachment 335691 [details]
Patch

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

> Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:41
> +PlatformContextCairo& getContext(PaintingOperationReplay& operationReplay)

I would rename this to platformContext() we don't use get in WebKit unless using an out parameter for the return value.

> Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:366
> +    UNUSED_PARAM(color);

Why don't you omit the name instead?

> Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:510
> +    UNUSED_PARAM(fontSmoothing);

Same here.

> Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:622
> +            Cairo::drawLine(getContext(replayer), arg<0>(), arg<1>(), arg<2>(), arg<3>(),
> +                arg<4>(), arg<5>());

This could be one line

[Zan Dobersek]

Comment on attachment 335691 [details]
Patch

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

>> Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:41
>> +PlatformContextCairo& getContext(PaintingOperationReplay& operationReplay)
> 
> I would rename this to platformContext() we don't use get in WebKit unless using an out parameter for the return value.

Somehow this clashes with GraphicsContext::platformContext(). I'll use contextForReplay() instead.

>> Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:366
>> +    UNUSED_PARAM(color);
> 
> Why don't you omit the name instead?

Because that then drops information about the purpose of that parameter, and it's more valuable to me to see what parameters are not being used for the given operation.

[Zan Dobersek]

Created attachment 335933 [details]
Patch for landing

[EWS Watchlist]

Attachment 335933 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:50:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:61:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:69:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

Comment on attachment 335933 [details]
Patch for landing

Clearing flags on attachment: 335933

Committed r229672: <https://trac.webkit.org/changeset/229672>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/38548942>

[Simon Fraser (smfr)]

Could this share code with Source/WebCore/platform/graphics/displaylists ?