Drop smallest inner rings for earcut performance

[yhahn]

• Adds unit tests for classifyRings()
• Adds sorting of inner rings for polygons by area (desc) and truncating to EARCUT_MAX_RINGS (currently set to 100)

Next actions

• coordinate benchmarks with @jakepruitt
• more IRL testing with various datasources to determine a good EARCUT_MAX_RINGS value
• mapbox-gl-native PR ([earcut] Drop smallest inner rings for earcut performance mapbox-gl-native#5145)

cc @jakepruitt @jfirebaugh

[mourner]

If you're feeling like doing something fancy, you can replace sort here with https://github.com/mourner/quickselect, which runs in O(n) rather than O(n log n), and would also allow you to avoid some copies like concat. :)

According to my unscientific tests, I think max rings will end up being something closer to 800, but let's wait to see what @jakepruitt figures out with real benchmarks.

[jfirebaugh]

LGTM. Will be interested to see integration with benchmarking from @jakepruitt.

Our plan is to move classifyRings into loadGeometry for the vector-tile-js 2.0 release (mapbox/vector-tile-js#33). I think ring-dropping could move along with it, such that loadGeometry takes an optional maxRings parameter. Or it could remain a post-processing step in gl-js.

[lucaswoj]

What went into the decision to simplify to a max # of rings rather than filter rings by area? I imagine the latter would result in

• more predictable visual artifacts because the amount of distortion isn't a function of the complexity of the input data
• improved rendering perf for small polygons
• O(n) filtering perf

[mourner]

Crossposting my comment from chat, answering @lucaswoj's question:

The source issue is that Earcut is slow on polygons with tons of holes, regardless of area. If we filter by area, we can’t guarantee that we’ll filter enough if the threshold is low, and that we won't introduce unwanted visual change if the threshold is high. E.g. I had to bump the area threshold pretty high in a recent test of Streets (800 in tile coords), but this may be too high for other datasets.

So we’ll guard against the source issue with Earcut, and leave the noise filtering as responsibility of the dataset designer.

[lucaswoj]

Once we bump EARCUT_MAX_RINGS to 500, this is ready to 🚢!

[lucaswoj]

I'm going to take a quick look at benchmarks using https://github.com/mourner/quickselect instead of Array#sort and then 🚢 this

[mourner]

@lucaswoj in theory, it should be about 5-7 times faster for a 500-hole polygon, but if the total sort is just a few millisecond, it probably doesn't matter much. Although worth pointing out that quickselect is already in the dep tree through supercluster->kdbush, so it won't add to the bundle size.

[mourner]

It seems that the 2nd arg to quickselect should be just maxRings — k is position independent of left.

[lucaswoj]

Got it! I was a little unsure of the relationship between k and maxRings from the quickselect docs.

[lucaswoj]

🚢 on 🍏 @mourner @yhahn @jakepruitt?