From c076d273e76b20da009845048f98763f1620cb93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sat, 28 Nov 2020 00:17:58 +0100 Subject: [PATCH 1/5] Cache hit detect indexes and check closest pixels first. --- src/ol/render/canvas/ExecutorGroup.js | 140 ++++++++---------- .../ol/render/canvas/executorgroup.test.js | 50 ++++++- 2 files changed, 103 insertions(+), 87 deletions(-) diff --git a/src/ol/render/canvas/ExecutorGroup.js b/src/ol/render/canvas/ExecutorGroup.js index a625c1ee2d..6d59379791 100644 --- a/src/ol/render/canvas/ExecutorGroup.js +++ b/src/ol/render/canvas/ExecutorGroup.js @@ -219,7 +219,7 @@ class ExecutorGroup { ); } - const mask = getCircleArray(hitTolerance); + const indexes = getPixelIndexArray(hitTolerance); let builderType; @@ -231,31 +231,24 @@ class ExecutorGroup { function featureCallback(feature, geometry) { const imageData = context.getImageData(0, 0, contextSize, contextSize) .data; - for (let i = 0; i < contextSize; i++) { - for (let j = 0; j < contextSize; j++) { - if (mask[i][j]) { - if (imageData[(j * contextSize + i) * 4 + 3] > 0) { - let result; - if ( - !( - declutteredFeatures && - (builderType == BuilderType.IMAGE || - builderType == BuilderType.TEXT) - ) || - declutteredFeatures.indexOf(feature) !== -1 - ) { - result = callback(feature, geometry); - } - if (result) { - return result; - } else { - context.clearRect(0, 0, contextSize, contextSize); - return undefined; - } + for (let i = 0, ii = indexes.length; i < ii; i++) { + if (imageData[indexes[i]] > 0) { + if ( + !declutteredFeatures || + (builderType !== BuilderType.IMAGE && + builderType !== BuilderType.TEXT) || + declutteredFeatures.indexOf(feature) !== -1 + ) { + const result = callback(feature, geometry); + if (result) { + return result; } } + context.clearRect(0, 0, contextSize, contextSize); + break; } } + return undefined; } /** @type {Array} */ @@ -372,78 +365,61 @@ class ExecutorGroup { } /** - * This cache is used for storing calculated pixel circles for increasing performance. + * This cache is used to store arrays of indexes for calculated pixel circles + * to increase performance. * It is a static property to allow each Replaygroup to access it. - * @type {Object>>} + * @type {Object>} */ -const circleArrayCache = { - 0: [[true]], -}; +const circlePixelIndexArrayCache = {}; /** - * This method fills a row in the array from the given coordinate to the - * middle with `true`. - * @param {Array>} array The array that will be altered. - * @param {number} x X coordinate. - * @param {number} y Y coordinate. - */ -function fillCircleArrayRowToMiddle(array, x, y) { - let i; - const radius = Math.floor(array.length / 2); - if (x >= radius) { - for (i = radius; i < x; i++) { - array[i][y] = true; - } - } else if (x < radius) { - for (i = x + 1; i < radius; i++) { - array[i][y] = true; - } - } -} - -/** - * This methods creates a circle inside a fitting array. Points inside the - * circle are marked by true, points on the outside are undefined. - * It uses the midpoint circle algorithm. + * This methods creates an array with indexes of all pixels within a circle, + * ordered by how close they are to the center. * A cache is used to increase performance. * @param {number} radius Radius. - * @returns {Array>} An array with marked circle points. + * @returns {Array} An array with indexes within a circle. */ -export function getCircleArray(radius) { - if (circleArrayCache[radius] !== undefined) { - return circleArrayCache[radius]; +export function getPixelIndexArray(radius) { + if (circlePixelIndexArrayCache[radius] !== undefined) { + return circlePixelIndexArrayCache[radius]; } - const arraySize = radius * 2 + 1; - const arr = new Array(arraySize); - for (let i = 0; i < arraySize; i++) { - arr[i] = new Array(arraySize); - } - - let x = radius; - let y = 0; - let error = 0; - - while (x >= y) { - fillCircleArrayRowToMiddle(arr, radius + x, radius + y); - fillCircleArrayRowToMiddle(arr, radius + y, radius + x); - fillCircleArrayRowToMiddle(arr, radius - y, radius + x); - fillCircleArrayRowToMiddle(arr, radius - x, radius + y); - fillCircleArrayRowToMiddle(arr, radius - x, radius - y); - fillCircleArrayRowToMiddle(arr, radius - y, radius - x); - fillCircleArrayRowToMiddle(arr, radius + y, radius - x); - fillCircleArrayRowToMiddle(arr, radius + x, radius - y); - - y++; - error += 1 + 2 * y; - if (2 * (error - x) + 1 > 0) { - x -= 1; - error += 1 - 2 * x; + const size = radius * 2 + 1; + const maxDistanceSq = radius * radius; + const distances = new Array(maxDistanceSq + 1); + for (let i = 0; i <= radius; ++i) { + for (let j = 0; j <= radius; ++j) { + const distanceSq = i * i + j * j; + if (distanceSq > maxDistanceSq) { + break; + } + let distance = distances[distanceSq]; + if (!distance) { + distance = []; + distances[distanceSq] = distance; + } + distance.push(((radius + i) * size + (radius + j)) * 4 + 3); + if (i > 0) { + distance.push(((radius - i) * size + (radius + j)) * 4 + 3); + } + if (j > 0) { + distance.push(((radius + i) * size + (radius - j)) * 4 + 3); + if (i > 0) { + distance.push(((radius - i) * size + (radius - j)) * 4 + 3); + } + } } } - circleArrayCache[radius] = arr; - return arr; + const pixelIndex = []; + for (let i = 0, ii = distances.length; i < ii; ++i) { + if (distances[i]) { + pixelIndex.push(...distances[i]); + } + } + + circlePixelIndexArrayCache[radius] = pixelIndex; + return pixelIndex; } export default ExecutorGroup; diff --git a/test/spec/ol/render/canvas/executorgroup.test.js b/test/spec/ol/render/canvas/executorgroup.test.js index 6fda89e3b5..1c8e083865 100644 --- a/test/spec/ol/render/canvas/executorgroup.test.js +++ b/test/spec/ol/render/canvas/executorgroup.test.js @@ -1,13 +1,25 @@ -import {getCircleArray} from '../../../../../src/ol/render/canvas/ExecutorGroup.js'; +import {getPixelIndexArray} from '../../../../../src/ol/render/canvas/ExecutorGroup.js'; describe('ol.render.canvas.ExecutorGroup', function () { - describe('#getCircleArray_', function () { - it('creates an array with a pixelated circle marked with true', function () { + describe('#getPixelIndexArray', function () { + it('creates an array with every index within distance', function () { const radius = 10; + const size = radius * 2 + 1; + const hitIndexes = getPixelIndexArray(radius); + + const circleArray = new Array(size); + for (let i = 0; i < size; i++) { + circleArray[i] = new Array(size); + } + + hitIndexes.forEach(function (d) { + const x = ((d - 3) / 4) % size; + const y = ((d - 3) / 4 / size) | 0; + circleArray[x][y] = true; + }); + const minRadiusSq = Math.pow(radius - Math.SQRT2, 2); const maxRadiusSq = Math.pow(radius + Math.SQRT2, 2); - const circleArray = getCircleArray(radius); - const size = radius * 2 + 1; expect(circleArray.length).to.be(size); for (let i = 0; i < size; i++) { @@ -24,5 +36,33 @@ describe('ol.render.canvas.ExecutorGroup', function () { } } }); + it('orders the indexes correctly from closest to farthest away', function () { + const radius = 10; + const size = radius * 2 + 1; + const hitIndexes = getPixelIndexArray(radius); + + // Center first + expect(hitIndexes[0]).to.be((size * radius + radius) * 4 + 3); + + // 4 Pixels above/below/left/right of center next + const begin = hitIndexes.slice(1, 5); + expect(begin).to.contain((radius * size + radius + 1) * 4 + 3); + expect(begin).to.contain(((radius + 1) * size + radius) * 4 + 3); + expect(begin).to.contain(((radius - 1) * size + radius) * 4 + 3); + expect(begin).to.contain((radius * size + radius - 1) * 4 + 3); + + // 4 Pixels in the middle of each side in the last 12 elements (at radius 10) + const last = hitIndexes.slice(hitIndexes.length - 12); + expect(last).to.contain((0 * size + radius) * 4 + 3); + expect(last).to.contain((radius * size + 0) * 4 + 3); + expect(last).to.contain((radius * size + size - 1) * 4 + 3); + expect(last).to.contain(((size - 1) * size + radius) * 4 + 3); + }); + it('has no duplicate indexes', function () { + const radius = 10; + const hitIndexes = getPixelIndexArray(radius); + + expect(new Set(hitIndexes).size).to.be(hitIndexes.length); + }); }); }); From f3a770319455cfe0624e3321e7212e04c383c722 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sat, 28 Nov 2020 22:35:07 +0100 Subject: [PATCH 2/5] Hit tolerance priority example --- examples/hit-tolerance-priority.html | 9 +++ examples/hit-tolerance-priority.js | 84 ++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 examples/hit-tolerance-priority.html create mode 100644 examples/hit-tolerance-priority.js diff --git a/examples/hit-tolerance-priority.html b/examples/hit-tolerance-priority.html new file mode 100644 index 0000000000..7e09d5a66e --- /dev/null +++ b/examples/hit-tolerance-priority.html @@ -0,0 +1,9 @@ +--- +layout: example.html +title: Hit tolerance priority +shortdesc: Shows bad behavior of hit detection with hit tolerance. +docs: > + Hover over the map and observe how the small circles get a black outline as you hover over them. Is the expected feature getting highlighted? +tags: "simple, openstreetmap" +--- +
diff --git a/examples/hit-tolerance-priority.js b/examples/hit-tolerance-priority.js new file mode 100644 index 0000000000..56a84f8de5 --- /dev/null +++ b/examples/hit-tolerance-priority.js @@ -0,0 +1,84 @@ +import CircleStyle from '../src/ol/style/Circle.js'; +import Feature from '../src/ol/Feature.js'; +import Map from '../src/ol/Map.js'; +import VectorLayer from '../src/ol/layer/Vector.js'; +import VectorSource from '../src/ol/source/Vector.js'; +import View from '../src/ol/View.js'; +import {Fill, Stroke, Style} from '../src/ol/style.js'; +import {Point} from '../src/ol/geom.js'; + +const map = new Map({ + target: 'map', + view: new View({ + center: [0, 0], + resolution: 1, + resolutions: [1], + }), +}); + +const vectorLayer = new VectorLayer({ + source: new VectorSource({ + features: [ + new Feature({ + geometry: new Point([0, 0]), + color: 'white', + }), + new Feature({ + geometry: new Point([-10, 0]), + color: 'fuchsia', + }), + new Feature({ + geometry: new Point([-10, -10]), + color: 'orange', + }), + new Feature({ + geometry: new Point([-10, 10]), + color: 'cyan', + }), + ], + }), + style: (feature) => { + return new Style({ + image: new CircleStyle({ + radius: 5, + fill: new Fill({ + color: feature.get('color'), + }), + stroke: new Stroke({ + color: 'gray', + width: 1, + }), + }), + }); + }, +}); +map.addLayer(vectorLayer); + +const highlightFeature = new Feature(new Point([NaN, NaN])); +highlightFeature.setStyle( + new Style({ + image: new CircleStyle({ + radius: 5, + stroke: new Stroke({ + color: 'black', + width: 2, + }), + }), + }) +); +vectorLayer.getSource().addFeature(highlightFeature); +map.on('pointermove', (e) => { + const hit = map.forEachFeatureAtPixel( + e.pixel, + (feature) => { + highlightFeature.setGeometry(feature.getGeometry().clone()); + return true; + }, + { + hitTolerance: 10, + } + ); + if (!hit) { + highlightFeature.setGeometry(new Point([NaN, NaN])); + } +}); From cde2dac19fad69455c314faa15d6b902e2d34dba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sun, 29 Nov 2020 18:14:26 +0100 Subject: [PATCH 3/5] Skip executor groups when tile does not contain coordinates --- src/ol/renderer/canvas/VectorTileLayer.js | 27 ++++++++--------------- 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/ol/renderer/canvas/VectorTileLayer.js b/src/ol/renderer/canvas/VectorTileLayer.js index 0012c44716..6a549a378c 100644 --- a/src/ol/renderer/canvas/VectorTileLayer.js +++ b/src/ol/renderer/canvas/VectorTileLayer.js @@ -389,7 +389,6 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { const rotation = frameState.viewState.rotation; hitTolerance = hitTolerance == undefined ? 0 : hitTolerance; const layer = this.getLayer(); - const declutter = layer.getDeclutter(); const source = layer.getSource(); const tileGrid = source.getTileGridForProjection( frameState.viewState.projection @@ -405,14 +404,8 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { for (i = 0, ii = renderedTiles.length; i < ii; ++i) { const tile = renderedTiles[i]; const tileExtent = tileGrid.getTileCoordExtent(tile.wrappedTileCoord); - const tileContainsCoordinate = containsCoordinate(tileExtent, coordinate); - - if (!declutter) { - // When not decluttering, we only need to consider the tile that contains the given - // coordinate, because each feature will be rendered for each tile that contains it. - if (!tileContainsCoordinate) { - continue; - } + if (!containsCoordinate(tileExtent, coordinate)) { + continue; } const layerUid = getUid(layer); const executorGroups = [tile.executorGroups[layerUid]]; @@ -436,15 +429,13 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { * @return {?} Callback result. */ function (feature, geometry) { - if (tileContainsCoordinate) { - let key = feature.getId(); - if (key === undefined) { - key = getUid(feature); - } - if (!(key in features)) { - features[key] = true; - return callback(feature, layer, 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 From 4546eff66e555e61927dbd295592752bae21206e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Sun, 29 Nov 2020 22:06:09 +0100 Subject: [PATCH 4/5] Also use hitTolerance to select tiles to search --- src/ol/renderer/canvas/VectorTileLayer.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ol/renderer/canvas/VectorTileLayer.js b/src/ol/renderer/canvas/VectorTileLayer.js index 6a549a378c..6efd9933a9 100644 --- a/src/ol/renderer/canvas/VectorTileLayer.js +++ b/src/ol/renderer/canvas/VectorTileLayer.js @@ -19,8 +19,8 @@ import { translate as translateTransform, } from '../../transform.js'; import { + boundingExtent, buffer, - containsCoordinate, containsExtent, equals, getIntersection, @@ -393,6 +393,10 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { const tileGrid = source.getTileGridForProjection( frameState.viewState.projection ); + + const hitExtent = boundingExtent([coordinate]); + buffer(hitExtent, resolution * hitTolerance, hitExtent); + /** @type {!Object} */ const features = {}; @@ -404,7 +408,7 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { for (i = 0, ii = renderedTiles.length; i < ii; ++i) { const tile = renderedTiles[i]; const tileExtent = tileGrid.getTileCoordExtent(tile.wrappedTileCoord); - if (!containsCoordinate(tileExtent, coordinate)) { + if (!intersects(tileExtent, hitExtent)) { continue; } const layerUid = getUid(layer); 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 5/5] 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) {