Currently, parseColor() creates a CSSParser every time it's called. This is not always necessary. Also, the fast path for #RRGGBB (or #RGB) colors doesn't work for color strings actually starting with '#'
Created attachment 61214 [details] Proposed patch
Comment on attachment 61214 [details] Proposed patch Do you have perf measurements for this? There was a canvas perf test from hixie that the #color optimisation was made for, are we sure it doesn't work? or was it regressed at some point?
(In reply to comment #2) > was it regressed at some point? This regressed with https://bugs.webkit.org/show_bug.cgi?id=38845 where I unintentionally removed your optimization from https://bugs.webkit.org/show_bug.cgi?id=3781
Hixies test: http://hixie.ch/tests/adhoc/perf/video/002.html Without this patch (current trunk): Elapsed wall-clock time: 1325ms (ideal: 640ms). Elapsed non-idle time: 685ms (ideal: 0ms). Speed: 11.32fps (ideal: 25.00fps). With this patch: Elapsed wall-clock time: 1011ms (ideal: 640ms). Elapsed non-idle time: 371ms (ideal: 0ms). Speed: 14.84fps (ideal: 25.00fps).
Comment on attachment 61214 [details] Proposed patch I would prefer it if you had a separate patch to revert the #color parsing regression
(In reply to comment #5) > I would prefer it if you had a separate patch to revert the #color parsing regression I'm not sure we want to revert that. Condensing the color parsing to one call (CSSParser::parseColor()) from the Canvas side is still desirable IMO. Better to fix parseColor() to take the appropriate fast-path instead of doing the Color(string) trick in a bunch of places.
Created attachment 61243 [details] Proposed patch v2 Updated patch to also avoid allocating a new string when getting the RRGGBB (or RGB) part of a '#' color. Elapsed wall-clock time: 996ms (ideal: 640ms). Elapsed non-idle time: 356ms (ideal: 0ms). Speed: 15.06fps (ideal: 25.00fps).
Comment on attachment 61243 [details] Proposed patch v2 Clearing flags on attachment: 61243 Committed r63099: <http://trac.webkit.org/changeset/63099>
All reviewed patches have been landed. Closing bug.
Comment on attachment 61243 [details] Proposed patch v2 Adding an include of PlatformString.h to Color.h is not correct. Why was that done?
(In reply to comment #10) > Adding an include of PlatformString.h to Color.h is not correct. Why was that done? For UChar. What is the correct way?
wtf/unicode/Unicode.h is how you get UChar.
Right, thanks. Filed bug 42109 with a fix.