Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Condense rules with 'or' filters #206

Open
ajashton opened this issue Oct 24, 2012 · 8 comments
Open

Condense rules with 'or' filters #206

ajashton opened this issue Oct 24, 2012 · 8 comments

Comments

@ajashton
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

tmcw commented Dec 19, 2012

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
Copy link

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
Copy link
Contributor

tmcw commented Dec 20, 2012

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
Copy link

@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
Copy link
Contributor

tmcw commented Dec 20, 2012

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
Copy link
Contributor

tmcw commented Dec 20, 2012

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

@springmeyer
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants