[webkit-reviews] review denied: [Bug 48074] [Cairo] Text underline is not shadowed when text-shadow is enabled : [Attachment 84137] Patch with Krit's suggestions and a good deal of cleanup

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 48074: [Cairo] Text underline is not shadowed when text-shadow is enabled
https://bugs.webkit.org/show_bug.cgi?id=48074

Attachment 84137: Patch with Krit's suggestions and a good deal of cleanup
https://bugs.webkit.org/attachment.cgi?id=84137&action=review

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

I'd like to see it a second time.

> Source/WebCore/platform/graphics/GraphicsContext.h:509
> +

Remove this line

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:326
> +    bool isVerticalLine = (point1.x() == point2.x());

avoid the braces

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:431
> +	   patternWidth = floor(strokeThickness() / 2.f);

floorf, but depends what strokeThickness returns. please check this.

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:434
> +	   patternWidth = 3 * floor(strokeThickness() / 2.f);

ditto

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:445
> +	       distance = floor((piFloat * hRadius) / 2.0);

floorf

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:447
> +	       distance = floor((piFloat * sqrtf((hRadius * hRadius + vRadius *
vRadius) / 2.0)) / 2.0);

ditto

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:448
> +	   double patternOffset = calculateStrokePatternOffset(distance,
patternWidth);

if patternOffset is double, you should may use double in general and ignore the
last two comments.