[Webkit-unassigned] [Bug 12171] Remove "points" hack from SVGPolyElement
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue May 20 14:04:50 PDT 2008
http://bugs.webkit.org/show_bug.cgi?id=12171
eric at webkit.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #21215|review? |review-
Flag| |
------- Comment #4 from eric at webkit.org 2008-05-20 14:04 PDT -------
(From update of attachment 21215)
A few comments:
1. Do the tests pass already? If so, you should feel free to land them even
before you complete this patch as is.
2. Ideally the tests would use the js-pre stuff in fast/js/resources We try
to use those common "shouldBe" functions, etc. in all new tests. I know that
could get kinda tricky with these being SVG tests. I eventually had planned to
augment the JS testing system to allow js-only svg tests. (where they would
correctly use an SVG template instead of an html template).
3. I'm not sure it's OK to make Attribute::value virtual. You should ask
hyatt or mjs. I have performance concerns about such a basic function being
virtual.
+ RefPtr<SVGPathSeg> seg = path->pathSegList()->getItem(i, ec);
+ d += seg->toString();
+ if (++i == len) break;
+= is a slow way to build strings. I think we have a Vector<UChar> way to
build strings these days. Or possibly a StringBuilder class, not sure.
Also, the if/break line needs a return before the break.
Again, slow:
+ points += String::format("%.6lg %.6lg", p.x(), p.y());
+ if (++i == len) break;
You could preallocate a buffer large enough for the entire string pretty
easily.
r- for the perf concerns.
--
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
More information about the webkit-unassigned
mailing list