[webkit-reviews] review denied: [Bug 46475] [Cairo] Activate ContextShadow in all places where shadows are drawn : [Attachment 70017] Patch for this issue using drawRectShadow

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 13 02:21:42 PDT 2010


Dirk Schulze <krit at webkit.org> has denied Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 46475: [Cairo] Activate ContextShadow in all places where shadows are drawn
https://bugs.webkit.org/show_bug.cgi?id=46475

Attachment 70017: Patch for this issue using drawRectShadow
https://bugs.webkit.org/attachment.cgi?id=70017&action=review

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

Patch looks great. Please update the patch to trunk and fix the style issues.

> WebCore/platform/graphics/cairo/FontCairo.cpp:51
> +	   cairo_matrix_t mat = {1, 0, -tanf(SYNTHETIC_OBLIQUE_ANGLE * acosf(0)
/ 90), 1, point.x(), point.y()};

Can you remove SYNTHETIC_OBLIQUE_ANGLE and replace it with a static const for
SYNTHETIC_OBLIQUE_ANGLE * acosf(0) / 90 in WebKit style? Something like:
static const float gSyntheticObliqueSkew = 14 * acosf(0) / 90;
and move it after the header includes?

> WebCore/platform/graphics/cairo/FontCairo.cpp:55
> +    } else {
> +	   cairo_translate(context, point.x(), point.y());
> +    }

No braces for 'else' here.

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:157
>      double x0, x1, y0, y1;

Please use multiple lines and a initialization for the values.

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:863
> +    } else {
> +	   m_data->shadow = ContextShadow(color, blur, FloatSize(size.width(),
size.height()));
>      }

no braces for 'else' here.

> WebCore/platform/graphics/cairo/GraphicsContextPlatformPrivateCairo.h:106
> +    bool hasShadow() const
> +    {
> +	   return shadow.m_type != ContextShadow::NoShadow;
> +    }

You can use one line, if a function just includes a one liner.


More information about the webkit-reviews mailing list