Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch render DOM elements to avoid reflow #10530

Merged
merged 3 commits into from
Apr 7, 2021
Merged

Batch render DOM elements to avoid reflow #10530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
846520d
Batch render DOM elements to avoid reflow
zarov Apr 1, 2021
bbeb2f1
Fixed flow and unit testing for DOM batch rendering
zarov Apr 1, 2021
27ab023
Removed aliases after review
zarov Apr 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/ui/control/navigation_control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ class NavigationControl {
`scale(${1 / Math.pow(Math.cos(this._map.transform.pitch * (Math.PI / 180)), 0.5)}) rotateX(${this._map.transform.pitch}deg) rotateZ(${this._map.transform.angle * (180 / Math.PI)}deg)` :
`rotate(${this._map.transform.angle * (180 / Math.PI)}deg)`;

this._compassIcon.style.transform = rotate;
this._map._domRenderTaskQueue.add(() => {
if (this._compassIcon) {
this._compassIcon.style.transform = rotate;
}
});
}

onAdd(map: Map) {
Expand Down
19 changes: 11 additions & 8 deletions src/ui/control/scale_control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,32 +96,35 @@ function updateScale(map, container, options) {
const left = map.unproject([0, y]);
const right = map.unproject([maxWidth, y]);
const maxMeters = left.distanceTo(right);
const domQueue = map._domRenderTaskQueue;
// The real distance corresponding to 100px scale length is rounded off to
// near pretty number and the scale length for the same is found out.
// Default unit of the scale is based on User's locale.
if (options && options.unit === 'imperial') {
const maxFeet = 3.2808 * maxMeters;
if (maxFeet > 5280) {
const maxMiles = maxFeet / 5280;
setScale(container, maxWidth, maxMiles, map._getUIString('ScaleControl.Miles'));
setScale(container, maxWidth, maxMiles, map._getUIString('ScaleControl.Miles'), domQueue);
} else {
setScale(container, maxWidth, maxFeet, map._getUIString('ScaleControl.Feet'));
setScale(container, maxWidth, maxFeet, map._getUIString('ScaleControl.Feet'), domQueue);
}
} else if (options && options.unit === 'nautical') {
const maxNauticals = maxMeters / 1852;
setScale(container, maxWidth, maxNauticals, map._getUIString('ScaleControl.NauticalMiles'));
setScale(container, maxWidth, maxNauticals, map._getUIString('ScaleControl.NauticalMiles'), domQueue);
} else if (maxMeters >= 1000) {
setScale(container, maxWidth, maxMeters / 1000, map._getUIString('ScaleControl.Kilometers'));
setScale(container, maxWidth, maxMeters / 1000, map._getUIString('ScaleControl.Kilometers'), domQueue);
} else {
setScale(container, maxWidth, maxMeters, map._getUIString('ScaleControl.Meters'));
setScale(container, maxWidth, maxMeters, map._getUIString('ScaleControl.Meters'), domQueue);
}
}

function setScale(container, maxWidth, maxDistance, unit) {
function setScale(container, maxWidth, maxDistance, unit, domQueue) {
const distance = getRoundNum(maxDistance);
const ratio = distance / maxDistance;
container.style.width = `${maxWidth * ratio}px`;
container.innerHTML = `${distance} ${unit}`;
domQueue.add(() => {
container.style.width = `${maxWidth * ratio}px`;
container.innerHTML = `${distance} ${unit}`;
});
}

function getDecimalRoundNum(d) {
Expand Down
12 changes: 8 additions & 4 deletions src/ui/handler/box_zoom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,14 @@ class BoxZoomHandler {
minY = Math.min(p0.y, pos.y),
maxY = Math.max(p0.y, pos.y);

DOM.setTransform(this._box, `translate(${minX}px,${minY}px)`);

this._box.style.width = `${maxX - minX}px`;
this._box.style.height = `${maxY - minY}px`;
this._map._domRenderTaskQueue.add(() => {
if (this._box) {
DOM.setTransform(this._box, `translate(${minX}px,${minY}px)`);

this._box.style.width = `${maxX - minX}px`;
this._box.style.height = `${maxY - minY}px`;
}
});
}

mouseupWindow(e: MouseEvent, point: Point) {
Expand Down
4 changes: 4 additions & 0 deletions src/ui/map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ class Map extends Camera {
_collectResourceTiming: boolean;
_optimizeForTerrain: boolean;
_renderTaskQueue: TaskQueue;
_domRenderTaskQueue: TaskQueue;
_controls: Array<IControl>;
_logoControl: IControl;
_mapId: number;
Expand Down Expand Up @@ -420,6 +421,7 @@ class Map extends Camera {
this._collectResourceTiming = options.collectResourceTiming;
this._optimizeForTerrain = options.optimizeForTerrain;
this._renderTaskQueue = new TaskQueue();
this._domRenderTaskQueue = new TaskQueue();
this._controls = [];
this._mapId = uniqueId();
this._locale = extend({}, defaultLocale, options.locale);
Expand Down Expand Up @@ -2526,6 +2528,7 @@ class Map extends Camera {
this.painter.setBaseState();

this._renderTaskQueue.run(paintStartTimeStamp);
this._domRenderTaskQueue.run(paintStartTimeStamp);
// A task queue callback may have fired a user event which may have removed the map
if (this._removed) return;

Expand Down Expand Up @@ -2770,6 +2773,7 @@ class Map extends Camera {
this._frame = null;
}
this._renderTaskQueue.clear();
this._domRenderTaskQueue.clear();
this.painter.destroy();
this.handlers.destroy();
delete this.handlers;
Expand Down
6 changes: 5 additions & 1 deletion src/ui/marker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,11 @@ export default class Marker extends Evented {
this._pos = this._pos.round();
}

DOM.setTransform(this._element, `${anchorTranslate[this._anchor]} translate(${this._pos.x}px, ${this._pos.y}px) ${pitch} ${rotation}`);
this._map._domRenderTaskQueue.add(() => {
if (this._element && this._pos && this._anchor) {
DOM.setTransform(this._element, `${anchorTranslate[this._anchor]} translate(${this._pos.x}px, ${this._pos.y}px) ${pitch} ${rotation}`);
}
});
}

/**
Expand Down
8 changes: 6 additions & 2 deletions src/ui/popup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,8 +572,12 @@ export default class Popup extends Evented {
}

const offsetedPos = pos.add(offset[anchor]).round();
DOM.setTransform(this._container, `${anchorTranslate[anchor]} translate(${offsetedPos.x}px,${offsetedPos.y}px)`);
applyAnchorClass(this._container, anchor, 'popup');
this._map._domRenderTaskQueue.add(() => {
if (this._container && anchor) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: anchor is always truthy here so no need to check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but flow is not understanding this. Is there any flag I can maybe add to make it understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same goes in marker.js btw.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See this previous job on CircleCI for reference

DOM.setTransform(this._container, `${anchorTranslate[anchor]} translate(${offsetedPos.x}px,${offsetedPos.y}px)`);
applyAnchorClass(this._container, anchor, 'popup');
}
});
}

_focusFirstElement() {
Expand Down
5 changes: 5 additions & 0 deletions test/unit/ui/control/scale.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import ScaleControl from '../../../../src/ui/control/scale_control.js';
test('ScaleControl appears in bottom-left by default', (t) => {
const map = createMap(t);
map.addControl(new ScaleControl());
map._domRenderTaskQueue.run();

t.equal(map.getContainer().querySelectorAll('.mapboxgl-ctrl-bottom-left .mapboxgl-ctrl-scale').length, 1);
t.end();
Expand All @@ -14,6 +15,7 @@ test('ScaleControl appears in bottom-left by default', (t) => {
test('ScaleControl appears in the position specified by the position option', (t) => {
const map = createMap(t);
map.addControl(new ScaleControl(), 'top-left');
map._domRenderTaskQueue.run();

t.equal(map.getContainer().querySelectorAll('.mapboxgl-ctrl-top-left .mapboxgl-ctrl-scale').length, 1);
t.end();
Expand All @@ -24,11 +26,13 @@ test('ScaleControl should change unit of distance after calling setUnit', (t) =>
const scale = new ScaleControl();
const selector = '.mapboxgl-ctrl-bottom-left .mapboxgl-ctrl-scale';
map.addControl(scale);
map._domRenderTaskQueue.run();

let contents = map.getContainer().querySelector(selector).innerHTML;
t.match(contents, /km/);

scale.setUnit('imperial');
map._domRenderTaskQueue.run();
contents = map.getContainer().querySelector(selector).innerHTML;
t.match(contents, /mi/);
t.end();
Expand All @@ -41,6 +45,7 @@ test('ScaleControl should respect the maxWidth regardless of the unit and actual
const selector = '.mapboxgl-ctrl-bottom-left .mapboxgl-ctrl-scale';
map.addControl(scale);
map.setZoom(12.5);
map._domRenderTaskQueue.run();

const el = map.getContainer().querySelector(selector);
t.ok(parseFloat(el.style.width, 10) <= maxWidth, 'ScaleControl respects maxWidth');
Expand Down
15 changes: 15 additions & 0 deletions test/unit/ui/marker.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ test('Marker anchor defaults to center', (t) => {
const marker = new Marker()
.setLngLat([0, 0])
.addTo(map);
map._domRenderTaskQueue.run();

t.ok(marker.getElement().classList.contains('mapboxgl-marker-anchor-center'));
t.match(marker.getElement().style.transform, /translate\(-50%,-50%\)/);
Expand All @@ -232,6 +233,7 @@ test('Marker anchors as specified by the anchor option', (t) => {
const marker = new Marker({anchor: 'top'})
.setLngLat([0, 0])
.addTo(map);
map._domRenderTaskQueue.run();

t.ok(marker.getElement().classList.contains('mapboxgl-marker-anchor-top'));
t.match(marker.getElement().style.transform, /translate\(-50%,0\)/);
Expand Down Expand Up @@ -259,6 +261,7 @@ test('Popup offsets around default Marker', (t) => {
.setLngLat([0, 0])
.setPopup(new Popup().setText('Test'))
.addTo(map);
map._domRenderTaskQueue.run();

t.ok(marker.getPopup().options.offset.bottom[1] < 0, 'popup is vertically offset somewhere above the tip');
t.ok(marker.getPopup().options.offset.top[1] === 0, 'popup is vertically offset at the tip');
Expand Down Expand Up @@ -296,34 +299,42 @@ test('Popup anchors around default Marker', (t) => {
Object.defineProperty(marker.getPopup()._container, 'offsetHeight', {value: 100});

// marker should default to above since it has enough space
map._domRenderTaskQueue.run();
t.ok(marker.getPopup()._container.classList.contains('mapboxgl-popup-anchor-bottom'), 'popup anchors above marker');

// move marker to the top forcing the popup to below
marker.setLngLat(map.unproject([mapHeight / 2, markerTop]));
map._domRenderTaskQueue.run();
t.ok(marker.getPopup()._container.classList.contains('mapboxgl-popup-anchor-top'), 'popup anchors below marker');

// move marker to the right forcing the popup to the left
marker.setLngLat(map.unproject([mapHeight - markerRight, mapHeight / 2]));
map._domRenderTaskQueue.run();
t.ok(marker.getPopup()._container.classList.contains('mapboxgl-popup-anchor-right'), 'popup anchors left of marker');

// move marker to the left forcing the popup to the right
marker.setLngLat(map.unproject([markerRight, mapHeight / 2]));
map._domRenderTaskQueue.run();
t.ok(marker.getPopup()._container.classList.contains('mapboxgl-popup-anchor-left'), 'popup anchors right of marker');

// move marker to the top left forcing the popup to the bottom right
marker.setLngLat(map.unproject([markerRight, markerTop]));
map._domRenderTaskQueue.run();
t.ok(marker.getPopup()._container.classList.contains('mapboxgl-popup-anchor-top-left'), 'popup anchors bottom right of marker');

// move marker to the top right forcing the popup to the bottom left
marker.setLngLat(map.unproject([mapHeight - markerRight, markerTop]));
map._domRenderTaskQueue.run();
t.ok(marker.getPopup()._container.classList.contains('mapboxgl-popup-anchor-top-right'), 'popup anchors bottom left of marker');

// move marker to the bottom left forcing the popup to the top right
marker.setLngLat(map.unproject([markerRight, mapHeight]));
map._domRenderTaskQueue.run();
t.ok(marker.getPopup()._container.classList.contains('mapboxgl-popup-anchor-bottom-left'), 'popup anchors top right of marker');

// move marker to the bottom right forcing the popup to the top left
marker.setLngLat(map.unproject([mapHeight - markerRight, mapHeight]));
map._domRenderTaskQueue.run();
t.ok(marker.getPopup()._container.classList.contains('mapboxgl-popup-anchor-bottom-right'), 'popup anchors top left of marker');

t.end();
Expand Down Expand Up @@ -725,11 +736,13 @@ test('Marker transforms rotation with the map', (t) => {
const marker = new Marker({rotationAlignment: 'map'})
.setLngLat([0, 0])
.addTo(map);
map._domRenderTaskQueue.run();

const rotationRegex = /rotateZ\(-?([0-9]+)deg\)/;
const initialRotation = marker.getElement().style.transform.match(rotationRegex)[1];

map.setBearing(map.getBearing() + 180);
map._domRenderTaskQueue.run();

const finalRotation = marker.getElement().style.transform.match(rotationRegex)[1];
t.notEqual(initialRotation, finalRotation);
Expand All @@ -745,11 +758,13 @@ test('Marker transforms pitch with the map', (t) => {
.addTo(map);

map.setPitch(0);
map._domRenderTaskQueue.run();

const rotationRegex = /rotateX\(-?([0-9]+)deg\)/;
const initialPitch = marker.getElement().style.transform.match(rotationRegex)[1];

map.setPitch(45);
map._domRenderTaskQueue.run();

const finalPitch = marker.getElement().style.transform.match(rotationRegex)[1];
t.notEqual(initialPitch, finalPitch);
Expand Down
9 changes: 9 additions & 0 deletions test/unit/ui/popup.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ test('Popup anchors as specified by the anchor option', (t) => {
.setLngLat([0, 0])
.setText('Test')
.addTo(map);
map._domRenderTaskQueue.run();

t.ok(popup.getElement().classList.contains('mapboxgl-popup-anchor-top-left'));
t.end();
Expand All @@ -419,13 +420,15 @@ test('Popup anchors as specified by the anchor option', (t) => {
.setLngLat([0, 0])
.setText('Test')
.addTo(map);
map._domRenderTaskQueue.run();

Object.defineProperty(popup.getElement(), 'offsetWidth', {value: 100});
Object.defineProperty(popup.getElement(), 'offsetHeight', {value: 100});

t.stub(map, 'project').returns(point);
t.stub(map.transform, 'locationPoint3D').returns(point);
popup.setLngLat([0, 0]);
map._domRenderTaskQueue.run();

t.ok(popup.getElement().classList.contains(`mapboxgl-popup-anchor-${anchor}`));
t.end();
Expand All @@ -440,6 +443,7 @@ test('Popup anchors as specified by the anchor option', (t) => {
.setLngLat([0, 0])
.setText('Test')
.addTo(map);
map._domRenderTaskQueue.run();

t.equal(popup.getElement().style.transform, transform);
t.end();
Expand All @@ -457,12 +461,14 @@ test('Popup automatically anchors to top if its bottom offset would push it off-
.setLngLat([0, 0])
.setText('Test')
.addTo(map);
map._domRenderTaskQueue.run();

Object.defineProperty(popup.getElement(), 'offsetWidth', {value: containerWidth / 2});
Object.defineProperty(popup.getElement(), 'offsetHeight', {value: containerHeight / 2});

t.stub(map, 'project').returns(point);
popup.setLngLat([0, 0]);
map._domRenderTaskQueue.run();

t.ok(popup.getElement().classList.contains('mapboxgl-popup-anchor-top'));
t.end();
Expand All @@ -477,6 +483,7 @@ test('Popup is offset via a PointLike offset option', (t) => {
.setLngLat([0, 0])
.setText('Test')
.addTo(map);
map._domRenderTaskQueue.run();

t.equal(popup.getElement().style.transform, 'translate(0,0) translate(5px,10px)');
t.end();
Expand All @@ -491,6 +498,7 @@ test('Popup is offset via an object offset option', (t) => {
.setLngLat([0, 0])
.setText('Test')
.addTo(map);
map._domRenderTaskQueue.run();

t.equal(popup.getElement().style.transform, 'translate(0,0) translate(5px,10px)');
t.end();
Expand All @@ -505,6 +513,7 @@ test('Popup is offset via an incomplete object offset option', (t) => {
.setLngLat([0, 0])
.setText('Test')
.addTo(map);
map._domRenderTaskQueue.run();

t.equal(popup.getElement().style.transform, 'translate(-100%,0) translate(0px,0px)');
t.end();
Expand Down