Skip to content

Comments

fix(mapbox): automatically inject 'mapbox' view in overlaid mode for multi-view consistency#9947

Open
chrisgervang wants to merge 6 commits intomasterfrom
mapbox-overlay-view-injection
Open

fix(mapbox): automatically inject 'mapbox' view in overlaid mode for multi-view consistency#9947
chrisgervang wants to merge 6 commits intomasterfrom
mapbox-overlay-view-injection

Conversation

@chrisgervang
Copy link
Collaborator

@chrisgervang chrisgervang commented Jan 20, 2026

Summary

Fixes an inconsistency in MapboxOverlay where custom views behave differently in interleaved vs overlaid modes.

Problem

When implementing multi-view examples in #9946, we discovered that when interleaved=false (overlaid mode), the multi-view example did not render airports/arcs layers because there was no 'mapbox' view for them to render in. In interleaved=true mode, the same example worked fine.

Root cause: In interleaved mode, getViewport() in deck-utils.ts automatically falls back to creating a 'mapbox' view when it's not found in custom views. However, in overlaid mode, custom views were used as-is without this fallback, creating inconsistent behavior.

Solution

Added a _getViews() helper method in MapboxOverlay that checks if custom views include a view with id 'mapbox'. If not, it automatically injects the default 'mapbox' view, matching the interleaved mode behavior.

This allows users to only specify additional views (like minimap, ortho) without having to manually include the base map view.

I kept the fallback within getViewport() as a defensive measure.. it shouldn't be needed anymore, but I'm leaving this cleanup for another PR for the sake of keeping this one focused.

Test Plan

  • All tests passing. New tests:
    1. AutoInjectMapboxView - No custom views initially, then tests setProps with custom views
    2. AutoInjectMapboxViewWithCustomViews - Custom views provided at construction time
    3. NoDuplicateWhenMapboxViewProvided - Explicit 'mapbox' view provided at construction time
  • Verified multi-view examples from feat(basemap-browser): add batched rendering toggle and multi-view examples #9946 now work in both interleaved and overlaid modes

Note

Medium Risk
Touches view selection and update paths (setProps, render loop, and style/projection updates), which could affect rendering order or multi-view setups; changes are localized and backed by new tests.

Overview
Fixes multi-view inconsistencies in MapboxOverlay by always ensuring a base-map-synchronized view exists: when custom views are provided without an id of "mapbox", a default MapView/GlobeView with id "mapbox" is auto-injected (including on setProps, render updates, and projection/style changes).

Adds tests to verify auto-injection, setProps behavior, and no-duplicate behavior when an explicit "mapbox" view is supplied, and updates docs to clearly document the "mapbox" view-id convention for layerFilter and custom view ordering.

Written by Cursor Bugbot for commit 9615b35. This will update automatically on new commits. Configure here.

…multi-view consistency

When custom views are provided without a view with id 'mapbox', MapboxOverlay
now automatically injects the default 'mapbox' view in overlaid mode, matching
the behavior in interleaved mode (where getViewport() already does this fallback).

This ensures consistent behavior between interleaved and overlaid modes, and
allows users to only specify additional views (like minimap, ortho) without
having to manually include the base map view.

The bug was discovered while implementing multi-view examples in #9946. When
interleaved was false (overlaid mode), the multi-view example did not render
airports/arcs layers because there was no 'mapbox' view for them to render in.
In interleaved mode, the same example worked because getViewport() in deck-utils
falls back to creating a 'mapbox' view when not found in custom views.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
cursor[bot]

This comment was marked as outdated.

@coveralls
Copy link

coveralls commented Jan 20, 2026

Coverage Status

coverage: 91.091% (+0.002%) from 91.089%
when pulling 9615b35 on mapbox-overlay-view-injection
into 6149b4c on master.

…ction

- Improved multi-view documentation to explain automatic 'mapbox' view injection
- Added 6 test cases covering view injection in both overlaid and interleaved modes
- Tests verify that 'mapbox' view is auto-injected when not provided and that no duplicates are created when explicitly provided

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Comment on lines +277 to +279
// Check if custom views include a view with id 'mapbox'
const views = Array.isArray(this._props.views) ? this._props.views : [this._props.views];
const hasMapboxView = views.some((v: any) => v.id === 'mapbox');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To use #9948

chrisgervang and others added 2 commits January 20, 2026 15:19
- Updated setProps() to call _getViews() for both overlaid and interleaved modes
- Added _getViews() call in _onAddInterleaved() to inject 'mapbox' view at initialization
- Updated _handleStyleChange() to always update views on projection change, even with custom views
- Extended AutoInjectMapboxView tests to verify setProps maintains view injection in both modes

This ensures both overlaid and interleaved modes consistently keep user props pure while
dynamically computing views at all usage points, preventing view inconsistencies when calling
setProps() or when map projection changes.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The interleavedAutoInjectMapboxView test was hanging because it called setProps()
inside a render callback without waiting for async operations to complete. Added
async/await pattern to match other interleaved tests.
@chrisgervang chrisgervang marked this pull request as draft January 20, 2026 23:50
@chrisgervang
Copy link
Collaborator Author

chrisgervang commented Jan 21, 2026

I'm considering a few paths here..

  1. inject the mapbox view into views when it isn't present in every usage at runtime in MapboxOverlay.
    Pros: this is the pattern we do for parameters and keeps the data flow uni-directional. It's predictable.
    Cons: need to maintain coverage on all setProps usages

  2. (current solution) somehow insert the view internally in getViewport in the overlaid case. We can currently do this in interleaved since
    Pros: Works for interleaved since it calls deck._drawLayers directly
    Cons: Overlaid does call deck._drawLayers currently since all drawing is handled internally. Overlaid would have to be done using a different hook or diverge from how interleaved works

  3. Always require the user to provide a MapView({id: 'mapbox'}) or GlobeView({id: 'mapbox'}) when they're supplying the views prop
    Pros: simplier internals
    Cons: the user is responsible for understanding internal workings and interactions between maplibre and deck.. there also isn't much benefit to them because they

With that said, it is still possible to take advantage of deck's multi-view system and render a mapbox base map onto any one `MapView` of your choice by setting the `views` array and a `layerFilter` callback.

- To use multiple views, define a `MapView` with the id `“mapbox”`. This view will receive the state that matches the base map at each render.
- If views are provided but the array does not contain this id, then a `MapView({id: 'mapbox'})` will be inserted at the bottom of the stack.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For context, this is what is not true for overlaid mode that this PR aims to fix

… pollution

Reduced overlaid mode tests to a single test due to WebGL context state
pollution when running multiple overlaid tests sequentially. Comprehensive
coverage for setProps, custom views, and duplicate prevention is maintained
through interleaved mode tests which don't exhibit the same issues.

See #9950 for details on the
underlying WebGL context cleanup issue.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@chrisgervang chrisgervang marked this pull request as ready for review January 21, 2026 22:33
@chrisgervang chrisgervang added this to the v9.3 milestone Feb 2, 2026
@chrisgervang chrisgervang mentioned this pull request Feb 3, 2026
12 tasks
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