Merge pull request #4391 from ahocevar/unmanaged-layer-foreachfeatureatpixel

Pass null as forEachFeatureAtPixel layer arg for unmanaged layers

changelog/upgrade-notes.md

@@ -46,6 +46,12 @@ but with additional css:
 
 With the introduction of true vector tile support, `ol.source.TileVector` becomes obsolete. Change your code to use `ol.layer.VectorTile` and `ol.source.VectorTile` instead of `ol.layer.Vector` and `ol.source.TileVector`.
 
+#### `ol.Map#forEachFeatureAtPixel` changes for unmanaged layers
+
+`ol.Map#forEachFeatureAtPixel` will still be called for unmanaged layers, but the 2nd argument to the callback function will be `null` instead of a reference to the unmanaged layer. This brings back the behavior of the abandoned `ol.FeatureOverlay` that was replaced by unmanaged layers.
+
+If you are affected by this change, please change your unmanaged layer to a regular layer by using e.g. `ol.Map#addLayer` instead of `ol.layer.Layer#setMap`.
+
 ### v3.10.0
 
 #### `ol.layer.Layer` changes

src/ol/interaction/selectinteraction.js

@@ -97,6 +97,8 @@ goog.inherits(ol.interaction.SelectEvent, goog.events.Event);
  * `toggle`, `add`/`remove`, and `multi` options; a `layers` filter; and a
  * further feature filter using the `filter` option.
  *
+ * Selected features are added to an internal unmanaged layer.
+ *
  * @constructor
  * @extends {ol.interaction.Interaction}
  * @param {olx.interaction.SelectOptions=} opt_options Options.
@@ -281,7 +283,7 @@ ol.interaction.Select.handleEvent = function(mapBrowserEvent) {
          * @param {ol.layer.Layer} layer Layer.
          */
         function(feature, layer) {
-          if (this.filter_(feature, layer)) {
+          if (layer && this.filter_(feature, layer)) {
             selected.push(feature);
             this.addFeatureLayerAssociation_(feature, layer);
             return !this.multi_;

src/ol/layer/layer.js

@@ -20,6 +20,11 @@ goog.require('ol.source.State');
  * Layers group together those properties that pertain to how the data is to be
  * displayed, irrespective of the source of that data.
  *
+ * Layers are usually added to a map with {@link ol.Map#addLayer}. Components
+ * like {@link ol.interaction.Select} use unmanaged layers internally. These
+ * unmanaged layers are associated with the map using
+ * {@link ol.layer.Layer#setMap} instead.
+ *
  * A generic `change` event is fired when the state of the source changes.
  *
  * @constructor

src/ol/map.js

@@ -599,8 +599,9 @@ ol.Map.prototype.disposeInternal = function() {
  *     called with two arguments. The first argument is one
  *     {@link ol.Feature feature} or
  *     {@link ol.render.Feature render feature} at the pixel, the second is
- *     the {@link ol.layer.Layer layer} of the feature. To stop detection,
- *     callback functions can return a truthy value.
+ *     the {@link ol.layer.Layer layer} of the feature and will be null for
+ *     unmanaged layers. To stop detection, callback functions can return a
+ *     truthy value.
  * @param {S=} opt_this Value to use as `this` when executing `callback`.
  * @param {(function(this: U, ol.layer.Layer): boolean)=} opt_layerFilter Layer
  *     filter function. The filter function will receive one argument, the

src/ol/renderer/maprenderer.js

@@ -174,7 +174,9 @@ ol.renderer.Map.prototype.forEachFeatureAtCoordinate =
       if (layer.getSource()) {
         result = layerRenderer.forEachFeatureAtCoordinate(
             layer.getSource().getWrapX() ? translatedCoordinate : coordinate,
-            frameState, callback, thisArg);
+            frameState,
+            layerState.managed ? callback : forEachFeatureAtCoordinate,
+            thisArg);
       }
       if (result) {
         return result;

test/spec/ol/interaction/selectinteraction.test.js

@@ -161,6 +161,29 @@ describe('ol.interaction.Select', function() {
   describe('filter features using the filter option', function() {
     var select;
 
+    describe('with unmanaged layers', function() {
+      it('does not call filter for unmanaged layers', function() {
+        var spy = sinon.spy();
+        var select = new ol.interaction.Select({
+          multi: false,
+          filter: spy
+        });
+        map.addInteraction(select);
+        var feature = new ol.Feature(
+            new ol.geom.Polygon([[[0, 0], [0, 40], [40, 40], [40, 0]]]));
+        var unmanaged = new ol.layer.Vector({
+          source: new ol.source.Vector({
+            features: [feature]
+          })
+        });
+        unmanaged.setMap(map);
+        map.renderSync();
+        simulateEvent(ol.MapBrowserEvent.EventType.SINGLECLICK, 10, -20);
+        expect(spy.firstCall.args[0]).to.not.equal(feature);
+        unmanaged.setMap(null);
+      });
+    });
+
     describe('with multi set to true', function() {
 
       it('only selects features that pass the filter', function() {

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

@@ -46,13 +46,23 @@ describe('ol.renderer.canvas.Map', function() {
       document.body.removeChild(target);
     });
 
-    it('always includes unmanaged layers', function() {
+    it('calls callback with layer for managed layers', function() {
+      map.addLayer(layer);
+      map.renderSync();
+      var cb = sinon.spy();
+      map.forEachFeatureAtPixel(map.getPixelFromCoordinate([0, 0]), cb);
+      expect(cb).to.be.called();
+      expect(cb.firstCall.args[1]).to.be(layer);
+    });
+
+    it('includes unmanaged layers, but calls callback with null', function() {
       layer.setMap(map);
       map.renderSync();
       var cb = sinon.spy();
       map.forEachFeatureAtPixel(map.getPixelFromCoordinate([0, 0]), cb, null,
           function() { return false; });
       expect(cb).to.be.called();
+      expect(cb.firstCall.args[1]).to.be(null);
     });
 
     it('filters managed layers', function() {