Bug 11734

Summary:

SVGPoint, SVGRect, etc are implemented as PODs

Product:

WebKit

Reporter:

Oliver Hunt <oliver>

Component:

SVG

Assignee:

Nikolas Zimmermann <zimmermann>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, sam

Priority:

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+

Oliver Hunt

Reported 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.

Attachments
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
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Oliver Hunt

Comment 1 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

Nikolas Zimmermann

Comment 2 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.

Nikolas Zimmermann

Comment 3 2006-12-02 09:17:53 PST

Created attachment 11713 [details] Initial set of LayoutTests.

Rob Buis

Comment 4 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.

Oliver Hunt

Comment 5 2006-12-02 16:18:57 PST

Comment on attachment 11715 [details] Improved patch FloatPoint::matrixTransform should be in FloatPointCG.cpp

Sam Weinig

Comment 6 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.

Oliver Hunt

Comment 7 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

Nikolas Zimmermann

Comment 8 2006-12-03 03:25:45 PST

*** Bug 11685 has been marked as a duplicate of this bug. ***

Nikolas Zimmermann

Comment 9 2006-12-03 05:22:51 PST

Created attachment 11718 [details] Updated patch Incorporated comments .

Oliver Hunt

Comment 10 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

Nikolas Zimmermann

Comment 11 2006-12-03 05:48:26 PST

Landed in r17993.

Note You need to log in before you can comment on or make changes to this bug.