Bug 25432

Summary: Simplify SVG hit testing (and make it floating point precise)
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs.webkit.org, krit, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on: 25431    
Bug Blocks: 25433    
Attachments:
Description Flags
Simplify nodeAtPoint for SVG simon.fraser: review+

Description Eric Seidel (no email) 2009-04-27 14:39:27 PDT
Simplify SVG hit testing (and make it floating point precise)

SVG hit testing is one of the last clients of absoluteTransform(), so fixing it will bring us one step closer to killing absoluteTransform.

I love that this patch killed at least one "this code is needlessly complicated and probably wrong" statements. :)
Comment 1 Eric Seidel (no email) 2009-04-27 14:46:03 PDT
Created attachment 29829 [details]
Simplify nodeAtPoint for SVG

 20 files changed, 185 insertions(+), 95 deletions(-)
Comment 2 Simon Fraser (smfr) 2009-04-28 11:27:27 PDT
Comment on attachment 29829 [details]
Simplify nodeAtPoint for SVG



> +TransformationMatrix RenderForeignObject::localToParentTransform() const
> +{
> +    // FIXME: This trasition is backwards!
> +    // It should be localTransform() * translationForAttributes()
> +    // but leaving it backwards for now for LayoutTest result compatibility
> +    return translationForAttributes() * localTransform();
> +}

Reference the bug you filed in the comment.

> +    // SVG uses a different hit-testing system from CSS
> +    virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const FloatPoint& pointInParent, HitTestAction);

I'd rather the new nodeAtPoint got a different name to make it easier to fix bug 23120.

Looks OK to me but I'd like hyatt to give the final word on whether we should add nodeAtPoint proliferation.
Comment 3 Simon Fraser (smfr) 2009-04-28 11:30:22 PDT
Comment on attachment 29829 [details]
Simplify nodeAtPoint for SVG

Dave says it's OK.
Comment 4 Eric Seidel (no email) 2009-04-28 13:56:30 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderForeignObject.cpp
	M	WebCore/rendering/RenderForeignObject.h
	M	WebCore/rendering/RenderObject.cpp
	M	WebCore/rendering/RenderObject.h
	M	WebCore/rendering/RenderPath.cpp
	M	WebCore/rendering/RenderPath.h
	M	WebCore/rendering/RenderSVGContainer.cpp
	M	WebCore/rendering/RenderSVGContainer.h
	M	WebCore/rendering/RenderSVGHiddenContainer.cpp
	M	WebCore/rendering/RenderSVGHiddenContainer.h
	M	WebCore/rendering/RenderSVGImage.cpp
	M	WebCore/rendering/RenderSVGImage.h
	M	WebCore/rendering/RenderSVGModelObject.cpp
	M	WebCore/rendering/RenderSVGModelObject.h
	M	WebCore/rendering/RenderSVGRoot.cpp
	M	WebCore/rendering/RenderSVGText.cpp
	M	WebCore/rendering/RenderSVGText.h
	M	WebCore/rendering/RenderSVGViewportContainer.cpp
	M	WebCore/rendering/RenderSVGViewportContainer.h
Committed r42960
Comment 5 Eric Seidel (no email) 2009-04-28 18:00:51 PDT
*** Bug 21892 has been marked as a duplicate of this bug. ***