Bug 11734 - SVGPoint, SVGRect, etc are implemented as PODs
Summary: SVGPoint, SVGRect, etc are implemented as PODs
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 416.x
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nikolas Zimmermann
: 11685 (view as bug list)
Depends on:
Reported: 2006-12-01 13:51 PST by Oliver Hunt
Modified: 2006-12-03 05:48 PST (History)
2 users (show)

See Also:

Testcase (828 bytes, application/xml)
2006-12-01 13:52 PST, Oliver Hunt
no flags Details
Initial patch adressing problems. (8.95 KB, patch)
2006-12-02 09:16 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Initial set of LayoutTests. (22.81 KB, patch)
2006-12-02 09:17 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Improved patch (47.80 KB, patch)
2006-12-02 15:43 PST, Rob Buis
oliver: review-
Details | Formatted Diff | Diff
Updated patch (63.13 KB, patch)
2006-12-03 05:22 PST, Nikolas Zimmermann
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 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.

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.