fix: Improve handling of auto-fitting with many features and the antimeridian on geo traces

draftlogs/7948_fix.md

@@ -0,0 +1 @@
+- Fix `geo.fitbounds: 'locations'` when mixing antimeridian-crossing territories with normal ones [[#7948](https://github.com/plotly/plotly.js/pull/7948)]

src/lib/geo_location_utils.js

@@ -401,16 +401,33 @@ function fetchTraceGeoData(calcData) {
  *   Returns `null` for input with no extractable coordinates (e.g. `Sphere`,
  *   empty FeatureCollection).
  */
-function computeBbox(d) {
-    // Extract an array containing all points contained in the GeoJSON object.
-    // coordAll throws an error on Sphere, malformed inputs, and nullish values.
-    // Treat any failure as "no bounds" so callers can null-guard uniformly.
-    let points;
+const computeBbox = (d) => boundsOfCoords(coordsOf(d));
+
+/**
+ * Return every coordinate contained in a GeoJSON object.
+ *
+ * @param {object} d - a GeoJSON Feature, Geometry, FeatureCollection, or
+ *   GeometryCollection.
+ * @return {Array} `[lon, lat]` pairs. Empty for input with nothing extractable:
+ *   coordAll throws on a Sphere, on malformed input and on nullish values, and
+ *   returns nothing for an empty collection.
+ */
+function coordsOf(d) {
     try {
-        points = coordAll(d);
+        return coordAll(d);
     } catch (_) {
-        return null;
+        return [];
     }
+}
+
+/**
+ * Bounding box of a list of coordinates, as `computeBbox` describes.
+ *
+ * @param {Array} points - `[lon, lat]` pairs
+ * @return {[number, number, number, number]|null} `[west, south, east, north]`,
+ *   or null when there are no points.
+ */
+function boundsOfCoords(points) {
     if (points.length === 0) return null;
     if (points.length === 1) {
         const [lon, lat] = points[0];
@@ -428,53 +445,28 @@ function computeBbox(d) {
     ];
 }
 
+const usesFitGeojson = (trace, geoLayout) => geoLayout.fitbounds === 'geojson' && trace.locationmode === 'geojson-id';
+
 /**
- * Pick a compact longitude range for `fitbounds`-style auto-framing when the
- * data straddles the antimeridian (±180°).
- *
- * Longitude is cyclic, so the naive [min, max] range used by the autorange
- * machinery can include a large empty span when points sit on both sides of
- * ±180° (e.g. lon = [131.8855, -179] spans ~311° the long way round, when the
- * compact view spans ~49° across the antimeridian). This finds the largest gap
- * between consecutive longitudes and, when that gap is wider than the gap across
- * the antimeridian, returns the complementary range so the map shows the dense
- * cluster of points rather than the empty ocean between them.
- *
- * The returned upper bound may exceed 180°; downstream `makeRangeBox` (and
- * MapLibre's `LngLatBounds`) handle ranges that cross the antimeridian without
- * ambiguity.
+ * Coordinates of a trace's whole geojson, for the `fitbounds: 'geojson'` mode.
  *
- * @param {Array} lons - longitude values (may contain non-finite entries)
- * @return {Array|null} [lonStart, lonEnd] when an antimeridian-crossing range is
- *   more compact, otherwise null (caller keeps the autorange result).
+ * @param {object} trace - a `fullData` trace
+ * @param {object} geoLayout - the subplot's `fullLayout` entry
+ * @return {Array} `[lon, lat]` pairs. Empty when the trace is in another mode, or
+ *   when the geojson has nothing extractable.
  */
-function getFitboundsLonRange(lons) {
-    const sorted = lons.filter(isFinite).sort((a, b) => a - b);
-    if (sorted.length < 2) return null;
-
-    const n = sorted.length;
-    const naiveSpan = sorted[n - 1] - sorted[0];
-    // Data already wraps the whole globe; there is nothing to compact.
-    if (naiveSpan >= 360) return null;
-
-    // Widest gap between consecutive longitudes.
-    let maxGap = -Infinity;
-    let gapStart = -1;
-    for (let i = 0; i < n - 1; i++) {
-        const gap = sorted[i + 1] - sorted[i];
-        if (gap > maxGap) {
-            maxGap = gap;
-            gapStart = i;
-        }
-    }
+const fitGeojsonCoords = (trace, geoLayout) =>
+    usesFitGeojson(trace, geoLayout) ? coordsOf(getTraceGeojson(trace)) : [];
 
-    // Only worth wrapping when an interior gap is wider than the gap that the
-    // naive [min, max] range already leaves open across the antimeridian.
-    const antimeridianGap = 360 - naiveSpan;
-    if (maxGap <= antimeridianGap) return null;
-
-    return [sorted[gapStart + 1], sorted[gapStart] + ANTIMERIDIAN_LON_SHIFT];
-}
+/**
+ * Bounding box of a trace's whole geojson, for the `fitbounds: 'geojson'` mode.
+ *
+ * @param {object} trace - a `fullData` trace
+ * @param {object} geoLayout - the subplot's `fullLayout` entry
+ * @return {Array|null} `[west, south, east, north]`, or null whenever
+ *   `fitGeojsonCoords` is empty.
+ */
+const fitGeojsonBbox = (trace, geoLayout) => boundsOfCoords(fitGeojsonCoords(trace, geoLayout));
 
 /**
  * Return an unwrapped version of a `[lon0, lon1]` longitude range.
@@ -502,9 +494,12 @@ module.exports = {
     getTraceGeojson,
     extractTraceFeature,
     fetchTraceGeoData,
+    boundsOfCoords,
     computeBbox,
+    coordsOf,
     doesCrossAntiMeridian,
-    getFitboundsLonRange,
+    fitGeojsonBbox,
+    fitGeojsonCoords,
     unwrapLonRange,
     ANTIMERIDIAN_LON_SHIFT
 };

src/plots/geo/constants.js

@@ -142,7 +142,7 @@ exports.lataxisSpan = {
 };
 
 // Projections whose math doesn't play well with fitbounds
-exports.fitboundsIncompatible = new Set(['albers usa', 'craig', 'satellite']);
+exports.fitboundsIncompatible = new Set(['albers usa', 'craig', 'peirce quincuncial', 'satellite']);
 
 // defaults for each scope
 exports.scopeDefaults = {

src/plots/geo/geo.js

@@ -16,7 +16,7 @@ var Drawing = require('../../components/drawing');
 var Fx = require('../../components/fx');
 var Plots = require('../plots');
 var Axes = require('../cartesian/axes');
-var getAutoRange = require('../cartesian/autorange').getAutoRange;
+const { concatExtremes, getAutoRange, makePadFn } = require('../cartesian/autorange');
 var dragElement = require('../../components/dragelement');
 var prepSelect = require('../../components/selections').prepSelect;
 var clearOutline = require('../../components/selections').clearOutline;
@@ -26,7 +26,7 @@ var createGeoZoom = require('./zoom');
 var constants = require('./constants');
 
 var geoUtils = require('../../lib/geo_location_utils');
-const { getFitboundsLonRange, unwrapLonRange } = geoUtils;
+const { unwrapLonRange } = geoUtils;
 var topojsonUtils = require('../../lib/topojson_utils');
 var topojsonFeature = require('topojson-client').feature;
 
@@ -237,51 +237,40 @@ proto.updateProjection = function (geoCalcData, fullLayout) {
         axLon.range = getAutoRange(gd, axLon);
         axLat.range = getAutoRange(gd, axLat);
 
-        // For point data straddling the antimeridian (±180°), the naive [min, max]
-        // longitude range above can include a large empty span; prefer the compact
-        // crossing range instead. Restricted to fitbounds='locations' with no
-        // region-bearing traces: choropleth, scattergeo `locations`, and the
-        // geojson-bbox path used by fitbounds='geojson' + locationmode='geojson-id'
-        // all carry region extents that per-point lonlat centroids don't capture.
-        if (!this.hasChoropleth && geoLayout.fitbounds === 'locations') {
-            var lons = [];
-            var hasLocationData = false;
-
-            for (var i = 0; i < geoCalcData.length; i++) {
-                var calcTrace = geoCalcData[i];
-                var fitTrace = calcTrace[0].trace;
-
-                // only visible traces contribute to the autorange above
-                if (fitTrace.visible !== true) continue;
-                if (fitTrace.locations?.length) {
-                    hasLocationData = true;
-                    break;
-                }
-                for (var j = 0; j < calcTrace.length; j++) {
-                    var lonlat = calcTrace[j].lonlat;
-                    if (lonlat) lons.push(lonlat[0]);
-                }
-            }
+        // Min/maxing the per-trace ranges above breaks when data crosses the
+        // antimeridian, since `computeBbox` unwraps an east edge past 180°. Bounding
+        // every coordinate at once lets `geoBounds` pick the compact range instead.
+        const fitCoordParts = [];
 
-            if (!hasLocationData) {
-                var fitLonRange = getFitboundsLonRange(lons);
-                if (fitLonRange) {
-                    // getFitboundsLonRange returns a tight [min, max]. getAutoRange
-                    // pads the naive range (for marker size and the standard
-                    // margin), so scale that padding to the narrower crossing range
-                    // and apply it, keeping markers off the frame edge as on any
-                    // other fitbounds map. The padding is symmetric, so the
-                    // mid-longitude the projection centers on is unchanged.
-                    var lonDataSpan = Lib.aggNums(Math.max, null, lons) - Lib.aggNums(Math.min, null, lons);
-                    var lonPad =
-                        lonDataSpan > 0
-                            ? (((axLon.range[1] - axLon.range[0] - lonDataSpan) / 2) *
-                                  (fitLonRange[1] - fitLonRange[0])) /
-                              lonDataSpan
-                            : 0;
-                    axLon.range = [fitLonRange[0] - lonPad, fitLonRange[1] + lonPad];
-                }
-            }
+        for (const calcTrace of geoCalcData) {
+            const fitTrace = calcTrace[0].trace;
+            if (fitTrace.visible !== true) continue;
+
+            if (fitTrace._module.fitCoords) fitCoordParts.push(fitTrace._module.fitCoords(calcTrace, geoLayout));
+        }
+
+        // Get the extents in the same manner as getAutoRange
+        const lonExtremes = concatExtremes(gd, axLon);
+        const lonDataMin = lonExtremes.min.reduce((min, { val }) => Math.min(min, val), Infinity);
+        const lonDataMax = lonExtremes.max.reduce((max, { val }) => Math.max(max, val), -Infinity);
+        const lonDataSpan = lonDataMax - lonDataMin;
+        const fitBbox = geoUtils.boundsOfCoords(fitCoordParts.flat());
+        const [fitWest, , fitEast] = fitBbox || [];
+        const fitSpan = fitEast - fitWest;
+        const useFit = Boolean(fitBbox) && (lonDataSpan > 360 || (lonDataSpan < 360 && fitSpan < lonDataSpan));
+
+        // Add padding in the same manner as getAutoRange. Ideally this could use an
+        // underlying helper function, but that doesn't exist yet so we handle it like this.
+        if (useFit) {
+            const getPadMin = makePadFn(fullLayout, axLon, 0);
+            const getPadMax = makePadFn(fullLayout, axLon, 1);
+            const padMin = lonExtremes.min.reduce((max, pt) => Math.max(max, getPadMin(pt)), 0);
+            const padMax = lonExtremes.max.reduce((max, pt) => Math.max(max, getPadMax(pt)), 0);
+            const usable = axLon._length - padMin - padMax;
+            const paddedSpan = usable > axLon._length / 10 ? (fitSpan * axLon._length) / usable : fitSpan;
+            const fitMid = (fitWest + fitEast) / 2;
+
+            axLon.range = [fitMid - paddedSpan / 2, fitMid + paddedSpan / 2];
         }
 
         var midLon = (axLon.range[0] + axLon.range[1]) / 2;

src/traces/choropleth/index.js

@@ -6,6 +6,7 @@ module.exports = {
     colorbar: require('../heatmap/colorbar'),
     calc: require('./calc'),
     calcGeoJSON: require('./plot').calcGeoJSON,
+    fitCoords: require('./plot').fitCoords,
     plot: require('./plot').plot,
     style: require('./style').style,
     styleOnSelect: require('./style').styleOnSelect,

src/traces/choropleth/plot.js

@@ -38,13 +38,9 @@ function calcGeoJSON(calcTrace, fullLayout) {
             ? geoUtils.extractTraceFeature(calcTrace)
             : getTopojsonFeatures(trace, geo.topojson);
 
-    // A falsy result (Sphere feature or malformed/empty geojson) here
-    // falls back to per-feature bounds — effectively the same as
-    // fitbounds === 'locations' behavior.
-    const bboxGeojson =
-        geoLayout.fitbounds === 'geojson' && locationmode === 'geojson-id'
-            ? geoUtils.computeBbox(geoUtils.getTraceGeojson(trace))
-            : null;
+    // A falsy result (another fitbounds mode, or a Sphere/malformed/empty geojson)
+    // falls back to per-feature bounds, similar to `fitbounds === 'locations'`.
+    const bboxGeojson = geoUtils.fitGeojsonBbox(trace, geoLayout);
 
     var lonArray = [];
     var latArray = [];
@@ -85,7 +81,29 @@ function calcGeoJSON(calcTrace, fullLayout) {
     trace._extremes.lat = findExtremes(geoLayout.lataxis._ax, latArray, opts);
 }
 
+/**
+ * Append the coordinates this trace contributes to a subplot-wide `fitbounds`
+ * bounding box. Keeping it all together allows for proper auto-fitting of
+ * geometry that crosses the antimeridian.
+ *
+ * @param {Array} calcTrace - calcdata for this trace
+ * @param {object} geoLayout - The subplot's `fullLayout` entry
+ * @return {Array} `[lon, lat]` pairs
+ */
+function fitCoords(calcTrace, geoLayout) {
+    const geojsonCoords = geoUtils.fitGeojsonCoords(calcTrace[0].trace, geoLayout);
+    if (geojsonCoords.length) return geojsonCoords;
+
+    const parts = [];
+    for (const calcPt of calcTrace) {
+        if (calcPt.geojson) parts.push(geoUtils.coordsOf(calcPt.geojson));
+    }
+
+    return parts.flat();
+}
+
 module.exports = {
     calcGeoJSON,
+    fitCoords,
     plot
 };

src/traces/scattergeo/index.js

@@ -7,6 +7,7 @@ module.exports = {
     formatLabels: require('./format_labels'),
     calc: require('./calc'),
     calcGeoJSON: require('./plot').calcGeoJSON,
+    fitCoords: require('./plot').fitCoords,
     plot: require('./plot').plot,
     style: require('./style'),
     styleOnSelect: require('../scatter/style').styleOnSelect,

src/traces/scattergeo/plot.js

@@ -100,11 +100,10 @@ function calcGeoJSON(calcTrace, fullLayout) {
     var opts = { padded: true };
     var lonArray;
     var latArray;
-
-    const bboxGeojson =
-        geoLayout.fitbounds === 'geojson' && trace.locationmode === 'geojson-id'
-            ? geoUtils.computeBbox(geoUtils.getTraceGeojson(trace))
-            : null;
+
+    // A falsy result (another fitbounds mode, or a Sphere/malformed/empty geojson)
+    // falls back to the plotted points, similar to `fitbounds === 'locations'`.
+    const bboxGeojson = geoUtils.fitGeojsonBbox(trace, geoLayout);
 
     if (bboxGeojson) {
         const [west, south, east, north] = bboxGeojson;
@@ -126,7 +125,29 @@ function calcGeoJSON(calcTrace, fullLayout) {
     trace._extremes.lat = findExtremes(geoLayout.lataxis._ax, latArray, opts);
 }
 
+/**
+ * Append the coordinates this trace contributes to a subplot-wide `fitbounds`
+ * bounding box. Keeping it all together allows for proper auto-fitting of
+ * geometry that crosses the antimeridian.
+ *
+ * @param {Array} calcTrace - calcdata for this trace
+ * @param {object} geoLayout - The subplot's `fullLayout` entry
+ * @return {Array} `[lon, lat]` pairs
+ */
+function fitCoords(calcTrace, geoLayout) {
+    const geojsonCoords = geoUtils.fitGeojsonCoords(calcTrace[0].trace, geoLayout);
+    if (geojsonCoords.length) return geojsonCoords;
+
+    const coords = [];
+    for (const calcPt of calcTrace) {
+        if (calcPt.lonlat && isFinite(calcPt.lonlat[0])) coords.push(calcPt.lonlat);
+    }
+
+    return coords;
+}
+
 module.exports = {
-    calcGeoJSON: calcGeoJSON,
-    plot: plot
+    calcGeoJSON,
+    fitCoords,
+    plot
 };

test/image/mocks/geo_country-names.json

@@ -1012,12 +1012,14 @@
     "geo": {
       "projection": {
         "type": "robinson"
-      }
+      },
+      "fitbounds": false
     },
     "geo2": {
       "projection": {
         "type": "robinson"
-      }
+      },
+      "fitbounds": false
     },
     "showlegend": false
   }