Add support for color string in JSON array format #189
Conversation
src/util/parse_color.ts
Outdated
| clamp(+b / 255, 0, 1), | ||
| clamp(isNaN(a) ? 1 : +a, 0, 1)]; | ||
| } | ||
| } catch(ex) { |
There was a problem hiding this comment.
My understanding previously was that try/catch was slower. But looks like modern browsers have improved and now I don't see any perf issue with using try/catch (tested on Edge/Chrome/Firefox). Adding try/catch here allow to catch all invalid JSON string.
There was a problem hiding this comment.
I think try-catch is slower compared to ifs, in case you go through the catch part of the try-catch, at least is what I was taught long time ago... :-)
There was a problem hiding this comment.
Yes, if code goes inside catch then its quite slow. But for any production style, it should never go into catch block as it will be a invalid color and thus production code does not suffer from any performance degradation in this case. Overhead of adding various checks to validate JSON format will eat away any performance benefits apart from adding code complications.
|
Can you post benchmark results against regular color parsing ( |
Here are the results using the code used in this PR from my home machine. RGB, RGBA and HSL is 50-65% slower then JSON array (depending on machine). https://jsbench.me/84li0k8nkg/1
|
|
As per discussion in #187, holding on to this PR (Moving to draft). Will first impliment color caching which will help improve color parsing by about 50%. If there is still need to further improve, will come back to this PR. |
|
@pramilk I'm closing this PR for now to reduce clutter. |
Please see discussion #187 for details
Launch Checklist
CHANGELOG.mdunder the## mainsection.