Condense rules with 'or' filters

[ajashton]

The example style below will result in an XML file with 9 separate rules. But Mapnik XML supports 'or' in filters like ([type] = 'foo' or [type] = 'bar') and it really only needs to be one rule.

Ideally Carto could just condense these automatically, but alternatively there could be some kind of key in value1, value2, ... syntax. Thoughts?

#areas {
  [type='camp_site'],
  [type='common'],
  [type='national_park'],
  [type='nature_reserve'],
  [type='park'],
  [type='protected_area'],
  [type='recreation_ground'],
  [type='village_green'],
  [type='zoo'] {
    polygon-fill: green;
  }
}

<Style name="areas" filter-mode="first" >
  <Rule>
    <Filter>([type] = 'camp_site')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
  <Rule>
    <Filter>([type] = 'common')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
  <Rule>
    <Filter>([type] = 'national_park')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
  <Rule>
    <Filter>([type] = 'nature_reserve')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
  <Rule>
    <Filter>([type] = 'park')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
  <Rule>
    <Filter>([type] = 'protected_area')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
  <Rule>
    <Filter>([type] = 'recreation_ground')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
  <Rule>
    <Filter>([type] = 'village_green')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
  <Rule>
    <Filter>([type] = 'zoo')</Filter>
    <PolygonSymbolizer fill="#008000" />
  </Rule>
</Style>

[springmeyer]

this seems quite useful to fix. I had a quick look but came up empty trying to see where or why this occurs. It seems like Filtersets assume and. That would likely be easy to fix, but I'm lost trying to see why ord selectors do not become filtersets right now.

Wild, likely wrong guess, does this has to do with missing combinators? https://github.com/cloudhead/less.js/blob/master/lib/less/parser.js#L1055

[tmcw]

Wild, likely wrong guess, does this has to do with missing combinators?

No, this is more about additional logic that would need to be implemented in filterset.js, which is currently constructing the filtersets.

That would likely be easy to fix, but I'm lost trying to see why ord selectors do not become filtersets right now.

There are no or selectors: .foo, .bar { } is shorthand for .foo {} .bar {}. In order to implement this, we'll need to implement some logic in filterset which detects when you have multiple foo={a,b,c} selectors.

[springmeyer]

Okay, make sense. Seems like implementing that logic, if doable, would be a solid benefit to both XML load time and Mapnik render time, since it would reduce the # of expressions that need to get parsed (which is the current overwhelming bottleneck in XML loading in mapnik) and would reduce the # of expressions and rules that get evaluated.

So, would be great to see this working.

[tmcw]

This is going to be a deep one. Right now the entire filter system is written with the assumption that there will be a single key=* in each filterset. Also this brings up weird cases like

#areas {
  [area>10],
  [area>20] { }

In which we do need to deduplicate styles rather than forming or filters.

[springmeyer]

@tmcw - would it be more feasible if we did not worry about de-duping?

Or, what do you think of @ajashton's idea of a new key in {list of values syntax that could fit in a single selector? We'd add support for it on the Mapnik side....

[tmcw]

I'll open up an experimental branch that doesn't do as much de-duping, but it'll both require the same kind of re-testing that we will need with #20 and might worsen rule-explosion problems a bit.

[tmcw]

Experimental branch at https://github.com/mapbox/carto/tree/natural-filters - it's quite a departure.

[springmeyer]

cool, more reason for starting to build up a visual test suite