[webkit-reviews] review denied: [Bug 60185] [Cairo] Move the global alpha property from GraphicsContext to PlatformContextCairo : [Attachment 92283] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 60185: [Cairo] Move the global alpha property from GraphicsContext to
PlatformContextCairo
https://bugs.webkit.org/show_bug.cgi?id=60185

Attachment 92283: Patch
https://bugs.webkit.org/attachment.cgi?id=92283&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92283&action=review

Does plaformContext depend on the current state? Does save() and restore() push
and pop the globalalpha like expected?
r- because of style issues.

> Source/WebCore/platform/graphics/cairo/FontCairo.cpp:120
> +	       if (platformContext->globalAlpha() < 1.0f) {

s/1.0f/1/

looks like you need globalAlpha for every condition and mostly more than once.
Maybe you can save it in a local variable before the if?

> Source/WebCore/platform/graphics/cairo/FontCairo.cpp:129
> +	       if (platformContext->globalAlpha() < 1.0f) {

Ditto

> Source/WebCore/platform/graphics/cairo/FontCairo.cpp:150
> +	       if (platformContext->globalAlpha() < 1.0f) {

Ditto

> Source/WebCore/platform/graphics/cairo/PlatformContextCairo.h:75
> +    float m_globalAlpha;

no initial value 1?