set further near plane to fix depth precision for deckgl

[ansis]

fix #7573

This fixes precision issues in custom layers that use deckgl. While the old near plane worked well for our fill-extrusion layers it was causing issues in deckgl layers that do more complex projection in the shaders. This moves the near plane further away to a distance that seems to fix both the z-fighting in deckgl while not creating camera-close-to-building clipping issues.

deckgl would have to change 1 / deck.height to 1 / 50 here to match our change:
https://github.com/uber/deck.gl/blob/bddc36902a2535fa9430a1b450944f83cf4d9d11/modules/mapbox/src/deck-utils.js#L118

@Pessimistress can you confirm this would work for you?

Testing

The debug-z-fighting-deckgl branch contains a debug page that can be used to test this. In the branch I've both patched deckgl and mapbox-gl-js. The third map pane shows both a deckgl layer and a fill-extrusion layer coexisting. To start the example: yarn run start-debug and then open http://localhost:9966/debug/

Launch Checklist

• briefly describe the changes in this PR
• write tests for all new functionality
• manually test the debug page

[chloekraw]

cc @tgorkin @chrisirhc @heshan0131 @ibgreen

[Pessimistress]

@ansis Thank you!
For backward compatibility, is there a way we can get the version of mapbox-gl from the Map instance?

[ansis]

For backward compatibility, is there a way we can get the version of mapbox-gl from the Map instance?

The version isn't on the map instance but it is exported by the library (mapboxgl.version). Does that work?

[astojilj]

LGTM
On native side, near set to 0.1 * state.getCameraToCenterDistance() evaluates to height / 90.

[asheemmamoowala]

Is there an existing render test that exercises this with extrusion layers?

[Pessimistress]

@ansis We need to account for both the script tag and node_modules. So we end up having to do something like

function getMapboxVersion() {
  if (typeof mapboxgl === 'undefined') {
    return mapboxgl.version;
  }
  try {
    return require('mapbox-gl').version;
  } catch (err) {
    return 'unknown';
  }
}

And also change bundler configs to avoid bundling mapbox-gl with deck.

[Pessimistress]

@ansis Just brainstorming, can layer.render receive a third argument that are miscellaneous parameters? Then we can do something like

render(gl, matrix, {near, far}) {
  ...
}

[ansis]

I've added the version to the map instance. Exposing the actual parameters is a good idea but in the interest of just getting this merged I've added the version.

[asheemmamoowala]

@ansis I'm not clear on why the Map object needs to expose version. This is a property of the bundle, and correctly exported on the mapboxgl object, as it applies to more than just the Map class.

[asheemmamoowala]

Is there an existing render test that exercises this with extrusion layers?

[asheemmamoowala]

We need to account for both the script tag and node_modules.

@Pessimistress can you say more about what the problem with your current approach is? It looks like you are trying to support multiple ways of importing mapboxgl in your project (or it is demand loaded?)

When a CustomLayerInterface is added to a map, and onAdd is invoked, you should be able to capture the mapboxgl.version and use it to determine how the layer behaves.

[Pessimistress]

@asheemmamoowala Our custom layer class is published to npm and it does not depend on mapbox-gl. This is clearly allowed as long as it implements the custom layer interface.

From the layer's perspective, we cannot tell how the user is including mapbox. If they are using a bundler, then there will not be a global mapboxgl object. Conditionally requiring an external module is dangerous, because

• It requires additional bundler setup for users who wish to include mapbox using the script tag
• It breaks the esm build by mixing require and export
• It generally violates our coding policy by requiring something that is not in the dependencies list

[ansis]

Is there an existing render test that exercises this with extrusion layers?

There are tests with extrusions but none that exercise this because all the values work well for us in mapbox-gl. It's only in deckgl that the issues arise.

When a CustomLayerInterface is added to a map, and onAdd is invoked, you should be able to capture the mapboxgl.version and use it to determine how the layer behaves.

onAdd provides the map, not mapboxgl which is what is motivating adding it to the map

[asheemmamoowala]

@Pessimistress Thanks for the quick feedback. In that case, adding Map#version seems like a simple enough addition.

LGTM!