[Webkit-unassigned] [Bug 48074] [Cairo] Text underline is not shadowed when text-shadow is enabled

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=48074


Dirk Schulze <krit at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71448|review?                     |review-
               Flag|                            |




--- Comment #2 from Dirk Schulze <krit at webkit.org>  2010-11-23 10:31:42 PST ---
(From update of attachment 71448)
View in context: https://bugs.webkit.org/attachment.cgi?id=71448&action=review

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:281
> +    int numSegments = (distance - remainder) / patternWidth;

You really want to use a integer division?

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:294
> +            return (patternWidth - remainder) + (remainder / 2.0);
> +        return patternWidth / 2.0;

2.f

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:299
> +        return (patternWidth - remainder) / 2.0;

2.f

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:341
> +        double patternWidthAsDouble = patternWidth;

You already defined patternWidth as double above :-P

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:359
> +    cairo_save(cairoContext);
> +    drawLineOnCairoContext(this, cairoContext, point1, point2);
> +    cairo_restore(cairoContext);

Why do you use cairo_save? What gets restored after drawLineOnCairoContext()? Maybe it's faster to do it manually?

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:421
> +        patternWidth = strokeThickness() / 2.0;

patternWidth is an integer, please use floorf or ceilf

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:424
> +        patternWidth = 3.0 * strokeThickness() / 2.0;

Ditto.

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:435
>              distance = static_cast<int>((M_PI * hRadius) / 2.0);

Can you use floor(f) or ceil(f) here please? And 2.f should be ok. Don't use M_PI, take piDouble or piFloat from <wtf/MathExtras.h>

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:437
>              distance = static_cast<int>((M_PI * sqrtf((hRadius * hRadius + vRadius * vRadius) / 2.0)) / 2.0);

Ditto.

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

why don't you use double directly and avoid copying the data?

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:441
> +        cairo_set_dash(cr, &patternWidthAsDouble, 1, patternOffset);

hm, does cairo_set_dash really use a double and a float? I thought cairo always uses double.

> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:681
> +    cairo_restore(cairoContext);

Again, is save and restore necessary here?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.