[webkit-reviews] review granted: [Bug 125718] Faster implementation of text-decoration-skip: ink : [Attachment 219450] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 125718: Faster implementation of text-decoration-skip: ink
https://bugs.webkit.org/show_bug.cgi?id=125718

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=219450&action=review


> Source/WebCore/platform/graphics/mac/FontMac.mm:350
> +static bool findIntersectionPoint(float y, CGPoint p1, CGPoint p2, CGFloat&
x)
> +{
> +    x = p1.x + (y - p1.y) * (p2.x - p1.x) / (p2.y - p1.y);
> +    return (p1.y < y && p2.y > y) || (p1.y > y && p2.y < y);
> +}

I wish we had a GeometryUtilities.h :\

> Source/WebCore/platform/graphics/mac/FontMac.mm:352
> +static void findPathIntersections(void* stateAsVoidPointer, const
CGPathElement* e)

I would like to see a comment saying what this function does. It's very hard to
figure out.

> Source/WebCore/platform/graphics/mac/FontMac.mm:393
> +DashArray Font::intersectionPoints(const TextRun& run, const FloatPoint&
textOrigin, int textRunStartIndex, int textRunEndIndex, const FloatRect&
lineExtents) const

"intersectionPoints": intersection with what?

> Source/WebCore/platform/graphics/mac/FontMac.mm:405
> +	   GlyphIterationState info = GlyphIterationState(CGPointMake(0, 0),
CGPointMake(0, 0), lineExtents.y(), lineExtents.y() + lineExtents.height(),
1e7, -1e7);

1e7, -1e7? Don't we have FLOAT_MAX or CGFLOATMAX you can use?