Skip to content
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

Color interpolation #26

Merged
merged 1 commit into from
Oct 5, 2016
Merged

Color interpolation #26

merged 1 commit into from
Oct 5, 2016

Conversation

tmcw
Copy link
Contributor

@tmcw tmcw commented Sep 22, 2016

interpolation

Support an interpolation property on functions that allows
users to choose LAB or HCL color interpolation.

@tmcw tmcw mentioned this pull request Sep 22, 2016
6 tasks
Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! My only concern is that conversion from RGB to LAB/HCL and back is relatively expensive and will happen on every function call, even though the stops will have the same values. Can we refactor the code in a way that caches the converted colors so that the work on an actual call is limited to simple interpolation?

@tmcw
Copy link
Contributor Author

tmcw commented Sep 23, 2016

Yep, I think that's very possible. We'd still have to convert back out to RGB, so it wouldn't be zero-cost. I'll work on a second iteration.

@tmcw
Copy link
Contributor Author

tmcw commented Sep 23, 2016

55bdcb3 now translates colors on the way in and allows for an "outputFunction" that translates them on the way out. To do this we need to clone the parameters to avoid mutating it, so a JSON.parse/JSON.stringify loop. Do you think that's okay?

@lucaswoj
Copy link

lucaswoj commented Sep 23, 2016

Thanks for tackling this @tmcw! I like your syntax and implementation 👍

My 2¢: the word interpolation is vague in the context of a generic function library relative to what it does

interpolation is a method of constructing new data points within the range of a discrete set of known data points

vs

interpolation property on functions that allows users to choose LAB or HCL color interpolation.

What do you think about using something like colorInterpolation or colorSpace?

@tmcw
Copy link
Contributor Author

tmcw commented Sep 23, 2016

Yep, I agree. Changed interpolation to colorSpace

@lucaswoj
Copy link

lucaswoj commented Oct 4, 2016

@mourner are you ready to give this a ✅ ?

Support a `colorSpace` property on functions that allows
users to choose LAB or HCL color interpolation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants