Skip to content

feat(core): asynchronous view resolution - #10865

Open
Pessimistress wants to merge 3 commits into
masterfrom
codex/async-view-resolution
Open

Pessimistress wants to merge 3 commits into
masterfrom
codex/async-view-resolution

Conversation

@Pessimistress

@Pessimistress Pessimistress commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

For #10739

Preparation for introducing math.gl's projectionEngine

Change List

  • Add optional View.initialize that resolves asynchronously
  • LayerManager defers layer updates when context.viewport is unavailable
  • Unit tests

Note

Medium Risk
Changes core view/layer initialization ordering and when layer lifecycle runs; regressions could affect startup, view swaps, and projection-dependent layers, though behavior is covered by new tests.

Overview
Adds asynchronous view resolution so Deck can wait for views to finish loading before layers use projection context—prep for async projection engines (e.g. math.gl projectionEngine).

Views gain an optional initialize() hook; subclasses can keep makeViewport() returning null until that work completes. ViewManager runs initialize() for new views, rebuilds viewports via updateViewports() when async init finishes, forwards init failures through onError, and only attaches controllers once a viewport exists. Equal view descriptors reuse the previous instance so in-flight loading is not reset on React re-renders.

LayerManager no longer fabricates a default viewport: context.viewport may be null, and setLayers / updateLayers are deferred until a real viewport is active. When a viewport appears, activateViewport marks layers with viewport/projection change flags and resumes the pending layer list.

Deck wires onError into ViewManager and, each frame, resolves pending viewports before updateLayers().

Reviewed by Cursor Bugbot for commit 782689c. Bugbot is set up for automated code reviews on this repo. Configure here.

@Pessimistress
Pessimistress requested a review from ibgreen October 13, 2026 20:57
@greptile-apps

greptile-apps Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Adds asynchronous view setup and waits for a viewport before updating layers.

  • Views can finish setup before they create a viewport.
  • Layers wait for a real viewport before starting lifecycle work.

Diagram

sequenceDiagram
  participant View
  participant VM as ViewManager
  participant Deck
  participant LM as LayerManager
  VM->>View: initialize()
  VM->>View: makeViewport()
  View-->>VM: null while pending
  Deck->>LM: activateViewport(undefined)
  Note over LM: Keep pending layers without updating them
  View-->>VM: Initialization finishes
  VM->>VM: setNeedsUpdate()
  Deck->>VM: updateViewports()
  VM->>View: makeViewport()
  View-->>VM: Ready viewport
  Deck->>LM: activateViewport(viewport)
  Deck->>LM: updateLayers()
Loading

Reviews (4) · Last reviewed commit: "address comments" · Reviewed by Greptile

Comment thread modules/core/src/lib/layer-manager.ts
Comment thread modules/core/src/lib/view-manager.ts Outdated
Comment thread modules/core/src/views/view.ts
@coveralls

coveralls commented Oct 13, 2026 •

Copy link
Copy Markdown

Coverage Status

coverage: 83.728% (+0.03%) from 83.703% — codex/async-view-resolution into master

@Pessimistress
Pessimistress force-pushed the codex/async-view-resolution branch from b7a263e to 782689c Compare October 15, 2026 17:37

@ibgreen ibgreen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The async viewport lifecycle changes look good overall. I found one missed group of callers that should be updated before merge.

setLayers(newLayers: LayersList, reason?: string): void {
debug(TRACE_SET_LAYERS, this, reason, newLayers);

if (!this.context.viewport) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new guard leaves getLayers() empty until a viewport is activated and updateLayers() runs. A couple of existing callers were missed: test/modules/extensions/mask/mask-pass.spec.ts constructs LayerManager(device, {}), calls setLayers(), and immediately passes getLayers() to MaskPass.render, so it now renders zero layers; test/apps/attribute-worker/worker.js never activates a viewport, so snapshot generation returns undefined. Please give these callers an initial viewport or otherwise preserve their viewport-independent lifecycle path.

This branch has not been deployed

No deployments
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