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

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,