-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hsl / hsla #28
Comments
I've got initial hsl(a) support plus some very basic tests working, added to context2d.js and canvas.test.js thus: RandomEtc@47032c1 Branch here: https://github.com/RandomEtc/node-canvas/tree/hsl At least in the short term, are you happy with the color parsing continuing to happen in javascript? If so, and the above looks good, do I need to do a pull request or can you take it from here? |
awesome thanks man. yeah I am fine with it being js, it is cached so c would not be much faster. In fact I re-wrote it in c, and it is of couse way faster than js, but with caching involved there is little difference |
Great! I see you have benchmark and optimize on your issues list too so if this is really hurting someone (drawing random color wheels or something?) then it can always be revisited. Thanks for an awesome library! Is there anywhere apart from these issue threads that we can discuss your plans? I'd love to help out more but I started hacking Image support in a day before you added it and that knocked the wind out of my sails a little bit :) |
for sure, I think that might be one of the only real use-cases for a faster parser. or the "ray.js" example I have. id like to keep them in issues for now, even if it is a feature request |
merged, thanks! |
Hey, looks like this went away with the move to a C color parser? Is it something you're hoping to add back into rgba_from_string in https://github.com/LearnBoost/node-canvas/blob/master/src/color.cc or should I move it into my own js utils? |
yeah sorry about that, I would be happy to get it in there, just had most of this done already and wanted to get it in |
No description provided.
The text was updated successfully, but these errors were encountered: