From b61bb032ad450cba648c6af7f7c70b481ef229a8 Mon Sep 17 00:00:00 2001 From: Nathan Olson Date: Thu, 20 Feb 2025 10:13:47 -0800 Subject: [PATCH] Fix 5486 (#5518) * add failing unit test * fix 5486. Do not allow variable zoom tiles just because the top padding is non-zero. Only allow variable zoom at high pitch or when terrain is enabled. * update changelog --- CHANGELOG.md | 1 + src/geo/projection/covering_tiles.test.ts | 16 ++++++++++++++++ .../mercator_covering_tiles_details_provider.ts | 2 +- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4befde27435..b2195549f00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - _...Add new stuff here..._ - Fix how padding is applied when using flyTo() with Globe ([#5406](https://github.com/maplibre/maplibre-gl-js/pull/5406)) - Fix URL hash validation to support bearing range -180 to 180 ([#5461](https://github.com/maplibre/maplibre-gl-js/issues/5461)) +- Fix variable zoom tile calculation when padding is set ([#5486](https://github.com/maplibre/maplibre-gl-js/issues/5486)) ## 5.1.0 diff --git a/src/geo/projection/covering_tiles.test.ts b/src/geo/projection/covering_tiles.test.ts index 182b5948a93..203c36d12ab 100644 --- a/src/geo/projection/covering_tiles.test.ts +++ b/src/geo/projection/covering_tiles.test.ts @@ -219,6 +219,22 @@ describe('coveringTiles', () => { new OverscaledTileID(10, 0, 10, 511, 512), new OverscaledTileID(10, 0, 10, 512, 512)]); + transform.resize(2048, 128); + transform.setZoom(9); + transform.setPadding({top: 16}); + expect(coveringTiles(transform, options)).toEqual([ + new OverscaledTileID(9, 0, 9, 255, 255), + new OverscaledTileID(9, 0, 9, 256, 255), + new OverscaledTileID(9, 0, 9, 255, 256), + new OverscaledTileID(9, 0, 9, 256, 256), + new OverscaledTileID(9, 0, 9, 254, 255), + new OverscaledTileID(9, 0, 9, 254, 256), + new OverscaledTileID(9, 0, 9, 257, 255), + new OverscaledTileID(9, 0, 9, 257, 256), + new OverscaledTileID(9, 0, 9, 253, 255), + new OverscaledTileID(9, 0, 9, 253, 256)]); + + transform.setPadding({top: 0}); transform.setZoom(5.1); transform.setPitch(60.0); transform.setBearing(32.0); diff --git a/src/geo/projection/mercator_covering_tiles_details_provider.ts b/src/geo/projection/mercator_covering_tiles_details_provider.ts index e5a9d4da6f0..06edf7133ae 100644 --- a/src/geo/projection/mercator_covering_tiles_details_provider.ts +++ b/src/geo/projection/mercator_covering_tiles_details_provider.ts @@ -42,7 +42,7 @@ export class MercatorCoveringTilesDetailsProvider implements CoveringTilesDetail allowVariableZoom(transform: IReadonlyTransform, options: CoveringTilesOptions): boolean { const zfov = transform.fov * (Math.abs(Math.cos(transform.rollInRadians)) * transform.height + Math.abs(Math.sin(transform.rollInRadians)) * transform.width) / transform.height; const maxConstantZoomPitch = clamp(78.5 - zfov / 2, 0.0, 60.0); - return (!!options.terrain || transform.pitch > maxConstantZoomPitch || transform.padding.top >= 0.1); + return (!!options.terrain || transform.pitch > maxConstantZoomPitch); } allowWorldCopies(): boolean {