Avoid recomputing the viewport size by reading the DOM everytime

Also clarify View#calculateExtent doc & remove the [data-view] attribute
on the viewport element (not needed anymore).

src/ol/PluggableMap.js

@@ -1,7 +1,6 @@
 /**
  * @module ol/PluggableMap
  */
-import {getUid} from './util.js';
 import Collection from './Collection.js';
 import CollectionEventType from './CollectionEventType.js';
 import MapBrowserEvent from './MapBrowserEvent.js';
@@ -1102,7 +1101,8 @@ class PluggableMap extends BaseObject {
     }
     const view = this.getView();
     if (view) {
-      this.viewport_.setAttribute('data-view', getUid(view));
+      this.updateViewportSize_();
+
       this.viewPropertyListenerKey_ = listen(
         view, ObjectEventType.PROPERTYCHANGE,
         this.handleViewPropertyChanged_, this);
@@ -1360,6 +1360,27 @@ class PluggableMap extends BaseObject {
             parseFloat(computedStyle['borderBottomWidth'])
       ]);
     }
+
+    this.updateViewportSize_();
+  }
+
+  /**
+   * Recomputes the viewport size and save it on the view object (if any)
+   * @private
+   */
+  updateViewportSize_() {
+    const view = this.getView();
+    if (view) {
+      let size = undefined;
+      const computedStyle = getComputedStyle(this.viewport_);
+      if (computedStyle.width && computedStyle.height) {
+        size = [
+          parseInt(computedStyle.width, 10),
+          parseInt(computedStyle.height, 10)
+        ];
+      }
+      view.setViewportSize(size);
+    }
   }
 }
 

src/ol/View.js

@@ -2,7 +2,6 @@
  * @module ol/View
  */
 import {DEFAULT_TILE_SIZE} from './tilegrid/common.js';
-import {getUid} from './util.js';
 import {VOID} from './functions.js';
 import {createExtent, none as centerNone} from './centerconstraint.js';
 import BaseObject from './Object.js';
@@ -294,6 +293,12 @@ class View extends BaseObject {
      */
     this.projection_ = createProjection(options.projection, 'EPSG:3857');
 
+    /**
+     * @private
+     * @type {import("./size.js").Size}
+     */
+    this.viewportSize_ = [100, 100];
+
     /**
      * @private
      * @type {import("./coordinate.js").Coordinate|undefined}
@@ -649,7 +654,7 @@ class View extends BaseObject {
             animation.targetResolution :
             animation.sourceResolution + progress * (animation.targetResolution - animation.sourceResolution);
           if (animation.anchor) {
-            const size = this.getSizeFromViewport_(this.getRotation());
+            const size = this.getViewportSize_(this.getRotation());
             const constrainedResolution = this.constraints_.resolution(resolution, 0, size, true);
             this.targetCenter_ = this.calculateCenterZoom(constrainedResolution, animation.anchor);
           }
@@ -722,26 +727,33 @@ class View extends BaseObject {
   }
 
   /**
+   * Returns the current viewport size.
    * @private
    * @param {number=} opt_rotation Take into account the rotation of the viewport when giving the size
    * @return {import("./size.js").Size} Viewport size or `[100, 100]` when no viewport is found.
    */
-  getSizeFromViewport_(opt_rotation) {
+  getViewportSize_(opt_rotation) {
-    const size = [100, 100];
+    const size = this.viewportSize_;
-    const selector = '.ol-viewport[data-view="' + getUid(this) + '"]';
-    const element = document.querySelector(selector);
-    if (element) {
-      const metrics = getComputedStyle(element);
-      size[0] = parseInt(metrics.width, 10);
-      size[1] = parseInt(metrics.height, 10);
-    }
     if (opt_rotation) {
       const w = size[0];
       const h = size[1];
-      size[0] = Math.abs(w * Math.cos(opt_rotation)) + Math.abs(h * Math.sin(opt_rotation));
+      return [
-      size[1] = Math.abs(w * Math.sin(opt_rotation)) + Math.abs(h * Math.cos(opt_rotation));
+        Math.abs(w * Math.cos(opt_rotation)) + Math.abs(h * Math.sin(opt_rotation)),
+        Math.abs(w * Math.sin(opt_rotation)) + Math.abs(h * Math.cos(opt_rotation))
+      ];
+    } else {
+      return size;
     }
-    return size;
+  }
+
+  /**
+   * Stores the viewport size on the view. The viewport size is not read every time from the DOM
+   * to avoid performance hit and layout reflow.
+   * This should be done on map size change.
+   * @param {import("./size.js").Size=} opt_size Viewport size; if undefined, [100, 100] is assumed
+   */
+  setViewportSize(opt_size) {
+    this.viewportSize_ = Array.isArray(opt_size) ? opt_size.slice() : [100, 100];
   }
 
   /**
@@ -792,8 +804,8 @@ class View extends BaseObject {
    * The size is the pixel dimensions of the box into which the calculated extent
    * should fit. In most cases you want to get the extent of the entire map,
    * that is `map.getSize()`.
-   * @param {import("./size.js").Size=} opt_size Box pixel size. If not provided, the size of the
+   * @param {import("./size.js").Size=} opt_size Box pixel size. If not provided, the size
-   * first map that uses this view will be used.
+   * of the map that uses this view will be used.
    * @return {import("./extent.js").Extent} Extent.
    * @api
    */
@@ -808,7 +820,7 @@ class View extends BaseObject {
    * @return {import("./extent.js").Extent} Extent.
    */
   calculateExtentInternal(opt_size) {
-    const size = opt_size || this.getSizeFromViewport_();
+    const size = opt_size || this.getViewportSize_();
     const center = /** @type {!import("./coordinate.js").Coordinate} */ (this.getCenterInternal());
     assert(center, 1); // The view center is not defined
     const resolution = /** @type {!number} */ (this.getResolution());
@@ -931,7 +943,7 @@ class View extends BaseObject {
    *     the given size.
    */
   getResolutionForExtentInternal(extent, opt_size) {
-    const size = opt_size || this.getSizeFromViewport_();
+    const size = opt_size || this.getViewportSize_();
     const xResolution = getWidth(extent) / size[0];
     const yResolution = getHeight(extent) / size[1];
     return Math.max(xResolution, yResolution);
@@ -1079,7 +1091,7 @@ class View extends BaseObject {
    * @api
    */
   fit(geometryOrExtent, opt_options) {
-    const options = assign({size: this.getSizeFromViewport_()}, opt_options || {});
+    const options = assign({size: this.getViewportSize_()}, opt_options || {});
 
     /** @type {import("./geom/SimpleGeometry.js").default} */
     let geometry;
@@ -1114,7 +1126,7 @@ class View extends BaseObject {
     const options = opt_options || {};
     let size = options.size;
     if (!size) {
-      size = this.getSizeFromViewport_();
+      size = this.getViewportSize_();
     }
     const padding = options.padding !== undefined ? options.padding : [0, 0, 0, 0];
     const nearest = options.nearest !== undefined ? options.nearest : false;
@@ -1261,7 +1273,7 @@ class View extends BaseObject {
    */
   adjustResolutionInternal(ratio, opt_anchor) {
     const isMoving = this.getAnimating() || this.getInteracting();
-    const size = this.getSizeFromViewport_(this.getRotation());
+    const size = this.getViewportSize_(this.getRotation());
     const newResolution = this.constraints_.resolution(this.targetResolution_ * ratio, 0, size, isMoving);
 
     if (opt_anchor) {
@@ -1385,7 +1397,7 @@ class View extends BaseObject {
 
     // compute rotation
     const newRotation = this.constraints_.rotation(this.targetRotation_, isMoving);
-    const size = this.getSizeFromViewport_(newRotation);
+    const size = this.getViewportSize_(newRotation);
     const newResolution = this.constraints_.resolution(this.targetResolution_, 0, size, isMoving);
     const newCenter = this.constraints_.center(this.targetCenter_, newResolution, size, isMoving);
 
@@ -1419,7 +1431,7 @@ class View extends BaseObject {
     const direction = opt_resolutionDirection || 0;
 
     const newRotation = this.constraints_.rotation(this.targetRotation_);
-    const size = this.getSizeFromViewport_(newRotation);
+    const size = this.getViewportSize_(newRotation);
     const newResolution = this.constraints_.resolution(this.targetResolution_, direction, size);
     const newCenter = this.constraints_.center(this.targetCenter_, newResolution, size);
 
@@ -1500,7 +1512,7 @@ class View extends BaseObject {
    * @return {import("./coordinate.js").Coordinate|undefined} Valid center position.
    */
   getConstrainedCenter(targetCenter, opt_targetResolution) {
-    const size = this.getSizeFromViewport_(this.getRotation());
+    const size = this.getViewportSize_(this.getRotation());
     return this.constraints_.center(targetCenter, opt_targetResolution || this.getResolution(), size);
   }
 
@@ -1529,7 +1541,7 @@ class View extends BaseObject {
    */
   getConstrainedResolution(targetResolution, opt_direction) {
     const direction = opt_direction || 0;
-    const size = this.getSizeFromViewport_(this.getRotation());
+    const size = this.getViewportSize_(this.getRotation());
 
     return this.constraints_.resolution(targetResolution, direction, size);
   }

test/spec/ol/view.test.js

@@ -1461,28 +1461,37 @@ describe('ol.View', function() {
     });
   });
 
-  describe('#getSizeFromViewport_()', function() {
+  describe('#getViewportSize_()', function() {
     let map, target;
     beforeEach(function() {
       target = document.createElement('div');
       target.style.width = '200px';
       target.style.height = '150px';
+      document.body.appendChild(target);
       map = new Map({
         target: target
       });
-      document.body.appendChild(target);
     });
     afterEach(function() {
       map.setTarget(null);
       document.body.removeChild(target);
     });
-    it('calculates the size correctly', function() {
+    it('correctly initializes the viewport size', function() {
-      let size = map.getView().getSizeFromViewport_();
+      const size = map.getView().getViewportSize_();
       expect(size).to.eql([200, 150]);
-      size = map.getView().getSizeFromViewport_(Math.PI / 2);
+    });
+    it('correctly updates the viewport size', function() {
+      target.style.width = '300px';
+      target.style.height = '200px';
+      map.updateSize();
+      const size = map.getView().getViewportSize_();
+      expect(size).to.eql([300, 200]);
+    });
+    it('calculates the size correctly', function() {
+      let size = map.getView().getViewportSize_(Math.PI / 2);
       expect(size[0]).to.roughlyEqual(150, 1e-9);
       expect(size[1]).to.roughlyEqual(200, 1e-9);
-      size = map.getView().getSizeFromViewport_(Math.PI);
+      size = map.getView().getViewportSize_(Math.PI);
       expect(size[0]).to.roughlyEqual(200, 1e-9);
       expect(size[1]).to.roughlyEqual(150, 1e-9);
     });