[webkit-reviews] review denied: [Bug 126205] Create clipping path from <box> value : [Attachment 222795] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Bem Jones-Bey <bjonesbe at adobe.com>'s
request for review:
Bug 126205: Create clipping path from <box> value
https://bugs.webkit.org/show_bug.cgi?id=126205

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

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


Some comments and questions to the patch.

> Source/WebCore/rendering/RenderLayer.cpp:3784
> +template <class ReferenceBoxClipPathOperation>
> +static inline LayoutRect computeReferenceBox(const RenderObject& renderer,
const ReferenceBoxClipPathOperation& clippingPath, const LayoutRect&
rootRelativeBounds)

Two questions here: Why does it need to be a template function and why can't
the code be added to ReferenceBoxClipPathOperation instead?

> Source/WebCore/rendering/RenderLayer.cpp:3827
> +    if (!rootRelativeBoundsComputed) {
> +	   rootRelativeBounds = calculateLayerBounds(paintingInfo.rootLayer,
&offsetFromRoot, 0);
> +	   rootRelativeBoundsComputed = true;
> +    }

Since we need this calculation for all cases now, it is ok to move it up here
(was an initial reviewer request just to do it when absolutely necessary). But
please remove it from the SVG clipPath code underneath as well.

> Source/WebCore/rendering/RenderLayer.cpp:3844
> +	   // FIXME this does not properly compute the rounded corners as
specified in all conditions

Real sentence please.

> Source/WebCore/rendering/shapes/ShapeInfo.cpp:95
> +	   // FIXME this does not properly compute the rounded corners as
specified in all conditions

Ditto.