From 9f0fefd42dc0f40fc9008c0776e0ad625f3b0b0d Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Wed, 17 Feb 2016 00:42:44 +0100 Subject: [PATCH 1/2] Hit-detect skipped features, but not on unmanaged layer --- src/ol/interaction/selectinteraction.js | 23 ++++++++----------- src/ol/map.js | 5 ++-- .../canvas/canvasvectorlayerrenderer.js | 3 +-- .../canvas/canvasvectortilelayerrenderer.js | 4 +--- src/ol/renderer/dom/domvectorlayerrenderer.js | 3 +-- src/ol/renderer/maprenderer.js | 16 +++++-------- .../webgl/webglvectorlayerrenderer.js | 2 +- src/ol/source/vectorsource.js | 10 -------- .../renderer/canvas/canvasmaprenderer.test.js | 21 +++++++++++++++-- 9 files changed, 40 insertions(+), 47 deletions(-) diff --git a/src/ol/interaction/selectinteraction.js b/src/ol/interaction/selectinteraction.js index d1bda17fb0..02da1f19ca 100644 --- a/src/ol/interaction/selectinteraction.js +++ b/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 (layer !== null) { - if (add || toggle) { - if (this.filter_(feature, layer) && - !ol.array.includes(features.getArray(), feature) && - !ol.array.includes(selected, feature)) { - selected.push(feature); - this.addFeatureLayerAssociation_(feature, layer); - } - } - } else if (this.featureOverlay_.getSource().hasFeature(feature)) { - if (remove || toggle) { + if (this.filter_(feature, layer)) { + if ((add || toggle) && + !ol.array.includes(features.getArray(), feature)) { + selected.push(feature); + this.addFeatureLayerAssociation_(feature, layer); + } else if ((remove || toggle) && + ol.array.includes(features.getArray(), feature)) { deselected.push(feature); this.removeFeatureLayerAssociation_(feature); } diff --git a/src/ol/map.js b/src/ol/map.js index ae2e6586f6..2c0a2a58bf 100644 --- a/src/ol/map.js +++ b/src/ol/map.js @@ -604,9 +604,8 @@ 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 and will be null for - * unmanaged layers. To stop detection, callback functions can return a - * truthy value. + * the {@link ol.layer.Layer layer} of the feature. 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 diff --git a/src/ol/renderer/canvas/canvasvectorlayerrenderer.js b/src/ol/renderer/canvas/canvasvectorlayerrenderer.js index 085f3c0717..e5bdc78a29 100644 --- a/src/ol/renderer/canvas/canvasvectorlayerrenderer.js +++ b/src/ol/renderer/canvas/canvasvectorlayerrenderer.js @@ -158,11 +158,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.} */ 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. diff --git a/src/ol/renderer/canvas/canvasvectortilelayerrenderer.js b/src/ol/renderer/canvas/canvasvectortilelayerrenderer.js index 2a22e9fc4e..648deae3e9 100644 --- a/src/ol/renderer/canvas/canvasvectortilelayerrenderer.js +++ b/src/ol/renderer/canvas/canvasvectortilelayerrenderer.js @@ -309,7 +309,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.} */ var features = {}; @@ -343,8 +342,7 @@ ol.renderer.canvas.VectorTileLayer.prototype.forEachFeatureAtCoordinate = functi } replayGroup = tile.getReplayState().replayGroup; found = found || replayGroup.forEachFeatureAtCoordinate( - tileSpaceCoordinate, resolution, rotation, - layerState.managed ? frameState.skippedFeatureUids : {}, + tileSpaceCoordinate, resolution, rotation, {}, /** * @param {ol.Feature|ol.render.Feature} feature Feature. * @return {?} Callback result. diff --git a/src/ol/renderer/dom/domvectorlayerrenderer.js b/src/ol/renderer/dom/domvectorlayerrenderer.js index 073d733a71..d35b8fc742 100644 --- a/src/ol/renderer/dom/domvectorlayerrenderer.js +++ b/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.} */ 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. diff --git a/src/ol/renderer/maprenderer.js b/src/ol/renderer/maprenderer.js index 49393f0898..aa3c8f82c9 100644 --- a/src/ol/renderer/maprenderer.js +++ b/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.} */ - 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)) { - features[key] = true; - return callback.call(thisArg, feature, null); + if (!(key in frameState.skippedFeatureUids && + !frameState.layerStates[goog.getUid(layer)].managed)) { + return callback.call(thisArg, feature, layer); } } @@ -172,9 +170,7 @@ ol.renderer.Map.prototype.forEachFeatureAtCoordinate = function(coordinate, fram if (layer.getSource()) { result = layerRenderer.forEachFeatureAtCoordinate( layer.getSource().getWrapX() ? translatedCoordinate : coordinate, - frameState, - layerState.managed ? callback : forEachFeatureAtCoordinate, - thisArg); + frameState, forEachFeatureAtCoordinate, thisArg); } if (result) { return result; diff --git a/src/ol/renderer/webgl/webglvectorlayerrenderer.js b/src/ol/renderer/webgl/webglvectorlayerrenderer.js index 7554e05f96..02824e630f 100644 --- a/src/ol/renderer/webgl/webglvectorlayerrenderer.js +++ b/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. diff --git a/src/ol/source/vectorsource.js b/src/ol/source/vectorsource.js index 1faaa14d2a..a35d3a4da8 100644 --- a/src/ol/source/vectorsource.js +++ b/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. */ diff --git a/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js b/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js index 6149ca3bcd..7c35c27462 100644 --- a/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js +++ b/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js @@ -55,13 +55,30 @@ describe('ol.renderer.canvas.Map', function() { expect(cb.firstCall.args[1]).to.be(layer); }); - it('calls callback with null for unmanaged layers', function() { + it('calls callback for unmanaged layers', function() { layer.setMap(map); 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(null); + expect(cb.firstCall.args[1]).to.be(layer); + }); + + 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() { From 5bc00d85351c084055758f92fa2da95cdb2f1f08 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Wed, 17 Feb 2016 08:16:21 +0100 Subject: [PATCH 2/2] Continue passing null instead of unmanaged layer to forEachFeatureAtCoordinate --- src/ol/map.js | 5 +++-- src/ol/renderer/maprenderer.js | 6 +++--- test/spec/ol/renderer/canvas/canvasmaprenderer.test.js | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ol/map.js b/src/ol/map.js index 2c0a2a58bf..ae2e6586f6 100644 --- a/src/ol/map.js +++ b/src/ol/map.js @@ -604,8 +604,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 diff --git a/src/ol/renderer/maprenderer.js b/src/ol/renderer/maprenderer.js index aa3c8f82c9..a716b2166c 100644 --- a/src/ol/renderer/maprenderer.js +++ b/src/ol/renderer/maprenderer.js @@ -139,9 +139,9 @@ ol.renderer.Map.prototype.forEachFeatureAtCoordinate = function(coordinate, fram function forEachFeatureAtCoordinate(feature, layer) { goog.asserts.assert(feature !== undefined, 'received a feature'); var key = goog.getUid(feature).toString(); - if (!(key in frameState.skippedFeatureUids && - !frameState.layerStates[goog.getUid(layer)].managed)) { - return callback.call(thisArg, feature, layer); + var managed = frameState.layerStates[goog.getUid(layer)].managed; + if (!(key in frameState.skippedFeatureUids && !managed)) { + return callback.call(thisArg, feature, managed ? layer : null); } } diff --git a/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js b/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js index 7c35c27462..fdb20ca524 100644 --- a/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js +++ b/test/spec/ol/renderer/canvas/canvasmaprenderer.test.js @@ -55,13 +55,13 @@ describe('ol.renderer.canvas.Map', function() { expect(cb.firstCall.args[1]).to.be(layer); }); - it('calls callback for unmanaged layers', 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); expect(cb).to.be.called(); - expect(cb.firstCall.args[1]).to.be(layer); + expect(cb.firstCall.args[1]).to.be(null); }); it('calls callback with main layer when skipped feature on unmanaged layer', function() {