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

Convert functions to expressions; add interpolation color spaces #5812

Closed
wants to merge 4 commits into from

Conversation

jfirebaugh
Copy link
Contributor

@jfirebaugh jfirebaugh commented Dec 5, 2017

Convert functions to expressions for evaluation (reverses #5379). Adds support for interpolate-hcl and interpolate-lab expression operators to replace the colorSpace function property (fixes #5326). Switches to shortest-path interpolation for HCL (fixes #5811).

Evaluation of converted expressions is slightly faster:
https://bl.ocks.org/anonymous/raw/ea7442aff28940abb9a358501aa9067b/

However, conversion+creation is slower. I may try to reimplement conversion to directly construct expression instances, rather than converting to a JSON representation.
https://bl.ocks.org/anonymous/raw/63b5d4b704f55e8e50dd5871297a1490/

What's still not working currently is the default property for functions. @anandthakker, I noticed the function tests for this were skipped. It looks like the skips were added as part of #4777. Do you recall if we ever came up with a plan for how to convert defaults into expression syntax?

@anandthakker
Copy link
Contributor

Do you recall if we ever came up with a plan for how to convert defaults into expression syntax?

I think now that we're handling spec-defined defaults outside of expression evaluation (i.e. in a catch block in the error-handling wrapper function), it would probably be simplest to handle user-defined defaults that way, too. Including functions' defaulting semantics in the converted expression would be possible, but undesirably complicated.

@jfirebaugh jfirebaugh mentioned this pull request Dec 5, 2017
@jfirebaugh jfirebaugh force-pushed the interpolate-color-spaces branch from 9b52b89 to 04ceada Compare December 6, 2017 16:05
Add support for interpolate-hcl and interpolate-lab expression operators to replace the colorSpace function property.
@jfirebaugh jfirebaugh force-pushed the interpolate-color-spaces branch from 04ceada to 98b2542 Compare December 8, 2017 18:30
@jfirebaugh
Copy link
Contributor Author

Shelving this for now. Conversion to JSON intermediate format isn't fast enough, and it turned out to be difficult and ugly to implement direct conversion to Expression subclasses.

@jfirebaugh jfirebaugh closed this Dec 11, 2017
@jfirebaugh jfirebaugh deleted the interpolate-color-spaces branch February 22, 2018 20:22
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.

Should hcl interpolation use shortest path? Implement LAB/HSL color interpolation for expressions
2 participants