[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