Repository navigation
feat(core): asynchronous view resolution - #10865
Pessimistress wants to merge 3 commits into
Conversation
[High impact] The PR appears safe to merge; no new actionable issues were found. SummaryAdds asynchronous view setup and waits for a viewport before updating layers.
|
b7a263e to
782689c
Compare
ibgreen
left a comment
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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.
For #10739
Preparation for introducing math.gl's
projectionEngineChange List
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 keepmakeViewport()returningnulluntil that work completes. ViewManager runsinitialize()for new views, rebuilds viewports viaupdateViewports()when async init finishes, forwards init failures throughonError, 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.viewportmay benull, andsetLayers/updateLayersare deferred until a real viewport is active. When a viewport appears,activateViewportmarks layers with viewport/projection change flags and resumes the pending layer list.Deck wires
onErrorinto ViewManager and, each frame, resolves pending viewports beforeupdateLayers().Reviewed by Cursor Bugbot for commit 782689c. Bugbot is set up for automated code reviews on this repo. Configure here.