Merge pull request #1089 from ahocevar/no-geometry-type-groups

No longer group features by geometry type

src/ol/layer/vectorlayer.js

@@ -6,13 +6,7 @@ goog.require('goog.asserts');
 goog.require('goog.events.EventType');
 goog.require('goog.object');
 goog.require('ol.Feature');
-goog.require('ol.expr');
-goog.require('ol.expr.Literal');
-goog.require('ol.expr.Logical');
-goog.require('ol.expr.LogicalOp');
-goog.require('ol.expr.functions');
 goog.require('ol.extent');
-goog.require('ol.geom.GeometryType');
 goog.require('ol.layer.Layer');
 goog.require('ol.proj');
 goog.require('ol.source.Vector');
@@ -34,12 +28,6 @@ ol.layer.FeatureCache = function() {
    */
   this.idLookup_;
 
-  /**
-   * @type {Object.<string, ol.Feature>}
-   * @private
-   */
-  this.geometryTypeIndex_;
-
   /**
    * @type {ol.structs.RTree}
    * @private
@@ -56,11 +44,6 @@ ol.layer.FeatureCache = function() {
  */
 ol.layer.FeatureCache.prototype.clear = function() {
   this.idLookup_ = {};
-  var geometryTypeIndex = {};
-  for (var key in ol.geom.GeometryType) {
-    geometryTypeIndex[ol.geom.GeometryType[key]] = {};
-  }
-  this.geometryTypeIndex_ = geometryTypeIndex;
   this.rTree_ = new ol.structs.RTree();
 };
 
@@ -75,99 +58,18 @@ ol.layer.FeatureCache.prototype.add = function(feature) {
 
   this.idLookup_[id] = feature;
 
-  // index by geometry type and bounding box
+  // index by bounding box
   if (!goog.isNull(geometry)) {
-    var geometryType = geometry.getType();
+    this.rTree_.insert(geometry.getBounds(), feature);
-    this.geometryTypeIndex_[geometryType][id] = feature;
-    this.rTree_.insert(geometry.getBounds(),
-        feature, geometryType);
   }
 };
 
 
 /**
- * @param {ol.expr.Expression=} opt_expr Expression for filtering.
  * @return {Object.<string, ol.Feature>} Object of features, keyed by id.
  */
-ol.layer.FeatureCache.prototype.getFeaturesObject = function(opt_expr) {
+ol.layer.FeatureCache.prototype.getFeaturesObject = function() {
-  var features;
+  return this.idLookup_;
-  if (!goog.isDef(opt_expr)) {
-    features = this.idLookup_;
-  } else {
-    // check for geometryType or extent expression
-    var name = ol.expr.isLibCall(opt_expr);
-    if (name === ol.expr.functions.GEOMETRY_TYPE) {
-      var args = /** @type {ol.expr.Call} */ (opt_expr).getArgs();
-      goog.asserts.assert(args.length === 1);
-      goog.asserts.assert(args[0] instanceof ol.expr.Literal);
-      var type = /** @type {ol.expr.Literal } */ (args[0]).evaluate();
-      goog.asserts.assertString(type);
-      features = this.geometryTypeIndex_[type];
-    } else if (name === ol.expr.functions.EXTENT) {
-      var args = /** @type {ol.expr.Call} */ (opt_expr).getArgs();
-      goog.asserts.assert(args.length === 4);
-      for (var i = 0; i < 4; ++i) {
-        goog.asserts.assert(args[i] instanceof ol.expr.Literal);
-      }
-      var extent = [
-        /** @type {ol.expr.Literal} */ (args[0]).evaluate(),
-        /** @type {ol.expr.Literal} */ (args[1]).evaluate(),
-        /** @type {ol.expr.Literal} */ (args[2]).evaluate(),
-        /** @type {ol.expr.Literal} */ (args[3]).evaluate()
-      ];
-      features = this.rTree_.searchReturningObject(extent);
-    } else {
-      // not a call expression, check logical
-      if (opt_expr instanceof ol.expr.Logical) {
-        var op = /** @type {ol.expr.Logical} */ (opt_expr).getOperator();
-        if (op === ol.expr.LogicalOp.AND) {
-          var expressions = [opt_expr.getLeft(), opt_expr.getRight()];
-          var expr, args, type, extent;
-          for (var i = 0; i <= 1; ++i) {
-            expr = expressions[i];
-            name = ol.expr.isLibCall(expr);
-            if (name === ol.expr.functions.GEOMETRY_TYPE) {
-              args = /** @type {ol.expr.Call} */ (expr).getArgs();
-              goog.asserts.assert(args.length === 1);
-              goog.asserts.assert(args[0] instanceof ol.expr.Literal);
-              type = /** @type {ol.expr.Literal } */ (args[0]).evaluate();
-              goog.asserts.assertString(type);
-            } else if (name === ol.expr.functions.EXTENT) {
-              args = /** @type {ol.expr.Call} */ (expr).getArgs();
-              goog.asserts.assert(args.length === 4);
-              for (var j = 0; j < 4; ++j) {
-                goog.asserts.assert(args[j] instanceof ol.expr.Literal);
-              }
-              extent = [[
-                /** @type {ol.expr.Literal} */ (args[0]).evaluate(),
-                /** @type {ol.expr.Literal} */ (args[1]).evaluate()
-              ], [
-                /** @type {ol.expr.Literal} */ (args[2]).evaluate(),
-                /** @type {ol.expr.Literal} */ (args[3]).evaluate()
-              ]];
-            }
-          }
-          if (type && extent) {
-            features = this.getFeaturesObjectForExtent(extent,
-                /** @type {ol.geom.GeometryType} */ (type));
-          }
-        }
-      }
-    }
-    if (!goog.isDef(features)) {
-      // TODO: support fast lane for other filter types
-      var candidates = this.idLookup_,
-          feature;
-      features = {};
-      for (i in candidates) {
-        feature = candidates[i];
-        if (ol.expr.evaluateFeature(opt_expr, feature)) {
-          features[i] = feature;
-        }
-      }
-    }
-  }
-  return features;
 };
 
 
@@ -175,19 +77,10 @@ ol.layer.FeatureCache.prototype.getFeaturesObject = function(opt_expr) {
  * Get all features whose bounding box intersects the provided extent.
  *
  * @param {ol.Extent} extent Bounding extent.
- * @param {ol.geom.GeometryType=} opt_type Optional geometry type.
  * @return {Object.<string, ol.Feature>} Features.
  */
-ol.layer.FeatureCache.prototype.getFeaturesObjectForExtent = function(extent,
+ol.layer.FeatureCache.prototype.getFeaturesObjectForExtent = function(extent) {
-    opt_type) {
+  return this.rTree_.searchReturningObject(extent);
-  var features;
-  if (goog.isDef(opt_type) &&
-      goog.object.isEmpty(this.geometryTypeIndex_[opt_type])) {
-    features = {};
-  } else {
-    features = this.rTree_.searchReturningObject(extent, opt_type);
-  }
-  return features;
 };
 
 
@@ -228,10 +121,8 @@ ol.layer.FeatureCache.prototype.remove = function(feature) {
 
   delete this.idLookup_[id];
 
-  // index by geometry type and bounding box
+  // index by bounding box
   if (!goog.isNull(geometry)) {
-    var geometryType = geometry.getType();
-    delete this.geometryTypeIndex_[geometryType][id];
     this.rTree_.remove(geometry.getBounds(), feature);
   }
 };
@@ -362,18 +253,17 @@ ol.layer.Vector.prototype.getStyle = function() {
  *
  * @param {ol.Extent} extent Bounding extent.
  * @param {ol.proj.Projection} projection Target projection.
- * @param {ol.geom.GeometryType=} opt_type Optional geometry type.
  * @param {Function=} opt_callback Callback to call when data is parsed.
  * @return {Object.<string, ol.Feature>} Features or null if source is loading
  *     data for `extent`.
  */
 ol.layer.Vector.prototype.getFeaturesObjectForExtent = function(extent,
-    projection, opt_type, opt_callback) {
+    projection, opt_callback) {
   var source = this.getSource();
   return source.prepareFeatures(this, extent, projection, opt_callback) ==
       ol.source.VectorLoadState.LOADING ?
           null :
-          this.featureCache_.getFeaturesObjectForExtent(extent, opt_type);
+          this.featureCache_.getFeaturesObjectForExtent(extent);
 };
 
 

src/ol/renderer/canvas/canvasvectorlayerrenderer.js

@@ -101,21 +101,6 @@ ol.renderer.canvas.VectorLayer = function(mapRenderer, layer) {
    */
   this.tileArchetype_ = null;
 
-  /**
-   * Geometry types in rendering order.
-   * TODO: these will go away shortly (in favor of one call per symbolizer type)
-   * @private
-   * @type {Array.<ol.geom.GeometryType>}
-   */
-  this.geometryTypes_ = [
-    ol.geom.GeometryType.POINT,
-    ol.geom.GeometryType.MULTIPOINT,
-    ol.geom.GeometryType.LINESTRING,
-    ol.geom.GeometryType.MULTILINESTRING,
-    ol.geom.GeometryType.POLYGON,
-    ol.geom.GeometryType.MULTIPOLYGON
-  ];
-
   /**
    * @private
    * @type {number}
@@ -464,14 +449,10 @@ ol.renderer.canvas.VectorLayer.prototype.renderFrame =
   var tilesOnSketchCanvas = {};
   // TODO make gutter configurable?
   var tileGutter = 15 * tileResolution;
-  var tile, tileCoord, key, x, y;
+  var tile, tileCoord, key, x, y, i, type;
-  // render features by geometry type
+  var deferred = false;
-  var types = this.geometryTypes_,
+  var dirty = false;
-      numTypes = types.length,
+  var tileExtent, groups, group, j, numGroups, featuresObject, tileHasFeatures;
-      deferred = false,
-      dirty = false,
-      i, type, tileExtent,
-      groups, group, j, numGroups, featuresObject, tileHasFeatures;
   fetchTileData:
   for (x = tileRange.minX; x <= tileRange.maxX; ++x) {
     for (y = tileRange.minY; y <= tileRange.maxY; ++y) {
@@ -486,21 +467,15 @@ ol.renderer.canvas.VectorLayer.prototype.renderFrame =
         tileExtent[1] -= tileGutter;
         tileExtent[3] += tileGutter;
         tileHasFeatures = false;
-        for (i = 0; i < numTypes; ++i) {
+        featuresObject = layer.getFeaturesObjectForExtent(tileExtent,
-          type = types[i];
+            projection, this.requestMapRenderFrame_);
-          if (!goog.isDef(featuresToRender[type])) {
+        if (goog.isNull(featuresObject)) {
-            featuresToRender[type] = {};
+          deferred = true;
-          }
+          break fetchTileData;
-          featuresObject = layer.getFeaturesObjectForExtent(tileExtent,
-              projection, type, this.requestMapRenderFrame_);
-          if (goog.isNull(featuresObject)) {
-            deferred = true;
-            break fetchTileData;
-          }
-          tileHasFeatures = tileHasFeatures ||
-              !goog.object.isEmpty(featuresObject);
-          goog.object.extend(featuresToRender[type], featuresObject);
         }
+        tileHasFeatures = tileHasFeatures ||
+            !goog.object.isEmpty(featuresObject);
+        goog.object.extend(featuresToRender, featuresObject);
         if (tileHasFeatures) {
           tilesOnSketchCanvas[key] = tileCoord;
         }
@@ -511,19 +486,15 @@ ol.renderer.canvas.VectorLayer.prototype.renderFrame =
   }
   this.dirty_ = dirty;
 
-  renderByGeometryType:
+  groups = layer.groupFeaturesBySymbolizerLiteral(featuresToRender,
-  for (type in featuresToRender) {
+      tileResolution);
-    groups = layer.groupFeaturesBySymbolizerLiteral(
+  numGroups = groups.length;
-        featuresToRender[type], tileResolution);
+  for (j = 0; j < numGroups; ++j) {
-    numGroups = groups.length;
+    group = groups[j];
-    for (j = 0; j < numGroups; ++j) {
+    deferred = sketchCanvasRenderer.renderFeatures(group[0], group[1],
-      group = groups[j];
+        group[2]);
-      deferred = sketchCanvasRenderer.renderFeaturesByGeometryType(
+    if (deferred) {
-          /** @type {ol.geom.GeometryType} */ (type),
+      break;
-          group[0], group[1], group[2]);
-      if (deferred) {
-        break renderByGeometryType;
-      }
     }
   }
 

src/ol/renderer/canvas/canvasvectorrenderer.js

@@ -114,42 +114,21 @@ ol.renderer.canvas.Vector.prototype.getMaxSymbolSize = function() {
 
 
 /**
- * @param {ol.geom.GeometryType} type Geometry type.
  * @param {Array.<ol.Feature>} features Array of features.
  * @param {ol.style.Literal} symbolizer Symbolizer.
  * @param {Array} data Additional data.
  * @return {boolean} true if deferred, false if rendered.
  */
-ol.renderer.canvas.Vector.prototype.renderFeaturesByGeometryType =
+ol.renderer.canvas.Vector.prototype.renderFeatures =
-    function(type, features, symbolizer, data) {
+    function(features, symbolizer, data) {
   var deferred = false;
-  if (!(symbolizer instanceof ol.style.TextLiteral)) {
+  if (symbolizer instanceof ol.style.PointLiteral) {
-    switch (type) {
+    deferred = this.renderPointFeatures_(features, symbolizer);
-      case ol.geom.GeometryType.POINT:
+  } else if (symbolizer instanceof ol.style.LineLiteral) {
-      case ol.geom.GeometryType.MULTIPOINT:
+    this.renderLineStringFeatures_(features, symbolizer);
-        goog.asserts.assert(symbolizer instanceof ol.style.PointLiteral,
+  } else if (symbolizer instanceof ol.style.PolygonLiteral) {
-            'Expected point symbolizer: ' + symbolizer);
+    this.renderPolygonFeatures_(features, symbolizer);
-        deferred = this.renderPointFeatures_(
+  } else if (symbolizer instanceof ol.style.TextLiteral) {
-            features, /** @type {ol.style.PointLiteral} */ (symbolizer));
-        break;
-      case ol.geom.GeometryType.LINESTRING:
-      case ol.geom.GeometryType.MULTILINESTRING:
-        goog.asserts.assert(symbolizer instanceof ol.style.LineLiteral,
-            'Expected line symbolizer: ' + symbolizer);
-        this.renderLineStringFeatures_(
-            features, /** @type {ol.style.LineLiteral} */ (symbolizer));
-        break;
-      case ol.geom.GeometryType.POLYGON:
-      case ol.geom.GeometryType.MULTIPOLYGON:
-        goog.asserts.assert(symbolizer instanceof ol.style.PolygonLiteral,
-            'Expected polygon symbolizer: ' + symbolizer);
-        this.renderPolygonFeatures_(
-            features, /** @type {ol.style.PolygonLiteral} */ (symbolizer));
-        break;
-      default:
-        throw new Error('Rendering not implemented for geometry type: ' + type);
-    }
-  } else {
     this.renderText_(features, symbolizer, data);
   }
   return deferred;

test/spec/ol/layer/vectorlayer.test.js

@@ -23,24 +23,6 @@ describe('ol.layer.Vector', function() {
         new ol.Feature({
           g: new ol.geom.Point([16.0, 48.0])
         }),
-        new ol.Feature({
-          g: new ol.geom.Point([16.1, 48.1])
-        }),
-        new ol.Feature({
-          g: new ol.geom.Point([16.2, 48.2])
-        }),
-        new ol.Feature({
-          g: new ol.geom.Point([16.3, 48.3])
-        }),
-        new ol.Feature({
-          g: new ol.geom.LineString([[16.4, 48.4], [16.5, 48.5]])
-        }),
-        new ol.Feature({
-          g: new ol.geom.LineString([[16.6, 48.6], [16.7, 48.7]])
-        }),
-        new ol.Feature({
-          g: new ol.geom.LineString([[16.8, 48.8], [16.9, 48.9]])
-        }),
         new ol.Feature({
           g: new ol.geom.LineString([[17.0, 49.0], [17.1, 49.1]])
         })
@@ -51,47 +33,9 @@ describe('ol.layer.Vector', function() {
       layer.addFeatures(features);
     });
 
-    var geomFilter = ol.expr.parse('geometryType("linestring")');
+    it('returns the features in an object', function() {
-    var extentFilter = ol.expr.parse('extent(16, 48, 16.3, 48.3)');
+      var featuresObject = layer.featureCache_.getFeaturesObject();
-
+      expect(goog.object.getCount(featuresObject)).to.eql(features.length);
-    it('can filter by geometry type using its GeometryType index', function() {
-      sinon.spy(geomFilter, 'evaluate');
-      var lineStrings = layer.featureCache_.getFeaturesObject(geomFilter);
-      expect(geomFilter.evaluate).to.not.be.called();
-      expect(goog.object.getCount(lineStrings)).to.eql(4);
-      expect(goog.object.getValues(lineStrings)).to.contain(features[4]);
-    });
-
-    it('can filter by extent using its RTree', function() {
-      sinon.spy(extentFilter, 'evaluate');
-      var subset = layer.featureCache_.getFeaturesObject(extentFilter);
-      expect(extentFilter.evaluate).to.not.be.called();
-      expect(goog.object.getCount(subset)).to.eql(4);
-      expect(goog.object.getValues(subset)).not.to.contain(features[7]);
-    });
-
-    it('can filter by extent and geometry type using its index', function() {
-      var filter1 = new ol.expr.Logical(
-          ol.expr.LogicalOp.AND, geomFilter, extentFilter);
-      var filter2 = new ol.expr.Logical(
-          ol.expr.LogicalOp.AND, extentFilter, geomFilter);
-      sinon.spy(filter1, 'evaluate');
-      sinon.spy(filter2, 'evaluate');
-      var subset1 = layer.featureCache_.getFeaturesObject(filter1);
-      var subset2 = layer.featureCache_.getFeaturesObject(filter2);
-      expect(filter1.evaluate).to.not.be.called();
-      expect(filter2.evaluate).to.not.be.called();
-      expect(goog.object.getCount(subset1)).to.eql(0);
-      expect(goog.object.getCount(subset2)).to.eql(0);
-    });
-
-    it('can handle query using the filter\'s evaluate function', function() {
-      var filter = new ol.expr.Logical(
-          ol.expr.LogicalOp.OR, geomFilter, extentFilter);
-      sinon.spy(filter, 'evaluate');
-      var subset = layer.featureCache_.getFeaturesObject(filter);
-      expect(filter.evaluate).to.be.called();
-      expect(goog.object.getCount(subset)).to.eql(8);
     });
 
   });
@@ -181,8 +125,6 @@ goog.require('goog.dispose');
 goog.require('goog.object');
 goog.require('ol.Feature');
 goog.require('ol.expr');
-goog.require('ol.expr.Logical');
-goog.require('ol.expr.LogicalOp');
 goog.require('ol.geom.LineString');
 goog.require('ol.geom.Point');
 goog.require('ol.proj');