Bug 11813

Summary:

SVGLength needs a rewrite

Product:

WebKit

Reporter:

Nikolas Zimmermann <zimmermann>

Component:

SVG

Assignee:

Nikolas Zimmermann <zimmermann>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
First patch, not ready for review yet	none
Second patch, not ready for review yet	none
No regressions	none
Updated patch	none
Updated patch II	none
Final patch	rwlbuis: review+

Nikolas Zimmermann

Reported 2006-12-12 10:16:00 PST

SVGLength needs to be passed around as value, not by pointer, just like it's done for SVGMatrix* -> AffineTransform, or SVGPoint/SVGRect. Also it's unnecessary to store both a value and a valueInSpecifiedUnits object in SVGLength itself, this can be changed. The only problem is that the JS put() problems have to be fixed in POD types first, that's also done in the patch I'm attaching soon.

Attachments
First patch, not ready for review yet (182.87 KB, patch) 2006-12-12 10:17 PST, Nikolas Zimmermann	no flags	Details Formatted Diff Diff
Second patch, not ready for review yet (156.11 KB, patch) 2006-12-14 15:57 PST, Nikolas Zimmermann	no flags	Details Formatted Diff Diff
No regressions (157.60 KB, patch) 2006-12-16 06:23 PST, Rob Buis	no flags	Details Formatted Diff Diff
Updated patch (208.58 KB, patch) 2006-12-16 13:01 PST, Nikolas Zimmermann	no flags	Details Formatted Diff Diff
Updated patch II (209.08 KB, patch) 2006-12-16 14:27 PST, Nikolas Zimmermann	no flags	Details Formatted Diff Diff
Final patch (211.19 KB, patch) 2006-12-17 04:11 PST, Nikolas Zimmermann	rwlbuis: review+	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Nikolas Zimmermann

Comment 1 2006-12-12 10:17:59 PST

Created attachment 11824 [details] First patch, not ready for review yet This patch compiles & links, but it still has some subtle bugs related to coordinate handling & fractions, and the new JS put() logic doesn't seem to work reliable yet. It doesn't crash but clearle shows memory corruption (ie. createSVGNumber() returns some weird 9.38E30 values....) I just want to keep the patch here so Rob & others can have a look at it, too.

Nikolas Zimmermann

Comment 2 2006-12-14 15:57:53 PST

Created attachment 11846 [details] Second patch, not ready for review yet Now that JS SVG POD types are fixed, SVGLength can be converted, finally. This patch is complete, compiles & links, but still has some issues with two tests (svg length fraction handling). This is likely, as SVGLength has been completely rewritten :-) I'll try to fix them tomorrow... Attaching patch for early birds..

Rob Buis

Comment 3 2006-12-16 06:23:57 PST

Created attachment 11880 [details] No regressions These changes to Niko's patch should fix the regressions. Cheers, Rob.

Nikolas Zimmermann

Comment 4 2006-12-16 13:01:19 PST

Created attachment 11885 [details] Updated patch Finally a complete patch, with no regressions! :-) Credits go to Rob for fixing the last (very important!) regression, which I was unable to find for quite some days...

Nikolas Zimmermann

Comment 5 2006-12-16 14:27:14 PST

Created attachment 11889 [details] Updated patch II We still found a problem when manually testing, uploading latest patch for Rob & Oliver.

Nikolas Zimmermann

Comment 6 2006-12-17 04:11:49 PST

Created attachment 11893 [details] Final patch All regressions fixed now! :-)

Nikolas Zimmermann

Comment 7 2006-12-17 04:30:48 PST

Landed in r18267.

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