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

Stylesheet restructure #356

Closed
edenh opened this issue Mar 24, 2014 · 66 comments
Closed

Stylesheet restructure #356

edenh opened this issue Mar 24, 2014 · 66 comments

Comments

@edenh
Copy link
Contributor

edenh commented Mar 24, 2014

After working with the stylesheet structure a bit, I think there are a few improvements we can make without losing configuration flexibility.

This is my personal wishlist, which draws from many of the previously discussed ideas in #244, #279, and #276. I'm not sure if this should be the content for a preprocessor, a replacement for the current stylesheet, or a step towards a styling language.

Here is an example to illustrate the idea:

{
    buckets: {
        'streets': {
            'poi': {
                layer: 'poi_label',
                point: true,
                children: {
                    'poi_park': {
                        filter: { 'maki': 'park' }
                        point: true,
                        text: {
                            textField: 'name',
                            path: 'curve',
                            font: 'Open Sans',
                            fontSize: 18
                        }
                    },
                    'poi_other': {
                        filter: { 'maki': ['airport', 'bus'] }
                        point: true,
                    }
                }
            },
            'road': {
                layer: 'road',
                line: true,
                'road_motorway': {
                    filter: { 'class': ['motorway', 'motorway_link'] },
                    line: {
                        cap: 'round',
                        join: 'bevel'
                    }
                },
                'road_main': {
                    filter: {
                        'class': 'main',
                        'oneway': 1
                    },
                    line: {
                        cap: 'round',
                        join: 'bevel'
                    }
                }
            },
            'building': {
                layer: 'building',
                fill: true
                stroke: {
                    cap: 'round',
                    join: 'round'
                }
            },
            'wetland': {
                layer: 'landuse_overlay',
                filter: { 'class': 'wetland' },
                fill: true
            }
        }
    },
    structure: [
        'wetland',
        'poi',
        'poi_park',
        'poi_other',
        { 
            composite: 'road',
            layers: [
                'road_motorway',
                'road_main'
            ] 
        },
        'building'
    ],
    layers: {
        'default': {
            'background': { fillColor: 'black' },
            'poi': {
                fillColor: 'white',
                pointRadius: 5
            },
            'poi_park': {
                pointImage: 'park-18',
                textColor: 'white',
                textTranslate: [40, 0]
            },
            'poi_other': {
                pointImage: 'star-stroked-18'
            },
            'road': {
                opacity: 0.9
            },
            'road_motorway': {
                fillColor: 'white',
                lineWidth: 3,
                strokeColor: 'green',
                strokeWidth: 1,
                transition: {
                    fillColor: { duration: 300 },
                    strokeColor: { 
                        duration: 300,
                        delay: 100
                    }
                }
            },
            'road_main': {
                fillColor: 'white',
                lineWidth: 1
            },
            'building': {
                fillColor: '#333333',
                fillOpacity: 0.8,
                strokeColor: '#555555',
                strokeWidth: 3
            }
        }
    }
}

Buckets

Bucket structure

- Nest buckets under sources, such as streets. This avoids unneeded repetition. - Allow for 'child' buckets that filter the parent bucket. These filters should support multiple dimensions, such as:
filter: {
    'class': ['motorway', 'motorway_link'],
    'oneway': 1
}
  • Specify type using fill: true or line: { cap: 'round', join: 'bevel' }

Multiple types in buckets

Specifying multiple types in a bucket can alleviate a lot of the confusion around line casings and fill strokes, cut down on repeating bucket code, more closely resemble carto patterns, and most importantly, remove the need for multiple layers pointing at a single bucket.

  • Allow for stroke control on fill buckets, which can later be styled in classes, such as:
'building': {
    layer: 'building',
    fill: true
    stroke: {
        cap: 'round',
        join: 'round'
    }
}
  • Allow for points and text to be specified in a single bucket:
'poi_park': {
    filter: { 'maki': 'park' }
    point: true,
    text: {
        textField: 'name',
        path: 'curve',
        font: 'Open Sans',
        fontSize: 18
    }
}

Structure

By baking multiple types in bucket specification and solving for the road casing issue in the class (see layer updates below), there doesn't seem to be a need for multiple layers pointing to a bucket. We can move to a flatter array for the structure:

structure: [
    'wetland',
    'poi',
    'poi_park',
    'poi_other',
    { 
        composite: 'road',
        layers: [
            'road_motorway',
            'road_main'
        ] 
    },
    'building'
]

This requires layer names to match up with bucket names.

Layers

- Rename classes to layers. - Use type-prepended style properties when appropriate. - Every layer has a fillColor property regardless of type. - Images on points use pointImage and tiled patterns on fills use pattern. - Properties such as translate are type-prepended: textTranslate: [40, 0] - Line casings are solved using stroke: strokeWitdth, strokeColor, strokeOpacity, etc. - Transition propertied are grouped like:
transition: {
    fillColor: { duration: 300 },
    strokeColor: { 
        duration: 300,
        delay: 100
    }
}

With these changes, I tried to cover all current functionality while simplifying the process conceptually.

cc @nickidlugash @ansis @mourner @kkaefer @tmcw

@edenh edenh added the style label Mar 24, 2014
@mourner
Copy link
Member

mourner commented Mar 24, 2014

I like these changes a lot. Quick question:

Line casings are solved using stroke: strokeWidth, strokeColor, strokeOpacity, etc.

How would you specify the z-order of the casings in the structure? How do you know from the current style example where to render them?

@tmcw
Copy link
Contributor

tmcw commented Mar 24, 2014

/cc @sgillies as arbiter of JSON style.

Random thoughts:

  • The fill/line attributes seem really close to SVG terminology, which is the same as simplestyle-spec but differ just slightly and I'm not sure they match anything else. Same with font - close to CSS, but just a little different. Naming sucks but I'd love to strive for similarity where it's cheap to do.
  • Instead of point: true, could we not have type: 'point'?
  • Is structure what determines z-index and nothing else? Should it be order?
  • Keep in mind that json and javascript objects are unordered, so if we don't have an order element, it'll be risky
  • Hard to tell what the use of class means here
  • The id: {} pattern is concise and guarantees no duplicate ids, but it can be weird to deal with, since you can't easily pull out an element from the object and refer to it after that. Doing more like [{ id: foo, ... }] might be preferable.
  • I'm not convinced we need to support a wide range of color formats here. This is a low-level, strict representation, and simplestyle-spec & kml both specify one valid color encoding.

@edenh
Copy link
Contributor Author

edenh commented Mar 24, 2014

@mourner it would probably always be an outer-stroke and be drawn on top of the line layer, but having more control there would also be nice: strokePosition: outer/inner/center

So if lineWidth is 5 and strokeWidth is 1, the 'casing' would be automatically offset with width of the line, instead of drawing a casing at 6 and a line on top at 5.

The fill/line attributes seem really close to SVG terminology, which is the same as simplestyle-spec but differ just slightly and I'm not sure they match anything else. Same with font - close to CSS, but just a little different. Naming sucks but I'd love to strive for similarity where it's cheap to do.

I agree. Going with something more similar to SVG has advantages.

Instead of point: true, could we not have type: 'point'?

We could, but when defining a line or text type, it would be nice to be able to nest the attributes specific to that type like line: { cap: 'round', join: 'bevel' }, without having to also include line: true. I could see how this may be confusing though.

Is structure what determines z-index and nothing else? Should it be order?

Order: 👍

Hard to tell what the use of class means here

This is specific field in mb streets: https://a.tiles.mapbox.com/v3/mapbox.mapbox-streets-v4.json
Not sure how to clarify/name this better.

@mourner
Copy link
Member

mourner commented Mar 24, 2014

it would probably always be an outer-stroke and be drawn on top of the line layer, but having more control there would also be nice: strokePosition: outer/inner/center

@edenh what I mean is that in many cases with several road types you first need to render all the casings of each type, and then render all the lines. You can't just assume casing-road-casing-road everywhere, as far as I understand. @ajashton right?

Is structure what determines z-index and nothing else? Should it be order?

@tmcw It also groups some layers into a named composite layer. That's not exactly "order", so I'm not sure.

Instead of point: true, could we not have type: 'point'?

We want to specify several types on one bucket. Do you suggest e.g. type: ['line', 'fill']?

@tmcw
Copy link
Contributor

tmcw commented Mar 24, 2014

Do you suggest e.g. type: ['line', 'fill']?

Sure, seems reasonable.

@edenh
Copy link
Contributor Author

edenh commented Mar 24, 2014

You can't just assume casing-road-casing-road everywhere, as far as I understand.

That's true. I'm wondering if we could rely on strokes being applied to composited layers in this case.

@mourner
Copy link
Member

mourner commented Mar 24, 2014

I'm wondering if we could rely on strokes being applied to composited layers in this case.

Looks a bit fishy. Besides imagine 4 different road types where we want to render 2 casings, 2 roads, 2 casings, 2 roads. This is why I suggested postfixes here #279 — covers cases like this and allows for fine-grained control.

@edenh
Copy link
Contributor Author

edenh commented Mar 25, 2014

Yep, #279 is better.

@ansis
Copy link
Contributor

ansis commented Mar 25, 2014

I think we need to get rid of the concept of buckets for outside users. We should only have layers and styles. Layers handle z order and data selection. Styles handle everything about appearance.

@edenh's example would look something like this:

{
    "layers": [
        {
            "streets": { "source": "mapbox-streets" },
            "layers": [
            {
                "wetland": { "layer": "landuse" }
            },
            {
                "poi": { "layer": "poi_label" },
                "layers": [
                    { "poi_park": { "maki": "park" } },
                    { "poi_other": { "maki": ["airport", "bus"] } }
                ]
            },
            {
                "road": { "layer": "road" },
                "layers": [
                    { "road_motorway_casing": { "class": ["motorway", "motorway_link"] } },
                    { "road_main_casing": { "class": "main", "oneway": 1 } },
                    { "road_motorway": { "class": ["motorway", "motorway_link"] } },
                    { "road_main": { "class": "main", "oneway": 1 } }
                ]
            },
            {
                "building": { "layer": "building" }
            }
        ]}
    ],
    "styles": [
        { "": {
            "wetland": {
                "fill-color": "#00ff00"
            },
            "poi": {
                "fill-clor": "#ffffff",
                "point-radius": 5
            },
            "poi_park": {
                "point-image": "park-18",
                "text-color": "#ffffff",
                "text-field": "name",
                "text-path": "curve",
                "text-font": "Open Sans",
                "text-size": 18,
                "text-translate": [0, 40]
            },
            "poi_other": {
                "point-image": "start-stroked-18"
            },
            "road": {
                "opacity": 0,
                "line-cap": "round",
                "line-join": "bevel"
            },
            "road_motorway_casing": {
                "fill-color": "#000000",
                "line-width": 3
            },
            "road_main_casing": {
                "fill-color": "#000000",
                "line-width": 1
            },
            "road_motorway": {
                "fill-color": "#ffffff",
                "line-width": 3
            },
            "road_main": {
                "fill-color": "#ffffff",
                "line-width": 1
            },
            "building": {
                "fill-color": "#333333",
                "fill-opacity": 0.8,
                "line-color": "#555555",
                "line-width": 3
            }
        }},
        { "superwideroads": {
            "streets": {
                "transition-line-width": { "duration": 2000, "delay": 500 }
            },
            "road_motorway_casing": {
                "line-width": 10
            },
            "road_motorway": {
                "line-width": 8
            }
        }}
    ]
}

Layers

Each layer looks like this: { "layername": filterobject }. The filter object describes how to select the data for that layer. The key/values in the object would be matched against feature tags. There are two special keys: source and layer. layer is a concept from mapbox vector tiles. source is for llmr sources.

If a layer object has a layers property, it is a layer group. The layers property is an array of layers. The filters from a layergroup get automatically included in its child layers' filters, to make them more concise.

Styles

Styles describe appearance. I think the example explains things fairly well. In the example superwideroads is a class that could be added with map.addClass("superwideroads") to transition motorways to a wider width. Styles on layer groups are included in their children's styles. Some of the style properties used to be found on buckets.

abstracting away buckets

Exposing buckets has only one potential "advantage". Forcing more of the underlying details on the designer may help them create less duplicate buckets which may make a noticeable performance impact. But in most cases it should be possible to automatically dedupe and share buckets. In the example above, road_motorway and road_motorway_casing have the same filters and linecaps, which would be automatically detected.

This style would be processed into an expanded renderer-friendly version with buckets similar to what we have now.

I think this approach reduces the number of concepts someone needs to know to write a style, and is pretty concise.

@edenh
Copy link
Contributor Author

edenh commented Mar 25, 2014

I think we need to get rid of the concept of buckets for outside users. We should only have layers and styles. Layers handle z order and data selection. Styles handle everything about appearance.

👍 looks great

@edenh
Copy link
Contributor Author

edenh commented Mar 25, 2014

@ansis how would bucket/layer types be defined?

With the proposed layout, I take it features would be automatically drawn based on the existence of style attributes, such as text-name for text?

@mourner
Copy link
Member

mourner commented Mar 25, 2014

@ansis I think this looks great. One minor thing is that we don't need to specify order of the classes, so we can just do "styles": {"myclass": { .... The idea to move all the geometry-specific styles out of bucket config is really great too. Should simplify the style a lot. I also like how you define the global "streets" layer and set a transition on it all at once.

@ansis
Copy link
Contributor

ansis commented Mar 25, 2014

I take it features would be automatically drawn based on the existence of style attributes, such as text-name for text?

Yeah, I think so. Each type of drawing would have its own prefix.

we don't need to specify order of the classes

If we created a sortofwideroads class that set motorways to a width of 7, and then did map.addClass('sortofwideroads').addClass('superwideroads') what would be the width of motorways? Would it be based on the order the classes were added? I think css precedence is based on the order rules are defined in the stylesheet.

define the global "streets" layer and set a transition on it all at once

There should probably also be a magic layer below everything that could be used for default styles across sources. #327

We might want to support everything mapnik does for filters (and, or, not, =, !=, <, <=, >, >=, regex), but I'm not sure what that would look like here.

{ "motorway": ["or", ["=", "class", "motorway"], ["=", "class", "motorway_link"]] }

or maybe?

{ "motorway": "[class]='motorway' or [class]='motorway_link'" }

not sure

@kkaefer
Copy link
Member

kkaefer commented Mar 25, 2014

  • Think about adding data/ids to the buffers we create from bucket information so that we can e.g. draw all poi icons in one go. Not sure how we could do this on a generic level. Think a about things like assigning colors/widths/icons/textures/translations based on a feature property. It gets complicated when you add zoom-level/latitude/longitude dependent functions into the mix, e.g. line width that depends on feature property, zoom level, latitude.
  • Mapnik has a boost spirit expression parser for the filters; we could do something similar in JS/native (e.g. use Lua/LuaJIT, parse the expressions ourselves...?). Are we going to support buckets that pull data from multiple datasources or datasource layers? (Think drawing overground roads, tunnels and bridges in one go)
  • I like the line: { ... }, point: { ... } approach. Lets us use the same data for multiple things, e.g. drawing wide outlines around a fill
  • Selecting data independent of layers/stylesheet allows less repetition if you have large/complex filters
  • Color functions might be interesting too, e.g. manipulate the hue or saturation based on zoom level (parks/forests fading in over zoom levels). currently we "fake" this with opacity functions, which is really just a special case for modifying a color component
  • To make it easier conceptually, we could also think about moving all of the geometry-related properties which are currently in buckets to the style, then parse the style and create the appropriate buckets. It's going to be hard to explain why line width is in the style and line-cap in the bucket.

@ansis
Copy link
Contributor

ansis commented Mar 25, 2014

Think about adding data/ids to the buffers we create from bucket information so that we can e.g. draw all poi icons in one go. Not sure how we could do this on a generic level. Think a about things like assigning colors/widths/icons/textures/translations based on a feature property. It gets complicated when you add zoom-level/latitude/longitude dependent functions into the mix, e.g. line width that depends on feature property, zoom level, latitude.

I think we should start with only supporting constant values. To implement functions that take properties as args we'd need to evaluate the functions in shaders. The complexity doesn't seem like its worth it.

Mapnik has a boost spirit expression parser for the filters; we could do something similar in JS/native (e.g. use Lua/LuaJIT, parse the expressions ourselves...?)

What approach to filters do you think is best?

To make it easier conceptually, we could also think about moving all of the geometry-related properties which are currently in buckets to the style, then parse the style and create the appropriate buckets. It's going to be hard to explain why line width is in the style and line-cap in the bucket.

@kkaefer What do you think of the example where buckets are removed entirely from the exposed style?

@sgillies
Copy link

@tmcw this is all very tastefully done :) I definitely like it better without the bucket concept.

@nickidlugash
Copy link

I think removing exposed buckets and moving all geometry-related properties to styles is a great idea – would make styling much simpler.

Question about layers: how would layer declaration work if you wanted to place different parts of an mb vector tile layer in different positions in the order? E.g. say you wanted to place poi labels for parks in one place, and poi labels for everything else further down in the order? Can different layers have the same layer filterobject to allow for this?

@edenh
Copy link
Contributor Author

edenh commented Mar 25, 2014

@nickidlugash I think using @ansis's example, the layers order would look something like this:

{"layers": [
    {
        "streets": {"source": "mapbox-streets"},
        "layers": [
            {
                "poi_labels1": {"layer": "poi_labels" },
                "layers": [
                    {"parks": {"maki": "park"}}
                ]
            },    
            {
                "buildings": {"layer":"buildings"}
            },
            {
                "poi_labels2": {"layer":"poi_labels"},
                "layers": [
                    {"other": {"[maki] != 'park'"}}
                ]
            }
        ]
    }
]}

This would place park poi's, then buildings, then all other poi's (still not sure about syntax for the != part). Do you see any holes here in comparison to working with carto?

Edit: just updated the syntax a bit

@kkaefer
Copy link
Member

kkaefer commented Mar 26, 2014

@edenh: not sure I understand that syntax; what does the "streets" key mean? How are we going to distinguish between the name poi_labels1 and layers (which is a magic key)? {"[maki] != 'park'"} is not valid JSON.

@mourner
Copy link
Member

mourner commented Mar 26, 2014

If we created a sortofwideroads class that set motorways to a width of 7, and then did map.addClass('sortofwideroads').addClass('superwideroads') what would be the width of motorways? Would it be based on the order the classes were added? I think css precedence is based on the order rules are defined in the stylesheet.

@ansis Yes, CSS precedence is based on definition order, but considering that our primary goal is to help people design maps, we should decide if making sure precedence logic fits CSS spec is more important than keeping things easy and simple, allowing easy manipulation of classes dynamically etc. I'm not sure.

not sure I understand that syntax; what does the "streets" key mean?

This is a good point, initially I found the syntax a bit confusing too. It's compact, but semantically confusing. Maybe it would be better to sacrifice terseness and introduce "id" or "name" key:

{
    "id": "streets",
    "source": "mapbox-streets",
    "layers": [{
        "id": "poi_labels1",
        "layer": "poi_labels",
        "layers": [{
            "id": "parks",
            "maki": "park"
        }]
    }, {
        "id": "buildings",
        "layer": "buildings"
    }, {
        "id": "poi_labels2",
        "layer": "poi_labels",
        "layers": [{
            "id": "other",
            "maki": '!park'
        }]
    }]
}

I also don't like the "layer"-"layers" ambiguity, because similar keys mean totally different things. Maybe "layer" should be "source_layer" or something.

@yhahn yhahn mentioned this issue Mar 26, 2014
3 tasks
@ansis
Copy link
Contributor

ansis commented Mar 26, 2014

not sure I understand that syntax; what does the "streets" key mean?

This is a good point, initially I found the syntax a bit confusing too. It's compact, but semantically confusing.

I agree. It feels really weird.

@ansis
Copy link
Contributor

ansis commented Mar 28, 2014

revised. Getting closer?

{
    "layers": [
        {
            "id": "streets",
            "filter": "source == 'mapbox-streets'",
            "layers": [
            {
                "id": "wetland",
                "filter": "source_layer == 'landuse'"
            },
            {
                "id": "poi",
                "filter": "source_layer == 'poi_label'",
                "layers": [
                    {
                        "id": "poi_park",
                        "filter": "maki == 'park'"
                    },
                    {
                        "id": "poi_other",
                        "filter": "maki == 'bus' or maki == 'airport'"
                    }
                ]
            },
            {
                "id": "road",
                "filter": "source_layer == 'road'",
                "layers": [
                    {
                        "id": "road_motorway_casing",
                        "filter": "class == 'motorway' or class == 'motorway_link'"
                    },
                    {
                        "id": "road_main_casing",
                        "filter": "class == 'main' and oneway == 1"
                    },
                    {
                        "id": "road_motorway",
                        "filter": "class == 'motorway' or class == 'motorway_link'"
                    },
                    {
                        "id": "road_main",
                        "filter": "class == 'main' and oneway == 1"
                    }
                ]
            },
            {
                "id": "building",
                "filter": "source_layer == 'building'"
            }
        ]}
    ],
    "styles": {
        "": {
            "wetland": {
                "fill-color": "#00ff00"
            },
            "poi": {
                "fill-clor": "#ffffff",
                "point-radius": 5
            },
            "poi_park": {
                "point-image": "park-18",
                "text-color": "#ffffff",
                "text-field": "name",
                "text-path": "curve",
                "text-font": "Open Sans",
                "text-size": 18,
                "text-translate": [0, 40]
            },
            "poi_other": {
                "point-image": "start-stroked-18"
            },
            "road": {
                "opacity": 0,
                "line-cap": "round",
                "line-join": "bevel"
            },
            "road_motorway_casing": {
                "fill-color": "#000000",
                "line-width": 3
            },
            "road_main_casing": {
                "fill-color": "#000000",
                "line-width": 1
            },
            "road_motorway": {
                "fill-color": "#ffffff",
                "line-width": 3
            },
            "road_main": {
                "fill-color": "#ffffff",
                "line-width": 1
            },
            "building": {
                "fill-color": "#333333",
                "fill-opacity": 0.8,
                "line-color": "#555555",
                "line-width": 3
            }
        },
        "superwideroads": {
            "streets": {
                "transition-line-width": { "duration": 2000, "delay": 500 }
            },
            "road_motorway_casing": {
                "line-width": 10
            },
            "road_motorway": {
                "line-width": 8
            }
        }
    }
}

Are these kind of filters clear and easy to write? The parsing side is fine: https://gist.github.com/ansis/764f11827c1a07d2e0a5

@yhahn
Copy link
Member

yhahn commented Mar 28, 2014

I'm not totally familiar with the problem space here so feel free to ignore.


Is it wise to make parsing the style more complex which will need to be handled and maintained in both JS and C++? To contrast, a preprocessor will likely only need to be written in one language, whether JS or C++, because it only needs to be run by authoring application/service.

I would aim to make the current style representation fit the data structure that the renderer needs to work with best. Other considerations like authoring convenience, etc. can always be delegated to preprocessing/templating/just doing the work.

@ansis
Copy link
Contributor

ansis commented Mar 28, 2014

Other considerations like authoring convenience, etc. can always be delegated to preprocessing/templating/just doing the work.

I should have written this explicitly, but (I think) this issue is about a style that would be pre-processed into something very similar to the current style. The currently implemented style is fairly close to where it should be for the renderer, and doesn't need huge restructuring.

@yhahn
Copy link
Member

yhahn commented Mar 28, 2014

👍 cool, i take it you are preserving this internal representation at some stage of style processing where you can always serialize it again for transport/reloading if necessary.

@mourner
Copy link
Member

mourner commented Mar 28, 2014

@ansis I like it, looks very readable and consistent. I'd go further with the filter syntax — values on the right side are always string so quotes can be dropped, and for consistency, I'd either go full-on on word operators or symbol operators or allowing both (the symbol ones look more readable):

"(maki == 'park' or maki == 'nope' and maki != 'airport') and maki =~ '.*'" // current
"(maki is park or maki is nope and maki isnt airport) and maki matches .*"  // words
"(maki == park || maki == nope && maki != airport) && maki =~ .*"           // symbols
"(maki = park | maki = nope & maki != airport) & maki =~ .*"                // one char

@ansis
Copy link
Contributor

ansis commented Apr 3, 2014

Does anyone have any suggestions for expressing filters nicely with json objects instead of strings?

@tmcw
Copy link
Contributor

tmcw commented Apr 3, 2014

There are a bunch of experimental JSON query languages, but the MongoDB query language is pretty deployed & tire-kicked

@mourner
Copy link
Member

mourner commented Apr 3, 2014

@ansis don't like the string filters?

@tmcw
Copy link
Contributor

tmcw commented Apr 3, 2014

I'd definitely favor non-string filters: if we can find a reasonably lightweight way of representing filters as JSON, it'll be easier to create filters graphically, and implementations that read the llmr format don't need a string tokenizer & parser

@mourner
Copy link
Member

mourner commented Apr 3, 2014

Regarding stops... If we want to be consistent with {fn: ..., params ...} convention, they would look something like this:

"line-width": {
  "fn": "stops",
  "stops": [
    {"z": 0, "val": 1.5}, 
    {"z": 6, "val": 1.5}, 
    {"z": 8, "val": 3}, 
    {"z": 22, "val": 3}
  ]
}

This looks too verbose to me — maybe this would be a better compromise of being obvious vs lightweight:

"line-width": ["stops", {
  "0": 1.5,
  "6": 1.5,
  "8": 3,
  "22": 3
}]

This way we could keep lightweight ["min", 15], etc., while still introducing named params in linear/exp functions:

"line-width": ["linear", {
  "z": 5,
  "val": 10,
  "slope": 2,
  "min": 1,
  "max": 20
}]

@tmcw
Copy link
Contributor

tmcw commented Apr 3, 2014

"line-width": ["stops", {
  "0": 1.5,
  "6": 1.5,
  "8": 3,
  "22": 3
}]

This makes the x values stringified, and it'll be harder to change these in code - objects aren't sorted and are harder to modify in-place. Also, if we have that format, and we want to add another argument to stops which isn't another stop, like interpolate with a function of how to interpolate between stops, where do we put it?

I think a decent middle ground would be:

"line-width": {
  "fn": "stops",
  "stops": [
    [0, 1.5],
    [6, 1.5],
    [8, 3],
    [22, 3]
  ]
}

@mourner
Copy link
Member

mourner commented Apr 4, 2014

@tmcw makes sense, I'll try that.

@kkaefer
Copy link
Member

kkaefer commented Apr 4, 2014

I'd definitely favor non-string filters: if we can find a reasonably lightweight way of representing filters as JSON, it'll be easier to create filters graphically, and implementations that read the llmr format don't need a string tokenizer & parser

String tokenization/expression parsing is complicated. If we can avoid it, we should. Mapnik's use of spirit for the expression parsing adds a lot of complexity and compile time. Lastly, if we do things like layer = "foo" as expressions, it'll be hard to optimize this because they could be in nexted and/or expressions.

@mourner
Copy link
Member

mourner commented Apr 4, 2014

I agree that JSON-based queries are much easier to parse/manipulate, but so far haven't seen any examples of an easy to use and readable JSON-based query language — the Mongo one is pretty confusing, many people really don't like it.

One thing that makes gl filters a better fit for a string-based language is that it's limited to basic property comparisons, by design there's not enough complexity to make tokenization/expression parsing very hard.

@tmcw
Copy link
Contributor

tmcw commented Apr 4, 2014

@mourner there is at least one person on the internet who hates each thing on the internet.

@springmeyer
Copy link
Contributor

String tokenization/expression parsing is complicated. If we can avoid it, we should. Mapnik's use of spirit for the expression parsing adds a lot of complexity and compile time.

I agree on complexity. Wrt to compile time and c++ specifically I presume we could do better with a fresh start + spirit is oddball because it was implemented before c++11

@tmcw
Copy link
Contributor

tmcw commented Apr 4, 2014

Okay, spitballing a simpler json encoding:

[{
    "and": [
        {"=": ["@class", "motorway_link"]},
        {"=": ["@oneway", "1"]}
    ]
}]

One lesson we learned the hard way in carto-land is that a comparison like class = motorway is extremely problematic: is class the column, or motorway? Is motorway a field, a string, or a variable?

Would strongly advise using sigils or something similar to make that clearer.

@edenh
Copy link
Contributor Author

edenh commented Apr 4, 2014

Can we just assume 'and' unless something else is specified? Something like:

"filter": {
    "layer": "waterway",
    "or": [
          {"type": "ditch"},
          {"type": "drain"}
    ]
}

@tmcw
Copy link
Contributor

tmcw commented Apr 4, 2014

@edenh you will always have someone who has a field that is named "and" or "or"

@edenh
Copy link
Contributor Author

edenh commented Apr 4, 2014

@tmcw

"filter": {
    "layer": "waterway",
    "$or": [
          {"type": "ditch"},
          {"type": "drain"}
    ]
}

@mourner
Copy link
Member

mourner commented Apr 7, 2014

Another option we have for filters is keeping the string syntax and then parsing it into JSON (however ugly it may be) on the preprocessing step (with a JS script).

@mourner
Copy link
Member

mourner commented Apr 7, 2014

FYI there's now a script in the gl-style repo that converts user-friendly styles into renderer-friendly styles. It generates buckets and structure from layers (handling duplicates), detects bucket types and moves bucket-related styles into bucket config.

Sample output (OSM Bright): https://github.com/mapbox/gl-style/blob/master/test/styles/bright-raw.js

@mourner
Copy link
Member

mourner commented Apr 11, 2014

@edenh if we go with the JSON filter approach, I'd go further and assume "or" for any array values, so your example would turn to:

{
  "layer": "waterway",
  "type": ["ditch", "drain"]
}

This way we could simplify most of the filters while still allowing $or and $and.

@mourner
Copy link
Member

mourner commented Apr 11, 2014

Looking at how more complex JSON filters would look like, compared to @ansis's first example:

// string-based
"(maki == 'park' or maki == 'nope' and maki != 'airport') and maki =~ '.*'"

// JSON
{
    "$or": [{
        "maki": "park"
    }, {
        "maki": "nope",
        "$not": {"maki": "airport"}
    }],
    "$match": {"maki": ".*"}
}

The JSON one looks much more verbose. On the other hand, I don't think complex filters are a common use case — most will be trivial, like here: https://github.com/mapbox/gl-style/blob/master/test/styles/bright-v1.js

@mourner
Copy link
Member

mourner commented Apr 11, 2014

OK, I went ahead with simple JSON filters in migrations, we can change this later if needed: mapbox/mapbox-gl-style-spec#10

@yhahn
Copy link
Member

yhahn commented Jun 10, 2014

@yhahn yhahn closed this as completed Jun 10, 2014
bensleveritt pushed a commit to bensleveritt/mapbox-gl-js that referenced this issue Oct 24, 2016
prevent adding extra nodes to polygons, fixes mapbox#345
lucaswoj pushed a commit that referenced this issue Jan 11, 2017
Updates to symbol render and layout property descriptions
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

No branches or pull requests