From 23dc768c2e105c1f9b741d6f1fe7b0dc727a961f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sun, 29 Nov 2020 02:32:04 +0100 Subject: [PATCH] Order callback calls by distance to click position All callback calls for hits with a tolerance > 0 are queued and called ordered by distance after all hits are detected. --- src/ol/render/canvas/Executor.js | 21 ++-- src/ol/render/canvas/ExecutorGroup.js | 14 ++- src/ol/renderer/Layer.js | 13 ++- src/ol/renderer/Map.js | 29 ++++- src/ol/renderer/canvas/VectorImageLayer.js | 17 ++- src/ol/renderer/canvas/VectorLayer.js | 100 +++++++++++------- src/ol/renderer/canvas/VectorTileLayer.js | 98 +++++++++++------ src/ol/renderer/webgl/PointsLayer.js | 14 ++- .../ol/renderer/canvas/vectorlayer.test.js | 16 +-- .../renderer/canvas/vectortilelayer.test.js | 36 ++++--- 10 files changed, 239 insertions(+), 119 deletions(-) diff --git a/src/ol/render/canvas/Executor.js b/src/ol/render/canvas/Executor.js index e4b7ae775d..058961edaf 100644 --- a/src/ol/render/canvas/Executor.js +++ b/src/ol/render/canvas/Executor.js @@ -55,7 +55,7 @@ import {transform2D} from '../../geom/flat/transform.js'; /** * @template T - * @typedef {function(import("../../Feature.js").FeatureLike, import("../../geom/SimpleGeometry.js").default): T=} FeatureCallback + * @typedef {function(import("../../Feature.js").FeatureLike, import("../../geom/SimpleGeometry.js").default): T} FeatureCallback */ /** @@ -602,9 +602,9 @@ class Executor { * @param {import("../../transform.js").Transform} transform Transform. * @param {Array<*>} instructions Instructions array. * @param {boolean} snapToPixel Snap point symbols and text to integer pixels. - * @param {FeatureCallback|undefined} featureCallback Feature callback. - * @param {import("../../extent.js").Extent=} opt_hitExtent Only check features that intersect this - * extent. + * @param {FeatureCallback=} opt_featureCallback Feature callback. + * @param {import("../../extent.js").Extent=} opt_hitExtent Only check + * features that intersect this extent. * @param {import("rbush").default=} opt_declutterTree Declutter tree. * @return {T|undefined} Callback result. * @template T @@ -615,7 +615,7 @@ class Executor { transform, instructions, snapToPixel, - featureCallback, + opt_featureCallback, opt_hitExtent, opt_declutterTree ) { @@ -1052,9 +1052,9 @@ class Executor { ++i; break; case CanvasInstruction.END_GEOMETRY: - if (featureCallback !== undefined) { + if (opt_featureCallback !== undefined) { feature = /** @type {import("../../Feature.js").FeatureLike} */ (instruction[1]); - const result = featureCallback(feature, currentGeometry); + const result = opt_featureCallback(feature, currentGeometry); if (result) { return result; } @@ -1174,10 +1174,9 @@ class Executor { * @param {CanvasRenderingContext2D} context Context. * @param {import("../../transform.js").Transform} transform Transform. * @param {number} viewRotation View rotation. - * @param {FeatureCallback} opt_featureCallback - * Feature callback. - * @param {import("../../extent.js").Extent=} opt_hitExtent Only check features that intersect this - * extent. + * @param {FeatureCallback=} opt_featureCallback Feature callback. + * @param {import("../../extent.js").Extent=} opt_hitExtent Only check + * features that intersect this extent. * @return {T|undefined} Callback result. * @template T */ diff --git a/src/ol/render/canvas/ExecutorGroup.js b/src/ol/render/canvas/ExecutorGroup.js index 6d59379791..12f5c7ca1e 100644 --- a/src/ol/render/canvas/ExecutorGroup.js +++ b/src/ol/render/canvas/ExecutorGroup.js @@ -161,7 +161,7 @@ class ExecutorGroup { * @param {number} resolution Resolution. * @param {number} rotation Rotation. * @param {number} hitTolerance Hit tolerance in pixels. - * @param {import("./Executor.js").FeatureCallback} callback Feature callback. + * @param {function(import("../../Feature.js").FeatureLike, import("../../geom/SimpleGeometry.js").default, number): T} callback Feature callback. * @param {Array} declutteredFeatures Decluttered features. * @return {T|undefined} Callback result. * @template T @@ -187,7 +187,8 @@ class ExecutorGroup { -coordinate[1] ); - if (!this.hitDetectionContext_) { + const newContext = !this.hitDetectionContext_; + if (newContext) { this.hitDetectionContext_ = createCanvasContext2D( contextSize, contextSize @@ -201,7 +202,7 @@ class ExecutorGroup { ) { context.canvas.width = contextSize; context.canvas.height = contextSize; - } else { + } else if (!newContext) { context.clearRect(0, 0, contextSize, contextSize); } @@ -226,7 +227,7 @@ class ExecutorGroup { /** * @param {import("../../Feature.js").FeatureLike} feature Feature. * @param {import("../../geom/SimpleGeometry.js").default} geometry Geometry. - * @return {?} Callback result. + * @return {T|undefined} Callback result. */ function featureCallback(feature, geometry) { const imageData = context.getImageData(0, 0, contextSize, contextSize) @@ -239,7 +240,10 @@ class ExecutorGroup { builderType !== BuilderType.TEXT) || declutteredFeatures.indexOf(feature) !== -1 ) { - const result = callback(feature, geometry); + const idx = (indexes[i] - 3) / 4; + const x = hitTolerance - (idx % contextSize); + const y = hitTolerance - ((idx / contextSize) | 0); + const result = callback(feature, geometry, x * x + y * y); if (result) { return result; } diff --git a/src/ol/renderer/Layer.js b/src/ol/renderer/Layer.js index 04344f881d..d16a5a1afd 100644 --- a/src/ol/renderer/Layer.js +++ b/src/ol/renderer/Layer.js @@ -106,10 +106,19 @@ class LayerRenderer extends Observable { * @param {import("../PluggableMap.js").FrameState} frameState Frame state. * @param {number} hitTolerance Hit tolerance in pixels. * @param {import("./vector.js").FeatureCallback} callback Feature callback. - * @return {T|void} Callback result. + * @param {Array>} matches The hit detected matches with tolerance. + * @return {T|undefined} Callback result. * @template T */ - forEachFeatureAtCoordinate(coordinate, frameState, hitTolerance, callback) {} + forEachFeatureAtCoordinate( + coordinate, + frameState, + hitTolerance, + callback, + matches + ) { + return undefined; + } /** * @abstract diff --git a/src/ol/renderer/Map.js b/src/ol/renderer/Map.js index 35368c9293..dc777be439 100644 --- a/src/ol/renderer/Map.js +++ b/src/ol/renderer/Map.js @@ -10,6 +10,16 @@ import {shared as iconImageCache} from '../style/IconImageCache.js'; import {inView} from '../layer/Layer.js'; import {wrapX} from '../coordinate.js'; +/** + * @typedef HitMatch + * @property {import("../Feature.js").FeatureLike} feature + * @property {import("../layer/Layer.js").default} layer + * @property {import("../geom/SimpleGeometry.js").default} geometry + * @property {number} distanceSq + * @property {import("./vector.js").FeatureCallback} callback + * @template T + */ + /** * @abstract */ @@ -92,7 +102,7 @@ class MapRenderer extends Disposable { * @param {import("../Feature.js").FeatureLike} feature Feature. * @param {import("../layer/Layer.js").default} layer Layer. * @param {import("../geom/Geometry.js").default} geometry Geometry. - * @return {?} Callback result. + * @return {T|undefined} Callback result. */ function forEachFeatureAtCoordinate(managed, feature, layer, geometry) { return callback.call(thisArg, feature, managed ? layer : null, geometry); @@ -111,11 +121,12 @@ class MapRenderer extends Disposable { const layerStates = frameState.layerStatesArray; const numLayers = layerStates.length; + const matches = /** @type {Array>} */ ([]); const tmpCoord = []; for (let i = 0; i < offsets.length; i++) { for (let j = numLayers - 1; j >= 0; --j) { const layerState = layerStates[j]; - const layer = /** @type {import("../layer/Layer.js").default} */ (layerState.layer); + const layer = layerState.layer; if ( layer.hasRenderer() && inView(layerState, viewState) && @@ -137,7 +148,8 @@ class MapRenderer extends Disposable { tmpCoord, frameState, hitTolerance, - callback + callback, + matches ); } if (result) { @@ -146,7 +158,16 @@ class MapRenderer extends Disposable { } } } - return undefined; + if (matches.length === 0) { + return undefined; + } + const order = 1 / matches.length; + matches.forEach((m, i) => (m.distanceSq += i * order)); + matches.sort((a, b) => a.distanceSq - b.distanceSq); + matches.some((m) => { + return (result = m.callback(m.feature, m.layer, m.geometry)); + }); + return result; } /** diff --git a/src/ol/renderer/canvas/VectorImageLayer.js b/src/ol/renderer/canvas/VectorImageLayer.js index 0908216610..2bb1aaa272 100644 --- a/src/ol/renderer/canvas/VectorImageLayer.js +++ b/src/ol/renderer/canvas/VectorImageLayer.js @@ -195,23 +195,32 @@ class CanvasVectorImageLayerRenderer extends CanvasImageLayerRenderer { * @param {import("../../PluggableMap.js").FrameState} frameState Frame state. * @param {number} hitTolerance Hit tolerance in pixels. * @param {import("../vector.js").FeatureCallback} callback Feature callback. - * @return {T|void} Callback result. + * @param {Array>} matches The hit detected matches with tolerance. + * @return {T|undefined} Callback result. * @template T */ - forEachFeatureAtCoordinate(coordinate, frameState, hitTolerance, callback) { + forEachFeatureAtCoordinate( + coordinate, + frameState, + hitTolerance, + callback, + matches + ) { if (this.vectorRenderer_) { return this.vectorRenderer_.forEachFeatureAtCoordinate( coordinate, frameState, hitTolerance, - callback + callback, + matches ); } else { return super.forEachFeatureAtCoordinate( coordinate, frameState, hitTolerance, - callback + callback, + matches ); } } diff --git a/src/ol/renderer/canvas/VectorLayer.js b/src/ol/renderer/canvas/VectorLayer.js index 6c46f35611..9747a00bca 100644 --- a/src/ol/renderer/canvas/VectorLayer.js +++ b/src/ol/renderer/canvas/VectorLayer.js @@ -407,55 +407,81 @@ class CanvasVectorLayerRenderer extends CanvasLayerRenderer { * @param {import("../../PluggableMap.js").FrameState} frameState Frame state. * @param {number} hitTolerance Hit tolerance in pixels. * @param {import("../vector.js").FeatureCallback} callback Feature callback. - * @return {T|void} Callback result. + * @param {Array>} matches The hit detected matches with tolerance. + * @return {T|undefined} Callback result. * @template T */ - forEachFeatureAtCoordinate(coordinate, frameState, hitTolerance, callback) { + forEachFeatureAtCoordinate( + coordinate, + frameState, + hitTolerance, + callback, + matches + ) { if (!this.replayGroup_) { return undefined; - } else { - const resolution = frameState.viewState.resolution; - const rotation = frameState.viewState.rotation; - const layer = this.getLayer(); + } + const resolution = frameState.viewState.resolution; + const rotation = frameState.viewState.rotation; + const layer = this.getLayer(); - /** @type {!Object} */ - const features = {}; + /** @type {!Object|true>} */ + const features = {}; - /** - * @param {import("../../Feature.js").FeatureLike} feature Feature. - * @param {import("../../geom/SimpleGeometry.js").default} geometry Geometry. - * @return {?} Callback result. - */ - const featureCallback = function (feature, geometry) { - const key = getUid(feature); - if (!(key in features)) { + /** + * @param {import("../../Feature.js").FeatureLike} feature Feature. + * @param {import("../../geom/SimpleGeometry.js").default} geometry Geometry. + * @param {number} distanceSq The squared distance to the click position + * @return {T|undefined} Callback result. + */ + const featureCallback = function (feature, geometry, distanceSq) { + const key = getUid(feature); + const match = features[key]; + if (!match) { + if (distanceSq === 0) { features[key] = true; return callback(feature, layer, geometry); } - }; - - let result; - const executorGroups = [this.replayGroup_]; - if (this.declutterExecutorGroup) { - executorGroups.push(this.declutterExecutorGroup); + matches.push( + (features[key] = { + feature: feature, + layer: layer, + geometry: geometry, + distanceSq: distanceSq, + callback: callback, + }) + ); + } else if (match !== true && distanceSq < match.distanceSq) { + if (distanceSq === 0) { + features[key] = true; + matches.splice(matches.lastIndexOf(match), 1); + return callback(feature, layer, geometry); + } + match.geometry = geometry; + match.distanceSq = distanceSq; } - executorGroups.forEach((executorGroup) => { - result = - result || - executorGroup.forEachFeatureAtCoordinate( - coordinate, - resolution, - rotation, - hitTolerance, - featureCallback, - executorGroup === this.declutterExecutorGroup - ? frameState.declutterTree.all().map((item) => item.value) - : null - ); - }); + return undefined; + }; - return result; + let result; + const executorGroups = [this.replayGroup_]; + if (this.declutterExecutorGroup) { + executorGroups.push(this.declutterExecutorGroup); } + executorGroups.some((executorGroup) => { + return (result = executorGroup.forEachFeatureAtCoordinate( + coordinate, + resolution, + rotation, + hitTolerance, + featureCallback, + executorGroup === this.declutterExecutorGroup + ? frameState.declutterTree.all().map((item) => item.value) + : null + )); + }); + + return result; } /** diff --git a/src/ol/renderer/canvas/VectorTileLayer.js b/src/ol/renderer/canvas/VectorTileLayer.js index 6efd9933a9..91cbf57935 100644 --- a/src/ol/renderer/canvas/VectorTileLayer.js +++ b/src/ol/renderer/canvas/VectorTileLayer.js @@ -381,10 +381,17 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { * @param {import("../../PluggableMap.js").FrameState} frameState Frame state. * @param {number} hitTolerance Hit tolerance in pixels. * @param {import("../vector.js").FeatureCallback} callback Feature callback. - * @return {T|void} Callback result. + * @param {Array>} matches The hit detected matches with tolerance. + * @return {T|undefined} Callback result. * @template T */ - forEachFeatureAtCoordinate(coordinate, frameState, hitTolerance, callback) { + forEachFeatureAtCoordinate( + coordinate, + frameState, + hitTolerance, + callback, + matches + ) { const resolution = frameState.viewState.resolution; const rotation = frameState.viewState.rotation; hitTolerance = hitTolerance == undefined ? 0 : hitTolerance; @@ -397,55 +404,82 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { const hitExtent = boundingExtent([coordinate]); buffer(hitExtent, resolution * hitTolerance, hitExtent); - /** @type {!Object} */ + /** @type {!Object|true>} */ const features = {}; + /** + * @param {import("../../Feature.js").FeatureLike} feature Feature. + * @param {import("../../geom/SimpleGeometry.js").default} geometry Geometry. + * @param {number} distanceSq The squared distance to the click position. + * @return {T|undefined} Callback result. + */ + const featureCallback = function (feature, geometry, distanceSq) { + let key = feature.getId(); + if (key === undefined) { + key = getUid(feature); + } + const match = features[key]; + if (!match) { + if (distanceSq === 0) { + features[key] = true; + return callback(feature, layer, geometry); + } + matches.push( + (features[key] = { + feature: feature, + layer: layer, + geometry: geometry, + distanceSq: distanceSq, + callback: callback, + }) + ); + } else if (match !== true && distanceSq < match.distanceSq) { + if (distanceSq === 0) { + features[key] = true; + matches.splice(matches.lastIndexOf(match), 1); + return callback(feature, layer, geometry); + } + match.geometry = geometry; + match.distanceSq = distanceSq; + } + return undefined; + }; + const renderedTiles = /** @type {Array} */ (this .renderedTiles); let found; - let i, ii; - for (i = 0, ii = renderedTiles.length; i < ii; ++i) { + for (let i = 0, ii = renderedTiles.length; !found && i < ii; ++i) { const tile = renderedTiles[i]; const tileExtent = tileGrid.getTileCoordExtent(tile.wrappedTileCoord); if (!intersects(tileExtent, hitExtent)) { continue; } + const layerUid = getUid(layer); const executorGroups = [tile.executorGroups[layerUid]]; const declutterExecutorGroups = tile.declutterExecutorGroups[layerUid]; if (declutterExecutorGroups) { executorGroups.push(declutterExecutorGroups); } - executorGroups.forEach((executorGroups) => { + executorGroups.some((executorGroups) => { + const declutteredFeatures = + executorGroups === declutterExecutorGroups + ? frameState.declutterTree.all().map((item) => item.value) + : null; for (let t = 0, tt = executorGroups.length; t < tt; ++t) { const executorGroup = executorGroups[t]; - found = - found || - executorGroup.forEachFeatureAtCoordinate( - coordinate, - resolution, - rotation, - hitTolerance, - /** - * @param {import("../../Feature.js").FeatureLike} feature Feature. - * @param {import("../../geom/SimpleGeometry.js").default} geometry Geometry. - * @return {?} Callback result. - */ - function (feature, geometry) { - let key = feature.getId(); - if (key === undefined) { - key = getUid(feature); - } - if (!(key in features)) { - features[key] = true; - return callback(feature, layer, geometry); - } - }, - executorGroups === declutterExecutorGroups - ? frameState.declutterTree.all().map((item) => item.value) - : null - ); + found = executorGroup.forEachFeatureAtCoordinate( + coordinate, + resolution, + rotation, + hitTolerance, + featureCallback, + declutteredFeatures + ); + if (found) { + return true; + } } }); } diff --git a/src/ol/renderer/webgl/PointsLayer.js b/src/ol/renderer/webgl/PointsLayer.js index 349ac05eff..8a01e04d19 100644 --- a/src/ol/renderer/webgl/PointsLayer.js +++ b/src/ol/renderer/webgl/PointsLayer.js @@ -598,13 +598,20 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { * @param {import("../../PluggableMap.js").FrameState} frameState Frame state. * @param {number} hitTolerance Hit tolerance in pixels. * @param {import("../vector.js").FeatureCallback} callback Feature callback. - * @return {T|void} Callback result. + * @param {Array>} matches The hit detected matches with tolerance. + * @return {T|undefined} Callback result. * @template T */ - forEachFeatureAtCoordinate(coordinate, frameState, hitTolerance, callback) { + forEachFeatureAtCoordinate( + coordinate, + frameState, + hitTolerance, + callback, + matches + ) { assert(this.hitDetectionEnabled_, 66); if (!this.hitRenderInstructions_) { - return; + return undefined; } const pixel = applyTransform( @@ -623,6 +630,7 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { if (feature) { return callback(feature, this.getLayer(), null); } + return undefined; } /** diff --git a/test/spec/ol/renderer/canvas/vectorlayer.test.js b/test/spec/ol/renderer/canvas/vectorlayer.test.js index b6bfd933b5..a951d8a7a7 100644 --- a/test/spec/ol/renderer/canvas/vectorlayer.test.js +++ b/test/spec/ol/renderer/canvas/vectorlayer.test.js @@ -189,7 +189,8 @@ describe('ol.renderer.canvas.VectorLayer', function () { }); describe('#forEachFeatureAtCoordinate', function () { - let layer, renderer; + /** @type {VectorLayer} */ let layer; + /** @type {CanvasVectorLayerRenderer} */ let renderer; beforeEach(function () { layer = new VectorLayer({ @@ -205,15 +206,17 @@ describe('ol.renderer.canvas.VectorLayer', function () { hitTolerance, callback ) { - const feature = new Feature(); - callback(feature); - callback(feature); + const feature = new Feature(new Point([0, 0])); + const distanceSq = 0; + callback(feature, feature.getGeometry(), distanceSq); + callback(feature, feature.getGeometry(), distanceSq); }; }); it('calls callback once per feature with a layer as 2nd arg', function () { const spy = sinon.spy(); const coordinate = [0, 0]; + const matches = []; const frameState = { layerStatesArray: [{}], viewState: { @@ -227,10 +230,11 @@ describe('ol.renderer.canvas.VectorLayer', function () { frameState, 0, spy, - undefined + matches ); expect(spy.callCount).to.be(1); - expect(spy.getCall(0).args[1]).to.equal(layer); + expect(spy.getCall(0).args[1]).to.be(layer); + expect(matches).to.be.empty(); }); }); diff --git a/test/spec/ol/renderer/canvas/vectortilelayer.test.js b/test/spec/ol/renderer/canvas/vectortilelayer.test.js index e1b4b75048..787bb8226d 100644 --- a/test/spec/ol/renderer/canvas/vectortilelayer.test.js +++ b/test/spec/ol/renderer/canvas/vectortilelayer.test.js @@ -114,7 +114,7 @@ describe('ol.renderer.canvas.VectorTileLayer', function () { it('creates a new instance', function () { const renderer = new CanvasVectorTileLayerRenderer(layer); expect(renderer).to.be.a(CanvasVectorTileLayerRenderer); - expect(renderer.getLayer()).to.equal(layer); + expect(renderer.getLayer()).to.be(layer); }); it('does not render replays for pure image rendering', function () { @@ -321,7 +321,10 @@ describe('ol.renderer.canvas.VectorTileLayer', function () { }); describe('#forEachFeatureAtCoordinate', function () { - let layer, renderer, executorGroup, source; + /** @type {VectorTileLayer] */ let layer; + /** @type {CanvasVectorTileLayerRenderer} */ let renderer; + /** @type {VectorTileSource} */ let source; + let executorGroup; class TileClass extends VectorRenderTile { constructor() { super(...arguments); @@ -339,6 +342,18 @@ describe('ol.renderer.canvas.VectorTileLayer', function () { }); source.sourceTileCache.set('0/0/0.mvt', sourceTile); executorGroup = {}; + executorGroup.forEachFeatureAtCoordinate = function ( + coordinate, + resolution, + rotation, + hitTolerance, + callback + ) { + const feature = new Feature(new Point([0, 0])); + const distanceSq = 0; + callback(feature, feature.getGeometry(), distanceSq); + callback(feature, feature.getGeometry(), distanceSq); + }; source.getTile = function () { const tile = VectorTileSource.prototype.getTile.apply( source, @@ -352,22 +367,12 @@ describe('ol.renderer.canvas.VectorTileLayer', function () { source: source, }); renderer = new CanvasVectorTileLayerRenderer(layer); - executorGroup.forEachFeatureAtCoordinate = function ( - coordinate, - resolution, - rotation, - hitTolerance, - callback - ) { - const feature = new Feature(); - callback(feature); - callback(feature); - }; }); it('calls callback once per feature with a layer as 2nd arg', function () { const spy = sinon.spy(); const coordinate = [0, 0]; + const matches = []; const frameState = { layerStatesArray: [{}], viewState: { @@ -384,10 +389,11 @@ describe('ol.renderer.canvas.VectorTileLayer', function () { frameState, 0, spy, - undefined + matches ); expect(spy.callCount).to.be(1); - expect(spy.getCall(0).args[1]).to.equal(layer); + expect(spy.getCall(0).args[1]).to.be(layer); + expect(matches).to.be.empty(); }); it('does not give false positives when overzoomed', function (done) {