Skip to content

fix(core): compare viewport projection state in equals - #10611

Merged
chrisgervang merged 5 commits into
masterfrom
codex/viewport-equality-projection-state
Aug 30, 2026
Merged

fix(core): compare viewport projection state in equals#10611
chrisgervang merged 5 commits into
masterfrom
codex/viewport-equality-projection-state

Conversation

@chrisgervang

@chrisgervang chrisgervang commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Problem

Viewport#equals is a cache-invalidation gate. Layer.activateViewport uses it to set viewportChanged, 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 true for viewports that project or tessellate the same input differently:

  • Changing Globe resolution changes path and polygon subdivision without changing the camera matrices. A resolution: 30 → 10 update could retain geometry generated for the old resolution.
  • Changing one axis of an OrthographicViewport changes distanceScales.unitsPerMeter. In the regression fixture, zoomX: 0 → 1 changes the common-space length of a [3, 4] segment from 5 to sqrt(52), but the old equality check suppressed the update.
  • Toggling Web Mercator legacyMeterSizes changes altitude projection and the pseudoMeters shader uniform while leaving camera and dimensions unchanged.

In each case, consumers that trusted Viewport#equals could retain stale tessellation, projected attributes, or shader inputs.

Changes

  • Compare projectionMode and Globe resolution.
  • Compare distanceScales.unitsPerMeter so anisotropic Orthographic scale changes are detected.
  • Compare Web Mercator's internal legacy-meter projection mode in a WebMercatorViewport#equals override while keeping _pseudoMeters private and out of the base viewport contract.
  • Add equality regressions for Globe resolution, Orthographic zoomX, and legacyMeterSizes while 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.
  • Commit-hook affected tests — 15/15 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#equals now treats viewports as different when projection-related state changes, not only when size, scalar scale, and camera matrices match. The base check adds projectionMode, resolution (Globe tessellation), and distanceScales.unitsPerMeter (e.g. anisotropic Orthographic zoomX), and drops the old distance-scales TODO.

WebMercatorViewport#equals extends 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 so equals correctly drives viewportChanged / 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.

@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown

Greptile Summary

The PR expands viewport equality to include projection-dependent state so cache invalidation occurs when projection semantics change.

  • Compares projection mode, globe resolution, and per-axis meter scales in the base viewport.
  • Compares Web Mercator legacy pseudo-meter mode in the subclass override.
  • Adds regression coverage for globe, orthographic, and Web Mercator projection settings.

Confidence Score: 5/5

The 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.

Important Files Changed

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

Comment thread modules/core/src/viewports/viewport.ts
@coveralls

coveralls commented Aug 30, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 82.241% (+0.005%) from 82.236% — codex/viewport-equality-projection-state into master

@chrisgervang chrisgervang added this to the v9.4 milestone Aug 30, 2026
@chrisgervang

Copy link
Copy Markdown
Collaborator Author

Cache-invalidation risk audit

I 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.

Scenario Comparisons Invalidations before Invalidations after Additional
Equivalent MapViewport reconstruction 99 0 0 0
Web Mercator latitude pan 99 99 99 0
Web Mercator continuous zoom 99 99 99 0
Web Mercator zoom-12 crossing 99 99 99 0
Globe longitude pan 99 99 99 0
Orthographic uniform zoom 99 99 99 0
Globe resolution animation 99 0 99 99
Orthographic X-only zoom while Y fixes scalar scale 99 0 99 99
Web Mercator legacy-meter mode toggle 1 0 1 1

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:

  • Animating Globe resolution now invalidates on every value change. Resolution is normally static view configuration; applications that animate it should expect resolution-aware geometry to rebuild.
  • Independent-axis Orthographic zoom now invalidates when the other axis keeps the scalar camera scale fixed. This can refresh viewport-dependent aggregation/effect work every animation frame, but the previous cache hit retained output projected with the wrong axis scale.

Integrated browser checks:

  • Globe PathLayer, resolution: 10 -> 5: exactly one tessellation rebuild; instances changed from 450 to 882.
  • Reconstructing the same resolution: zero rebuilds.
  • Changing camera state with unchanged resolution: zero tessellation rebuilds.
  • A 2,000-point ScreenGridLayer under 20 X-only Orthographic zoom changes received 20 viewport-dependent updates; ten equivalent viewport reconstructions received zero.
  • The Globe example rendered with nine layers and no page or console errors.

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 legacyMeterSizes are configuration changes rather than ordinary controller state, so this should be rare.

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.

@chrisgervang
chrisgervang merged commit 9aaab5b into master Aug 30, 2026
8 checks passed
@chrisgervang
chrisgervang deleted the codex/viewport-equality-projection-state branch August 30, 2026 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants