fix(mapbox): In interleaved, batched mode don't render layers twice

[zbigg]

Followup of #9939

Background

#9939 introduced experimental "batched" rendering mode forMapboxOverlay, but it's broken as we missed that we have second render hook that now renders everything second time.

Change List

• Fix double rendering of layers in MapboxOverlay in interlaved mode when _renderLayersInGroups is set

---

Note

Eliminates duplicate rendering in interleaved/batched mode by refining external render behavior.

• Simplifies afterRender to only redraw when non-Mapbox views exist and replaces the layer filter to exclude MAPBOX_VIEW_ID rather than checking mapboxLayers
• Removes mapboxLayers-based non-Mapbox layer detection to avoid rendering Mapbox layers twice

^{Written by Cursor Bugbot for commit 2f4c199. This will update automatically on new commits. Configure here.}

[coveralls]

coverage: 91.114% (-0.001%) from 91.115%
when pulling 2f4c199 on zbigg/fix-mapboxoverlay-double-render-in-groups
into 83ac412 on master.

[zbigg]

Seems like MapboxOverlay has many modes of operations and afterRender does following:

• checks if there are layers that are not "rendered by MapboxLayer"
• or some non-mapbox viewports

Now, since deprecation of MapboxLayer API (it's private since v9 as i understand) at least in interleaved mode (batched or not), all layers are always managed internally by resolveLayers and always rendered in "main render pass managed by maplibre", so actually feels like hasNonMapboxLayers is not necessary.

I looked very briefly and it looks like afterRender isn't used at all in overlaid mode.

Am i right, that whole hasNonMapboxLayers cannot be effectively true?

[chrisgervang]

Hmm, maybe.. what about a multi-view case where you have an minimap or an orthographic view on top of the main map with layers that only appear in those views?

Are those layer render passes still "managed" by this overlay?

[chrisgervang]

If all layers are in userData.mapboxLayerIds then I think this can be simplified like you've proposed.

From a user perspective, they are always providing all layers to the overlay's layers props.

[zbigg]

maybe.. what about a multi-view case

I have no experience with multiview, but this patch should multiview behavior.
If there are other views, all layers will be rendered there.

I am only removing case for layers in mapbox view (the main, the default one embedded in mapbox/libre map) ...

If all layers are in userData.mapboxLayerIds ...

Yes, this is what i believe. We cannot add MapboxLayer to "deck" in any other way than by passing normal deck Layer instance through overlay.setProps({layers: ....}) - uncontrolled map.addLayer(new MapboxLayer()) i. impossible now.

So ultimately mapboxLayerIds is equal to all layers in deck instance.

So this PR clears flow, so it's basically two steps:

• mapbox/libre renders its own layers and all deck layers (in groups or not)
• afterRender renders other views

To complete description, this is how i understand current flow:

• (1) mapbox/libre renders own layers and some deck layers added manually with map.add(new MapboxLayer({deck: someOtherDeckInstance}) (?)
• mapbox/libre all viewports, but
  • for mapbox view, it skips layers rendered in (1) renders rest on top
  • for other views renders all deck layers

The main mystery for me is that i don't know if/how it was possible to (1) to skip some layers. It must be some legacy path that was impossible now in v9.

I believe current code has just many unusable branches that are not working at all. Possibly remnants from old incarnations of MapboxOverlay.

[chrisgervang]

So this PR clears flow, so it's basically two steps:

• mapbox/libre renders its own layers and all deck layers (in groups or not)
• afterRender renders other views

Very interesting - is this the order it always occurs in as well? Layers and maplibre first, then other deck views (which may draw a clear color for a background)? That would explain why adding a view with a clear cannot be done before maplibre draws. Or, view draw order respect the views prop order?

I was trying to add a background color to the framebuffer while using Maplibre's globe view. Maplibre has no API for setting the background color, but deck can draw a full frame view with a clear color. I couldn't figure out why the background always drew over everything - perhaps this is why.

[chrisgervang]

The main mystery for me is that i don't know if/how it was possible to (1) to skip some layers. It must be some legacy path that was impossible now in v9.

I believe current code has just many unusable branches that are not working at all. Possibly remnants from old incarnations of MapboxOverlay.

I can't tell you how much I appreciate your work here. There are many remnants of old MapboxLayer features in MapboxOverlay since we wanted to offer a full migration path. But I agree this seems like a feature that no longer is used and I'd like to remove it

[zbigg]

I just've found that MapboxLayer docs are still available online even if it's not supported in v9

[chrisgervang]

I just've found that MapboxLayer docs are still available online even if it's not supported in v9

Yes, we hid it from the table of contents but kept it as an internal doc since the class was still in use.

[chrisgervang]

@Pessimistress can you please gut check our conclusion in the comments that it's fine to remove these layer filters since it's for a feature no longer supported?

[zbigg]

I looked at test apps created in #9946 and confirm that we're ok and we still support mutliple views.
I'll perform some more tests and merge, so #9955 can follow.