[webkit-reviews] review denied: [Bug 39965] SVG repaintRect should be empty if content got clipped away : [Attachment 57487] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Dirk Schulze
<krit at webkit.org>'s request for review:
Bug 39965: SVG repaintRect should be empty if content got clipped away
https://bugs.webkit.org/show_bug.cgi?id=39965

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

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
Hi Dirk, some comments:

WebCore/ChangeLog:10
 +	    The reference of the RenderObjects get already applied to the
resource
I don't understand this comment.
I guess it's related to the fact that you're creating ClipperData/MaskerData
earlier now, but I don't understand why this is needed.

WebCore/rendering/RenderSVGImage.cpp:154
 +	    
Please remove the extra spaces.

WebCore/rendering/RenderSVGResourceClipper.cpp:274
 +	SVGClipPathElement* clipElement =
static_cast<SVGClipPathElement*>(node());
That's not needed, you can leave it as it was before. You're not using
'clipElement' anywhere but in the if some lines below.

WebCore/rendering/SVGRenderSupport.cpp:306
 +	       
repaintRect.intersect(masker->resourceBoundingBox(const_cast<RenderObject*>(obj
ect)));
No need for the const_cast again, you already have a local variable 'renderer'
above.

Okay, not much to fixup, but r-, as long as I don't understand why you have to
early-create ClipperData/MaskerData etc I don't see the immediate need to do
so, especially not if (selfNeedsLayout()) is true :-)
Please enlighten me.

Cheers,
Niko