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.

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

min and max pitch options #8834

Merged
merged 13 commits into from
Nov 7, 2019
25 changes: 22 additions & 3 deletions src/geo/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,24 @@ class Transform {
_renderWorldCopies: boolean;
_minZoom: number;
_maxZoom: number;
_minPitch: number;
_maxPitch: number;
_center: LngLat;
_constraining: boolean;
_posMatrixCache: {[number]: Float32Array};
_alignedPosMatrixCache: {[number]: Float32Array};

constructor(minZoom: ?number, maxZoom: ?number, renderWorldCopies: boolean | void) {
constructor(minZoom: ?number, maxZoom: ?number, minPitch: ?number, maxPitch: ?number, renderWorldCopies: boolean | void) {
this.tileSize = 512; // constant
this.maxValidLatitude = 85.051129; // constant

this._renderWorldCopies = renderWorldCopies === undefined ? true : renderWorldCopies;
this._minZoom = minZoom || 0;
this._maxZoom = maxZoom || 22;

this._minPitch = (minPitch === undefined || minPitch === null) ? 0 : minPitch;
this._maxPitch = (maxPitch === undefined || maxPitch === null) ? 60 : maxPitch;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't simply use maxPitch || 60 because it will not accept maxPitch: 0.
I need to check both undefined and null because flow is picky.


this.setMaxBounds();

this.width = 0;
Expand All @@ -74,7 +79,7 @@ class Transform {
}

clone(): Transform {
const clone = new Transform(this._minZoom, this._maxZoom, this._renderWorldCopies);
const clone = new Transform(this._minZoom, this._maxZoom, this._minPitch, this.maxPitch, this._renderWorldCopies);
clone.tileSize = this.tileSize;
clone.latRange = this.latRange;
clone.width = this.width;
Expand Down Expand Up @@ -103,6 +108,20 @@ class Transform {
this.zoom = Math.min(this.zoom, zoom);
}

get minPitch(): number { return this._minPitch; }
set minPitch(pitch: number) {
if (this._minPitch === pitch) return;
this._minPitch = pitch;
this.pitch = Math.max(this.pitch, pitch);
}

get maxPitch(): number { return this._maxPitch; }
set maxPitch(pitch: number) {
if (this._maxPitch === pitch) return;
this._maxPitch = pitch;
this.pitch = Math.min(this.pitch, pitch);
}

get renderWorldCopies(): boolean { return this._renderWorldCopies; }
set renderWorldCopies(renderWorldCopies?: ?boolean) {
if (renderWorldCopies === undefined) {
Expand Down Expand Up @@ -145,7 +164,7 @@ class Transform {
return this._pitch / Math.PI * 180;
}
set pitch(pitch: number) {
const p = clamp(pitch, 0, 60) / 180 * Math.PI;
const p = clamp(pitch, this.minPitch, this.maxPitch) / 180 * Math.PI;
if (this._pitch === p) return;
this._unmodified = false;
this._pitch = p;
Expand Down
98 changes: 96 additions & 2 deletions src/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ type MapOptions = {
scrollZoom?: boolean,
minZoom?: ?number,
maxZoom?: ?number,
minPitch?: ?number,
maxPitch?: ?number,
boxZoom?: boolean,
dragRotate?: boolean,
dragPan?: boolean,
Expand All @@ -99,6 +101,11 @@ type MapOptions = {

const defaultMinZoom = 0;
const defaultMaxZoom = 22;

// the default values, but also the valid range
const defaultMinPitch = 0;
const defaultMaxPitch = 60;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead of defining both the default and limits, since the defaults are going to be the limits, I've used this constant for both the default and the limits.


const defaultOptions = {
center: [0, 0],
zoom: 0,
Expand All @@ -108,6 +115,9 @@ const defaultOptions = {
minZoom: defaultMinZoom,
maxZoom: defaultMaxZoom,

minPitch: defaultMinPitch,
maxPitch: defaultMaxPitch,

interactive: true,
scrollZoom: true,
boxZoom: true,
Expand Down Expand Up @@ -150,6 +160,8 @@ const defaultOptions = {
* @param {HTMLElement|string} options.container The HTML element in which Mapbox GL JS will render the map, or the element's string `id`. The specified element must have no children.
* @param {number} [options.minZoom=0] The minimum zoom level of the map (0-24).
* @param {number} [options.maxZoom=22] The maximum zoom level of the map (0-24).
* @param {number} [options.minPitch=0] The minimum pitch of the map (0-60).
* @param {number} [options.maxPitch=60] The maximum pitch of the map (0-60).
* @param {Object|string} [options.style] The map's Mapbox style. This must be an a JSON object conforming to
* the schema described in the [Mapbox Style Specification](https://mapbox.com/mapbox-gl-style-spec/), or a URL to
* such JSON.
Expand Down Expand Up @@ -326,10 +338,22 @@ class Map extends Camera {
options = extend({}, defaultOptions, options);

if (options.minZoom != null && options.maxZoom != null && options.minZoom > options.maxZoom) {
throw new Error(`maxZoom must be greater than minZoom`);
throw new Error(`maxZoom must be greater than or equal to minZoom`);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could split this out to a separate PR if needed as it is unrelated.

}

if (options.minPitch != null && options.maxPitch != null && options.minPitch > options.maxPitch) {
throw new Error(`maxPitch must be greater than or equal to minPitch`);
}

if (options.minPitch != null && options.minPitch < defaultMinPitch) {
throw new Error(`minPitch must be greater than or equal to ${defaultMinPitch}`);
}

if (options.maxPitch != null && options.maxPitch > defaultMaxPitch) {
throw new Error(`maxPitch must be less than or equal to ${defaultMaxPitch}`);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So you can't set a minPitch of -10 or a maxPitch of 90.


const transform = new Transform(options.minZoom, options.maxZoom, options.renderWorldCopies);
const transform = new Transform(options.minZoom, options.maxZoom, options.minPitch, options.maxPitch, options.renderWorldCopies);
super(transform, options);

this._interactive = options.interactive;
Expand Down Expand Up @@ -647,6 +671,76 @@ class Map extends Camera {
*/
getMaxZoom() { return this.transform.maxZoom; }

/**
* Sets or clears the map's minimum pitch.
* If the map's current pitch is lower than the new minimum,
* the map will ease to the new minimum.
*
* @param {number | null | undefined} minPitch The minimum pitch to set (0-60).
* If `null` or `undefined` is provided, the function removes the current minimum pitch (i.e. sets it to 0).
* @returns {Map} `this`
*/
setMinPitch(minPitch?: ?number) {

minPitch = minPitch === null || minPitch === undefined ? defaultMinPitch : minPitch;

if (minPitch < defaultMinPitch) {
throw new Error(`minPitch must be greater than or equal to ${defaultMinPitch}`);
}

if (minPitch >= defaultMinPitch && minPitch <= this.transform.maxPitch) {
this.transform.minPitch = minPitch;
this._update();

if (this.getPitch() < minPitch) this.setPitch(minPitch);

return this;

} else throw new Error(`minPitch must be between ${defaultMinPitch} and the current maxPitch, inclusive`);
}

/**
* Returns the map's minimum allowable pitch.
*
* @returns {number} minPitch
*/
getMinPitch() { return this.transform.minPitch; }

/**
* Sets or clears the map's maximum pitch.
* If the map's current pitch is higher than the new maximum,
* the map will zoom to the new maximum.
*
* @param {number | null | undefined} maxPitch The maximum pitch to set.
* If `null` or `undefined` is provided, the function removes the current maximum pitch (sets it to 60).
* @returns {Map} `this`
*/
setMaxPitch(maxPitch?: ?number) {

maxPitch = maxPitch === null || maxPitch === undefined ? defaultMaxPitch : maxPitch;

if (maxPitch > defaultMaxPitch) {
throw new Error(`maxPitch must be less than or equal to ${defaultMaxPitch}`);
}

if (maxPitch >= this.transform.minPitch) {
this.transform.maxPitch = maxPitch;
this._update();

if (this.getPitch() > maxPitch) this.setPitch(maxPitch);

return this;

} else throw new Error(`maxPitch must be greater than the current minPitch`);
}

/**
* Returns the map's maximum allowable pitch.
*
* @returns {number} maxPitch
*/
getMaxPitch() { return this.transform.maxPitch; }

/**
* Returns the state of `renderWorldCopies`. If `true`, multiple copies of the world will be rendered side by side beyond -180 and 180 degrees longitude. If set to `false`:
* - When the map is zoomed out far enough that a single representation of the world does not fill the map's entire
Expand Down
3 changes: 3 additions & 0 deletions test/unit/geo/transform.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ test('transform', (t) => {
t.equal(transform.worldSize, 512, 'worldSize');
t.equal(transform.width, 500, 'width');
t.equal(transform.minZoom, 0, 'minZoom');
t.equal(transform.minPitch, 0, 'minPitch');
t.equal(transform.bearing, 0, 'bearing');
t.equal(transform.bearing = 1, 1, 'set bearing');
t.equal(transform.bearing, 1, 'bearing');
Expand All @@ -26,6 +27,8 @@ test('transform', (t) => {
t.equal(transform.minZoom, 10);
t.deepEqual(transform.center, {lng: 0, lat: 0});
t.equal(transform.maxZoom, 10);
t.equal(transform.minPitch = 10, 10);
t.equal(transform.minPitch = 10, 10);
t.equal(transform.size.equals(new Point(500, 500)), true);
t.equal(transform.centerPoint.equals(new Point(250, 250)), true);
t.equal(transform.scaleZoom(0), -Infinity);
Expand Down
97 changes: 93 additions & 4 deletions test/unit/ui/map.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -807,14 +807,103 @@ test('Map', (t) => {
t.test('throw on maxZoom smaller than minZoom at init', (t) => {
t.throws(() => {
createMap(t, {minZoom:10, maxZoom:5});
}, new Error(`maxZoom must be greater than minZoom`));
}, new Error(`maxZoom must be greater than or equal to minZoom`));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

part of the unrelated change earlier

t.end();
});

t.test('throw on maxZoom smaller than minZoom at init with falsey maxZoom', (t) => {
t.test('#setMinPitch', (t) => {
const map = createMap(t, {pitch: 20});
map.setMinPitch(10);
map.setPitch(0);
t.equal(map.getPitch(), 10);
t.end();
});

t.test('unset minPitch', (t) => {
const map = createMap(t, {minPitch: 20});
map.setMinPitch(null);
map.setPitch(0);
t.equal(map.getPitch(), 0);
t.end();
});

t.test('#getMinPitch', (t) => {
const map = createMap(t, {pitch: 0});
t.equal(map.getMinPitch(), 0, 'returns default value');
map.setMinPitch(10);
t.equal(map.getMinPitch(), 10, 'returns custom value');
t.end();
});

t.test('ignore minPitchs over maxPitch', (t) => {
const map = createMap(t, {pitch: 0, maxPitch: 10});
t.throws(() => {
map.setMinPitch(20);
});
map.setPitch(0);
t.equal(map.getPitch(), 0);
t.end();
});

t.test('#setMaxPitch', (t) => {
const map = createMap(t, {pitch: 0});
map.setMaxPitch(10);
map.setPitch(20);
t.equal(map.getPitch(), 10);
t.end();
});

t.test('unset maxPitch', (t) => {
const map = createMap(t, {maxPitch:10});
map.setMaxPitch(null);
map.setPitch(20);
t.equal(map.getPitch(), 20);
t.end();
});

t.test('#getMaxPitch', (t) => {
const map = createMap(t, {pitch: 0});
t.equal(map.getMaxPitch(), 60, 'returns default value');
map.setMaxPitch(10);
t.equal(map.getMaxPitch(), 10, 'returns custom value');
t.end();
});

t.test('ignore maxPitchs over minPitch', (t) => {
const map = createMap(t, {minPitch:10});
t.throws(() => {
map.setMaxPitch(0);
});
map.setPitch(10);
t.equal(map.getPitch(), 10);
t.end();
});

t.test('throw on maxPitch smaller than minPitch at init', (t) => {
t.throws(() => {
createMap(t, {minPitch: 10, maxPitch: 5});
}, new Error(`maxPitch must be greater than or equal to minPitch`));
t.end();
});

t.test('throw on maxPitch smaller than minPitch at init with falsey maxPitch', (t) => {
t.throws(() => {
createMap(t, {minPitch: 1, maxPitch: 0});
}, new Error(`maxPitch must be greater than or equal to minPitch`));
t.end();
});

t.test('throw on maxPitch greater than valid maxPitch at init', (t) => {
t.throws(() => {
createMap(t, {maxPitch: 90});
}, new Error(`maxPitch must be less than or equal to 60`));
t.end();
});

t.test('throw on minPitch less than valid minPitch at init', (t) => {
t.throws(() => {
createMap(t, {minZoom:1, maxZoom:0});
}, new Error(`maxZoom must be greater than minZoom`));
createMap(t, {minPitch: -10});
}, new Error(`minPitch must be greater than or equal to 0`));
t.end();
});

Expand Down