Merge pull request #201 from mapbox/nested-classes

Fix nested class behavior.

lib/carto/renderer.js

@@ -215,6 +215,9 @@ function inheritRules(definitions, env) {
     return result;
 }
 
+// Sort styles by the minimum index of their rules.
+// This sorts a slice of the styles, so it returns a sorted
+// array but does not change the input.
 function sortStyles(styles, env) {
     styles.forEach(function(style) {
         style.index = Infinity;

lib/carto/tree/ruleset.js

@@ -8,8 +8,9 @@ tree.Ruleset = function Ruleset(selectors, rules) {
     this.is = 'ruleset';
 };
 tree.Ruleset.prototype = {
-    eval: function(env) {
-        var ruleset = new tree.Ruleset(this.selectors, this.rules.slice(0));
+    'eval': function(env) {
+        var i,
+            ruleset = new tree.Ruleset(this.selectors, this.rules.slice(0));
         ruleset.root = this.root;
 
         // push the current ruleset to the frames stack
@@ -17,7 +18,7 @@ tree.Ruleset.prototype = {
 
         // Evaluate imports
         if (ruleset.root) {
-            for (var i = 0; i < ruleset.rules.length; i++) {
+            for (i = 0; i < ruleset.rules.length; i++) {
                 if (ruleset.rules[i] instanceof tree.Import) {
                     Array.prototype.splice
                          .apply(ruleset.rules, [i, 1].concat(ruleset.rules[i].eval(env)));
@@ -26,7 +27,7 @@ tree.Ruleset.prototype = {
         }
 
         // Evaluate everything else
-        for (var i = 0, rule; i < ruleset.rules.length; i++) {
+        for (i = 0, rule; i < ruleset.rules.length; i++) {
             rule = ruleset.rules[i];
             ruleset.rules[i] = rule.eval ? rule.eval(env) : rule;
         }
@@ -71,7 +72,8 @@ tree.Ruleset.prototype = {
         this.rulesets().forEach(function(rule) {
             if (rule !== self) {
                 for (var j = 0; j < rule.selectors.length; j++) {
-                    if (match = selector.match(rule.selectors[j])) {
+                    match = selector.match(rule.selectors[j]);
+                    if (match) {
                         if (selector.elements.length > 1) {
                             Array.prototype.push.apply(rules, rule.find(
                                 new tree.Selector(null, null, selector.elements.slice(1)), self));
@@ -86,18 +88,21 @@ tree.Ruleset.prototype = {
         return this._lookups[key] = rules;
     },
     flatten: function(result, parents, env) {
-        var selectors = [];
+        var selectors = [], i, j;
         if (this.selectors.length === 0) {
             env.frames = env.frames.concat(this.rules);
         }
-        for (var i = 0; i < this.selectors.length; i++) {
+        for (i = 0; i < this.selectors.length; i++) {
             var child = this.selectors[i];
 
-            // This is an invalid filterset.
-            if (!child.filters) continue;
+            if (!child.filters) {
+                // TODO: is this internal inconsistency?
+                // This is an invalid filterset.
+                continue;
+            }
 
             if (parents.length) {
-                for (var j = 0; j < parents.length; j++) {
+                for (j = 0; j < parents.length; j++) {
                     var parent = parents[j];
 
                     var mergedFilters = parent.filters.cloneWith(child.filters);
@@ -106,7 +111,8 @@ tree.Ruleset.prototype = {
                         // filters. This means that we only have to clone when
                         // the zoom levels or the attachment is different too.
                         if (parent.zoom === (parent.zoom & child.zoom) &&
-                            parent.attachment === child.attachment) {
+                            parent.attachment === child.attachment &&
+                            parent.elements.join() === child.elements.join()) {
                             continue;
                         } else {
                             mergedFilters = parent.filters;
@@ -135,7 +141,7 @@ tree.Ruleset.prototype = {
         }
 
         var rules = [];
-        for (var i = 0; i < this.rules.length; i++) {
+        for (i = 0; i < this.rules.length; i++) {
             var rule = this.rules[i];
 
             if (rule instanceof tree.Ruleset) {
@@ -148,7 +154,7 @@ tree.Ruleset.prototype = {
         }
 
         var index = rules.length ? rules[0].index : false;
-        for (var i = 0; i < selectors.length; i++) {
+        for (i = 0; i < selectors.length; i++) {
             // For specificity sort, use the position of the first rule to allow
             // defining attachments that are under current element as a descendant
             // selector.

test/rendering/combined_class.mml

@@ -0,0 +1,27 @@
+{
+    "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over",
+    "Stylesheet": [
+        "combined_class.mss"
+    ],
+    "Layer": [
+    {
+        "name": "just_land",
+        "class": "land",
+        "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over",
+        "Datasource": {
+            "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip",
+            "type": "shape"
+        }
+    },
+    {
+        "name": "lakes",
+        "id": "lakes",
+        "class": "land",
+        "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over",
+        "Datasource": {
+            "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip",
+            "type": "shape"
+        }
+    }
+    ]
+}

test/rendering/combined_class.mss

@@ -0,0 +1,11 @@
+/* Applies to all layers with .land class */
+.land {
+  line-color: #ccc;
+  line-width: 0.5;
+  polygon-fill: #eee;
+}
+.land#lakes {
+  line-color: #ccc;
+  line-width: 0.5;
+  polygon-fill: #000;
+}

test/rendering/combined_class.result

@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!DOCTYPE Map[]>
+<Map srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over" maximum-extent="-20037508.34,-20037508.34,20037508.34,20037508.34">
+
+
+<Style name="just_land" filter-mode="first" >
+  <Rule>
+    <LineSymbolizer stroke="#cccccc" stroke-width="0.5" />
+    <PolygonSymbolizer fill="#eeeeee" />
+  </Rule>
+</Style>
+<Layer name="just_land"
+  srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over">
+    <StyleName>just_land</StyleName>
+    <Datasource>
+       <Parameter name="file"><![CDATA[[absolute path]]]></Parameter>
+       <Parameter name="type"><![CDATA[shape]]></Parameter>
+    </Datasource>
+  </Layer>
+
+<Style name="lakes" filter-mode="first" >
+  <Rule>
+    <LineSymbolizer stroke="#cccccc" stroke-width="0.5" />
+    <PolygonSymbolizer fill="#000000" />
+  </Rule>
+</Style>
+<Layer name="lakes"
+  srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over">
+    <StyleName>lakes</StyleName>
+    <Datasource>
+       <Parameter name="file"><![CDATA[[absolute path]]]></Parameter>
+       <Parameter name="type"><![CDATA[shape]]></Parameter>
+    </Datasource>
+  </Layer>
+
+</Map>

test/rendering/complex_cascades.result

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!DOCTYPE Map[]>
-<Map srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over"  maximum-extent="-20037508.34,-20037508.34,20037508.34,20037508.34">
+<Map srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over" maximum-extent="-20037508.34,-20037508.34,20037508.34,20037508.34">
 
 
 <Style name="world" filter-mode="first">
@@ -45,22 +45,22 @@
   <Rule>
     <MinScaleDenominator>12500000</MinScaleDenominator>
     <Filter>([NAME] = 'United States')</Filter>
-    <PolygonSymbolizer fill="#eeeeee" />
     <LineSymbolizer stroke="#cccccc" stroke-width="1" />
+    <PolygonSymbolizer fill="#cccccc" />
   </Rule>
   <Rule>
     <MaxScaleDenominator>12500000</MaxScaleDenominator>
-    <PolygonSymbolizer fill="#eeeeee" />
     <LineSymbolizer stroke-width="0.5" stroke="#cccccc" />
+    <PolygonSymbolizer fill="#cccccc" />
   </Rule>
   <Rule>
     <MinScaleDenominator>12500000</MinScaleDenominator>
-    <PolygonSymbolizer fill="#eeeeee" />
     <LineSymbolizer stroke="#cccccc" stroke-width="1" />
+    <PolygonSymbolizer fill="#cccccc" />
   </Rule>
 </Style>
 <Layer name="countries"
-   srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over">
+  srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over">
     <StyleName>countries</StyleName>
     <Datasource>
        <Parameter name="file"><![CDATA[[absolute path]]]></Parameter>

test/rendering/nesting_class.mml

@@ -0,0 +1,27 @@
+{
+    "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over",
+    "Stylesheet": [
+        "nesting_class.mss"
+    ],
+    "Layer": [
+    {
+        "name": "foo",
+        "class": "land",
+        "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over",
+        "Datasource": {
+            "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip",
+            "type": "shape"
+        }
+    },
+    {
+        "name": "lakes",
+        "id": "lakes",
+        "class": "land",
+        "srs": "+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over",
+        "Datasource": {
+            "file": "http://tilemill-data.s3.amazonaws.com/test_data/shape_demo.zip",
+            "type": "shape"
+        }
+    }
+    ]
+}

test/rendering/nesting_class.mss

@@ -0,0 +1,10 @@
+/* Applies to all layers with .land class */
+.land {
+  line-color: #ccc;
+  line-width: 0.5;
+  polygon-fill: #eee;
+  /* Applies to #lakes.land */
+  #lakes {
+    polygon-fill: #000;
+  }
+}

test/rendering/nesting_class.result

@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!DOCTYPE Map[]>
+<Map srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over" maximum-extent="-20037508.34,-20037508.34,20037508.34,20037508.34">
+
+
+<Style name="foo" filter-mode="first" >
+  <Rule>
+    <LineSymbolizer stroke="#cccccc" stroke-width="0.5" />
+    <PolygonSymbolizer fill="#eeeeee" />
+  </Rule>
+</Style>
+<Layer name="foo"
+  srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over">
+    <StyleName>foo</StyleName>
+    <Datasource>
+       <Parameter name="file"><![CDATA[[absolute path]]]></Parameter>
+       <Parameter name="type"><![CDATA[shape]]></Parameter>
+    </Datasource>
+  </Layer>
+
+<Style name="lakes" filter-mode="first" >
+  <Rule>
+    <LineSymbolizer stroke="#cccccc" stroke-width="0.5" />
+    <PolygonSymbolizer fill="#000000" />
+  </Rule>
+</Style>
+<Layer name="lakes"
+  srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over">
+    <StyleName>lakes</StyleName>
+    <Datasource>
+       <Parameter name="file"><![CDATA[[absolute path]]]></Parameter>
+       <Parameter name="type"><![CDATA[shape]]></Parameter>
+    </Datasource>
+  </Layer>
+
+</Map>