Skip to content

Commit f813651

Browse files
authored
Add constrain parameter to ITransform.apply(); constrain when migrating projections (#6917)
* Draft fix #6892: Add constrain to ITransform.apply * Test map.setProjection() constrains zoom, center * Add CHANGELOG entry * Remove constrain param camera.migrateProjection() Always call constrain=true on newTransform.apply() * Require `constrain` param for `ITransform.apply()` * Add @param tags to `ITransform.apply()`
1 parent 5118ea1 commit f813651

File tree

11 files changed

+37
-21
lines changed

11 files changed

+37
-21
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
- Fix setting visibility on custom layer ([#6883](https://github.com/maplibre/maplibre-gl-js/issues/6883) (by [melitele](https://github.com/melitele))
1010
- Hide leading and trailing control characters in `format` expressions ([#6907](https://github.com/maplibre/maplibre-gl-js/pull/6907)) (by [1ec5](https://github.com/1ec5))
1111
- Fix image sources being clipped at -180 and 180 longitude when terrain is enabled ([#4088](https://github.com/maplibre/maplibre-gl-js/issues/4088)) (by [pstaszek](https://github.com/pstaszek))
12+
- Fix map not immediately constraining to a valid zoom and center when changing projections ([#6892](https://github.com/maplibre/maplibre-gl-js/issues/6892)) (by [larsmaxfield](https://github.com/larsmaxfield))
1213
- _...Add new stuff here..._
1314

1415
## 5.15.0

src/geo/projection/globe_transform.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ describe('GlobeTransform', () => {
598598
const globeTransform = createGlobeTransform();
599599
globeTransform.setRenderWorldCopies(true);
600600
const mercator = new MercatorTransform({minZoom: 0, maxZoom: 1, minPitch: 2, maxPitch: 3, renderWorldCopies: false});
601-
mercator.apply(globeTransform);
601+
mercator.apply(globeTransform, false);
602602

603603
expect(mercator.renderWorldCopies).toBeTruthy();
604604
});

src/geo/projection/globe_transform.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -267,14 +267,14 @@ export class GlobeTransform implements ITransform {
267267
const clone = new GlobeTransform();
268268
clone._globeness = this._globeness;
269269
clone._globeLatitudeErrorCorrectionRadians = this._globeLatitudeErrorCorrectionRadians;
270-
clone.apply(this);
270+
clone.apply(this, false);
271271
return clone;
272272
}
273273

274-
public apply(that: IReadonlyTransform): void {
275-
this._helper.apply(that);
276-
this._mercatorTransform.apply(this);
277-
this._verticalPerspectiveTransform.apply(this, this._globeLatitudeErrorCorrectionRadians);
274+
public apply(that: IReadonlyTransform, constrain: boolean): void {
275+
this._helper.apply(that, constrain);
276+
this._mercatorTransform.apply(this, false);
277+
this._verticalPerspectiveTransform.apply(this, false, this._globeLatitudeErrorCorrectionRadians);
278278
}
279279

280280
public get projectionMatrix(): mat4 { return this.currentTransform.projectionMatrix; }
@@ -332,7 +332,7 @@ export class GlobeTransform implements ITransform {
332332
// - if autoCalculateNearFarZ is true then it computes globe Z values
333333
// - if autoCalculateNearFarZ is false then it inherits our Z values
334334
// In either case, its Z values are consistent with out settings and we want to copy its Z values to our helper.
335-
this._verticalPerspectiveTransform.apply(this, this._globeLatitudeErrorCorrectionRadians);
335+
this._verticalPerspectiveTransform.apply(this, false, this._globeLatitudeErrorCorrectionRadians);
336336
this._helper._nearZ = this._verticalPerspectiveTransform.nearZ;
337337
this._helper._farZ = this._verticalPerspectiveTransform.farZ;
338338

@@ -418,11 +418,11 @@ export class GlobeTransform implements ITransform {
418418
setLocationAtPoint(lnglat: LngLat, point: Point): void {
419419
if (!this.isGlobeRendering) {
420420
this._mercatorTransform.setLocationAtPoint(lnglat, point);
421-
this.apply(this._mercatorTransform);
421+
this.apply(this._mercatorTransform, false);
422422
return;
423423
}
424424
this._verticalPerspectiveTransform.setLocationAtPoint(lnglat, point);
425-
this.apply(this._verticalPerspectiveTransform);
425+
this.apply(this._verticalPerspectiveTransform, false);
426426
return;
427427
}
428428

src/geo/projection/mercator_transform.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,11 @@ export class MercatorTransform implements ITransform {
253253

254254
public clone(): ITransform {
255255
const clone = new MercatorTransform();
256-
clone.apply(this);
256+
clone.apply(this, false);
257257
return clone;
258258
}
259259

260-
public apply(that: IReadonlyTransform, constrain?: boolean, forceOverrideZ?: boolean): void {
260+
public apply(that: IReadonlyTransform, constrain: boolean, forceOverrideZ?: boolean): void {
261261
this._helper.apply(that, constrain, forceOverrideZ);
262262
}
263263

src/geo/projection/vertical_perspective_transform.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,13 +268,13 @@ export class VerticalPerspectiveTransform implements ITransform {
268268

269269
clone(): ITransform {
270270
const clone = new VerticalPerspectiveTransform();
271-
clone.apply(this);
271+
clone.apply(this, false);
272272
return clone;
273273
}
274274

275-
public apply(that: IReadonlyTransform, globeLatitudeErrorCorrectionRadians?: number): void {
275+
public apply(that: IReadonlyTransform, constrain: boolean, globeLatitudeErrorCorrectionRadians?: number): void {
276276
this._globeLatitudeErrorCorrectionRadians = globeLatitudeErrorCorrectionRadians || 0;
277-
this._helper.apply(that);
277+
this._helper.apply(that, constrain);
278278
}
279279

280280
public get projectionMatrix(): mat4 { return this._projectionMatrix; }

src/geo/transform_helper.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe('TransformHelper', () => {
3939
original.setZoom(2.3);
4040

4141
const cloned = new TransformHelper(emptyCallbacks);
42-
cloned.apply(original);
42+
cloned.apply(original, false);
4343

4444
// Check all getters from the ITransformGetters interface
4545
expect(cloned.constrainOverride).toEqual(original.constrainOverride);

src/geo/transform_helper.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ export class TransformHelper implements ITransformGetters {
184184
this._autoCalculateNearFarZ = true;
185185
}
186186

187-
public apply(thatI: ITransformGetters, constrain?: boolean, forceOverrideZ?: boolean): void {
187+
public apply(thatI: ITransformGetters, constrain: boolean, forceOverrideZ?: boolean): void {
188188
this._constrainOverride = thatI.constrainOverride;
189189
this._latRange = thatI.latRange;
190190
this._lngRange = thatI.lngRange;

src/geo/transform_interface.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,12 @@ export interface ITransformGetters {
106106
interface ITransformMutators {
107107
clone(): ITransform;
108108

109-
apply(that: IReadonlyTransform): void;
109+
/**
110+
* Applies a transform to the current transform.
111+
* @param that - The transform to apply to the current transform.
112+
* @param constrain - Whether to constrain the transform's center and zoom and recompute internal matricies once applied.
113+
*/
114+
apply(that: IReadonlyTransform, constrain: boolean): void;
110115

111116
/**
112117
* Sets the transform's minimal allowed zoom level.

src/ui/camera.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ export abstract class Camera extends Evented {
346346
* When the style's projection is changed (or first set), this function should be called.
347347
*/
348348
migrateProjection(newTransform: ITransform, newCameraHelper: ICameraHelper) {
349-
newTransform.apply(this.transform);
349+
newTransform.apply(this.transform, true);
350350
this.transform = newTransform;
351351
this.cameraHelper = newCameraHelper;
352352
}
@@ -1293,9 +1293,9 @@ export abstract class Camera extends Evented {
12931293
if (roll !== undefined) nextTransform.setRoll(roll);
12941294
if (pitch !== undefined) nextTransform.setPitch(pitch);
12951295
if (bearing !== undefined) nextTransform.setBearing(bearing);
1296-
finalTransform.apply(nextTransform);
1296+
finalTransform.apply(nextTransform, false);
12971297
}
1298-
this.transform.apply(finalTransform);
1298+
this.transform.apply(finalTransform, false);
12991299
}
13001300

13011301
_fireMoveEvents(eventData?: any) {

src/ui/map_tests/map_transform_constrains.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,16 @@ test('Creating a map with style defining globe projection uses Globe transform c
8484
expect(fixedNum(map.getZoom(), 3)).toBe(-2);
8585
});
8686

87+
test('Changing a map projection to Mercator constrains the zoom and center to valid values', async () => {
88+
const map = createMap( {zoom: -2, center: [0, 90], style: {version: 8, sources: {}, layers: [], projection: {type: 'globe'}}});
89+
90+
await map.once('style.load');
91+
map.setProjection({type: 'mercator'});
92+
93+
expect(fixedNum(map.getZoom(), 3)).toBe(-1.356);
94+
expect(fixedLngLat(map.getCenter(), 4)).toEqual({lng: 0, lat: 0});
95+
});
96+
8797
describe('transformConstrain', () => {
8898

8999
test('Creating a single-copy map with an identity transform constrain allows the map to underzoom and overpan', () => {

0 commit comments

Comments
 (0)