[webkit-reviews] review granted: [Bug 25276] m_absoluteBounds hack should be removed from SVG renderers : [Attachment 29591] Remove m_absoluteBounds hack from SVG renderers and move outlineBoundsForRepaint into RenderSVGModelObject

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Eric Seidel
<eric at webkit.org>'s request for review:
Bug 25276: m_absoluteBounds hack should be removed from SVG renderers
https://bugs.webkit.org/show_bug.cgi?id=25276

Attachment 29591: Remove m_absoluteBounds hack from SVG renderers and move
outlineBoundsForRepaint into RenderSVGModelObject
https://bugs.webkit.org/attachment.cgi?id=29591&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>

> +void RenderSVGModelObject::mapLocalToContainer(RenderBoxModelObject*
repaintContainer, bool fixed , bool useTransforms, TransformState&
transformState) const
> +{
> +    ASSERT(!fixed); // We should have no fixed content in the SVG rendering
tree.
> +
> +    // FIXME: If we don't respect useTransforms we break SVG text rendering.
 Seems SVG text has its own broken translation hacks.
> +    if (useTransforms)
> +	   transformState.applyTransform(localToParentTransform());

I think calling mapLocalToContainer() with useTransforms == false in SVG is
just wrong, and should never happen. We should either assert and
fix callers, or just force it to true.

> +// Copied from RenderBox, this method likely requires further refactoring to
work easily for both SVG and CSS Box Model content.
> +IntRect RenderSVGModelObject::outlineBoundsForRepaint(RenderBoxModelObject*
/*repaintContainer*/) const
> +{
> +    IntRect box = enclosingIntRect(repaintRectInLocalCoordinates());
> +    adjustRectForOutlineAndShadow(box);
> +
> +    FloatQuad absOutlineQuad = localToAbsoluteQuad(FloatRect(box));
> +    box = absOutlineQuad.enclosingBoundingBox();

Rather than follow the (broken: bug 25282) RenderBox, I think this should do:

FloatQuad containerOutlineQuad = localToContainerQuad(FloatRect(box),
repaintContainer)

r=me