Merge pull request #4472 from ahocevar/layer-filter

Do not ignore layer filter for unmanaged layers

changelog/upgrade-notes.md

@@ -1,5 +1,18 @@
 ## Upgrade notes
 
+### v3.12.0
+
+#### `ol.Map#forEachFeatureAtPixel` changes
+
+The optional `layerFilter` function is now also called for unmanaged layers. To get the same behaviour as before, wrap your layer filter code in an if block like this:
+```js
+function layerFilter(layer) {
+  if (map.getLayers().getArray().indexOf(layer) !== -1) {
+    // existing layer filter code
+  }
+}
+```
+
 ### v3.11.0
 
 #### `ol.format.KML` changes

src/ol/interaction/selectinteraction.js

@@ -283,7 +283,7 @@ ol.interaction.Select.handleEvent = function(mapBrowserEvent) {
          * @param {ol.layer.Layer} layer Layer.
          */
         function(feature, layer) {
-          if (!layer || this.filter_(feature, layer)) {
+          if (this.filter_(feature, layer)) {
             selected.push(feature);
             this.addFeatureLayerAssociation_(feature, layer);
             return !this.multi_;

src/ol/layer/layer.js

@@ -153,11 +153,11 @@ ol.layer.Layer.prototype.handleSourcePropertyChange_ = function() {
 
 
 /**
- * Sets the layer to be rendered on a map. The map will not manage this layer in
- * its layers collection, layer filters in {@link ol.Map#forEachLayerAtPixel}
- * will not filter the layer, and it will be rendered on top. This is useful for
- * temporary layers. To remove an unmanaged layer from the map, use
- * `#setMap(null)`.
+ * Sets the layer to be rendered on top of other layers on a map. The map will
+ * not manage this layer in its layers collection, and the callback in
+ * {@link ol.Map#forEachLayerAtPixel} will receive `null` as layer. This
+ * is useful for temporary layers. To remove an unmanaged layer from the map,
+ * use `#setMap(null)`.
  *
  * To add the layer to a map and have it managed by the map, use
  * {@link ol.Map#addLayer} instead.

src/ol/renderer/maprenderer.js

@@ -167,9 +167,8 @@ ol.renderer.Map.prototype.forEachFeatureAtCoordinate =
   for (i = numLayers - 1; i >= 0; --i) {
     var layerState = layerStates[i];
     var layer = layerState.layer;
-    if (!layerState.managed ||
-        (ol.layer.Layer.visibleAtResolution(layerState, viewResolution) &&
-        layerFilter.call(thisArg2, layer))) {
+    if (ol.layer.Layer.visibleAtResolution(layerState, viewResolution) &&
+        layerFilter.call(thisArg2, layer)) {
       var layerRenderer = this.getLayerRenderer(layer);
       if (layer.getSource()) {
         result = layerRenderer.forEachFeatureAtCoordinate(

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

@@ -161,29 +161,6 @@ 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.getCalls().length).to.be(0);
-        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

@@ -55,12 +55,11 @@ describe('ol.renderer.canvas.Map', function() {
       expect(cb.firstCall.args[1]).to.be(layer);
     });
 
-    it('includes unmanaged layers, but calls callback with null', function() {
+    it('calls callback with null for unmanaged layers', function() {
       layer.setMap(map);
       map.renderSync();
       var cb = sinon.spy();
-      map.forEachFeatureAtPixel(map.getPixelFromCoordinate([0, 0]), cb, null,
-          function() { return false; });
+      map.forEachFeatureAtPixel(map.getPixelFromCoordinate([0, 0]), cb);
       expect(cb).to.be.called();
       expect(cb.firstCall.args[1]).to.be(null);
     });