visgl / deck.gl

WebGL2 powered visualization framework
https://deck.gl
MIT License
12.17k stars 2.09k forks source link

[maplibre] checkerboard rendering regression when `interleaved: true` #8602

Closed chrisgervang closed 5 months ago

chrisgervang commented 7 months ago

I'm noticing a rendering issue in Maplibre GL JS when deck renders into its WebGL context (interleaved: true). The tiles aren't rendering correctly, they "checkerboard" while navigating.

This issue appears to only effect Maplibre v3 & Deck v9. I've tested and found no issue with Mapbox GL JS v3 & Deck v9, or with either basemap lib in Deck v8.9. Note that I can't test Maplibre v2 or v1 since Deck v9 requires WebGL2 for interleave.

Reproduction: https://codesandbox.io/p/sandbox/deck-v9-maplibre-interleave-repro-8602-lzx4z8?file=%2Findex.html%3A8%2C54

https://github.com/visgl/deck.gl/assets/2461547/6f8b9500-2c70-4d17-b7ee-c92fc6cd4d18

Originally posted by @chrisgervang in https://github.com/visgl/deck.gl/issues/8551#issuecomment-1984254002

chrisgervang commented 7 months ago

@donmccurdy @felixpalmer @Pessimistress any idea what causes this regression?

donmccurdy commented 7 months ago

I'll try testing tomorrow, but this looks similar to the issues fixed by https://github.com/visgl/luma.gl/pull/2032, just released in luma v9.0.2.

donmccurdy commented 7 months ago

Seems not to have been resolved by the earlier fix. Possibly a depth buffer issue. I've started a branch and copied the repro case into an example for testing...

https://github.com/visgl/deck.gl/compare/donmccurdy/fix-maplibre-interleave

... will see what I can find on a fix there!

chrisgervang commented 7 months ago

Thanks! You can also use https://github.com/visgl/deck.gl/tree/master/test/apps/mapbox-integration or https://github.com/visgl/deck.gl/blob/master/examples/gallery/src/maplibre-overlay.html

chrisgervang commented 6 months ago

Hey @birkskyum and @HarelM, we're so far unable to track down the cause of this rendering issue with deck v9 and maplibre. We aren't noticing it with other base maps. Do know anyone who could help us resolve this? Your help is much appreciated!

HarelM commented 6 months ago

Can you clarify what the issue is? The statement about v3 not working while v3 works is probably the root cause. Also we've released version 4 of maplibre. Let me know how I can help.

chrisgervang commented 6 months ago

Did you notice the checkerboard rendering artifacts in the video or Codesandbox? That only seems to happen in Maplibre. I'll try Maplibre 4. Also to clarify, interleaved rendering (shared context) works without artifacts in Mapbox (and Google maps) but not Maplibre.

birkskyum commented 6 months ago

The CodeSandbox repro is already on v4, and it's still showing the error, so it's not fixed yet.

Hmm, my first intuition was it's some z-fighting, similar to the flickering in the MVTLayer on zoom, so a negligible offset on either side might potentially be able to resolve it until WebGPU comes to the rescue.

HarelM commented 6 months ago

I'm guessing it is related to "fighting z order", but my expertise here are limited when it comes to webgl context stuff...

birkskyum commented 6 months ago

Might be worth checking how we can increase the depth buffer precision, and what it'll cost.

birkskyum commented 6 months ago

There might be two issues at play here.

Checkerboard when moving around issue (8.9.35 works - 9.0.0-beta.3 breaks)

The checkerboard issue that appear when moving around wasn't present in 8.9.35 where everything worked as expected with all version of MapLibre (2.4.0, 3.x, 4.x). For none of the following do i see version differences between maplibre versions.

Screenshot 2024-03-26 at 18 23 58

It's appearance started with the earliest v9 version i can run, 9.0.0-beta.3.

Screenshot 2024-03-26 at 18 22 32

z-fighting on Initial load issue (9.0.0-beta.5 works - 9.0.0-beta.6 breaks)

This is what I see on initial load with 9.0.0-beta.5

Screenshot 2024-03-26 at 18 20 20

And with 9.0.0-beta.6 and later

Screenshot 2024-03-26 at 18 20 27
birkskyum commented 6 months ago

For the issue introduced in 9.0.0-beta.6

This could very well come as a result of bumping luma.gl from 9.0.0-beta.4 to 9.0.0-beta.6, which cuts away webgl1 support entirely, as mentioned here.

It's maybe not the only thing, but I do see a mention to "Use device.createTexture instead of WEBGLRenderBuffer". I believe MapLibre LG JS still use the WEBGLRenderBuffer here:

Updating these places, potentially by introducing more conditional webgl1/2 logic, or it might help to simply go webgl2-only like deck.gl, which we had mixed results trying last May with the v3 release.

ibgreen commented 6 months ago

I believe MapLibre LG JS still use the WEBGLRenderBuffer here:

Good observation, worth some thought... But I can't think of a reason why creating and binding a Renderbuffer with the WebGL 2 API outside of the luma.gl API would cause problems.

There is no deep motivation behind the renderbuffer removal in v9. It just didn't seem worth the trouble to expose Renderbuffers through the luma.gl v9 Texture API - since it seems that textures can be used in all cases that required Renderbuffers in WebGL 1, and I could not find clear information stating that there are convincing perf advantages to renderbuffers.

arthurgailes commented 5 months ago

I'm also having this issue via mapbox-gl when interleaving since updating to deckgl v9/mapbox v3

arthurgailes commented 5 months ago

I'm also having this issue via MapboxOverlay when interleaving since updating to deckgl v9/mapbox v3

Repro works when using mapbox-gl with a mapbox basemap: Codesandbox

image

ibgreen commented 5 months ago
chrisgervang commented 5 months ago

These layers (e.g. roads) are native basemap layers though, so I don't think there is a PolygonLayer to apply props to. Maybe there's a maplibre equivalent. I do agree it looks like z-fighting.

arthurgailes commented 5 months ago

Just noting that the problem is present in both mapboxgl v2 and v3, but not in deck 8.9.

ibgreen commented 5 months ago

I don't think there is a PolygonLayer to apply props to

Sorry, I meant the ScatterplotLayer. And to be clear, I was talking about making a change in the linked codesandbox. I was going to try to but it seemed read-only.

      const deckOverlay = new MapboxOverlay({
        interleaved: true,
        layers: [
          new ScatterplotLayer({
            id: "deckgl-circle",
            beforeId: "water",
            data: [
              { position: [-122.402, 37.79], color: [255, 0, 0], radius: 1000 },
            ],
            getPosition: (d) => d.position,
            getFillColor: (d) => d.color,
            getRadius: (d) => d.radius,
            opacity: 0.3,
          }),
          new ArcLayer({
            id: "deckgl-arc",
            data: [
              {
                source: [-122.3998664, 37.7883697],
                target: [-122.400068, 37.7900503],
              },
            ],
            getSourcePosition: (d) => d.source,
            getTargetPosition: (d) => d.target,
            getSourceColor: [255, 208, 0],
            getTargetColor: [0, 128, 255],
            getWidth: 8,
          }),
        ],
      });
chrisgervang commented 5 months ago

Now that luma can enforce WebGL2 on WebGL1 libraries, I'm attempting to see how mapboxgl v1 and maplibre v2 fair in this codesandbox. However, I haven't figured out how to access enforceWebGL2 from our scripting env so I've opened up #8822

birkskyum commented 5 months ago

@chrisgervang , that's interesting - I can think of some places that likely will be problematic as a result of some missing webgl2 extensions in maplibre v2. Cases like the maplibre heatmap layer come to mind, but I'm curious to see how it handles it.

ibgreen commented 5 months ago

likely will be problematic as a result of some missing webgl2 extensions

I assume you are referring to the fact that WebGL2 contexts are not allowed to support WebGL1 extensions that offer functionality that is built-in to WebGL2. It would probably not be hard to make shims for those extensions if you had a list. After all the reason they are removed is that the functionality is already supported in the core, so we just need to override context.getExtension() and return some objects that forward calls to the new context methods.

birkskyum commented 5 months ago

All conditional loading of extensions is contained here: https://github.com/maplibre/maplibre-gl-js/blob/main/src/gl/context.ts

And a single VAO check here: https://github.com/maplibre/maplibre-gl-js/blob/main/src/gl/value.ts#L429-L433

ibgreen commented 5 months ago

All conditional loading of extensions is contained here:

I looked and realized your code handles both WebGL1 and WebGL2 with dynamic checks so you should not have any issues as far as I can tell, even if we force feed you a WebGL2 context.

birkskyum commented 5 months ago

It does have webgl1/2 compat from v3 and forward, but it didn't have that in maplibre gl js v2 which @chrisgervang wanted to try, so comparing these two versions can give an idea what a shim should add.