Bug 11666

Summary: .getScreenCTM() returns wrong values
Product: WebKit Reporter: Andreas Neumann <a.neumann>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://www.carto.net/neumann/webkitsvgbugs/SVGLocatableTest.svg
Attachments:
Description Flags
First attempt
none
Different approach
none
Even better patch
none
Fix ChangeLog mitz: review+

Description Andreas Neumann 2006-11-21 08:29:57 PST
The method .getScreenCTM() of the object SVGLocatable returns wrong values. Correct values (that are matched by Batik/Opera/Firefox) are indicated in the example.
Comment 1 Rob Buis 2006-11-24 07:46:50 PST
Created attachment 11621 [details]
First attempt

What can I say, this should fix the problem :)
Cheers,

Rob.
Comment 2 mitz 2006-11-24 11:34:50 PST
Comment on attachment 11621 [details]
First attempt

The * is on the wrong side:

+    SVGMatrix *ctm = SVGLocatable::getScreenCTM(this);

But I'm somewhat confused by this patch. SVGTextElement inherits from SVGTransformable, yet it calls directly to SVGLocatable's implementations of getCTM and getScreenCTM. The only other class that inherits directly from SVGTransformable is SVGStyledTransformableElement, whose getCTM and getScreenCTM implementations also awkwardly call SVGLocatable's implementation (and then apply the local transformation).

Wouldn't it be possible to implement getCTM and getScreenCTM in SVGTransformable (by invoking SVGLocatable's implementation and multiplying by the localMatrix()) and get rid of the implementations in both SVGTextElement and SVGStyledTransformableElement?
Comment 3 Rob Buis 2006-11-24 14:08:33 PST
Created attachment 11624 [details]
Different approach

As Mitz pointed out, there is some code duplication. This patch shares a bit more code.
Cheers,

Rob.
Comment 4 Rob Buis 2006-11-24 14:33:56 PST
Created attachment 11625 [details]
Even better patch

Removing the unneeded if now.
Cheers,

Rob.
Comment 5 Rob Buis 2006-11-24 14:41:52 PST
Created attachment 11626 [details]
Fix ChangeLog
Comment 6 mitz 2006-11-24 14:42:56 PST
Comment on attachment 11626 [details]
Fix ChangeLog

r=me
Comment 7 mitz 2006-11-24 15:35:19 PST
Landed by Rob in r17879.
Comment 8 Andreas Neumann 2006-11-25 06:50:40 PST
I can confirm that this is now working - thanks for implementing!

The next road block on getting my examples work is:

SVGPoint.matrixTransform() - see http://bugs.webkit.org/show_bug.cgi?id=11685

Thanks,
Andreas