[webkit-reviews] review denied: [Bug 48516] GraphicsContext: Remove "current path" and have strokePath, fillPath and clipPath take a Path argument : [Attachment 74629] Proposed patch v5

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Andreas Kling
<kling at webkit.org>'s request for review:
Bug 48516: GraphicsContext: Remove "current path" and have strokePath, fillPath
and clipPath take a Path argument
https://bugs.webkit.org/show_bug.cgi?id=48516

Attachment 74629: Proposed patch v5
https://bugs.webkit.org/attachment.cgi?id=74629&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=74629&action=review

Looks great to me, still a nitpick though:

> WebCore/platform/graphics/GraphicsContext.h:304
> +#if !PLATFORM(QT) && !PLATFORM(CAIRO) && !PLATFORM(CG) && !PLATFORM(HAIKU)

Could you only list the platforms that actually need begin/addPath here,
instead of blacklisting those who don't?

> WebCore/rendering/svg/SVGInlineTextBox.cpp:347
> +    releasePaintingResource(context, Path());

Hm, this seems wrong to me. The text code path doesn't need any Path usage at
all for painting text.
This implies that postApplyResource() should take a Path* pointer, so that we
can avoid creating a temporary Path() object here. Agreed?

> WebCore/rendering/svg/SVGInlineTextBox.cpp:505
> +	   releasePaintingResource(context, path);

You'd just have to use &path here then.