RESOLVED FIXED 11813
SVGLength needs a rewrite 
https://bugs.webkit.org/show_bug.cgi?id=11813
Summary SVGLength needs a rewrite
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
DetailsFormatted DiffDiff
Second patch, not ready for review yet (156.11 KB, patch)
2006-12-14 15:57 PST, Nikolas Zimmermann 		no flags
DetailsFormatted DiffDiff
No regressions (157.60 KB, patch)
2006-12-16 06:23 PST, Rob Buis 		no flags
DetailsFormatted DiffDiff
Updated patch (208.58 KB, patch)
2006-12-16 13:01 PST, Nikolas Zimmermann 		no flags
DetailsFormatted DiffDiff
Updated patch II (209.08 KB, patch)
2006-12-16 14:27 PST, Nikolas Zimmermann 		no flags
DetailsFormatted DiffDiff
Final patch (211.19 KB, patch)
2006-12-17 04:11 PST, Nikolas Zimmermann 		rwlbuis: review+
DetailsFormatted DiffDiff
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.