implement zoom and data driven functions #2454

ansis · 2016-04-15T20:29:49Z

For zoom+data driven functions, the buffers contain the values for four stops. Each shader reads all four values and uses a uniform, like u_radius_t to interpolate between the stops. Attributes with one component, like radius, are packed into a vec4. Attributes with more than one component are passed as separate glsl attributes.

Since functions may have more than four zoom stops the buffers may not hold all the stops. It adds the four nearest stops to the buffers (two stops before the tile's zoom, two stops after). The uniform used for interpolation accounts for this.

Zoom+data driven functions currently look like this:

"circle-radius": {
    stops: [
        [0, { stops: [[0,  2], [100, 10]], property: 'mapbox' }], 
        [6, { stops: [[0,  20], [100, 100]], property: 'mapbox' }]
    ]   
},  
"circle-color": {
    stops: [
        [0, { stops: [[0, 'red'], [100, 'violet']], property: 'mapbox' }], 
        [6, { stops: [[0, 'blue'], [100, 'green']], property: 'mapbox' }]
    ]   
}

The current implementation does not require that all the data subfunctions have the same stops or the same 'property' value. We may want to add that restriction to make UIs for editing functions simpler. Should these be valid? https://gist.github.com/ansis/625ed5b14e046a09b79a4ddb78cf214f

Does this function definition look ok, or does anyone have other ideas? I'll fix style validation after we settle on a format.

https://github.com/mapbox/mapbox-gl-function/tree/zoom-and-data implements the mapbox-gl-function part of this.

👀 @lucaswoj @jfirebaugh

ansis · 2016-04-15T20:41:40Z

example: http://bl.ocks.org/ansis/raw/00c2af09ba26e3e56c055a9ad24876b4/#1.5/38.9/-77.0

ansis · 2016-04-15T20:53:08Z

@scothis how does the proposed function syntax look from a Studio perspective? what restrictions would help make the UI simpler?

lucaswoj · 2016-04-15T21:00:54Z

I would prefer this simpler but equally expressive syntax:

{
  'circle-radius': {
    stops: [
      [{$zoom: 0, mapbox: 0}, 2],
      [{$zoom: 0, mapbox: 100}, 10],
      [{$zoom: 6, mapbox: 0}, 20],
      [{$zoom: 6, mapbox: 100}, 100],
  }
}

scothis · 2016-04-15T21:17:36Z

I lean toward the syntax @lucaswoj proposes because it follows the current key-value pattern we have now for stops. The difference is that we have a compound key.

However, the syntax does lead me to believe that the property being data-driven can change. For example:

{
  'circle-radius': {
    stops: [
      [{$zoom: 0, landarea: 0}, 2],
      [{$zoom: 0, population: 100}, 10],
      [{$zoom: 6, landarea: 0}, 20],
      [{$zoom: 6, population: 100}, 100],
  }
}

I might propose yet another option that enforces a single property to derive values from:

{
  'circle-radius': {
    property: 'landarea',
    stops: [
      [{$zoom: 0, $value: 0}, 2],
      [{$zoom: 0, $value: 100}, 10],
      [{$zoom: 6, $value: 0}, 20],
      [{$zoom: 6, $value: 100}, 100],
  }
}

Either syntax is workable if we apply the right validation constraints. It will be better to strongly enforce validation constraints early on and slowly relax them as needed than the other way around.

lucaswoj · 2016-04-15T21:20:04Z

I would like to eventually support functions with stops based on multiple properties like:

[{$zoom: 0, population: 100, landarea: 0}, 2]

I agree that we should enforce homogeneous properties programmatically

ansis · 2016-04-15T22:17:10Z

The main constraint would be that you need to have a stop for every pair of zoom and value used in other stops. If you imagine it as a spreadsheet with zoom on one axis and data values on the other axis then every cell needs a style value. In pseudo code:

zoomsUsedInStops = [0, 6];
valuesUsedInStops = [0, 100];
foreach z in zoomsUsedInStops:
    foreach v in valuesUsedInStops:
        assert(stop for { zoom: z, value: v } exists)

There would be exactly zoomsUsedInStops.length * valuesUsedInStops.length stops.

I might propose yet another option that enforces a single property to derive values from:

{
  'circle-radius': {
    property: 'landarea',
    stops: [
      [{$zoom: 0, $value: 0}, 2],
      [{$zoom: 0, $value: 100}, 10],
      [{$zoom: 6, $value: 0}, 20],
      [{$zoom: 6, $value: 100}, 100],
  }
}

This looks good to me. Maybe without the $. And maybe with data instead of value.

I slightly prefer syntaxes with nested arrays because it's closer to how the implementation works. The data functions are evaluated for a property to create a custom zoom function for each feature. The stops can be sorted into nested arrays easily so it's not a big difference.

I would like to eventually support functions with stops based on multiple properties like:
[{$zoom: 0, population: 100, landarea: 0}, 2]

I'm imagining the zoom+data functions as spreadsheets and 3d, 4d, etc spreadsheets are scary. I think the number of stops would get unwieldy really fast if stops support multiple properties. There might be simpler ways can handle these use cases.

I think it makes sense to stick with 2d functions for now.

ansis · 2016-04-16T00:58:52Z

spec and function pull requests:
mapbox/mapbox-gl-function#16
mapbox/mapbox-gl-style-spec#443

lucaswoj · 2016-04-18T16:45:20Z

I think it makes sense to stick with 2d functions for now.

Let's think carefully about this spec so that we never have to break backwards compatibility. This should be orthogonal to our short term vs long term priorities.

I'm imagining the zoom+data functions as spreadsheets and 3d, 4d, etc spreadsheets are scary. I think the number of stops would get unwieldy really fast if stops support multiple properties. There might be simpler ways can handle these use cases.

I'm 👎 on a solution that allows only interpolation in some cases and allows only arbitrary functions in other cases. We should go all-in on interpolation or all-in on arbitrary functions.

@scothis and I talked on Friday about the implications of supporting arbitrary functions in studio. He was in favor of arbitrary functions built atop the existing filter syntax. We could expose our existing interpolation functions as primitives to these functions:

"circle-radius": [
    'exponential',            // function name
    [[0, 10], [20, 100]]     // stops
]

Spitballing additional syntax here:

"circle-radius": [
    'exponential', // function name
    [[0, 10], [20, 100]], // stops
    '$zoom', // input
    2 // base
]

"circle-radius": [
    'exponential', // function name
    [[0, 10], [20, 100]], // stops
    ['*', '$zoom', 'population'], // input
]

"circle-radius": ['*', '$zoom', 'population']

"circle-color": ['if',
    [['>=', 'population', 10000], 'red'],
    ['else', 'blue'],
]

To support this, we'd probably have to move to a model of dynamically writing more shader code.

scothis · 2016-04-18T16:52:43Z

@scothis and I talked on Friday about the implications of supporting arbitrary functions in studio. He was in favor of arbitrary functions built atop the existing filter syntax.

I added a caveat that we will need the ability to have and detect pre-built functions in Studio. Most users should not need to drop to the low level syntax in order to do basic interpolation.

lucaswoj · 2016-04-18T18:17:56Z

I added a caveat that we will need the ability to have and detect pre-built functions in Studio. Most users should not need to drop to the low level syntax in order to do basic interpolation.

To clarify what this means:

we support abitrary functions in the spec
we provide point & click UIs to create and edit common function patterns
we provide a raw JSON editor in Studio to create and edit complex functions

ansis · 2016-04-18T19:40:24Z

I'm strongly 👎 on arbitrary functions that mix zoom and data inputs.

The current zoom+data implementation is designed so that the data part of the function can be evaluated separately from the zoom part. This let's us evaluate the data part in -js when creating the buffers and the zoom part in the shaders on every frame. The current limitations allow for highly optimized implementation, both in terms of shader complexity and buffer size.

For example, to evaluate "circle-radius": ['*', '$zoom', 'population'] in a shader you'd need to store population in the buffers. With what type? what precision? How do you deal with strings? And you'd also need complex code generation that you can't do with just the preprocessor language.

I'm 👎 on a solution that allows only interpolation in some cases and allows only arbitrary functions in other cases.

I'm against 3d, 4d, etc interpolation because I can't think of a case where n-dimensional interpolation is what you actually want. I also think that in order for the interpolation to be unambiguous you would need to provide an exponential number of stops, which seems really unwieldy.

My suggestion was to use an expression of data properties as the input to the scale, instead of just a direct data property. There would be no conditionals or comparisons. Just math operators that combine data values. I don't think we need to do this yet.

We should go all-in on interpolation or all-in on arbitrary functions.

We've already committed to scales/interpolation for data driven functions. It makes sense zoom+data functions work the same way. This is what's currently implemented.

The current implementation covers a lot of use cases and fits in with the existing spec. Any large changes that would be breaking for these zoom+data functions would likely also be breaking for the existing separate zoom functions and data functions. I think we should go with something similar to what's implemented here. It might be helpful to voice about this today.

lucaswoj · 2016-04-18T20:41:33Z

I agree with all of your points above ☝️

I'm not particularly tied to any particular syntax specification for this feature but I have a couple requirements that aren't met by the nested function syntax:

backwards-compatible extensibility for more powerful function features
style JSON readability & maintainability
encapsulation of implementation details
conceptual simplicity

Let's voice on this later today and see if we can figure out a function syntax specification that meets all of our requirements.

ansis · 2016-04-18T20:59:19Z

the nested function syntax

Sorry, I never mentioned that I switched the syntax to the one proposed by you and @scothis. It now looks like this:

mapbox-gl-js/debug/site.js

Lines 53 to 69 in 8abd688

    
           "circle-radius": { 
        
               property: "mapbox", 
        
               stops: [ 
        
                   [{ zoom: 0, data: 0 }, 2], 
        
                   [{ zoom: 0, data: 100 }, 10], 
        
                   [{ zoom: 6, data: 0 }, 20], 
        
                   [{ zoom: 6, data: 100 }, 100] 
        
               ] 
        
           }, 
        
           "circle-color": { 
        
               property: "mapbox", 
        
               stops: [ 
        
                   [{ zoom: 0, data: 0 }, 'red'], 
        
                   [{ zoom: 0, data: 100 }, 'violet'], 
        
                   [{ zoom: 6, data: 0 }, 'blue'], 
        
                   [{ zoom: 6, data: 100 }, 'green'] 
        
               ]

lucaswoj · 2016-04-18T22:51:23Z

js/data/bucket.js


        for (var c = 0; c < bucket.childLayers.length; c++) {
            var childLayer = bucket.childLayers[c];
-            layerPaintAttributes[childLayer.id] = { enabled: [], disabled: [] };
+            layerPaintAttributes[childLayer.id] = { enabled: [], uniforms: [] };


Would it make more sense to stick to either a) enabled/disabled or b) attributes/uniforms?

I'll switch this to attributes / uniforms. Zoom + data functions create uniforms

lucaswoj · 2016-04-19T18:30:13Z

All the code in this PR looks great!

Next Steps

merge support for zoom + data driven functions mapbox-gl-function#16
merge update validation for new data + zoom driven functions mapbox-gl-style-spec#443
release new mapbox-gl-function version
release new mapbox-gl-style-spec version
add mapbox-gl-test-suite tests
update package.json with new mapbox-gl-function and mapbox-gl-style-spec versions & new mapbox-gl-test-suite SHA

mourner · 2016-04-20T04:55:05Z

shaders/circle.vertex.glsl

+        return mix(value2, value3, t - 2.0);
+    }
+}
+


If we want to bring this to all other shaders, it will start growing very complex and repetitive... Perhaps we should consider a GLSL build system like https://github.com/stackgl/glslify

We could alternately use a simpler approach than glslify ref 50f2cf5#commitcomment-16958568

We could make a zoom_functions.glsl and prepend it to all vertex shaders.

dpieri · 2016-05-01T19:17:36Z

How do I start using this? Will this be implemented in Studio? Documentation on how to do this via style JSON?

lucaswoj · 2016-05-02T17:44:06Z

@dpieri This feature will be implemented in Studio once it has been expanded to all paint properties and is usable across both GL JS and GL Native platforms. Think of this like a dev preview for now.

lucaswoj · 2016-05-02T17:45:34Z

The syntax we eventually decided upon is:

{
  'circle-radius': {
    property: 'landarea',
    stops: [
      [{zoom: 0, value: 0}, 2],
      [{zoom: 0, value: 100}, 10],
      [{zoom: 6, value: 0}, 20],
      [{zoom: 6, value: 100}, 100],
  }
}

ansis force-pushed the zoom-and-data branch 2 times, most recently from bb9aa63 to 8abd688 Compare April 16, 2016 00:57

lucaswoj reviewed Apr 18, 2016
View reviewed changes

ansis force-pushed the zoom-and-data branch from 0f51d48 to ce20539 Compare April 19, 2016 17:51

ansis force-pushed the zoom-and-data branch from ce20539 to 3d8a1c7 Compare April 19, 2016 22:44

ansis added 2 commits April 19, 2016 16:06

separate layout and paint attribute definitions

7e38b63

implement zoom and data driven functions

b4ce4b7

ansis force-pushed the zoom-and-data branch from 3d8a1c7 to b4ce4b7 Compare April 19, 2016 23:06

ansis merged commit b4ce4b7 into master Apr 19, 2016

mourner reviewed Apr 20, 2016
View reviewed changes

ansis deleted the zoom-and-data branch April 21, 2016 19:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement zoom and data driven functions #2454

implement zoom and data driven functions #2454

ansis commented Apr 15, 2016 •

edited

Loading

ansis commented Apr 15, 2016

ansis commented Apr 15, 2016

lucaswoj commented Apr 15, 2016

scothis commented Apr 15, 2016

lucaswoj commented Apr 15, 2016 •

edited

Loading

ansis commented Apr 15, 2016

ansis commented Apr 16, 2016

lucaswoj commented Apr 18, 2016 •

edited

Loading

scothis commented Apr 18, 2016

lucaswoj commented Apr 18, 2016

ansis commented Apr 18, 2016 •

edited

Loading

lucaswoj commented Apr 18, 2016

ansis commented Apr 18, 2016

lucaswoj Apr 18, 2016

ansis Apr 19, 2016

lucaswoj commented Apr 19, 2016 •

edited by ansis

Loading

mourner Apr 20, 2016

lucaswoj Apr 20, 2016

ansis Apr 20, 2016

dpieri commented May 1, 2016

lucaswoj commented May 2, 2016

lucaswoj commented May 2, 2016

implement zoom and data driven functions #2454

implement zoom and data driven functions #2454

Conversation

ansis commented Apr 15, 2016 • edited Loading

ansis commented Apr 15, 2016

ansis commented Apr 15, 2016

lucaswoj commented Apr 15, 2016

scothis commented Apr 15, 2016

lucaswoj commented Apr 15, 2016 • edited Loading

ansis commented Apr 15, 2016

ansis commented Apr 16, 2016

lucaswoj commented Apr 18, 2016 • edited Loading

scothis commented Apr 18, 2016

lucaswoj commented Apr 18, 2016

ansis commented Apr 18, 2016 • edited Loading

lucaswoj commented Apr 18, 2016

ansis commented Apr 18, 2016

lucaswoj Apr 18, 2016

Choose a reason for hiding this comment

ansis Apr 19, 2016

Choose a reason for hiding this comment

lucaswoj commented Apr 19, 2016 • edited by ansis Loading

Next Steps

mourner Apr 20, 2016

Choose a reason for hiding this comment

lucaswoj Apr 20, 2016

Choose a reason for hiding this comment

ansis Apr 20, 2016

Choose a reason for hiding this comment

dpieri commented May 1, 2016

lucaswoj commented May 2, 2016

lucaswoj commented May 2, 2016

ansis commented Apr 15, 2016 •

edited

Loading

lucaswoj commented Apr 15, 2016 •

edited

Loading

lucaswoj commented Apr 18, 2016 •

edited

Loading

ansis commented Apr 18, 2016 •

edited

Loading

lucaswoj commented Apr 19, 2016 •

edited by ansis

Loading