Merge pull request #4854 from ahocevar/fix-select-and-everything-that-relies-on-unmanaged-layers-and-skipped-features-this-time-for-real---hopefully

Always report skipped feature hits for the original layer

src/ol/interaction/selectinteraction.js

@@ -177,7 +177,7 @@ ol.interaction.Select = function(opt_options) {
        */
       layerFilter = function(layer) {
         goog.asserts.assertFunction(options.layers);
-        return layer === featureOverlay || options.layers(layer);
+        return options.layers(layer);
       };
     } else {
       var layers = options.layers;
@@ -186,7 +186,7 @@ ol.interaction.Select = function(opt_options) {
        * @return {boolean} Include.
        */
       layerFilter = function(layer) {
-        return layer === featureOverlay || ol.array.includes(layers, layer);
+        return ol.array.includes(layers, layer);
       };
     }
   } else {
@@ -322,18 +322,13 @@ ol.interaction.Select.handleEvent = function(mapBrowserEvent) {
          * @param {ol.layer.Layer} layer Layer.
          */
         function(feature, layer) {
-          goog.asserts.assertInstanceof(feature, ol.Feature);
+          if (this.filter_(feature, layer)) {
-          if (layer !== null) {
+            if ((add || toggle) &&
-            if (add || toggle) {
+                !ol.array.includes(features.getArray(), feature)) {
-              if (this.filter_(feature, layer) &&
+              selected.push(feature);
-                  !ol.array.includes(features.getArray(), feature) &&
+              this.addFeatureLayerAssociation_(feature, layer);
-                  !ol.array.includes(selected, feature)) {
+            } else if ((remove || toggle) &&
-                selected.push(feature);
+                ol.array.includes(features.getArray(), feature)) {
-                this.addFeatureLayerAssociation_(feature, layer);
-              }
-            }
-          } else if (this.featureOverlay_.getSource().hasFeature(feature)) {
-            if (remove || toggle) {
               deselected.push(feature);
               this.removeFeatureLayerAssociation_(feature);
             }

src/ol/renderer/canvas/canvasvectorlayerrenderer.js

@@ -165,11 +165,10 @@ ol.renderer.canvas.VectorLayer.prototype.forEachFeatureAtCoordinate = function(c
     var resolution = frameState.viewState.resolution;
     var rotation = frameState.viewState.rotation;
     var layer = this.getLayer();
-    var layerState = frameState.layerStates[goog.getUid(layer)];
     /** @type {Object.<string, boolean>} */
     var features = {};
     return this.replayGroup_.forEachFeatureAtCoordinate(coordinate, resolution,
-        rotation, layerState.managed ? frameState.skippedFeatureUids : {},
+        rotation, {},
         /**
          * @param {ol.Feature|ol.render.Feature} feature Feature.
          * @return {?} Callback result.

src/ol/renderer/canvas/canvasvectortilelayerrenderer.js

@@ -306,7 +306,6 @@ ol.renderer.canvas.VectorTileLayer.prototype.forEachFeatureAtCoordinate = functi
   var resolution = frameState.viewState.resolution;
   var rotation = frameState.viewState.rotation;
   var layer = this.getLayer();
-  var layerState = frameState.layerStates[goog.getUid(layer)];
   /** @type {Object.<string, boolean>} */
   var features = {};
 
@@ -340,8 +339,7 @@ ol.renderer.canvas.VectorTileLayer.prototype.forEachFeatureAtCoordinate = functi
     }
     replayGroup = tile.getReplayState().replayGroup;
     found = found || replayGroup.forEachFeatureAtCoordinate(
-        tileSpaceCoordinate, resolution, rotation,
+        tileSpaceCoordinate, resolution, rotation, {},
-        layerState.managed ? frameState.skippedFeatureUids : {},
         /**
          * @param {ol.Feature|ol.render.Feature} feature Feature.
          * @return {?} Callback result.

src/ol/renderer/dom/domvectorlayerrenderer.js

@@ -179,11 +179,10 @@ ol.renderer.dom.VectorLayer.prototype.forEachFeatureAtCoordinate = function(coor
     var resolution = frameState.viewState.resolution;
     var rotation = frameState.viewState.rotation;
     var layer = this.getLayer();
-    var layerState = frameState.layerStates[goog.getUid(layer)];
     /** @type {Object.<string, boolean>} */
     var features = {};
     return this.replayGroup_.forEachFeatureAtCoordinate(coordinate, resolution,
-        rotation, layerState.managed ? frameState.skippedFeatureUids : {},
+        rotation, {},
         /**
          * @param {ol.Feature|ol.render.Feature} feature Feature.
          * @return {?} Callback result.

src/ol/renderer/maprenderer.js

@@ -131,19 +131,17 @@ ol.renderer.Map.prototype.forEachFeatureAtCoordinate = function(coordinate, fram
   var viewState = frameState.viewState;
   var viewResolution = viewState.resolution;
 
-  /** @type {Object.<string, boolean>} */
-  var features = {};
-
   /**
    * @param {ol.Feature|ol.render.Feature} feature Feature.
+   * @param {ol.layer.Layer} layer Layer.
    * @return {?} Callback result.
    */
-  function forEachFeatureAtCoordinate(feature) {
+  function forEachFeatureAtCoordinate(feature, layer) {
     goog.asserts.assert(feature !== undefined, 'received a feature');
     var key = goog.getUid(feature).toString();
-    if (!(key in features)) {
+    var managed = frameState.layerStates[goog.getUid(layer)].managed;
-      features[key] = true;
+    if (!(key in frameState.skippedFeatureUids && !managed)) {
-      return callback.call(thisArg, feature, null);
+      return callback.call(thisArg, feature, managed ? layer : null);
     }
   }
 
@@ -172,9 +170,7 @@ ol.renderer.Map.prototype.forEachFeatureAtCoordinate = function(coordinate, fram
       if (layer.getSource()) {
         result = layerRenderer.forEachFeatureAtCoordinate(
             layer.getSource().getWrapX() ? translatedCoordinate : coordinate,
-            frameState,
+            frameState, forEachFeatureAtCoordinate, thisArg);
-            layerState.managed ? callback : forEachFeatureAtCoordinate,
-            thisArg);
       }
       if (result) {
         return result;

src/ol/renderer/webgl/webglvectorlayerrenderer.js

@@ -115,7 +115,7 @@ ol.renderer.webgl.VectorLayer.prototype.forEachFeatureAtCoordinate = function(co
     return this.replayGroup_.forEachFeatureAtCoordinate(coordinate,
         context, viewState.center, viewState.resolution, viewState.rotation,
         frameState.size, frameState.pixelRatio, layerState.opacity,
-        layerState.managed ? frameState.skippedFeatureUids : {},
+        {},
         /**
          * @param {ol.Feature} feature Feature.
          * @return {?} Callback result.

src/ol/source/vectorsource.js

@@ -738,16 +738,6 @@ ol.source.Vector.prototype.handleFeatureChange_ = function(event) {
 };
 
 
-/**
- * @param {ol.Feature} feature Feature.
- * @return {boolean} Feature is in source.
- */
-ol.source.Vector.prototype.hasFeature = function(feature) {
-  var id = feature.getId();
-  return id ? id in this.idIndex_ : goog.getUid(feature) in this.undefIdIndex_;
-};
-
-
 /**
  * @return {boolean} Is empty.
  */

test/spec/ol/renderer/canvas/canvasmaprenderer.test.js

@@ -64,6 +64,23 @@ describe('ol.renderer.canvas.Map', function() {
       expect(cb.firstCall.args[1]).to.be(null);
     });
 
+    it('calls callback with main layer when skipped feature on unmanaged layer', function() {
+      var feature = layer.getSource().getFeatures()[0];
+      var managedLayer = new ol.layer.Vector({
+        source: new ol.source.Vector({
+          features: [feature]
+        })
+      });
+      map.addLayer(managedLayer);
+      map.skipFeature(feature);
+      layer.setMap(map);
+      map.renderSync();
+      var cb = sinon.spy();
+      map.forEachFeatureAtPixel(map.getPixelFromCoordinate([0, 0]), cb);
+      expect(cb.callCount).to.be(1);
+      expect(cb.firstCall.args[1]).to.be(managedLayer);
+    });
+
     it('filters managed layers', function() {
       map.addLayer(layer);
       map.renderSync();