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 #3245

Merged
merged 1 commit into from
Oct 10, 2016
Merged

Color interpolation #3245

merged 1 commit into from
Oct 10, 2016

Conversation

tmcw
Copy link
Contributor

@tmcw tmcw commented Sep 22, 2016

interpolation

Fixes #483

Adds color interpolation in perceptual colorspaces: LAB and HCL. Majority of the work is in mapbox-gl-function, and this will need a spec treatment as well to pass through the option and choose how to represent choices.

Launch Checklist

Pass-through function interpolation property to mapbox-gl-function
for customizable color space interpolation

@jfirebaugh
Copy link
Contributor

I always envisioned that we'd implement this by adding support for "lab(...)" color strings and then interpolating in the color space that the property value is using, thus not requiring an extension of the function syntax.

@tmcw
Copy link
Contributor Author

tmcw commented Sep 23, 2016

I'm flexible and we should consider multiple syntaxes for this.

Interpolating in the syntax given" is, I think, going to be difficult. For instance, we already support hsl() colors thanks to csscolorparser, but hsl is widely used as a more convenient syntax, not a separate color space: I don't believe users will expect hsl colors to interpolate in HSL coordinates and RGB colors to interpolate in RGB.

Additionally, requiring specific color formats for colorspace interpolation may be tricky with respect to default templates. For instance, if a person is using the "Light" style we provide by default, and they want to interpolate between Light's "gray" color and a red, in Mapbox Studio they are encouraged to use exactly the same color - we have a UI for value unification. The template is likely to have RGB or HSL values, not LAB or HCL values. In this case, would they have to convert any colors they want to interpolate?

Finally, the question of interpolating between colors in different spaces would be open - what's the interpolation between RGB and HSL, or HSL and HCL?


Those are the practical concerns I have. Conceptually, I think there's another one: color syntax is often treated as a way of representation: hex and rgb syntaxes are essentially equivalent in power, but one might be more convenient. I think keeping these syntaxes to representation and encoding, not semantic meaning, would make our approach to colors simpler to grasp and work with.

@lucaswoj
Copy link
Contributor

GL JS side looks good to me 👍

I left some feedback on the mapbox-gl-function PR and added a few items to the Launch Checklist above.

Your'e a 🚀, @tmcw.

@jfirebaugh
Copy link
Contributor

I don't believe users will expect hsl colors to interpolate in HSL coordinates and RGB colors to interpolate in RGB

I believe that users who aren't familiar with the existing nuances of interpolation will expect HSL colors to interpolate in HSL space by default. Also, I believe that the the set of users who actively rely on the current interpolation behavior and use HSL coordinates is very likely empty. I would not mind accepting a small behavioral change here.

I didn't quite follow your second point. Something about the fact that default styles have to choose a default space for the colors they use? Wouldn't they have to choose a default for the function property as well? Seems like if it's a given either way that if the default style doesn't do what the user wants, they have to modify it.

Finally, the question of interpolating between colors in different spaces would be open - what's the interpolation between RGB and HSL, or HSL and HCL?

We could define that the color space of the destination value is used. (Also, this question exists independently of whether or not it's a function property -- when performing a style transition, we may need to interpolate between two functions with two different values.)

Making the interpolation behavior reflect the color space of the value seems like the more intuitive behavior to me, and avoids adding yet another dimension to our function syntax, which is already complex and getting more so.

@tmcw
Copy link
Contributor Author

tmcw commented Sep 23, 2016

Something I also forgot to mention is that LAB and HCL colors are almost never actually written as such: d3-color, for instance, the library I'm using these interpolations from, doesn't parse lab or hcl syntax, there's no documented/standardized syntax, and it also isn't supported by csscolorparser. Adding HCL and LAB color syntax would mean that we can no longer say "we support CSS colors" - we'd support a custom superset of CSS colors that includes syntax of our own making.

Or put another way, I can't find any example in any codebase of actually specifying colors as LAB or HCL. They're popular as color spaces, but totally unknown as "encodings" or "syntax". I think inventing a syntax and making users adopt that syntax in order to get nice interpolation would be unexpected and a rough UX.

For the studio side:

2016-09-23 at 12 40 pm

Studio is flexible in its color interpretation: it's not perfect by any means, but it has the basic principle that "the way you edit colors is not necessarily the color space you're in", and there's no toggle for changing the "color space" on the UI. There could be, but I can't imagine a good implementation right now that would accurately frame this feature for users.

Re: choosing existing colors:

2016-09-23 at 12 41 pm

Basically:

  • You can share color values from different parts of the stylesheet
  • And it doesn't make sense to me that sharing a value here means sharing interpolation.

Like: if you have a certain color in your stylesheet, and you want to use it in a curve, you'd have to change it in the stylesheet - describing a road color as LAB or something - in order to share it with a visualization-heavy part of the design. That doesn't seem right to me: colors are values with multiple representations - having their representation change the way they behave in ramps would make cases like this harder to work with.

@jfirebaugh
Copy link
Contributor

That doesn't seem right to me: colors are values with multiple representations - having their representation change the way they behave in ramps would make cases like this harder to work with.

I'd expect that I specify a ramp in a data-driven style property using LAB colors, that the features take on values on that ramp according to LAB interpolation by default. I wouldn't expect that I also have to explicitly set another property to "LAB", or get RGB interpolation of my LAB colors.

@jfirebaugh
Copy link
Contributor

jfirebaugh commented Sep 23, 2016

Went looking to see if prior art matched my intuition, and it does not.

So looks like the consensus is with @tmcw. I withdraw my concerns.

Separate issue, but I do believe that we'll need to implement the alternate interpolations in GLSL as well, for the case that a data-driven style property uses non-RGB interpolation. (See here for existing color interpolation implementation.) And we'll need to either flag for the shader which interpolation is wanted, or adjust the dynamically-generated shader code for the layer appropriately.

@ansis
Copy link
Contributor

ansis commented Sep 23, 2016

For zoom + data driven functions part of the colour interpolation happens in the shaders. To allow interpolation with other colour spaces to work we'd need to add some sort of transformation between colour spaces that happens in the shaders. This could be pretty tricky to incorporate into the shader compilation process because we don't have anything like it yet.

I think it's completely fine to punt on this part. Maybe even never implement it. It's rare for colours to change with zoom and when they do it's probably to a similar colour that works ok for rgb interpolation.

@lucaswoj
Copy link
Contributor

To allow interpolation with other colour spaces to work we'd need to add some sort of transformation between colour spaces that happens in the shaders. This could be pretty tricky to incorporate into the shader compilation process because we don't have anything like it yet.

Our #pragma system was designed to accommodate use cases like this 😄

@lucaswoj
Copy link
Contributor

lucaswoj commented Oct 5, 2016

It seems the consensus is that we can 🚢 this, document the caveat about zoom-and-property functions, and ticket out future support for zoom-and-property functions.

Fire away, @tmcw!

@tmcw
Copy link
Contributor Author

tmcw commented Oct 6, 2016

I've rebased on master, but need to flag that we've currently got some version drift: mapbox-gl-js points to mapbox-gl-style-spec 8.9.0, but neither the git tag nor the package.json bump is pushed to GitHub.

@lucaswoj
Copy link
Contributor

lucaswoj commented Oct 7, 2016

We have resolved the issue mentioned in #3245 (comment) through mapbox/mapbox-gl-style-spec#518. Apologies for the mistake.

@tmcw
Copy link
Contributor Author

tmcw commented Oct 7, 2016

👍 rebased and updated the SHA to point to the style spec. Back up for review.

Copy link
Contributor

@lucaswoj lucaswoj left a comment

Choose a reason for hiding this comment

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

🚢 it!

Pass-through function interpolation property to mapbox-gl-function
for customizable color space 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.

4 participants