fix(core): compare viewport projection state in equals - #10611
Conversation
Greptile SummaryThe PR expands viewport equality to include projection-dependent state so cache invalidation occurs when projection semantics change.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported pseudo-meter equality issue is addressed by the WebMercatorViewport equality override and regression coverage.
|
| Filename | Overview |
|---|---|
| modules/core/src/viewports/viewport.ts | Extends base viewport equality with projection mode, resolution, and anisotropic distance-scale comparisons. |
| modules/core/src/viewports/web-mercator-viewport.ts | Adds Web Mercator-specific equality for the legacy pseudo-meter projection setting. |
| test/modules/core/viewports/viewport.spec.ts | Adds regression tests covering the newly compared projection state. |
Reviews (5): Last reviewed commit: "Merge branch 'master' into codex/viewpor..." | Re-trigger Greptile
Cache-invalidation risk auditI evaluated the broader invalidation risk by comparing the pre-PR equality fields with this PR across 100-step viewport sequences, then exercised representative consumers in the synchronized PathStyle branch.
The common interactions—MapView pan/zoom, Globe camera movement, uniform Orthographic zoom, and the zoom-12 projection-mode transition—already invalidated through scale or camera matrices. This PR does not increase their invalidation frequency. Equivalent viewport reconstruction also remains cacheable. The two potentially high-frequency additions are intentional semantic changes:
Integrated browser checks:
Other observable behavior is conservative but bounded: a visible TooltipWidget is cleared when these projection semantics change because its prior hover result may no longer correspond to the same screen position. Globe resolution and Conclusion: the blast radius justifies the automated Medium Risk label, but the audit found no added churn in ordinary map navigation. The new repeated work is limited to settings that genuinely change tessellation or common-space projection. No code changes were made as part of this audit. |
Problem
Viewport#equalsis a cache-invalidation gate.Layer.activateViewportuses it to setviewportChanged, and tilesets, terrain, masks, collision filtering, tooltips, and widgets also use it to decide whether projection-dependent state must be refreshed.The old comparison covered dimensions, scalar scale, and view/projection matrices, but not all state that controls common-space geometry. It could therefore return
truefor viewports that project or tessellate the same input differently:resolutionchanges path and polygon subdivision without changing the camera matrices. Aresolution: 30 → 10update could retain geometry generated for the old resolution.OrthographicViewportchangesdistanceScales.unitsPerMeter. In the regression fixture,zoomX: 0 → 1changes the common-space length of a[3, 4]segment from5tosqrt(52), but the old equality check suppressed the update.legacyMeterSizeschanges altitude projection and thepseudoMetersshader uniform while leaving camera and dimensions unchanged.In each case, consumers that trusted
Viewport#equalscould retain stale tessellation, projected attributes, or shader inputs.Changes
projectionModeand Globeresolution.distanceScales.unitsPerMeterso anisotropic Orthographic scale changes are detected.WebMercatorViewport#equalsoverride while keeping_pseudoMetersprivate and out of the base viewport contract.zoomX, andlegacyMeterSizeswhile preserving equality for matching viewports.Performance
No draw-time benchmark applies. The additional scalar/three-component comparisons run only when viewport state is compared. They cause extra invalidation only when projection semantics actually differ.
Validation
yarn vitest run --project headless test/modules/core/viewports/viewport.spec.ts— 6/6 passed.yarn lint— passed.git diff --check— passed.Note
Medium Risk
Changes when layers and widgets invalidate projection-dependent caches; fixes stale-state bugs but may increase refresh work when projection semantics differ while matrices look the same.
Overview
Viewport#equalsnow treats viewports as different when projection-related state changes, not only when size, scalar scale, and camera matrices match. The base check addsprojectionMode,resolution(Globe tessellation), anddistanceScales.unitsPerMeter(e.g. anisotropic OrthographiczoomX), and drops the old distance-scales TODO.WebMercatorViewport#equalsextends that with_pseudoMeters/legacyMeterSizes, so altitude projection and shader inputs stay in sync without exposing that flag on the base viewport API.Tests cover Globe resolution, Orthographic
zoomX, and legacy vs current meter sizing soequalscorrectly drivesviewportChanged/ refresh paths (layers, tooltips, view manager) instead of keeping stale geometry or uniforms.Reviewed by Cursor Bugbot for commit 33de218. Bugbot is set up for automated code reviews on this repo. Configure here.