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

Style format issues #27

Closed
5 tasks done
kkaefer opened this issue Jun 10, 2014 · 7 comments
Closed
5 tasks done

Style format issues #27

kkaefer opened this issue Jun 10, 2014 · 7 comments

Comments

@kkaefer
Copy link
Member

kkaefer commented Jun 10, 2014

  • opacity vs. fill/line/...-opacity? => remains prefixed
  • bucket has "fill": true, but we don't support buckets of multiple types. Instead use type: "fill"?
  • filter contains source, but they could also be feature properties. Why is source and layer part of the filter object? We should either prefix them with $, or move them back up the hierarchy.
  • generic keys like opacity, translate or enabled are prefixed too, which makes for a lot of code duplication
  • what happens if you add a layer, but don't define any styles for it? how do we know what symbolizer we should use for displaying it? currently, the symbolizer is defined by a (the first?) style applied to a layer.
@kkaefer
Copy link
Member Author

kkaefer commented Jun 10, 2014

Going to create a new version v2 style format with these changes:

v2 changes:

  • move source, layer, and feature_type out of the filter object again, we can still cascade them if we want to.
  • generic attributes remain prefixed for now
  • rename feature_type to geometry
  • layers gain a type attribute that defines the bucket type
  • rename point to icon
  • rename stroke-color to fill-stroke-color
  • icon-size is no longer an array, instead it is only a number and icons are always square
  • dropped icon-antialias, it's not used

raw changes

  • move source, layer, and feature_type out of the filter object again, we can still cascade them if we want to.
  • new complex filter format (backwards compatible with the old format)
  • buckets gain a type attribute that defines the bucket type

implemented in cutting-room-floor/mapbox-gl-style-lint@3bd2ab3

@mourner
Copy link
Member

mourner commented Jun 10, 2014

Why change without discussion? I agree to all points except:

move source, layer, and feature_type out of the filter object again, we can still cascade them if we want to.

I'm not convinced about this change. Semantically this is a part of filter — it narrows the data that we have using some criteria, as other attributes are. I'd rather use prefixed $source, $layer, $geometry.

buckets gain a type attribute that defines the bucket type

We want to support multiple formats per bucket soon. Are you changing this back to simplify porting native?

@yhahn
Copy link
Member

yhahn commented Jun 10, 2014

My understanding of the motivations here:

move source, layer, and feature_type out of the filter object again, we can still cascade them if we want to.

source, layer, feature_type -- the first two are required and they are all handled quite differently from filter expressions in the code. Having them nested under filter is deceptive and it confused me as a dev and styling initially -- I thought they might behave like filters but they really don't at all.

buckets gain a type attribute that defines the bucket type

We did some chatting around mapbox/mapbox-gl-js#244 -- I'm not sure what this would get us beyond grouping of icons and labels together. The complexity this causes in a style is scary to me -- every time I've seen this by accident in styles it's left me confused as to what should actually happen.

If there's no amazing reason to have this I don't think it'll earn its rent.

@mourner
Copy link
Member

mourner commented Jun 10, 2014

Yeah, that's a nice explanation — makes sense. Could we at least make the style less verbose and repetitive by grouping buckets by source in the style? Like this:

"buckets": {
    "mapbox-streets.v5": {
        "road": { ... }
    }

@kkaefer
Copy link
Member Author

kkaefer commented Jun 11, 2014

@mourner the v2 changes so far are a proposal; no parser is implemented yet. I wrote the conversion code to make sure this is a sound proposal. Me posting here was meant as a way of discussing it.

I want to move source and layer back to the main level, because they are not like other filters. In the source code, we are first looking up the correct source layer, then iterating through all the features and select using the filter attributes. If they were filter properties, they could be arbitrarily nested, so we'd have to first figure out what sources and layers we have to iterate, then iterate through all of them (this gets really complex with xor expressions and the like...). My understanding is that gljs doesn't do this either; it only ever looks up one layer in one source for a bucket.

As Young is saying, we aim for a different way of grouping things for label placement. Multi-type buckets are a little too complex to do that.

@jfirebaugh
Copy link
Contributor

👍 to moving source and layer out of filters -- this was one of the first things that seemed strange to me as well.

feature_type seems to be handled more like other filter properties though, in that is actually a condition on features, albeit one with some built in semantics about a mapping between 'point', 'line', and 'fill' and integer constants. I suggest leaving it inside filter but moving the string -> integer mapping knowledge into bucket-filter.js.

@yhahn
Copy link
Member

yhahn commented Jun 13, 2014

#29

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

4 participants