Bug 11734

Summary: SVGPoint, SVGRect, etc are implemented as PODs
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, sam
Priority: P2    
Version: 416.x   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Testcase
none
Initial patch adressing problems.
none
Initial set of LayoutTests.
none
Improved patch
oliver: review-
Updated patch oliver: review+

Description Oliver Hunt 2006-12-01 13:51:40 PST
SVGPoint, SVGRect etc are implemented as PODs and therefore have copy by value semantics that prevent a number of SVG+JS examples from working.
Comment 1 Oliver Hunt 2006-12-01 13:52:52 PST
Created attachment 11705 [details]
Testcase

Testcase demonstrates using SVGPoint as a reference.  Behaviour is borked in Opera, correct in FFX
Comment 2 Nikolas Zimmermann 2006-12-02 09:16:20 PST
Created attachment 11712 [details]
Initial patch adressing problems.

This patch doesn't contain any ChangeLogs, Rob wants to finish
it and implement matrixTransform() - as I don't have any time
anymore tonight.
Comment 3 Nikolas Zimmermann 2006-12-02 09:17:53 PST
Created attachment 11713 [details]
Initial set of LayoutTests.
Comment 4 Rob Buis 2006-12-02 15:43:21 PST
Created attachment 11715 [details]
Improved patch

This patch adds matrixTransform handling and a test for it. Sam Weinig added the objC part as well as the FloatPoint additions. I think we may need more testing seeing olliej's test, OTOH it is a big improvement IMHO.
Cheers,

Rob.
Comment 5 Oliver Hunt 2006-12-02 16:18:57 PST
Comment on attachment 11715 [details]
Improved patch

FloatPoint::matrixTransform should be in FloatPointCG.cpp
Comment 6 Sam Weinig 2006-12-02 17:11:02 PST
(In reply to comment #5)
> (From update of attachment 11715 [details] [edit])
> FloatPoint::matrixTransform should be in FloatPointCG.cpp
> 

I disagree, I think FloatPoint::matrixTransform should be in FloatPoint.cpp as it is entirely platform independant.  I also think static_casts should be used instead of the c-style casts.
Comment 7 Oliver Hunt 2006-12-02 18:25:32 PST
True, i'd completely forgotten that AffineTransform isn't a CG specific type, in either case it still needs to be pulled from FloatPointMac.cpp
Comment 8 Nikolas Zimmermann 2006-12-03 03:25:45 PST
*** Bug 11685 has been marked as a duplicate of this bug. ***
Comment 9 Nikolas Zimmermann 2006-12-03 05:22:51 PST
Created attachment 11718 [details]
Updated patch

Incorporated comments
.
Comment 10 Oliver Hunt 2006-12-03 05:37:17 PST
Comment on attachment 11718 [details]
Updated patch

looks fine to me

I'd normally expect layout tests to produce green/red rects -- but "passed" failed" is good too :D
Comment 11 Nikolas Zimmermann 2006-12-03 05:48:26 PST
Landed in r17993.