minimize scale changes when panning in globe view #11951

ansis · 2022-06-01T23:04:27Z

Attempts to fix #11353 and partially mitigate #11811

The underlying problem is that the zoom is a muddy concept used for all of:

rendering scale (camera zoom)
level-of-detail data loading (tile zoom)
styling (style zoom)

Globe view diverges from mercator on rendering scale inherently but we're still using mercator tiles and styles designed with mercator zoom levels. This leads to conflicting requirements.

This solution compromises. It tries to:

keep rendering scale exactly the same as mercator at z >= 5
keep rendering scale roughly similar to mercator at z < 5
avoid zoom level changes when panning
avoid (minimize) rendering scale changes when panning
keep tile zoom levels as close to the current zoom levels for as many tiles as possible
keep the number of tiles as low as possible (conflicts with previous bullet point)

This tries to find a compromise between all these conflicting goals. The real solution here is to introduce a new scale concept and support that properly. In the mean time, this should be better than what we currently have.

TODO full description

Launch Checklist

mpulkki-mapbox · 2022-06-02T08:53:40Z

This looks great! One bug I noticed is that the camera starts jumping if the terrain has been enabled. This is caused by transform.recenterOnTerrain which should be disabled if the globe view is active. (Btw that function is also a good candidate for future refactoring :) ).

mpulkki-mapbox · 2022-06-02T09:15:58Z

I also noticed an existing "bug" that is more visible now that the reference scale is higher than previously. Draped lines are more blurred around ~1.99 zoom level when approaching from a lower zoom level (e.g. zooming from 0 to 1.99) compared to zooming out (e.g. zooming out from 3 to 1.99).

After a quick investigation I believe this is caused by the function getPixelsToTileUnitsMatrix using transform.zoom for computing the scale which shouldn't be done in draped rendering. While draping the zoom value is either ~1 or ~1.99 depending on direction of the zoom leading to this kind of blurriness. Fixed tile zoom level should be used instead. cc @karimnaaji.

globe_pan_magnification.mp4

src/geo/transform.js

SnailBones · 2022-06-02T17:08:06Z

I'm noticing that around level 4, #11353 is still quite visible. Should we consider continuing this behavior past zoom level 5 or is this outweighed by the issues with styling and tile loading?

zoomies.mov

SnailBones · 2022-06-03T17:17:03Z

I also noticed an existing "bug" that is more visible now that the reference scale is higher than previously. Draped lines are more blurred around ~1.99 zoom level when approaching from a lower zoom level (e.g. zooming from 0 to 1.99) compared to zooming out (e.g. zooming out from 3 to 1.99).

@mpulkki-mapbox Opened a new ticket for discussion of this issue: #11956

ansis · 2022-06-06T14:52:43Z

This is caused by transform.recenterOnTerrain which should be disabled if the globe view is active.

Does 320bf81 look right?

ansis · 2022-06-06T14:59:45Z

I'm noticing that around level 4, #11353 is still quite visible. Should we consider continuing this behavior past zoom level 5 or is this outweighed by the issues with styling and tile loading?

The zoom range is adjusted by const t = getProjectionInterpolationT(this, 1024); to account for change in viewport size. If the viewport is smaller, start earlier. For globe view it's currently limited to not change further past 1024 in order to make sure it's done before the globe-to-mercator transition.

Fixing this visible scaling at z4 for massive maps would mean either:

making the globe-to-mercator transition range dynamic based on viewport size
maintaining scale during panning and changing zoom level here

Both are reasonable options, but I think we may want to put this off for now. (Massive maps also have pretty bad performance...)

SnailBones · 2022-06-06T15:08:47Z

Fixing this visible scaling at z4 for massive maps would mean either:
* making the globe-to-mercator transition range dynamic based on viewport size

* maintaining scale during panning and changing zoom level here

One option to add would be moving the globe-to-mercator transition to a higher zoom per discussion in https://github.com/mapbox/gl-js-team/issues/352.

karimnaaji

LGTM 🟢 but let's hold merging until @mpulkki-mapbox has a chance to review again.

On the topic of gestures, there's a very slight improvement that we could look at: it appears that panning requires a stronger pan distance and has less inertia when done around the poles compared to the equator during the zoom range where the scale adjustment is active.

It's a very subtle issue so it might not be visible to most users, but that's something I noticed when comparing against the older behavior.

mpulkki-mapbox

LGTM! 👍

* adjust scale at low zoom levels in globe view * improve * Adding render test catching missing tiles with minzoom * Allowing lower zoom level tiles toward the poles * Fix lint * Updating render tests to pass * Updating tolerances to fix failing tests * Updating one more tolerance * Fixing query tests * use GLOBE_ZOOM_THRESHOLD_MIN * rename to mercatorScaleRatio * add comment about matching scale * avoid recentering on terrain if rendering globe * update unit tests * lint Co-authored-by: Aidan H <aidan.hendrickson@mapbox.com>

* Add fog render test for empty update parameters (#11900) * Add precision qualifiers to globe_raster fragment shader (#11903) * Add filter features within map view with globe release test (#11888) * add filter features within map view with globe release test * Fix incorrect circle bucket up direction (#11904) * Fix for https://github.com/mapbox/mapbox-gl-native-internal/issues/3426 * Update baselines * ScaleControl: add i18n support (#11850) * ScaleControl: add i18n support * hardcode nautical miles as `nm` for all locales * Add minimum/maximum fields for center/parallels (#11912) * Add new render test for globe antialiasing (#11933) * Fix potential flickering on image source updates (#11928) * Fix flickering of image source on reload * Add unit tests * Fix setStyle breaking with globe view (#11913) * set draping for terrain * use map option for terrain draping check * move to one line * wip: add render test * fixed error with render test * demonstrate issue with mercator * fix background id to update in second setStyle call * bring zoom out to show sphere * Updating expected.png * better readability * fix conditional * fix lint Co-authored-by: Aidan H <aidan.hendrickson@mapbox.com> * Fix tiles not rendering across antimeridian in globe (#11943) * Add video to globe release tests (#11929) * add ozone video on globe to release tests * formatting * formatting * fix lint issues * simplify play/pause button * added atmosphere * Do not render atmosphere when fog isn't supported by the projection (#11947) * Clone stylesheet to allow toggling different styles with `setStyle` (#11942) * clone stylesheet instead of manipulating it directly * style should not contain terrain since it is no longer being overwritten * add unit test * add in default values for terrain when updating * Add test for projections * unit test for terrain exaggeration does not need sources * should have two calls to setStyle to fully test toggling * default value does not need to be added if previous terrain style also used the default value * hasOwnProperty is already a boolean * no need to check currSpec because checks for deep equals and 1 is already set for default * fix unit tests * fix style setting in unit test * accidental deletion * switch to deep clone from extend * better way to find default value * use hasOwnProperty instead * convert to false from undefined * create reusable function for setting transition parameters * update _setTransitionParameters * mix up between duraton/delay values * reduce repetitive code to update style spec and transitions between terrain and fog * revert change * minimize scale changes when panning in globe view (#11951) * adjust scale at low zoom levels in globe view * improve * Adding render test catching missing tiles with minzoom * Allowing lower zoom level tiles toward the poles * Fix lint * Updating render tests to pass * Updating tolerances to fix failing tests * Updating one more tolerance * Fixing query tests * use GLOBE_ZOOM_THRESHOLD_MIN * rename to mercatorScaleRatio * add comment about matching scale * avoid recentering on terrain if rendering globe * update unit tests * lint Co-authored-by: Aidan H <aidan.hendrickson@mapbox.com> * Do not set the map language to the user's preferred language by default (#11952) * Do not set the map language to the user's preferred language by default * fix unit test * cleanup * cleanup * fix worldview setter Co-authored-by: Mikko Pulkki <55925868+mpulkki-mapbox@users.noreply.github.com> Co-authored-by: Anna Peery <42715836+avpeery@users.noreply.github.com> Co-authored-by: Stepan Kuzmin <stepan.kuzmin@mapbox.com> Co-authored-by: Tristen Brown <tristen@users.noreply.github.com> Co-authored-by: Aidan H <aidan.hendrickson@mapbox.com> Co-authored-by: Ansis Brammanis <ansis@mapbox.com>

karimnaaji · 2022-11-28T20:40:48Z

@mpulkki-mapbox @endanke , the behavior adopted here hasn't been propagated in the mobile SDKs, and it has been reported as mismatching between web and native platform by some users (especially for those trying to get a consistent user experience between web and mobile on end customers products). Should we keep track of this change to back-port for consistency between the SDKs? Unless there is any reason we shouldn't adopt the same behavior on mobile.

endanke · 2022-11-29T13:54:16Z

@karimnaaji as I see this PR was ported by Mikko to GL-Native: https://github.com/mapbox/mapbox-gl-native-internal/pull/3574 (This was part of 10.7 so it's worth to check if the customer was using that version.)
Or do you mean that it would require some other changes in the platform SDKs?

karimnaaji · 2022-11-29T16:22:31Z

Thanks for confirming @endanke , it's possibly a mismatching customer version, I'll check and confirm with them.

ansis requested a review from karimnaaji June 1, 2022 23:04

karimnaaji requested a review from mpulkki-mapbox June 2, 2022 00:11

karimnaaji reviewed Jun 2, 2022

View reviewed changes

src/geo/transform.js Outdated Show resolved Hide resolved

src/geo/transform.js Outdated Show resolved Hide resolved

src/geo/transform.js Outdated Show resolved Hide resolved

SnailBones added the skip changelog Used for PRs that do not need a changelog entry label Jun 2, 2022

SnailBones mentioned this pull request Jun 3, 2022

Blur on draped lines inconsistent depending on zoom direction #11956

Closed

ansis and others added 14 commits June 6, 2022 12:40

adjust scale at low zoom levels in globe view

0c8baab

improve

b5f38ee

Adding render test catching missing tiles with minzoom

101e1ec

Allowing lower zoom level tiles toward the poles

c118aca

Fix lint

191697f

Updating render tests to pass

fb93d8d

Updating tolerances to fix failing tests

7594054

Updating one more tolerance

2218fdd

Fixing query tests

8770616

use GLOBE_ZOOM_THRESHOLD_MIN

2681218

rename to mercatorScaleRatio

6249316

add comment about matching scale

d369985

avoid recentering on terrain if rendering globe

0baad51

update unit tests

93d4fe3

ansis force-pushed the ansis/globe-scaling branch from 320bf81 to 93d4fe3 Compare June 6, 2022 18:12

lint

faa44c9

ansis marked this pull request as ready for review June 6, 2022 18:28

avpeery mentioned this pull request Jun 6, 2022

Map with globe flickers when using requestAnimationFrame to slowly zoom in #11875

Open

karimnaaji reviewed Jun 6, 2022

View reviewed changes

mpulkki-mapbox approved these changes Jun 7, 2022

View reviewed changes

karimnaaji approved these changes Jun 7, 2022

View reviewed changes

karimnaaji merged commit 3424632 into main Jun 7, 2022

karimnaaji deleted the ansis/globe-scaling branch June 7, 2022 16:04

karimnaaji mentioned this pull request Jun 7, 2022

Missing tiles on globe when using bathymetry example #11811

Closed

SnailBones mentioned this pull request Jun 29, 2022

API to access the camera parameters when using terrain #12040

Open

SnailBones mentioned this pull request Jul 12, 2022

Revert tile loading changes from #11988 #12075

Merged

7 tasks

karimnaaji mentioned this pull request Sep 6, 2022

Add support for cameraForBounds on globe projection #12138

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

minimize scale changes when panning in globe view #11951

minimize scale changes when panning in globe view #11951

ansis commented Jun 1, 2022

mpulkki-mapbox commented Jun 2, 2022

mpulkki-mapbox commented Jun 2, 2022

SnailBones commented Jun 2, 2022

SnailBones commented Jun 3, 2022

ansis commented Jun 6, 2022

ansis commented Jun 6, 2022

SnailBones commented Jun 6, 2022

karimnaaji left a comment

mpulkki-mapbox left a comment

karimnaaji commented Nov 28, 2022

endanke commented Nov 29, 2022

karimnaaji commented Nov 29, 2022

minimize scale changes when panning in globe view #11951

minimize scale changes when panning in globe view #11951

Conversation

ansis commented Jun 1, 2022

Launch Checklist

mpulkki-mapbox commented Jun 2, 2022

mpulkki-mapbox commented Jun 2, 2022

SnailBones commented Jun 2, 2022

SnailBones commented Jun 3, 2022

ansis commented Jun 6, 2022

ansis commented Jun 6, 2022

SnailBones commented Jun 6, 2022

karimnaaji left a comment

Choose a reason for hiding this comment

mpulkki-mapbox left a comment

Choose a reason for hiding this comment

karimnaaji commented Nov 28, 2022

endanke commented Nov 29, 2022

karimnaaji commented Nov 29, 2022