From 4440994ec8f6b2268a6da269e01491d9e5770d81 Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Fri, 20 Dec 2019 15:27:16 +0100 Subject: [PATCH] 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 | 25 ++++++++++++++-- src/ol/View.js | 62 +++++++++++++++++++++++---------------- test/spec/ol/view.test.js | 21 +++++++++---- 3 files changed, 75 insertions(+), 33 deletions(-) diff --git a/src/ol/PluggableMap.js b/src/ol/PluggableMap.js index bc771866d8..8ddf303321 100644 --- a/src/ol/PluggableMap.js +++ b/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); + } } } diff --git a/src/ol/View.js b/src/ol/View.js index 5fc955d351..9331d9a740 100644 --- a/src/ol/View.js +++ b/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) { - const size = [100, 100]; - 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); - } + getViewportSize_(opt_rotation) { + const size = this.viewportSize_; 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)); - size[1] = Math.abs(w * Math.sin(opt_rotation)) + Math.abs(h * Math.cos(opt_rotation)); + return [ + 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 - * first map that uses this view will be used. + * @param {import("./size.js").Size=} opt_size Box pixel size. If not provided, the size + * 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); } diff --git a/test/spec/ol/view.test.js b/test/spec/ol/view.test.js index 94957a7cef..17c2f2733e 100644 --- a/test/spec/ol/view.test.js +++ b/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() { - let size = map.getView().getSizeFromViewport_(); + it('correctly initializes the viewport size', function() { + 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); });