Bug 25276 - m_absoluteBounds hack should be removed from SVG renderers
Summary: m_absoluteBounds hack should be removed from SVG renderers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 23881
  Show dependency treegraph
 
Reported: 2009-04-17 16:06 PDT by Eric Seidel (no email)
Modified: 2009-04-20 12:11 PDT (History)
1 user (show)

See Also:


Attachments
Remove m_absoluteBounds hack from SVG renderers and move outlineBoundsForRepaint into RenderSVGModelObject (15.83 KB, patch)
2009-04-17 16:12 PDT, Eric Seidel (no email)
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-04-17 16:06:23 PDT
m_absoluteBounds hack should be removed from SVG renderers

Now that I've cleaned up the SVG render tree a little, we can!

See attached patch.
Comment 1 Eric Seidel (no email) 2009-04-17 16:12:18 PDT
Created attachment 29591 [details]
Remove m_absoluteBounds hack from SVG renderers and move outlineBoundsForRepaint into RenderSVGModelObject

 10 files changed, 96 insertions(+), 63 deletions(-)
Comment 2 Simon Fraser (smfr) 2009-04-17 17:38:54 PDT
Comment on attachment 29591 [details]
Remove m_absoluteBounds hack from SVG renderers and move outlineBoundsForRepaint into RenderSVGModelObject


> +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
Comment 3 Eric Seidel (no email) 2009-04-20 12:08:59 PDT
I did not change that back to an ASSERT, but I plan to do that once I fix <text>

I fixed the localToContainerQuad issue.
Comment 4 Eric Seidel (no email) 2009-04-20 12:10:48 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderPath.cpp
	M	WebCore/rendering/RenderPath.h
	M	WebCore/rendering/RenderSVGContainer.cpp
	M	WebCore/rendering/RenderSVGContainer.h
	M	WebCore/rendering/RenderSVGModelObject.cpp
	M	WebCore/rendering/RenderSVGModelObject.h
	M	WebCore/rendering/RenderSVGRoot.cpp
	M	WebCore/rendering/RenderSVGRoot.h
	M	WebCore/rendering/RenderSVGViewportContainer.cpp
Committed r42677