Moving all style functionality from source to style

src/ol/renderer/canvas/canvasvectorlayerrenderer.js

@@ -18,6 +18,7 @@ goog.require('ol.layer.VectorLayerRenderIntent');
 goog.require('ol.renderer.canvas.Layer');
 goog.require('ol.renderer.canvas.Vector');
 goog.require('ol.source.VectorEventType');
+goog.require('ol.style');
 goog.require('ol.tilegrid.TileGrid');
 
 
@@ -475,7 +476,7 @@ ol.renderer.canvas.VectorLayer.prototype.renderFrame =
   var dirty = false;
   var layer = this.getVectorLayer();
   var source = layer.getSource();
-  var tileExtent, groups, group, j, numGroups, featuresObject, tileHasFeatures;
+  var tileExtent, featuresObject, tileHasFeatures;
   fetchTileData:
   for (x = tileRange.minX; x <= tileRange.maxX; ++x) {
     for (y = tileRange.minY; y <= tileRange.maxY; ++y) {
@@ -509,10 +510,15 @@ ol.renderer.canvas.VectorLayer.prototype.renderFrame =
   }
   this.dirty_ = dirty;
 
-  groups = source.groupFeaturesBySymbolizerLiteral(layer.getStyle(),
+  var style = layer.getStyle();
+  if (goog.isNull(style)) {
+    style = ol.style.getDefault();
+  }
+  var groups = style.groupFeaturesBySymbolizerLiteral(
       featuresToRender, tileResolution);
-  numGroups = groups.length;
+  var numGroups = groups.length;
-  for (j = 0; j < numGroups; ++j) {
+  var group;
+  for (var j = 0; j < numGroups; ++j) {
     group = groups[j];
     deferred = sketchCanvasRenderer.renderFeatures(group[0], group[1],
         group[2]);

src/ol/source/vectorsource.js

@@ -13,9 +13,6 @@ goog.require('ol.extent');
 goog.require('ol.proj');
 goog.require('ol.source.Source');
 goog.require('ol.structs.RTree');
-goog.require('ol.style');
-goog.require('ol.style.Style');
-goog.require('ol.style.TextLiteral');
 
 
 /**
@@ -150,66 +147,6 @@ ol.source.Vector.prototype.getFeaturesObjectForExtent = function(extent,
 };
 
 
-/**
- * TODO: This should be a ol.style.Style method.
- * @param {ol.style.Style} style Style.
- * @param {Object.<string, ol.Feature>} features Features.
- * @param {number} resolution Map resolution.
- * @return {Array.<Array>} symbolizers for features. Each array in this array
- *     contains 3 items: an array of features, the symbolizer literal, and
- *     an array with optional additional data for each feature.
- */
-ol.source.Vector.prototype.groupFeaturesBySymbolizerLiteral =
-    function(style, features, resolution) {
-  var uniqueLiterals = {},
-      featuresBySymbolizer = [],
-      i, j, l, feature, symbolizers, literals, numLiterals, literal,
-      uniqueLiteral, key, item;
-  for (i in features) {
-    feature = features[i];
-    // feature level symbolizers take precedence
-    symbolizers = feature.getSymbolizers();
-    if (!goog.isNull(symbolizers)) {
-      literals = ol.style.Style.createLiterals(symbolizers, feature);
-    } else {
-      // layer style second
-      if (goog.isNull(style)) {
-        style = ol.style.getDefault();
-      }
-      literals = style.createLiterals(feature, resolution);
-    }
-    numLiterals = literals.length;
-    for (j = 0; j < numLiterals; ++j) {
-      literal = literals[j];
-      for (l in uniqueLiterals) {
-        uniqueLiteral = featuresBySymbolizer[uniqueLiterals[l]][1];
-        if (literal.equals(uniqueLiteral)) {
-          literal = uniqueLiteral;
-          break;
-        }
-      }
-      key = goog.getUid(literal);
-      if (!goog.object.containsKey(uniqueLiterals, key)) {
-        uniqueLiterals[key] = featuresBySymbolizer.length;
-        featuresBySymbolizer.push([
-          /** @type {Array.<ol.Feature>} */ ([]),
-          /** @type {ol.style.Literal} */ (literal),
-          /** @type {Array} */ ([])
-        ]);
-      }
-      item = featuresBySymbolizer[uniqueLiterals[key]];
-      item[0].push(feature);
-      if (literal instanceof ol.style.TextLiteral) {
-        item[2].push(literals[j].text);
-      }
-    }
-  }
-  // TODO: move sort function to ol.style.Style
-  featuresBySymbolizer.sort(this.sortByZIndex_);
-  return featuresBySymbolizer;
-};
-
-
 /**
  * @param {Object|Element|Document|string} data Feature data.
  * @param {ol.proj.Projection} projection This sucks.  The layer should be a
@@ -365,18 +302,6 @@ ol.source.Vector.prototype.removeFeatures = function(features) {
 };
 
 
-/**
- * Sort function for `groupFeaturesBySymbolizerLiteral`.
- * @private
- * @param {Array} a 1st item for the sort comparison.
- * @param {Array} b 2nd item for the sort comparison.
- * @return {number} Comparison result.
- */
-ol.source.Vector.prototype.sortByZIndex_ = function(a, b) {
-  return a[1].zIndex - b[1].zIndex;
-};
-
-
 
 /**
  * @constructor

src/ol/style/style.js

@@ -15,6 +15,7 @@ goog.require('ol.style.Rule');
 goog.require('ol.style.Shape');
 goog.require('ol.style.Stroke');
 goog.require('ol.style.Symbolizer');
+goog.require('ol.style.TextLiteral');
 
 
 
@@ -82,6 +83,71 @@ ol.style.Style.prototype.createLiterals = function(feature, resolution) {
 };
 
 
+/**
+ * @param {Object.<string, ol.Feature>} features Features.
+ * @param {number} resolution Map resolution.
+ * @return {Array.<Array>} symbolizers for features. Each array in this array
+ *     contains 3 items: an array of features, the symbolizer literal, and
+ *     an array with optional additional data for each feature.
+ */
+ol.style.Style.prototype.groupFeaturesBySymbolizerLiteral =
+    function(features, resolution) {
+  var uniqueLiterals = {},
+      featuresBySymbolizer = [],
+      i, j, l, feature, symbolizers, literals, numLiterals, literal,
+      uniqueLiteral, key, item;
+  for (i in features) {
+    feature = features[i];
+    // feature level symbolizers take precedence
+    symbolizers = feature.getSymbolizers();
+    if (!goog.isNull(symbolizers)) {
+      literals = ol.style.Style.createLiterals(symbolizers, feature);
+    } else {
+      literals = this.createLiterals(feature, resolution);
+    }
+    numLiterals = literals.length;
+    for (j = 0; j < numLiterals; ++j) {
+      literal = literals[j];
+      for (l in uniqueLiterals) {
+        uniqueLiteral = featuresBySymbolizer[uniqueLiterals[l]][1];
+        if (literal.equals(uniqueLiteral)) {
+          literal = uniqueLiteral;
+          break;
+        }
+      }
+      key = goog.getUid(literal);
+      if (!goog.object.containsKey(uniqueLiterals, key)) {
+        uniqueLiterals[key] = featuresBySymbolizer.length;
+        featuresBySymbolizer.push([
+          /** @type {Array.<ol.Feature>} */ ([]),
+          /** @type {ol.style.Literal} */ (literal),
+          /** @type {Array} */ ([])
+        ]);
+      }
+      item = featuresBySymbolizer[uniqueLiterals[key]];
+      item[0].push(feature);
+      if (literal instanceof ol.style.TextLiteral) {
+        item[2].push(literals[j].text);
+      }
+    }
+  }
+  featuresBySymbolizer.sort(this.sortByZIndex_);
+  return featuresBySymbolizer;
+};
+
+
+/**
+ * Sort function for `groupFeaturesBySymbolizerLiteral`.
+ * @private
+ * @param {Array} a 1st item for the sort comparison.
+ * @param {Array} b 2nd item for the sort comparison.
+ * @return {number} Comparison result.
+ */
+ol.style.Style.prototype.sortByZIndex_ = function(a, b) {
+  return a[1].zIndex - b[1].zIndex;
+};
+
+
 /**
  * The default style.
  * @type {ol.style.Style}

test/spec/ol/source/vectorsource.test.js

@@ -21,106 +21,6 @@ describe('ol.source.Vector', function() {
     });
   });
 
-  describe('#groupFeaturesBySymbolizerLiteral()', function() {
-
-    var source = new ol.source.Vector({
-      projection: ol.proj.get('EPSG:4326')
-    });
-
-    var style = new ol.style.Style({
-      symbolizers: [
-        new ol.style.Stroke({
-          width: 2,
-          color: ol.expr.parse('colorProperty'),
-          opacity: 1
-        })
-      ]
-    });
-    var features;
-
-    it('groups equal symbolizers', function() {
-      features = [
-        new ol.Feature({
-          g: new ol.geom.LineString([[-10, -10], [10, 10]]),
-          colorProperty: '#BADA55'
-        }),
-        new ol.Feature({
-          g: new ol.geom.LineString([[-10, 10], [10, -10]]),
-          colorProperty: '#013'
-        }),
-        new ol.Feature({
-          g: new ol.geom.LineString([[10, -10], [-10, -10]]),
-          colorProperty: '#013'
-        })
-      ];
-
-      var groups = source.groupFeaturesBySymbolizerLiteral(style, features, 1);
-      expect(groups.length).to.be(2);
-      expect(groups[0][0].length).to.be(1);
-      expect(groups[0][1].color).to.be('#BADA55');
-      expect(groups[1][0].length).to.be(2);
-      expect(groups[1][1].color).to.be('#013');
-    });
-
-    it('groups equal symbolizers also when defined on features', function() {
-      var symbolizer = new ol.style.Stroke({
-        width: 3,
-        color: ol.expr.parse('colorProperty'),
-        opacity: 1
-      });
-      var anotherSymbolizer = new ol.style.Stroke({
-        width: 3,
-        color: '#BADA55',
-        opacity: 1
-      });
-      var featureWithSymbolizers = new ol.Feature({
-        g: new ol.geom.LineString([[-10, -10], [-10, 10]]),
-        colorProperty: '#BADA55'
-      });
-      featureWithSymbolizers.setSymbolizers([symbolizer]);
-      var anotherFeatureWithSymbolizers = new ol.Feature({
-        g: new ol.geom.LineString([[-10, 10], [-10, -10]])
-      });
-      anotherFeatureWithSymbolizers.setSymbolizers([anotherSymbolizer]);
-      features.push(featureWithSymbolizers, anotherFeatureWithSymbolizers);
-
-      var groups = source.groupFeaturesBySymbolizerLiteral(style, features, 1);
-      expect(groups).to.have.length(3);
-      expect(groups[2][0].length).to.be(2);
-      expect(groups[2][1].width).to.be(3);
-
-    });
-
-    it('sorts groups by zIndex', function() {
-      var symbolizer = new ol.style.Stroke({
-        width: 3,
-        color: '#BADA55',
-        opacity: 1,
-        zIndex: 1
-      });
-      var anotherSymbolizer = new ol.style.Stroke({
-        width: 3,
-        color: '#BADA55',
-        opacity: 1
-      });
-      var featureWithSymbolizers = new ol.Feature({
-        g: new ol.geom.LineString([[-10, -10], [-10, 10]])
-      });
-      featureWithSymbolizers.setSymbolizers([symbolizer]);
-      var anotherFeatureWithSymbolizers = new ol.Feature({
-        g: new ol.geom.LineString([[-10, 10], [-10, -10]])
-      });
-      anotherFeatureWithSymbolizers.setSymbolizers([anotherSymbolizer]);
-      features = [featureWithSymbolizers, anotherFeatureWithSymbolizers];
-
-      var groups = source.groupFeaturesBySymbolizerLiteral(style, features, 1);
-      expect(groups).to.have.length(2);
-      expect(groups[0][1].zIndex).to.be(0);
-      expect(groups[1][1].zIndex).to.be(1);
-    });
-
-  });
-
   describe('#prepareFeatures_', function() {
     it('loads and parses data from a file', function(done) {
       var source = new ol.source.Vector({
@@ -249,7 +149,6 @@ goog.require('goog.dispose');
 goog.require('goog.events');
 goog.require('goog.object');
 goog.require('ol.Feature');
-goog.require('ol.expr');
 goog.require('ol.geom.LineString');
 goog.require('ol.geom.Point');
 goog.require('ol.parser.GeoJSON');
@@ -258,5 +157,3 @@ goog.require('ol.source.FeatureCache');
 goog.require('ol.source.Source');
 goog.require('ol.source.Vector');
 goog.require('ol.source.VectorEventType');
-goog.require('ol.style.Stroke');
-goog.require('ol.style.Style');

test/spec/ol/style/style.test.js

@@ -122,6 +122,102 @@ describe('ol.style.Style', function() {
 
   });
 
+  describe('#groupFeaturesBySymbolizerLiteral()', function() {
+
+    var style = new ol.style.Style({
+      symbolizers: [
+        new ol.style.Stroke({
+          width: 2,
+          color: ol.expr.parse('colorProperty'),
+          opacity: 1
+        })
+      ]
+    });
+    var features;
+
+    it('groups equal symbolizers', function() {
+      features = [
+        new ol.Feature({
+          g: new ol.geom.LineString([[-10, -10], [10, 10]]),
+          colorProperty: '#BADA55'
+        }),
+        new ol.Feature({
+          g: new ol.geom.LineString([[-10, 10], [10, -10]]),
+          colorProperty: '#013'
+        }),
+        new ol.Feature({
+          g: new ol.geom.LineString([[10, -10], [-10, -10]]),
+          colorProperty: '#013'
+        })
+      ];
+
+      var groups = style.groupFeaturesBySymbolizerLiteral(features, 1);
+      expect(groups.length).to.be(2);
+      expect(groups[0][0].length).to.be(1);
+      expect(groups[0][1].color).to.be('#BADA55');
+      expect(groups[1][0].length).to.be(2);
+      expect(groups[1][1].color).to.be('#013');
+    });
+
+    it('groups equal symbolizers also when defined on features', function() {
+      var symbolizer = new ol.style.Stroke({
+        width: 3,
+        color: ol.expr.parse('colorProperty'),
+        opacity: 1
+      });
+      var anotherSymbolizer = new ol.style.Stroke({
+        width: 3,
+        color: '#BADA55',
+        opacity: 1
+      });
+      var featureWithSymbolizers = new ol.Feature({
+        g: new ol.geom.LineString([[-10, -10], [-10, 10]]),
+        colorProperty: '#BADA55'
+      });
+      featureWithSymbolizers.setSymbolizers([symbolizer]);
+      var anotherFeatureWithSymbolizers = new ol.Feature({
+        g: new ol.geom.LineString([[-10, 10], [-10, -10]])
+      });
+      anotherFeatureWithSymbolizers.setSymbolizers([anotherSymbolizer]);
+      features.push(featureWithSymbolizers, anotherFeatureWithSymbolizers);
+
+      var groups = style.groupFeaturesBySymbolizerLiteral(features, 1);
+      expect(groups).to.have.length(3);
+      expect(groups[2][0].length).to.be(2);
+      expect(groups[2][1].width).to.be(3);
+
+    });
+
+    it('sorts groups by zIndex', function() {
+      var symbolizer = new ol.style.Stroke({
+        width: 3,
+        color: '#BADA55',
+        opacity: 1,
+        zIndex: 1
+      });
+      var anotherSymbolizer = new ol.style.Stroke({
+        width: 3,
+        color: '#BADA55',
+        opacity: 1
+      });
+      var featureWithSymbolizers = new ol.Feature({
+        g: new ol.geom.LineString([[-10, -10], [-10, 10]])
+      });
+      featureWithSymbolizers.setSymbolizers([symbolizer]);
+      var anotherFeatureWithSymbolizers = new ol.Feature({
+        g: new ol.geom.LineString([[-10, 10], [-10, -10]])
+      });
+      anotherFeatureWithSymbolizers.setSymbolizers([anotherSymbolizer]);
+      features = [featureWithSymbolizers, anotherFeatureWithSymbolizers];
+
+      var groups = style.groupFeaturesBySymbolizerLiteral(features, 1);
+      expect(groups).to.have.length(2);
+      expect(groups[0][1].zIndex).to.be(0);
+      expect(groups[1][1].zIndex).to.be(1);
+    });
+
+  });
+
   describe('ol.style.getDefault()', function() {
     var style = ol.style.getDefault();