Extrusions: Do not try to triangulate non-polygon type features

[greysail]

Trying to triangulate the base area of a feature without base area (e.g.
GeoJSON LineStrings) just creates invalid excess triangles,
yielding visual artifacts

There is an equivalent pull request for mapbox-gl-native, which I will post shortly

Launch Checklist

• briefly describe the changes in this PR
• write tests for all new functionality (not actually new functionality; more about making mapbox behave as expected)
• document any changes to public APIs (none)
• post benchmark scores (i guess this should not affect performace measurably)
• manually test the debug page
• tagged @mapbox/studio and/or @mapbox/maps-design if this PR includes style spec changes (none)

[greysail]

[v2] added fix to lint styleguide errors

[ansis]

@greysail thanks for the contribution! Are you interested in trying to add a render test for this fix?

If you are, the way to do this is:

• create a new directory in test/integration/render-tests/ called fill-extrusion-geometry with a subdirectory called linestring
• add a style.json to the directory (look at other fill-extrusion render tests for examples)
• run it with UPDATE=1 yarn run test-render fill-extrusion-geometry and tweak it until you get the results you expect

[greysail]

[v3] added render test

[mourner]

Should we put the check somewhere up the method chain? It might be confusing to have it this way down, inside a loop where non-polygon features will simply have all iterations skipped with continue.

[mourner]

Also, can we make the render test as small as possible (while still testing what we need)? Each new test impacts the running time of the test suite and the bigger the image, the longer it runs.

[greysail]

I have reduced the render image size to 32x32. Below that, the overhead for setting up the render pipeline is probably bigger than the time for rendering itself.

[greysail]

The rationale for putting the code into addFeature() in fill_extrusion_bucket.js is that it actually does not skip loops:

Extrusions consist of two parts:

• the base area
• the vertical parts.

For polygon-type features, we want to generate geometry for both parts while for line-type features we only want to generate geometry for the vertical parts since they do not have a base area.

If one were to move the logic for distinguishing between line-type and polygon-type features somewhere higher up, this would from my understanding mean having to split up addFeature() into two seperate routines (one for the base area and one for the vertical parts of the extrusion).

[mourner]

You're right, I misread the code. Looks good to me — pending on @ansis signing this off.

[greysail]

[v4] rebase on master & squash commits for test case into a single patch

[greysail]

Note: the render test actually fails on native, so please do not merge until this is fixed

[greysail]

[v5]: fix test for native implementation

[greysail]

[v6] rebase on master

[mourner]

Thank you!