[webkit-reviews] review granted: [Bug 72011] Move unit resolving for all resources to rendering/ : [Attachment 114491] Patch

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 72011: Move unit resolving for all resources to rendering/
https://bugs.webkit.org/show_bug.cgi?id=72011

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

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=114491&action=review


r=me with some nits.

> Source/WebCore/rendering/svg/RenderSVGInlineText.h:56
> -    virtual FloatRect objectBoundingBox() const { return FloatRect(); }
> +    virtual FloatRect objectBoundingBox() const { return linesBoundingBox();
}

Awesome! :)

> Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp:188
> -    filterData->builder = buildPrimitives(filterData->filter.get());
> +    filterData->builder =
buildPrimitives(static_cast<SVGFilter*>(filterData->filter.get()));

This cast looks unnecessary.

> Source/WebCore/rendering/svg/RenderSVGResourceRadialGradient.cpp:65
> +    // Eventually adjust focal points, as described below

Missing period at the end of this sentence.

> Source/WebCore/rendering/svg/RenderSVGResourceRadialGradient.h:50
> +    void adjustFocalPointIfNeeded(float radius, const FloatPoint&
centerPoint, FloatPoint&) const;

I think we should name the last argument here (focalPoint.)