Fix: #11516, #11041. Terrain: buildings on tile borders. (#11530)

* Fix #11516: unecessary optimization approach disabled border intersection calculation for complex polygons: when enumerating polygon rings, if previous rings were outside bookkeeped aabb would prevent checking if edge crosses tile border and disable joining and displaying features split and asserting on check (in dev build).
Backporting fill extrusions on terrain #11041 fix from gl-native is related: when adjacent tiles are of different zooms, breaks or no building parts were visible.

Fixes: #11516, #11041

* Enable flat-roof-over-border-of-different-zoom-zoomin render test

There is occasional flakiness on rendering fill color (zoom 16 evaluation is not seen on drape but zoom 15) and fill color is made constant to prevent it.

Co-authored-by: Aleksandar Stojiljković <aleksandar.stojijkovic@mapbox.com>

src/data/bucket.js

@@ -16,6 +16,7 @@ export type BucketParameters<Layer: TypedStyleLayer> = {
     index: number,
     layers: Array<Layer>,
     zoom: number,
+    canonical: CanonicalTileID,
     pixelRatio: number,
     overscaling: number,
     collisionBoxArray: CollisionBoxArray,

src/data/bucket/fill_extrusion_bucket.js

@@ -95,32 +95,23 @@ class PartMetadata {
         this.currentPolyCount.edges++;
 
         this.acc._add(p);
-        let checkBorders = !!this.borders;
-
         const min = this.min, max = this.max;
         if (p.x < min.x) {
             min.x = p.x;
-            checkBorders = true;
         } else if (p.x > max.x) {
             max.x = p.x;
-            checkBorders = true;
         }
         if (p.y < min.y) {
             min.y = p.y;
-            checkBorders = true;
         } else if (p.y > max.y) {
             max.y = p.y;
-            checkBorders = true;
         }
         if (((p.x === 0 || p.x === EXTENT) && p.x === prev.x) !== ((p.y === 0 || p.y === EXTENT) && p.y === prev.y)) {
             // Custom defined geojson buildings are cut on borders. Points are
             // repeated when edge cuts tile corner (reason for using xor).
             this.processBorderOverlap(p, prev);
         }
-        if (checkBorders) this.checkBorderIntersection(p, prev);
-    }
-
-    checkBorderIntersection(p: Point, prev: Point) {
+        // check border intersection
         if ((prev.x < 0) !== (p.x < 0)) {
             this.addBorderIntersection(0, interpolate(prev.y, p.y, (0 - prev.x) / (p.x - prev.x)));
         }
@@ -180,6 +171,7 @@ class PartMetadata {
 class FillExtrusionBucket implements Bucket {
     index: number;
     zoom: number;
+    canonical: CanonicalTileID;
     overscaling: number;
     enableTerrain: boolean;
     layers: Array<FillExtrusionStyleLayer>;
@@ -206,15 +198,16 @@ class FillExtrusionBucket implements Bucket {
     features: Array<BucketFeature>;
 
     featuresOnBorder: Array<PartMetadata>;
-    // borders / borderDone: 0 - left, 1, right, 2 - top, 3 - bottom
+    // borders / borderDoneWithNeighborZ: 0 - left, 1, right, 2 - top, 3 - bottom
     borders: Array<Array<number>>; // For each side, indices into featuresOnBorder array.
-    borderDone: Array<boolean>;
+    borderDoneWithNeighborZ: Array<number>;
     needsCentroidUpdate: boolean;
     tileToMeter: number; // cache conversion.
     projection: string;
 
     constructor(options: BucketParameters<FillExtrusionStyleLayer>) {
         this.zoom = options.zoom;
+        this.canonical = options.canonical;
         this.overscaling = options.overscaling;
         this.layers = options.layers;
         this.layerIds = this.layers.map(layer => layer.id);
@@ -236,7 +229,7 @@ class FillExtrusionBucket implements Bucket {
         this.hasPattern = hasPattern('fill-extrusion', this.layers, options);
         this.featuresOnBorder = [];
         this.borders = [[], [], [], []];
-        this.borderDone = [false, false, false, false];
+        this.borderDoneWithNeighborZ = [-1, -1, -1, -1];
         this.tileToMeter = tileToMeter(canonical);
 
         for (const {feature, id, index, sourceLayerIndex} of features) {

src/render/draw_fill_extrusion.js

@@ -180,23 +180,26 @@ function flatRoofsUpdate(context, source, coord, bucket, layer, terrain) {
     ];
 
     const getLoadedBucket = (nid) => {
-        const maxzoom = source.getSource().maxzoom;
+        const minzoom = source.getSource().minzoom;
         const getBucket = (key) => {
             const n = source.getTileByID(key);
             if (n && n.hasData()) {
                 return n.getBucket(layer);
             }
         };
-        // In overscale range, we look one tile zoom above and under. We do this to avoid
-        // flickering and use the content in Z-1 and Z+1 buckets until Z bucket is loaded.
-        let b0, b1, b2;
-        if (nid.overscaledZ === nid.canonical.z || nid.overscaledZ >= maxzoom)
-            b0 = getBucket(nid.key);
-        if (nid.overscaledZ >= maxzoom)
-            b1 = getBucket(nid.calculateScaledKey(nid.overscaledZ + 1));
-        if (nid.overscaledZ > maxzoom)
-            b2 = getBucket(nid.calculateScaledKey(nid.overscaledZ - 1));
-        return b0 || b1 || b2;
+        // Look one tile zoom above and under. We do this to avoid flickering and
+        // use the content in Z-1 and Z+1 buckets until Z bucket is loaded or handle
+        // behavior on borders between different zooms.
+        const zoomLevels = [0, -1, 1];
+        for (const i of zoomLevels) {
+            const z = nid.overscaledZ + i;
+            if (z < minzoom) continue;
+            const key = nid.calculateScaledKey(nid.overscaledZ + i);
+            const b = getBucket(key);
+            if (b) {
+                return b;
+            }
+        }
     };
 
     const projectedToBorder = [0, 0, 0]; // [min, max, maxOffsetFromBorder]
@@ -241,13 +244,19 @@ function flatRoofsUpdate(context, source, coord, bucket, layer, terrain) {
 
     // Process all four borders: get neighboring tile
     for (let i = 0; i < 4; i++) {
+        // borders / borderDoneWithNeighborZ: 0 - left, 1, right, 2 - top, 3 - bottom
+        // bucket's border i is neighboring bucket's border j:
+        const j = (i < 2 ? 1 : 5) - i;
         // Sort by border intersection area minimums, ascending.
         const a = bucket.borders[i];
-        if (a.length === 0) { bucket.borderDone[i] = true; }
-        if (bucket.borderDone[i]) continue;
+        if (a.length === 0) continue;
         const nid = neighborTileID = neighborCoord[i](coord);
         const nBucket = getLoadedBucket(nid);
         if (!nBucket || !(nBucket instanceof FillExtrusionBucket) || !nBucket.enableTerrain) continue;
+        if (bucket.borderDoneWithNeighborZ[i] === nBucket.canonical.z &&
+            nBucket.borderDoneWithNeighborZ[j] === bucket.canonical.z) {
+            continue;
+        }
 
         neighborDEMTile = terrain.findDEMTileFor(nid);
         if (!neighborDEMTile || !neighborDEMTile.dem) continue;
@@ -256,9 +265,28 @@ function flatRoofsUpdate(context, source, coord, bucket, layer, terrain) {
             if (!(dem && dem.dem)) return; // defer update until an elevation tile is available.
             demTile = dem;
         }
-        const j = (i < 2 ? 1 : 5) - i;
         const b = nBucket.borders[j];
         let ib = 0;
+
+        const updateNeighbor = nBucket.borderDoneWithNeighborZ[j] !== bucket.canonical.z;
+        // If neighbors are of different canonical z, we cannot join parts but show
+        // all without flat roofs.
+        if (bucket.canonical.z !== nBucket.canonical.z) {
+            for (const index of a) {
+                bucket.encodeCentroid(undefined, bucket.featuresOnBorder[index], false);
+            }
+            if (updateNeighbor) {
+                for (const index of b) {
+                    nBucket.encodeCentroid(undefined, nBucket.featuresOnBorder[index], false);
+                }
+                nBucket.borderDoneWithNeighborZ[j] = bucket.canonical.z;
+                nBucket.needsCentroidUpdate = true;
+            }
+            bucket.borderDoneWithNeighborZ[i] = nBucket.canonical.z;
+            bucket.needsCentroidUpdate = true;
+            continue;
+        }
+
         for (let ia = 0; ia < a.length; ia++) {
             const parta = bucket.featuresOnBorder[a[ia]];
             const partABorderRange = parta.borders[i];
@@ -269,7 +297,7 @@ function flatRoofsUpdate(context, source, coord, bucket, layer, terrain) {
                 partb = nBucket.featuresOnBorder[b[ib]];
                 const partBBorderRange = partb.borders[j];
                 if (partBBorderRange[1] > partABorderRange[0] + error) break;
-                if (!nBucket.borderDone[j]) nBucket.encodeCentroid(undefined, partb, false);
+                if (updateNeighbor) nBucket.encodeCentroid(undefined, partb, false);
                 ib++;
             }
             if (partb && ib < b.length) {
@@ -292,7 +320,7 @@ function flatRoofsUpdate(context, source, coord, bucket, layer, terrain) {
                     }
 
                     bucket.encodeCentroid(undefined, parta, false);
-                    if (!nBucket.borderDone[j]) nBucket.encodeCentroid(undefined, partb, false);
+                    if (updateNeighbor) nBucket.encodeCentroid(undefined, partb, false);
                     continue;
                 }
 
@@ -307,16 +335,18 @@ function flatRoofsUpdate(context, source, coord, bucket, layer, terrain) {
                 bucket.encodeCentroid(centroid, parta, false);
 
                 assert(partb.vertexArrayOffset !== undefined && partb.vertexArrayOffset < nBucket.layoutVertexArray.length);
-                if (!nBucket.borderDone[j]) nBucket.encodeCentroid(centroid, partb, false);
+                if (updateNeighbor) nBucket.encodeCentroid(centroid, partb, false);
             } else {
                 assert(parta.intersectsCount() > 1 || (partb && partb.intersectsCount() > 1)); // expected at the end of border, when buildings cover corner (show building w/o flat roof).
                 bucket.encodeCentroid(undefined, parta, false);
             }
         }
 
-        bucket.borderDone[i] = bucket.needsCentroidUpdate = true;
-        if (!nBucket.borderDone[j]) {
-            nBucket.borderDone[j] = nBucket.needsCentroidUpdate = true;
+        bucket.borderDoneWithNeighborZ[i] = nBucket.canonical.z;
+        bucket.needsCentroidUpdate = true;
+        if (updateNeighbor) {
+            nBucket.borderDoneWithNeighborZ[j] = bucket.canonical.z;
+            nBucket.needsCentroidUpdate = true;
         }
     }
 

test/ignores.json

@@ -35,8 +35,6 @@
   "render-tests/distance/layout-text-size": "skip - distance expression is not implemented",
   "render-tests/skybox/atmosphere-padding": "skip - https://github.com/mapbox/mapbox-gl-js/issues/10314",
   "render-tests/terrain/symbol-draping/style.json": "skip - https://github.com/mapbox/mapbox-gl-js/issues/10365",
-  "render-tests/fill-extrusion-terrain/flat-roof-over-border-of-different-zoom-zoomin": "skip - https://github.com/mapbox/mapbox-gl-js/issues/11041",
-  "render-tests/fill-extrusion-terrain/flat-roof-over-border-of-different-zoom": "skip - https://github.com/mapbox/mapbox-gl-js/issues/11041",
   "render-tests/text-variable-anchor/pitched": "skip - non-deterministic symbol placement on tile boundaries",
   "render-tests/video/projected": "https://github.com/mapbox/mapbox-gl-js/issues/11234",
   "query-tests/terrain/draped/lines/slope-occlusion-box-query": "skip - non-deterministic",

test/integration/render-tests/fill-extrusion-terrain/flat-roof-gl-js#11516/style.json

@@ -0,0 +1,85 @@
+{
+  "version": 8,
+  "metadata": {
+    "description": "Verifies fix for bug https://github.com/mapbox/mapbox-gl-js/issues/11516",
+    "test": {
+      "height": 512,
+      "width": 512,
+      "operations": [
+        ["wait"]
+      ]
+    }
+  },
+  "sources": {
+    "rgbterrain": {
+      "type": "raster-dem",
+      "tiles": [
+        "local://tiles/{z}-{x}-{y}.terrain.512.png"
+      ],
+      "maxzoom": 14,
+      "tileSize": 512
+    },
+    "country-boundaries": {
+      "type": "vector",
+      "maxzoom": 2,
+      "tiles": [
+        "local://tiles/mapbox.country-boundaries-v1/{z}-{x}-{y}.mvt"
+      ]
+    }
+  },
+  "terrain": {
+    "source": "rgbterrain",
+    "exaggeration": ["interpolate",["linear"],["zoom"],0,50,11,1,12,0]
+  },
+  "zoom": 2.18,
+  "center": [
+    -100,
+    19
+  ],
+  "layers": [
+    {
+      "id": "background",
+      "type": "background",
+      "paint": {
+        "background-color": "lightblue"
+      }
+    },
+    {
+      "id": "country-boundaries",
+      "type": "fill-extrusion",
+      "paint": {
+        "fill-extrusion-color": [
+          "interpolate",
+          [
+            "linear"
+          ],
+          [
+            "get",
+            "color_group"
+          ],
+          1,
+          "hsl(0, 87%, 68%)",
+          6,
+          "hsl(43, 88%, 58%)"
+        ],
+        "fill-extrusion-height": [
+          "interpolate",
+          [
+            "linear"
+          ],
+          [
+            "get",
+            "color_group"
+          ],
+          1,
+          1000000,
+          6,
+          100000
+        ],
+        "fill-extrusion-opacity": 0.8
+      },
+      "source": "country-boundaries",
+      "source-layer": "country_boundaries"
+    }
+  ]
+}

test/integration/render-tests/fill-extrusion-terrain/flat-roof-over-border-of-different-zoom-zoomin/style.json

@@ -64,19 +64,6 @@
       "id": "building",
       "source": "mapbox",
       "paint": {
-        "fill-opacity": [
-          "interpolate",
-          [
-            "linear"
-          ],
-          [
-            "zoom"
-          ],
-          15.0,
-          0.0,
-          16.0,
-          1.0
-        ],
         "fill-outline-color": [
           "rgba",
           205.00001525878907,