Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Tracked by #8541
chrisgervang opened this issue Mar 7, 2024 · 25 comments · Fixed by #8844
Closed
Tracked by #8541

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

chrisgervang opened this issue Mar 7, 2024 · 25 comments · Fixed by #8844
Labels
Milestone

Comments

@chrisgervang
Copy link
Collaborator

chrisgervang commented Mar 7, 2024

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

interleaved.mp4

Originally posted by @chrisgervang in #8551 (comment)

@chrisgervang
Copy link
Collaborator Author

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

@donmccurdy
Copy link
Collaborator

donmccurdy commented Mar 14, 2024

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

@donmccurdy
Copy link
Collaborator

donmccurdy commented Mar 14, 2024

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
Copy link
Collaborator Author

chrisgervang commented Mar 14, 2024

@chrisgervang
Copy link
Collaborator Author

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
Copy link
Contributor

HarelM commented Mar 26, 2024

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
Copy link
Collaborator Author

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
Copy link

birkskyum commented Mar 26, 2024

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
Copy link
Contributor

HarelM commented Mar 26, 2024

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

@birkskyum
Copy link

birkskyum commented Mar 26, 2024

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

@birkskyum
Copy link

birkskyum commented Mar 26, 2024

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
Copy link

birkskyum commented Mar 26, 2024

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
Copy link
Collaborator

ibgreen commented Mar 26, 2024

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
Copy link

arthurgailes commented Apr 23, 2024

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

@arthurgailes
Copy link

arthurgailes commented Apr 23, 2024

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
Copy link
Collaborator

ibgreen commented Apr 23, 2024

  • Looks like basic z-fighting.
  • You could try to set getPolygonOffset prop on the PolygonLayer to fix it in the app.
  • Wonder why this is a regression... it does look like deck should apply a minimal offset to all layers in interleaved mode.

@chrisgervang
Copy link
Collaborator Author

chrisgervang commented Apr 23, 2024

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
Copy link

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

@ibgreen
Copy link
Collaborator

ibgreen commented Apr 23, 2024

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
Copy link
Collaborator Author

chrisgervang commented Apr 23, 2024

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
Copy link

birkskyum commented Apr 23, 2024

@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
Copy link
Collaborator

ibgreen commented Apr 23, 2024

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
Copy link

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
Copy link
Collaborator

ibgreen commented Apr 24, 2024

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
Copy link

birkskyum commented Apr 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants