functions for pattern properties #976

ansis · 2015-02-03T00:08:17Z

This implements functions for line, fill and background patterns. They look like this:

"fill-image": {
    "stops": [[0, "waves-small"], [10, "waves-medium"], [15, "waves-large"]],
    "duration": 300
}

Duration is optional.

todo:

style spec changes so that linting works allow functions for everything mapbox-gl-style-spec#243
add more specific types so parsing doesn't need to depend on property names. This could be done later
native https://github.com/mapbox/mapbox-gl-native/tree/pattern-functions

jfirebaugh · 2015-02-03T00:14:23Z

js/style/style_declaration.js

    return type === 'color' ? parseColor(value) :
        type === 'number' ? parseNumber(value) :
        type === 'boolean' ? Boolean(value) :
        type === 'image' ? String(value) :
+        type === 'string' && (prop === 'fill-image' || prop === 'line-image' || prop === 'background-image') ? parsePattern(value) :


Can we do this in a way that's not hard-coded to particular property names? Either support non-interpolated functions for all properties, or look to the style spec to tell whether functions are supported for a particular property.

Yeah, this is one of the todo's. I was thinking that would need to create a separate type to handle these, but now that we have layout property functions I think I can implement them for the other string properties.

ansis · 2015-02-03T03:15:47Z

js/style/style_declaration.js

    return type === 'color' ? parseColor(value) :
        type === 'number' ? parseNumber(value) :
        type === 'boolean' ? Boolean(value) :
        type === 'image' ? String(value) :
-        type === 'string' ? String(value) :
+        type === 'string' ? parsePattern(value) :
+        type === 'array' && prop === 'line-dasharray' ? parsePattern(value) :


@jfirebaugh I added functions for all the other string properties, but I think we'll need to make dasharrays a different type than other number arrays. Other number arrays would use interpolated stops functions instead of faded stops functions.

We should distinguish them in the style spec. "function": "interpolated" vs "function": "piecewise-constant" or something.

jfirebaugh · 2015-02-03T20:04:50Z

Is there any reason not to support non-interpolated functions for every remaining property (i.e. enums and booleans too)?

ansis · 2015-02-03T22:49:59Z

Is there any reason not to support non-interpolated functions for every remaining property (i.e. enums and booleans too)?

Nope. Everything can be a function.

How about this:

There are two types of functions:

interpolated
piecewise

Colors, numbers, arrays and paint string properties can all be either interpolated or piecewise.
They all default to interpolated except for *-image and line-dasharray.
Enums, booleans and layout string properties are always piecewise.

Non-default types can be specified like this:

"fill-image": {
    "type": "interpolated",
    "stops": []
}

How would this look in the spec?
Would "function": "interpolated" specify the default function type?
Should the spec specify that enums, booleans and layout strings are only piecewise, or is it fine if that restriction is just hardcoded?

jfirebaugh · 2015-02-03T23:20:47Z

Is is necessary / useful to give style authors a choice between interpolated and piecewise for properties that could support both? I'd rather just spec that each property is one or the other.

ansis · 2015-02-03T23:27:34Z

Is is necessary / useful to give style authors a choice between interpolated and piecewise for properties that could support both? I'd rather just spec that each property is one or the other.

Probably not that useful. I'll just stick to one. thanks

ansis · 2015-02-04T00:48:32Z

There are now three types of functions:

interpolated - what used to be the old stops functions
piecewise-constant - returns the nearest step with stepZ <= z
transitioned-piecewise-constant - like piecewise-constant, except it transitions between values (for patterns, dasharrays).

By default everything is a piecewise-constant. If a function is not piecewise-constant it's function type is specified with "function": "interpolated" or "function": "transitioned-piecewise-constant" in the spec. mapbox/mapbox-gl-style-spec#243

@jfirebaugh how does this seem?

jfirebaugh · 2015-02-04T00:54:53Z

The spec uses "transition": true for other transitionable properties -- can we use that to mark the transitioned piecewise-constant properties instead of having a third function type?

Relatedly, instead of:

"fill-image": {
    "stops": [[0, "waves-small"], [10, "waves-medium"], [15, "waves-large"]],
    "duration": 300
}

it should be like other transitionable properties:

"fill-image": {
    "stops": [[0, "waves-small"], [10, "waves-medium"], [15, "waves-large"]]
},
"fill-image-transition": {
    "duration": 300
}

ansis · 2015-02-04T01:08:13Z

I avoided this approach earlier because I thought it would be weird that "*-transition" would affect transitions between classes for some properties and affect zoom transitions for others. Is it weird that two different kinds of things are specified the same way? or maybe they are less different than I am imagining

jfirebaugh · 2015-02-04T01:11:59Z

Interesting point. I think that *-transition should affect between-class transitions though. Like if you switched between night and day classes and both had function-valued fill-image properties, it would ideally fade between the two, right?

ansis · 2015-02-04T01:30:03Z

Yes, it could affect both. I'm not sure that if you change the duration for one you'll want the duration for the other to be the same, but maybe changing either is rare enough that it doesn't matter.

I'll make the change. If it's actually a problem we can change it later.

ansis · 2015-02-04T03:08:05Z

This should now be all implemented.

Everything is a function. By default properties are piecewise-constant functions, otherwise they have "function": "interpolated" in the spec.

Transitions are set using *-transition. This value is also used for class transitions.

jfirebaugh · 2015-02-04T20:24:48Z

There are a number of failures in the test-suite:

# fill-translate function
not ok 35 (unnamed assert)
...
# icon-translate function
not ok 80 (unnamed assert)
...
# line-dasharray literal
not ok 97 (unnamed assert)
...
# line-translate function
not ok 121 (unnamed assert)
...
# raster-brightness function

/Users/john/Development/mapbox-gl-js/node_modules/tape/index.js:75
        throw err
              ^
TypeError: Expected uniform1f(WebGLUniformLocation location, number x)
    at WebGLRenderingContext.uniform1f (/Users/john/Development/mapbox-gl-js/node_modules/gl/webgl.js:850:11)
    at Object.drawRaster [as raster] (/Users/john/Development/mapbox-gl-js/js/render/draw_raster.js:17:8)
    at GLPainter.drawLayers (/Users/john/Development/mapbox-gl-js/js/render/painter.js:256:25)
    at GLPainter.drawTile (/Users/john/Development/mapbox-gl-js/js/render/painter.js:242:10)
    at Object.exports._renderTiles [as render] (/Users/john/Development/mapbox-gl-js/js/source/source.js:58:17)
    at GLPainter.render (/Users/john/Development/mapbox-gl-js/js/render/painter.js:232:20)
    at util.extend.render (/Users/john/Development/mapbox-gl-js/js/ui/map.js:329:22)
    at Timer.listOnTimeout [as ontimeout] (timers.js:112:15)

ansis · 2015-02-04T21:28:08Z

The implementation now wants

"raster-brightness": {
    "stops": [[16, [0.5, 0.6]], [17, [0.5, 0.4]]]
}

instead of

"raster-brightness": [{
    "stops": [[16, 0.5], [17, 0.5]]
  }, {
    "stops": [[16, 0.6], [17, 0.4]]
  }]

This was briefly discussed at mapbox/mapbox-gl-style-spec#237 (comment) but wasn't decisively decided.

Should I work on migrations, or switch the implementation back?

ansis · 2015-02-05T19:33:21Z

migrations for mapbox/mapbox-gl-style-spec#237 changes:

cutting-room-floor/mapbox-gl-style-lint#41
mapbox/mapbox-gl-test-suite#14
mapbox/mapbox-gl-styles#31

ansis · 2015-02-06T02:50:08Z

Implemented in -native in mapbox/mapbox-gl-native#821

I think this is ready now

mapbox/mapbox-gl-style-spec#237

jfirebaugh mentioned this pull request Feb 3, 2015

v7 mapbox/mapbox-gl-style-spec#238

Merged

jfirebaugh reviewed Feb 3, 2015
View reviewed changes

This was referenced Feb 3, 2015

functions for pattern properties mapbox/mapbox-gl-native#804

Closed

allow functions for everything mapbox/mapbox-gl-style-spec#243

Merged

ansis reviewed Feb 3, 2015
View reviewed changes

jfirebaugh mentioned this pull request Feb 3, 2015

update v7 styles to use layout property functions mapbox/mapbox-gl-styles#30

Closed

jfirebaugh mentioned this pull request Feb 5, 2015

functions without interpolation mapbox/mapbox-gl-style-spec#237

Closed

ansis mentioned this pull request Feb 6, 2015

functions for everything mapbox/mapbox-gl-native#821

Merged

Function support for all properties

ba3899f

mapbox/mapbox-gl-style-spec#237

jfirebaugh force-pushed the pattern-functions branch from 74253d6 to ba3899f Compare February 6, 2015 22:37

jfirebaugh merged commit ba3899f into master Feb 6, 2015

jfirebaugh deleted the pattern-functions branch February 6, 2015 22:48

nickidlugash mentioned this pull request Mar 10, 2015

Implement transitions for all (or more) piecewise functions? mapbox/mapbox-gl-style-spec#254

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

functions for pattern properties #976

functions for pattern properties #976

ansis commented Feb 3, 2015

jfirebaugh Feb 3, 2015

ansis Feb 3, 2015

ansis Feb 3, 2015

jfirebaugh Feb 3, 2015

jfirebaugh commented Feb 3, 2015

ansis commented Feb 3, 2015

jfirebaugh commented Feb 3, 2015

ansis commented Feb 3, 2015

ansis commented Feb 4, 2015

jfirebaugh commented Feb 4, 2015

ansis commented Feb 4, 2015

jfirebaugh commented Feb 4, 2015

ansis commented Feb 4, 2015

ansis commented Feb 4, 2015

jfirebaugh commented Feb 4, 2015

ansis commented Feb 4, 2015

ansis commented Feb 5, 2015

ansis commented Feb 6, 2015

functions for pattern properties #976

functions for pattern properties #976

Conversation

ansis commented Feb 3, 2015

jfirebaugh Feb 3, 2015

Choose a reason for hiding this comment

ansis Feb 3, 2015

Choose a reason for hiding this comment

ansis Feb 3, 2015

Choose a reason for hiding this comment

jfirebaugh Feb 3, 2015

Choose a reason for hiding this comment

jfirebaugh commented Feb 3, 2015

ansis commented Feb 3, 2015

jfirebaugh commented Feb 3, 2015

ansis commented Feb 3, 2015

ansis commented Feb 4, 2015

jfirebaugh commented Feb 4, 2015

ansis commented Feb 4, 2015

jfirebaugh commented Feb 4, 2015

ansis commented Feb 4, 2015

ansis commented Feb 4, 2015

jfirebaugh commented Feb 4, 2015

ansis commented Feb 4, 2015

ansis commented Feb 5, 2015

ansis commented Feb 6, 2015