From 449686527b753a2b87fa378fc55770aaf89e5fae Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Mon, 23 Sep 2019 17:34:49 +0200 Subject: [PATCH 1/2] Document how to avoid canvas reuse for layers This restores the `map.forEachLayerAtPixel` functionality. This is intended to help with openlayers#9720 --- changelog/upgrade-notes.md | 17 +++++++++++++++++ src/ol/PluggableMap.js | 4 ++++ src/ol/layer/Layer.js | 6 ++++++ 3 files changed, 27 insertions(+) diff --git a/changelog/upgrade-notes.md b/changelog/upgrade-notes.md index 478c58eff3..b69be2d8ed 100644 --- a/changelog/upgrade-notes.md +++ b/changelog/upgrade-notes.md @@ -4,6 +4,23 @@ #### Backwards incompatible changes +##### Usage of `Map.forEachLayerAtPixel` + +Due to performance considerations, the layers in a map will sometimes be rendered into one +single canvas instead of separate elements. +This means `Map.forEachLayerAtPixel` will bring up false positives. + +The easiest solution to avoid that is to assign different `className` properties to each layer like so: +```js +new Layer({ + // ... + className: 'my-layer' +}) +``` + +Please note that this may incur a significant performance loss when dealing with many layers and/or +targetting mobile devices. + ##### Removal of `TOUCH` constant from `ol/has` If you were previously using this constant, you can check if `'ontouchstart'` is defined in `window` instead. diff --git a/src/ol/PluggableMap.js b/src/ol/PluggableMap.js index 6e5e14391d..09c540e418 100644 --- a/src/ol/PluggableMap.js +++ b/src/ol/PluggableMap.js @@ -595,6 +595,10 @@ class PluggableMap extends BaseObject { * Detect layers that have a color value at a pixel on the viewport, and * execute a callback with each matching layer. Layers included in the * detection can be configured through `opt_layerFilter`. + * + * Note: this may give false positives unless the map layers have had different `className` + * properties assigned to them. + * * @param {import("./pixel.js").Pixel} pixel Pixel. * @param {function(this: S, import("./layer/Layer.js").default, (Uint8ClampedArray|Uint8Array)): T} callback * Layer callback. This callback will receive two arguments: first is the diff --git a/src/ol/layer/Layer.js b/src/ol/layer/Layer.js index 4665dafdb9..58f2ef44f2 100644 --- a/src/ol/layer/Layer.js +++ b/src/ol/layer/Layer.js @@ -35,6 +35,7 @@ import SourceState from '../source/State.js'; * @property {import("../PluggableMap.js").default} [map] Map. * @property {RenderFunction} [render] Render function. Takes the frame state as input and is expected to return an * HTML element. Will overwrite the default rendering for the layer. + * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. */ @@ -71,6 +72,11 @@ import SourceState from '../source/State.js'; * * A generic `change` event is fired when the state of the source changes. * + * Please note that for performance reasons several layers might get rendered to + * the same HTML element, which will cause {@link module:ol/Map~Map#forEachLayerAtPixel} to + * give false positives. To avoid this, apply different `className` properties to the + * layers at creation time. + * * @fires import("../render/Event.js").RenderEvent#prerender * @fires import("../render/Event.js").RenderEvent#postrender * From 167fa6b8a0f215708456f4d4724d37b9a639f29a Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Tue, 24 Sep 2019 10:57:23 +0200 Subject: [PATCH 2/2] Add property to all layers api doc --- src/ol/layer/BaseImage.js | 1 + src/ol/layer/BaseVector.js | 1 + src/ol/layer/Graticule.js | 1 + src/ol/layer/Heatmap.js | 1 + src/ol/layer/Layer.js | 2 +- src/ol/layer/VectorImage.js | 1 + src/ol/layer/VectorTile.js | 1 + 7 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ol/layer/BaseImage.js b/src/ol/layer/BaseImage.js index 9183382986..59b971c657 100644 --- a/src/ol/layer/BaseImage.js +++ b/src/ol/layer/BaseImage.js @@ -6,6 +6,7 @@ import Layer from './Layer.js'; /** * @typedef {Object} Options + * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. * @property {number} [opacity=1] Opacity (0, 1). * @property {boolean} [visible=true] Visibility. * @property {import("../extent.js").Extent} [extent] The bounding extent for layer rendering. The layer will not be diff --git a/src/ol/layer/BaseVector.js b/src/ol/layer/BaseVector.js index 441ea30742..8c2024ffd6 100644 --- a/src/ol/layer/BaseVector.js +++ b/src/ol/layer/BaseVector.js @@ -8,6 +8,7 @@ import {createDefaultStyle, toFunction as toStyleFunction} from '../style/Style. /** * @typedef {Object} Options + * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. * @property {number} [opacity=1] Opacity (0, 1). * @property {boolean} [visible=true] Visibility. * @property {import("../extent.js").Extent} [extent] The bounding extent for layer rendering. The layer will not be diff --git a/src/ol/layer/Graticule.js b/src/ol/layer/Graticule.js index 56ea5a5be5..d395bc13fa 100644 --- a/src/ol/layer/Graticule.js +++ b/src/ol/layer/Graticule.js @@ -52,6 +52,7 @@ const INTERVALS = [ /** * @typedef {Object} Options + * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. * @property {number} [opacity=1] Opacity (0, 1). * @property {boolean} [visible=true] Visibility. * @property {import("../extent.js").Extent} [extent] The bounding extent for layer rendering. The layer will not be diff --git a/src/ol/layer/Heatmap.js b/src/ol/layer/Heatmap.js index 4b85070355..d1380103d5 100644 --- a/src/ol/layer/Heatmap.js +++ b/src/ol/layer/Heatmap.js @@ -10,6 +10,7 @@ import WebGLPointsLayerRenderer from '../renderer/webgl/PointsLayer.js'; /** * @typedef {Object} Options + * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. * @property {number} [opacity=1] Opacity (0, 1). * @property {boolean} [visible=true] Visibility. * @property {import("../extent.js").Extent} [extent] The bounding extent for layer rendering. The layer will not be diff --git a/src/ol/layer/Layer.js b/src/ol/layer/Layer.js index 58f2ef44f2..1e0491608c 100644 --- a/src/ol/layer/Layer.js +++ b/src/ol/layer/Layer.js @@ -17,6 +17,7 @@ import SourceState from '../source/State.js'; /** * @typedef {Object} Options + * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. * @property {number} [opacity=1] Opacity (0, 1). * @property {boolean} [visible=true] Visibility. * @property {import("../extent.js").Extent} [extent] The bounding extent for layer rendering. The layer will not be @@ -35,7 +36,6 @@ import SourceState from '../source/State.js'; * @property {import("../PluggableMap.js").default} [map] Map. * @property {RenderFunction} [render] Render function. Takes the frame state as input and is expected to return an * HTML element. Will overwrite the default rendering for the layer. - * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. */ diff --git a/src/ol/layer/VectorImage.js b/src/ol/layer/VectorImage.js index 980385da57..b121cc1b83 100644 --- a/src/ol/layer/VectorImage.js +++ b/src/ol/layer/VectorImage.js @@ -7,6 +7,7 @@ import CanvasVectorImageLayerRenderer from '../renderer/canvas/VectorImageLayer. /** * @typedef {Object} Options + * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. * @property {number} [opacity=1] Opacity (0, 1). * @property {boolean} [visible=true] Visibility. * @property {import("../extent.js").Extent} [extent] The bounding extent for layer rendering. The layer will not be diff --git a/src/ol/layer/VectorTile.js b/src/ol/layer/VectorTile.js index aa548fbba7..644ac16c48 100644 --- a/src/ol/layer/VectorTile.js +++ b/src/ol/layer/VectorTile.js @@ -11,6 +11,7 @@ import {assign} from '../obj.js'; /** * @typedef {Object} Options + * @property {string} [className='ol-layer'] A CSS class name to set to the layer element. * @property {number} [opacity=1] Opacity (0, 1). * @property {boolean} [visible=true] Visibility. * @property {import("../extent.js").Extent} [extent] The bounding extent for layer rendering. The layer will not be