Merge pull request #10239 from ahocevar/simpler-container-reuse

Simpler container reuse

src/ol/layer/Base.js

@@ -105,8 +105,7 @@ class BaseLayer extends BaseObject {
     /** @type {import("./Layer.js").State} */
     const state = this.state_ || /** @type {?} */ ({
       layer: this,
-      managed: opt_managed === undefined ? true : opt_managed,
+      managed: opt_managed === undefined ? true : opt_managed
-      hasOverlay: false
     });
     const zIndex = this.getZIndex();
     state.opacity = clamp(Math.round(this.getOpacity() * 100) / 100, 0, 1);

src/ol/layer/Layer.js

@@ -46,7 +46,6 @@ import SourceState from '../source/State.js';
  * @property {SourceState} sourceState
  * @property {boolean} visible
  * @property {boolean} managed
- * @property {boolean} hasOverlay Set by the renderer when an overlay for points and text is used.
  * @property {import("../extent.js").Extent} [extent]
  * @property {number} zIndex
  * @property {number} maxResolution

src/ol/render/canvas/Executor.js

@@ -537,6 +537,7 @@ class Executor extends Disposable {
     let lastStrokeInstruction = null;
     const coordinateCache = this.coordinateCache_;
     const viewRotation = this.viewRotation_;
+    const viewRotationFromTransform = Math.round(Math.atan2(-transform[1], transform[0]) * 1e12) / 1e12;
 
     const state = /** @type {import("../../render.js").State} */ ({
       context: context,
@@ -667,8 +668,12 @@ class Executor extends Disposable {
             backgroundFill = backgroundStroke = false;
           }
 
-          if (rotateWithView) {
+          if (rotateWithView && viewRotationFromTransform) {
+            // Canvas is expected to be rotated to reverse view rotation.
             rotation += viewRotation;
+          } else if (!rotateWithView && !viewRotationFromTransform) {
+            // Canvas is not rotated, images need to be rotated back to be north-up.
+            rotation -= viewRotation;
           }
           let widthIndex = 0;
           let declutterGroupIndex = 0;

src/ol/renderer/Composite.js

@@ -98,11 +98,9 @@ class CompositeMapRenderer extends MapRenderer {
     const viewState = frameState.viewState;
 
     this.children_.length = 0;
-    let hasOverlay = false;
     let previousElement = null;
     for (let i = 0, ii = layerStatesArray.length; i < ii; ++i) {
       const layerState = layerStatesArray[i];
-      hasOverlay = hasOverlay || layerState.hasOverlay;
       frameState.layerIndex = i;
       if (!inView(layerState, viewState) ||
         (layerState.sourceState != SourceState.READY && layerState.sourceState != SourceState.UNDEFINED)) {
@@ -114,13 +112,8 @@ class CompositeMapRenderer extends MapRenderer {
       if (!element) {
         continue;
       }
-      const childElementCount = element.childElementCount;
-      if ((element !== previousElement || i == ii - 1) && childElementCount === 2 && !hasOverlay) {
-        element.removeChild(element.lastElementChild);
-      }
       if (element !== previousElement) {
         this.children_.push(element);
-        hasOverlay = childElementCount === 2;
         previousElement = element;
       }
     }

src/ol/renderer/canvas/ImageLayer.js

@@ -7,7 +7,7 @@ import {containsExtent, intersects} from '../../extent.js';
 import {fromUserExtent} from '../../proj.js';
 import {getIntersection, isEmpty} from '../../extent.js';
 import CanvasLayerRenderer from './Layer.js';
-import {compose as composeTransform, makeInverse, toString as transformToString} from '../../transform.js';
+import {compose as composeTransform, makeInverse} from '../../transform.js';
 
 /**
  * @classdesc
@@ -104,7 +104,9 @@ class CanvasImageLayerRenderer extends CanvasLayerRenderer {
     );
     makeInverse(this.inversePixelTransform, this.pixelTransform);
 
-    this.useContainer(target, this.pixelTransform, layerState.opacity);
+    const canvasTransform = this.createTransformString(this.pixelTransform);
+
+    this.useContainer(target, canvasTransform, layerState.opacity);
 
     const context = this.context;
     const canvas = context.canvas;
@@ -162,7 +164,6 @@ class CanvasImageLayerRenderer extends CanvasLayerRenderer {
       context.restore();
     }
 
-    const canvasTransform = transformToString(this.pixelTransform);
     if (canvasTransform !== canvas.style.transform) {
       canvas.style.transform = canvasTransform;
     }

src/ol/renderer/canvas/Layer.js

@@ -7,7 +7,7 @@ import RenderEvent from '../../render/Event.js';
 import RenderEventType from '../../render/EventType.js';
 import {rotateAtOffset} from '../../render/canvas.js';
 import LayerRenderer from '../Layer.js';
-import {create as createTransform, apply as applyTransform, compose as composeTransform, toString as transformToString} from '../../transform.js';
+import {create as createTransform, apply as applyTransform, compose as composeTransform, toString} from '../../transform.js';
 
 /**
  * @abstract
@@ -69,12 +69,18 @@ class CanvasLayerRenderer extends LayerRenderer {
      */
     this.containerReused = false;
 
+    /**
+     * @type {HTMLCanvasElement}
+     * @private
+     */
+    this.createTransformStringCanvas_ = createCanvasContext2D(1, 1).canvas;
+
   }
 
   /**
    * Get a rendering container from an existing target, if compatible.
    * @param {HTMLElement} target Potential render target.
-   * @param {import("../../transform").Transform} transform Transform.
+   * @param {string} transform CSS Transform.
    * @param {number} opacity Opacity.
    */
   useContainer(target, transform, opacity) {
@@ -86,7 +92,7 @@ class CanvasLayerRenderer extends LayerRenderer {
         context = canvas.getContext('2d');
       }
     }
-    if (context && context.canvas.style.transform === transformToString(transform)) {
+    if (context && context.canvas.style.transform === transform) {
       // Container of the previous layer renderer can be used.
       this.container = target;
       this.context = context;
@@ -263,6 +269,15 @@ class CanvasLayerRenderer extends LayerRenderer {
     return data;
   }
 
+  /**
+   * @param {import("../../transform.js").Transform} transform Transform.
+   * @return {string} CSS transform.
+   */
+  createTransformString(transform) {
+    this.createTransformStringCanvas_.style.transform = toString(transform);
+    return this.createTransformStringCanvas_.style.transform;
+  }
+
 }
 
 export default CanvasLayerRenderer;

src/ol/renderer/canvas/TileLayer.js

@@ -7,7 +7,7 @@ import TileRange from '../../TileRange.js';
 import TileState from '../../TileState.js';
 import {createEmpty, equals, getIntersection, getTopLeft} from '../../extent.js';
 import CanvasLayerRenderer from './Layer.js';
-import {apply as applyTransform, compose as composeTransform, makeInverse, toString as transformToString} from '../../transform.js';
+import {apply as applyTransform, compose as composeTransform, makeInverse} from '../../transform.js';
 import {numberSafeCompareFunction} from '../../array.js';
 
 /**
@@ -242,7 +242,9 @@ class CanvasTileLayerRenderer extends CanvasLayerRenderer {
       -width / 2, -height / 2
     );
 
-    this.useContainer(target, this.pixelTransform, layerState.opacity);
+    const canvasTransform = this.createTransformString(this.pixelTransform);
+
+    this.useContainer(target, canvasTransform, layerState.opacity);
     const context = this.context;
     const canvas = context.canvas;
 
@@ -368,7 +370,6 @@ class CanvasTileLayerRenderer extends CanvasLayerRenderer {
       context.restore();
     }
 
-    const canvasTransform = transformToString(this.pixelTransform);
     if (canvasTransform !== canvas.style.transform) {
       canvas.style.transform = canvasTransform;
     }

src/ol/renderer/canvas/VectorLayer.js

@@ -127,7 +127,9 @@ class CanvasVectorLayerRenderer extends CanvasLayerRenderer {
     makeScale(this.pixelTransform, 1 / pixelRatio, 1 / pixelRatio);
     makeInverse(this.inversePixelTransform, this.pixelTransform);
 
-    this.useContainer(target, this.pixelTransform, layerState.opacity);
+    const canvasTransform = transformToString(this.pixelTransform);
+
+    this.useContainer(target, canvasTransform, layerState.opacity);
     const context = this.context;
     const canvas = context.canvas;
 
@@ -145,7 +147,6 @@ class CanvasVectorLayerRenderer extends CanvasLayerRenderer {
     if (canvas.width != width || canvas.height != height) {
       canvas.width = width;
       canvas.height = height;
-      const canvasTransform = transformToString(this.pixelTransform);
       if (canvas.style.transform !== canvasTransform) {
         canvas.style.transform = canvasTransform;
       }

src/ol/renderer/canvas/VectorTileLayer.js

@@ -2,7 +2,6 @@
  * @module ol/renderer/canvas/VectorTileLayer
  */
 import {getUid} from '../../util.js';
-import {createCanvasContext2D} from '../../dom.js';
 import TileState from '../../TileState.js';
 import ViewHint from '../../ViewHint.js';
 import {listen, unlistenByKey} from '../../events.js';
@@ -20,9 +19,8 @@ import {
   reset as resetTransform,
   scale as scaleTransform,
   translate as translateTransform,
-  toString as transformToString,
+  multiply,
-  makeScale,
+  scale
-  makeInverse
 } from '../../transform.js';
 import CanvasExecutorGroup, {replayDeclutter} from '../../render/canvas/ExecutorGroup.js';
 import {clear} from '../../obj.js';
@@ -64,31 +62,6 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer {
     /** @private */
     this.boundHandleStyleImageChange_ = this.handleStyleImageChange_.bind(this);
 
-    /**
-     * @private
-     * @type {CanvasRenderingContext2D}
-     */
-    this.overlayContext_ = null;
-
-    /**
-     * @type {string}
-     */
-    this.overlayContextUid_;
-
-    /**
-     * The transform for rendered pixels to viewport CSS pixels for the overlay canvas.
-     * @private
-     * @type {import("../../transform.js").Transform}
-     */
-    this.overlayPixelTransform_ = createTransform();
-
-    /**
-     * The transform for viewport CSS pixels to rendered pixels for the overlay canvas.
-     * @private
-     * @type {import("../../transform.js").Transform}
-     */
-    this.inverseOverlayPixelTransform_ = createTransform();
-
     /**
      * @private
      * @type {boolean}
@@ -131,37 +104,6 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer {
     this.tmpTransform_ = createTransform();
   }
 
-  /**
-   * @inheritDoc
-   */
-  useContainer(target, transform, opacity) {
-    let overlayContext;
-    if (target && target.childElementCount === 2) {
-      overlayContext = target.lastElementChild.getContext('2d');
-      if (!overlayContext) {
-        target = null;
-      }
-    }
-    const containerReused = this.containerReused;
-    super.useContainer(target, transform, opacity);
-    if (containerReused) {
-      this.overlayContext_ = overlayContext || null;
-      this.overlayContextUid_ = overlayContext ? getUid(overlayContext) : undefined;
-    }
-    if (!this.overlayContext_) {
-      const overlayContext = createCanvasContext2D();
-      const style = overlayContext.canvas.style;
-      style.position = 'absolute';
-      style.left = '0';
-      style.transformOrigin = 'top left';
-      this.overlayContext_ = overlayContext;
-      this.overlayContextUid_ = getUid(overlayContext);
-    }
-    if (this.container.childElementCount === 1) {
-      this.container.appendChild(this.overlayContext_.canvas);
-    }
-  }
-
   /**
    * @param {import("../../VectorRenderTile.js").default} tile Tile.
    * @param {number} pixelRatio Pixel ratio.
@@ -239,8 +181,6 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer {
    * @inheritDoc
    */
   prepareFrame(frameState) {
-    const layerState = frameState.layerStatesArray[frameState.layerIndex];
-    layerState.hasOverlay = true;
     const layerRevision = this.getLayer().getRevision();
     if (this.renderedLayerRevision_ != layerRevision) {
       this.renderedTiles.length = 0;
@@ -506,7 +446,7 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer {
       }
     }
 
-    const context = this.overlayContext_;
+    const context = this.context;
     const declutterReplays = layer.getDeclutter() ? {} : null;
     const replayTypes = VECTOR_REPLAYS[renderMode];
     const pixelRatio = frameState.pixelRatio;
@@ -516,24 +456,8 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer {
     const rotation = viewState.rotation;
     const size = frameState.size;
 
-    // set forward and inverse pixel transforms
-    makeScale(this.overlayPixelTransform_, 1 / pixelRatio, 1 / pixelRatio);
-    makeInverse(this.inverseOverlayPixelTransform_, this.overlayPixelTransform_);
-
-    // resize and clear
-    const canvas = context.canvas;
     const width = Math.round(size[0] * pixelRatio);
     const height = Math.round(size[1] * pixelRatio);
-    if (canvas.width != width || canvas.height != height) {
-      canvas.width = width;
-      canvas.height = height;
-      const canvasTransform = transformToString(this.overlayPixelTransform_);
-      if (canvas.style.transform !== canvasTransform) {
-        canvas.style.transform = canvasTransform;
-      }
-    } else if (getUid(context) === this.overlayContextUid_) {
-      context.clearRect(0, 0, width, height);
-    }
 
     const tiles = this.renderedTiles;
     const tileGrid = source.getTileGridForProjection(frameState.viewState.projection);
@@ -547,7 +471,10 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer {
       const tileCoord = tile.tileCoord;
       const tileExtent = tileGrid.getTileCoordExtent(tile.wrappedTileCoord);
       const worldOffset = tileGrid.getTileCoordExtent(tileCoord, this.tmpExtent)[0] - tileExtent[0];
-      const transform = this.getRenderTransform(center, resolution, rotation, pixelRatio, width, height, worldOffset);
+      const transform = multiply(
+        scale(this.inversePixelTransform.slice(), 1 / pixelRatio, 1 / pixelRatio),
+        this.getRenderTransform(center, resolution, rotation, pixelRatio, width, height, worldOffset)
+      );
       const executorGroups = tile.executorGroups[getUid(layer)];
       let clipped = false;
       for (let t = 0, tt = executorGroups.length; t < tt; ++t) {
@@ -707,34 +634,6 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer {
     replayState.renderedTileResolution = tile.wantedResolution;
   }
 
-  /**
-   * @inheritDoc
-   */
-  getDataAtPixel(pixel, frameState, hitTolerance) {
-    let data = super.getDataAtPixel(pixel, frameState, hitTolerance);
-    if (data) {
-      return data;
-    }
-
-    const renderPixel = applyTransform(this.inverseOverlayPixelTransform_, pixel.slice());
-    const context = this.overlayContext_;
-
-    try {
-      data = context.getImageData(Math.round(renderPixel[0]), Math.round(renderPixel[1]), 1, 1).data;
-    } catch (err) {
-      if (err.name === 'SecurityError') {
-        // tainted canvas, we assume there is data at the given pixel (although there might not be)
-        return new Uint8Array();
-      }
-      return data;
-    }
-
-    if (data[3] === 0) {
-      return null;
-    }
-    return data;
-  }
-
 }
 
 

test/spec/ol/layer/group.test.js

@@ -39,7 +39,6 @@ describe('ol.layer.Group', function() {
         opacity: 1,
         visible: true,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: undefined,
         zIndex: 0,
@@ -165,7 +164,6 @@ describe('ol.layer.Group', function() {
         opacity: 0.5,
         visible: false,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: undefined,
         zIndex: 10,
@@ -209,7 +207,6 @@ describe('ol.layer.Group', function() {
         opacity: 0.5,
         visible: false,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: groupExtent,
         zIndex: 0,
@@ -254,7 +251,6 @@ describe('ol.layer.Group', function() {
         opacity: 0.3,
         visible: false,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: groupExtent,
         zIndex: 10,
@@ -273,7 +269,6 @@ describe('ol.layer.Group', function() {
         opacity: 0,
         visible: false,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: undefined,
         zIndex: 0,
@@ -290,7 +285,6 @@ describe('ol.layer.Group', function() {
         opacity: 1,
         visible: true,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: undefined,
         zIndex: 0,
@@ -465,7 +459,6 @@ describe('ol.layer.Group', function() {
         opacity: 0.25,
         visible: false,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: undefined,
         zIndex: 0,

test/spec/ol/layer/layer.test.js

@@ -57,7 +57,6 @@ describe('ol.layer.Layer', function() {
         opacity: 1,
         visible: true,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: undefined,
         zIndex: 0,
@@ -99,7 +98,6 @@ describe('ol.layer.Layer', function() {
         opacity: 0.5,
         visible: false,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: undefined,
         zIndex: 10,
@@ -373,7 +371,6 @@ describe('ol.layer.Layer', function() {
         opacity: 0.33,
         visible: false,
         managed: true,
-        hasOverlay: false,
         sourceState: 'ready',
         extent: undefined,
         zIndex: 10,

test/spec/ol/renderer/canvas/vectortilelayer.test.js

@@ -215,12 +215,9 @@ describe('ol.renderer.canvas.VectorTileLayer', function() {
           url: 'rendering/ol/data/tiles/osm/{z}/{x}/{y}.png'
         })
       }));
-      map.once('postcompose', function(e) {
-        expect(e.frameState.layerStatesArray[1].hasOverlay).to.be(true);
-      });
       map.once('rendercomplete', function() {
         expect(document.querySelector('.ol-layers').childElementCount).to.be(1);
-        expect(document.querySelector('.ol-layer').childElementCount).to.be(2);
+        expect(document.querySelector('.ol-layer').childElementCount).to.be(1);
         map.removeLayer(map.getLayers().item(1));
         map.renderSync();
         expect(document.querySelector('.ol-layer').childElementCount).to.be(1);