Repository navigation
fix(core): let GlobeController zoom past level 12 - #10737
dchaudhari7177 wants to merge 5 commits into
Conversation
Above zoom 12 GlobeView renders with a WebMercatorViewport, which has no getZoomAnchorStrength, so a pointer-anchored wheel zoom threw 'viewport.getZoomAnchorStrength is not a function' and the view could not zoom in any further. Only ask a GlobeViewport for its anchor strength; a WebMercatorViewport's map covers the screen, so anchor at full strength. Fixes visgl#10736
|
| initialViewState: { | ||
| width: 1280, | ||
| height: 720, | ||
| longitude: 10, | ||
| latitude: 45, | ||
| zoom: 12.5, |
There was a problem hiding this comment.
Viewport handoff remains untested
This test begins at zoom 12.5 and zooms further in, so it uses WebMercatorViewport throughout. It does not cover the zoom-12 handoff, where the viewport changes and anchor strength switches from the globe-specific calculation to 1. A pointer jump during the first wheel event that crosses this boundary could therefore escape the regression suite. Please add a case that starts at or below zoom 12, crosses above it, and verifies that the pointer coordinate remains stable.
Knowledge Base Used: Distribution and quality
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Heads-up: #10713, merged on 09-28, reverted #10598. That revert removed the I see two options:
I'm happy to do either. Let me know which you'd prefer. If I don't hear back, I'll close it in a few days rather than leave a stale conflict. |
|
Hey @dchaudhari7177, thanks for working on this and re-evaluating after we reverted #10598. I'm in favor of adding the regression tests you've described and re-purposing this PR for that. Please let me know when the tests are up and I'll review. |
Resolve conflicts with the visgl#10713 revert: the getZoomAnchorStrength call now lives in GlobeController.getZoomPosition, so guard it there.
Adds the zoom-12 handoff case asked for in visgl#10737 review: a wheel gesture that starts at zoom 11.9 on a GlobeViewport and lands above 12 on a WebMercatorViewport keeps the coordinate under the pointer.
Closes #10736
Background
Above zoom 12,
GlobeViewrenders with aWebMercatorViewport. That class has nogetZoomAnchorStrength, so a pointer-anchored wheel zoom threw inGlobeController.applyConstraints, and the view couldn't zoom in any further. Now only aGlobeViewportis asked for its anchor strength. AWebMercatorViewport's map covers the whole screen, so the anchor holds at full strength there.Change List
GlobeController.applyConstraints: checkinstanceof GlobeViewportbefore callinggetZoomAnchorStrengthviewport.getZoomAnchorStrength is not a function.Checked locally:
controllers.spec.ts(86 tests) passes,tsc --noEmit -p modules/corepasses, and prettier is clean. The spec normally runs in theheadlessproject, but I don't have Playwright's Chromium on this machine, so I ran it through thenodeproject from a temporary*.node.spec.tscopy. CI will run it inheadless.Note
Low Risk
Small, targeted change to globe zoom anchor logic with new unit tests; no auth, data, or broad API surface impact.
Overview
Fixes GlobeController pointer-anchored wheel zoom above zoom level 12, where GlobeView switches from
GlobeViewporttoWebMercatorViewport(#10736).getZoomPositionno longer assumes a globe viewport or callsgetZoomAnchorStrengthon every viewport. For aWebMercatorViewportit keeps zooming around the pointer (full-screen map = full anchor strength); globe-only off-globe fallback behavior is unchanged.Adds controller tests for wheel zoom starting above 12.5 and for a single gesture crossing the zoom-12 handoff, asserting zoom increases and the coordinate under the pointer stays stable.
Reviewed by Cursor Bugbot for commit 9a83120. Bugbot is set up for automated code reviews on this repo. Configure here.