Skip to content
This repository has been archived by the owner on Apr 10, 2018. It is now read-only.

Support color operation functions #38

Closed
nickidlugash opened this issue Jun 13, 2014 · 18 comments
Closed

Support color operation functions #38

nickidlugash opened this issue Jun 13, 2014 · 18 comments
Milestone

Comments

@nickidlugash
Copy link
Contributor

Can we support color operation functions similar to the LESS-type color functions supported in cartoCSS?

E.g.
lighten(constant, %)
darken(constant, %)
desaturate(constant, %)
spin(constant, %)
mix(constant1, constant2, %)
fadeout(constant, %)
fadein(constant, %)

These are useful when you want colors to maintain a relationship with another color – e.g., landuse or text halo colors to the land color.

@edenh
Copy link
Contributor

edenh commented Jun 20, 2014

Syntax idea. I assume we need to support multiple operations.

{
    "text-color": {
        "value": "#0000FF",
        "operation": [["lighten", 10], ["spin", 35]]
    },
    "text-halo-color": {
        "value": "white",
        "operation": ["mix", "#FFCCFF", 50]
    }
}

@jfirebaugh
Copy link
Contributor

@nickidlugash What's your current sense on the importance of this feature?

@nickidlugash
Copy link
Contributor Author

@jfirebaugh My current sense is that it is still very important to be able to transform colors (specifically color constants), but I don't have a strong feeling as to whether it's better to implement this in the style spec or only in the editor. @edenh @tmcw @lbud ?

@lbud
Copy link
Contributor

lbud commented Jan 22, 2015

If I recall correctly trying to implement this in the editor was a beast (in that compiling and decompiling transforms written into a style in the editor would never really work). I would think having these in the spec would be a much cleaner option…

@nickidlugash
Copy link
Contributor Author

I would think having these in the spec would be a much cleaner option…

@jfirebaugh @tmcw @edenh thoughts?

@edenh
Copy link
Contributor

edenh commented Jan 22, 2015

👍 what @lbud said.

@tmcw tmcw closed this as completed in 06747a4 Mar 12, 2015
@jfirebaugh jfirebaugh reopened this Mar 12, 2015
@lbud
Copy link
Contributor

lbud commented Mar 16, 2015

Per chat with @nickidlugash we're comfortable going with the syntax @edenh suggested above with the exception of "mix", which we're leaning toward doing as follows:

    "text-halo-color": {
        "value": ["white", "#FFCCFF"],
        "operation": [["mix", 50]]
    }

so as to accommodate the idea that mix is more about mixing two colors together rather than an idea of one being primary and one being secondary.
cc @quinnvd

edit: we'll also be enforcing always using an array of arrays for operation.

@jfirebaugh
Copy link
Contributor

One important feature that the syntax should support is composition of operations. For example, think about how you would express a composed operations like lighten(20, mix(50, red, green)) and mix(50, red, lighten(20, green)). You might want to take inspiration from the filter syntax.

@lbud
Copy link
Contributor

lbud commented Mar 16, 2015

@jfirebaugh interesting. If we want to support that my inclination would just be to nest as much as desired, right? Like

    "text-halo-color": {
        "value": {
            "value": ["red", "green"],
            "operation": ["mix", 50]
        },
        "operation": ["lighten", 20]
    }

— so we'd drop the concept of arrays of arrays of operations, as they could all be expressed like this. It's a little less nice but I guess it's the only way to represent mix right (unless we wanted to do something like "operation": ["mix", "#FFCCFF", 50] after all)

@lbud
Copy link
Contributor

lbud commented Mar 16, 2015

(similarly:)

    "text-halo-color": {
        "value": [{
            "value": "red",
            "operation": ["lighten", 20]
        }, "green"],
        "operation": ["mix", 50]
    }

@jfirebaugh
Copy link
Contributor

I'm wondering if we need named properties versus a more compact array-based syntax:

"text-halo-color": ["lighten", 20, ["mix", 50, "red", "green"]]
"text-halo-color": ["mix", 50, ["lighten", 20, "red"], "green"]

@mourner
Copy link
Member

mourner commented Mar 16, 2015

I like the array syntax. We also want to be consistent with other nested constructs like filters.

@lbud
Copy link
Contributor

lbud commented Mar 16, 2015

Cool — going down that (array) path now.
(@mourner no surprise there :) )

@mourner
Copy link
Member

mourner commented Mar 20, 2015

Just wondering — would it be better to have the syntax [op, color, value] instead of [op, value, color]? This would be more consistent with the filter syntax and a bit easier to understand. E.g.:

"text-halo-color": ["lighten", ["mix", "red", "green", 50], 20]
"text-halo-color": ["mix", ["lighten", "red", 20], "green", 50]

@lbud
Copy link
Contributor

lbud commented Mar 20, 2015

@mourner hm...since mix takes two colors and all the rest take one color, I think I'd prefer it [op, value, color, ?color] so that the value is always reliably at op[1] (especially if for some reason this syntax gets extended to use more colors later…). (I also think as-is it's more consistent with filter syntax because of this.) Willing to defer to alternate consensus though if others would rather switch it.

@lbud
Copy link
Contributor

lbud commented Mar 26, 2015

Status on this implementation:

#264 (comment) means some refactors to -spec (#271), -gl-js (mapbox/mapbox-gl-js#1111), -native (https://github.com/mapbox/mapbox-gl-native/tree/color_operations), and mapbox-gl-color-operations (https://github.com/mapbox/mapbox-gl-color-operations). Other than that, this is mostly close:

  • gl-js: @jfirebaugh and I talked about the possibility of refactoring the constant resolution so that it happens within the confines of color-op resolution — its modularity makes that a bit weird but it is a possibility.
  • native: this is most of the remaining work as it just doesn't resolve constants yet and needs tests

@jfirebaugh jfirebaugh added this to the v8 milestone Jul 4, 2015
@jfirebaugh
Copy link
Contributor

So this is in v8 and implemented. Is it actually going to be used? How will the app expose this? Is it still valuable in a constantless world?

@jfirebaugh
Copy link
Contributor

#308

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants