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

Issues rendering coastline in the Baltic Sea #773

Open
ianthetechie opened this issue Dec 27, 2020 · 6 comments
Open

Issues rendering coastline in the Baltic Sea #773

ianthetechie opened this issue Dec 27, 2020 · 6 comments
Milestone

Comments

@ianthetechie
Copy link

TANGRAM VERSION:

Tangram v0.21.1

ENVIRONMENT:

macOS Big Sur running either Firefox 83 or Chrome 87.

TO REPRODUCE THE ISSUE, FOLLOW THESE STEPS:

  • Load the following Tangram scene (paste into play.tangram.city, for example; sorry I had to zip it as GitHub rejects yaml files): scene.yaml.zip
  • Zoom to the Baltic Sea area at z6/z7 (Tangram calls it z7, but MVTs are sort of off-by-one and double the standard slippy tile size so the loaded tiles will be z6).

RESULT:

It will take approximately 30 seconds to fully render the tiles containing the coastline of Sweden and the Åland Islands on a loaded 16" MBP.

EXPECTED RESULT:

It will render the tiles in a reasonable period of time ;)

I'm sure that the detail level of our tiles bears some of the blame, but comparable renderers such as Mapbox/MapLibre GL JS and OpenLayers (using the https://github.com/openlayers/ol-mapbox-style plugin) have no issues rendering this area so I think we've hit some sort of edge case/inefficiency in the geometry functions.

I initially thought this was due to the rather obscene number of town+village labels in Latvia (large numbers of labels in places like Latvia and Germany ARE extremely slow, but that's not the main point of this ticket and we can probably mitigate that by thinning our tiles), but profiling it in Chrome revealed that most of the time was actually spent in intersectsPolygon. I am pretty sure the water layer is mostly to blame, as disabling wide ranges of other layers (including all label layers) has no noticeable effect. However, removing only the water layer from the scene allows it to render very quickly.

IMAGE 2020-12-27 14:37:48

I'm still familiarizing myself with the code, but if this is something I can help with, I'd be happy to give it a shot if you can point me in the right direction.

@bcamper
Copy link
Member

bcamper commented Dec 27, 2020

Thanks - haven't taken a look at the scene yet, but the trace you posted is inside the earcut, which is Mapbox's own (very good!) polygon tessellation library. So if you are not seeing this behavior in Mapbox GL when you are rendering the same geometries (assuming you're pretty sure the style is translating such that the same geoms are rendering 1:1 in both renderers), one possibility is that Tangram's version of the library is out of date and there's an edge case performance issue in an earlier version? Or, something about how Tangram is handling these before they are passed to earcut could be at play, though nothing obvious there comes to mind right away.

Re: zoom levels, for the outward-facing interface, Tangram maintains the "traditional" Web Mercator zoom levels that were established for 256px raster tiles, and which Leaflet itself uses by default. But you are correct, it will load them "off one" when tile_size: 512 (and theoretically off two for 1024px tiles etc., though I don't think I've ever seen them in use). MVT itself doesn't specify this size or relationship though, you can generate MVTs of different internal resolutions and intended display scenarios.

@bcamper
Copy link
Member

bcamper commented Dec 27, 2020

Oh, so, if you want to give it a try, you are welcome to look into whether an earcut upgrade is needed :) Otherwise I will look when I am able.

@meetar
Copy link
Member

meetar commented Dec 27, 2020 via email

@ianthetechie
Copy link
Author

Wow, do you guys ever sleep? ;)

Yes, it's (ostensibly :D) the exact same geometries styled in substantially the same way (barring any bugs in my translation script; the output looks right though).

It looks like the earcut library is up to date (at least in tangram master; v2.2.2 is referenced in package.json, and that was released almost a year ago). Additionally, our tiles (at the ocean level) haven't changed substantially over the last 2 years and never had issues in either renderer, so it's probably something else.

It could still very well could be that I'm asking Tangram to do something expensive without knowing it, but the water layer itself (below) is really quite simple and doesn't do any funky JavaScript functions or weird coloring.

  water:
    data:
      source: openmaptiles
    draw:
      polygons:
        blend_order: 9
        color: '#222'
        order: 9
        style: inlay_polygons
    filter:
      $geometry: polygon

@bcamper
Copy link
Member

bcamper commented Feb 5, 2021

OK, finally got back to this :) After confirming that the code was bottlenecking in earcut, as your profile showed, I wanted to validate that Mapbox indeed had no problem rendering this same geometry. Since the earcut code that Tangram uses for triangulation is in fact Mapbox's own code, my suspicion was that there was higher-level logic that was preempting this from happening in MBGL. Comparing Stadia's Tangram and MBGL styles showed that the Tangram version is indeed rendering much more detail in these water polygons:

Tangram
Screen Shot 2021-02-04 at 8 04 08 PM

Mapbox
Screen Shot 2021-02-04 at 8 03 50 PM

The Tangram version is rendering upwards of 160k teeny tiny water polygons. You can see the MBGL version drops a bunch of these, but maintains a good overall level of detail. A quick dive into MBGL found this possible explanation re: earcut performance: https://github.com/mapbox/mapbox-gl-js/blob/ef7b2cb243f983fef7482502021e26bda93896e2/src/util/classify_rings.js#L37-L45

Which limits the number of rings in a polygon and is invoked with a limit of 500 rings here: https://github.com/mapbox/mapbox-gl-js/blob/ef7b2cb243f983fef7482502021e26bda93896e2/src/data/bucket/fill_bucket.js#L170

I did a quick and dirty test of similar logic locally in Tangram and it solved the problem! 🎉 I will keep this issue open to work on a slightly more polished fix.

@bcamper bcamper added this to the v0.21.2 milestone Feb 5, 2021
@Azbesciak
Copy link

Still opened :(

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

No branches or pull requests

4 participants