second part of fix for #343

mapbox · Jun 4, 2017 · 848dc87 · 848dc87
1 parent c227f77
commit 848dc87
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 19 deletions.
diff --git a/lib/carto/tree/definition.js b/lib/carto/tree/definition.js
@@ -12,13 +12,14 @@ var assert = require('assert'),
 tree.Definition = function Definition(selector, rules) {
     this.elements = selector.elements;
     assert.ok(selector.filters instanceof tree.Filterset);
-    this.rules = rules;
     this.ruleIndex = {};
-    for (var i = 0; i < this.rules.length; i++) {
-        if ('zoom' in this.rules[i]) this.rules[i] = this.rules[i].clone();
-        this.rules[i].zoom = selector.zoom;
-        this.ruleIndex[this.rules[i].updateID()] = true;
+    for (var i = 0; i < rules.length; i++) {
+        if ('zoom' in rules[i]) rules[i] = rules[i].clone();
+        rules[i].zoom = selector.zoom;
+        rules[i].updateID();
     }
+    this.rules = [];
+    this.addRules(rules);
     this.filters = selector.filters;
     this.zoom = selector.zoom;
     this.attachment = selector.attachment || '__default__';
@@ -51,12 +52,14 @@ tree.Definition.prototype.clone = function(filters) {
 };
 
 tree.Definition.prototype.addRules = function(rules) {
-    var added = 0;
+    var added = 0,
+        idx = 0;
 
     // Add only unique rules.
     for (var i = 0; i < rules.length; i++) {
         if (!this.ruleIndex[rules[i].id]) {
-            this.rules.push(rules[i]);
+            idx = _.sortedIndexBy(this.rules, rules[i], 'zoom');
+            this.rules.splice(idx, 0, rules[i]);
             this.ruleIndex[rules[i].id] = true;
             added++;
         }
@@ -226,7 +229,7 @@ tree.Definition.prototype.collectSymbolizers = function(zooms, i) {
 
 // The tree.Zoom.toString function ignores the holes in zoom ranges and outputs
 // scaledenominators that cover the whole range from the first to last bit set.
-// This algorithm can produces zoom ranges that may have holes. However,
+// This algorithm can produce zoom ranges that may have holes. However,
 // when using the filter-mode="first", more specific zoom filters will always
 // end up before broader ranges. The filter-mode will pick those first before
 // resorting to the zoom range with the hole and stop processing further rules.

diff --git a/test/rendering-mss/issue_343a.xml b/test/rendering-mss/issue_343a.xml
@@ -1,23 +1,23 @@
 <Style name="style" filter-mode="first">
   <Rule>
     <MaxScaleDenominator>200000</MaxScaleDenominator>
-    <Filter>([tourism] = 'attraction') and ([tourism].match('anythingelse'))</Filter>
-    <MarkersSymbolizer marker-type="ellipse" fill="#ff0000" width="20" />
+    <Filter>([tourism].match('anythingelse')) and ([tourism] = 'attraction')</Filter>
+    <MarkersSymbolizer fill="#ff0000" marker-type="ellipse" width="20" />
   </Rule>
   <Rule>
     <MaxScaleDenominator>400000</MaxScaleDenominator>
     <MinScaleDenominator>200000</MinScaleDenominator>
-    <Filter>([tourism] = 'attraction') and ([tourism].match('anythingelse'))</Filter>
-    <MarkersSymbolizer marker-type="ellipse" fill="#0000ff" width="6" />
+    <Filter>([tourism].match('anythingelse')) and ([tourism] = 'attraction')</Filter>
+    <MarkersSymbolizer fill="#0000ff" marker-type="ellipse" width="6" />
   </Rule>
   <Rule>
     <MaxScaleDenominator>200000</MaxScaleDenominator>
     <Filter>([tourism] = 'attraction')</Filter>
-    <MarkersSymbolizer marker-type="ellipse" fill="#ff0000" width="20" />
+    <MarkersSymbolizer fill="#ff0000" marker-type="ellipse" width="20" />
   </Rule>
   <Rule>
     <MaxScaleDenominator>400000</MaxScaleDenominator>
     <Filter>([tourism].match('anythingelse'))</Filter>
-    <MarkersSymbolizer marker-type="ellipse" fill="#0000ff" width="6" />
+    <MarkersSymbolizer fill="#0000ff" marker-type="ellipse" width="6" />
   </Rule>
 </Style>
diff --git a/test/rendering-mss/issue_343b.xml b/test/rendering-mss/issue_343b.xml
@@ -2,22 +2,22 @@
   <Rule>
     <MaxScaleDenominator>200000</MaxScaleDenominator>
     <Filter>([tourism] = 'attraction') and ([tourism].match('anythingelse'))</Filter>
-    <MarkersSymbolizer marker-type="ellipse" fill="#ff0000" width="20" />
+    <MarkersSymbolizer fill="#ff0000" marker-type="ellipse" width="20" />
   </Rule>
   <Rule>
     <MaxScaleDenominator>400000</MaxScaleDenominator>
     <MinScaleDenominator>200000</MinScaleDenominator>
     <Filter>([tourism] = 'attraction') and ([tourism].match('anythingelse'))</Filter>
-    <MarkersSymbolizer marker-type="ellipse" fill="#0000ff" width="6" />
+    <MarkersSymbolizer fill="#0000ff" marker-type="ellipse" width="6" />
   </Rule>
   <Rule>
     <MaxScaleDenominator>200000</MaxScaleDenominator>
     <Filter>([tourism] = 'attraction')</Filter>
-    <MarkersSymbolizer marker-type="ellipse" fill="#ff0000" width="20" />
+    <MarkersSymbolizer fill="#ff0000" marker-type="ellipse" width="20" />
   </Rule>
   <Rule>
     <MaxScaleDenominator>400000</MaxScaleDenominator>
     <Filter>([tourism].match('anythingelse'))</Filter>
-    <MarkersSymbolizer marker-type="ellipse" fill="#0000ff" width="6" />
+    <MarkersSymbolizer fill="#0000ff" marker-type="ellipse" width="6" />
   </Rule>
-</Style>
+</Style>