Merge pull request #4216 from plotly/automargin-max-number-of-redraws

More auto-margin fixes (some of them temporary)

Author: etpinard

src/plots/cartesian/axes.js

@@ -1643,6 +1643,7 @@ axes.draw = function(gd, arg, opts) {
  * - ax._anchorAxis
  * - ax._subplotsWith
  * - ax._counterDomainMin, ax._counterDomainMax (optionally, from linkSubplots)
+ * - ax._tickAngles (on redraw only, old value relinked during supplyDefaults)
  * - ax._mainLinePosition (from lsInner)
  * - ax._mainMirrorPosition
  * - ax._linepositions
@@ -1684,6 +1685,8 @@ axes.drawOne = function(gd, ax, opts) {
     // - stash tickLabels selection, so that drawTitle can use it to scoot title
     ax._selections = {};
     // stash tick angle (including the computed 'auto' values) per tick-label class
+    // linkup 'previous' tick angles on redraws
+    if(ax._tickAngles) ax._prevTickAngles = ax._tickAngles;
     ax._tickAngles = {};
     // measure [in px] between axis position and outward-most part of bounding box
     // (touching either the tick label or ticks)
@@ -1897,12 +1900,12 @@ axes.drawOne = function(gd, ax, opts) {
                 if(llbbox.width > 0) {
                     var rExtra = llbbox.right - (ax._offset + ax._length);
                     if(rExtra > 0) {
-                        push.x = 1;
+                        push.xr = 1;
                         push.r = rExtra;
                     }
                     var lExtra = ax._offset - llbbox.left;
                     if(lExtra > 0) {
-                        push.x = 0;
+                        push.xl = 0;
                         push.l = lExtra;
                     }
                 }
@@ -1917,12 +1920,12 @@ axes.drawOne = function(gd, ax, opts) {
                 if(llbbox.height > 0) {
                     var bExtra = llbbox.bottom - (ax._offset + ax._length);
                     if(bExtra > 0) {
-                        push.y = 0;
+                        push.yb = 0;
                         push.b = bExtra;
                     }
                     var tExtra = ax._offset - llbbox.top;
                     if(tExtra > 0) {
-                        push.y = 1;
+                        push.yt = 1;
                         push.t = tExtra;
                     }
                 }
@@ -2400,6 +2403,7 @@ axes.drawZeroLine = function(gd, ax, opts) {
  *  - {number} tickangle
  *  - {object (optional)} _selections
  *  - {object} (optional)} _tickAngles
+ *  - {object} (optional)} _prevTickAngles
  * @param {object} opts
  * - {array of object} vals (calcTicks output-like)
  * - {d3 selection} layer
@@ -2416,13 +2420,14 @@ axes.drawZeroLine = function(gd, ax, opts) {
 axes.drawLabels = function(gd, ax, opts) {
     opts = opts || {};
 
+    var fullLayout = gd._fullLayout;
     var axId = ax._id;
     var axLetter = axId.charAt(0);
     var cls = opts.cls || axId + 'tick';
     var vals = opts.vals;
     var labelFns = opts.labelFns;
     var tickAngle = opts.secondary ? 0 : ax.tickangle;
-    var lastAngle = (ax._tickAngles || {})[cls];
+    var prevAngle = (ax._prevTickAngles || {})[cls];
 
     var tickLabels = opts.layer.selectAll('g.' + cls)
         .data(ax.showticklabels ? vals : [], tickDataFn);
@@ -2507,17 +2512,17 @@ axes.drawLabels = function(gd, ax, opts) {
     // do this without waiting, using the last calculated angle to
     // minimize flicker, then do it again when we know all labels are
     // there, putting back the prescribed angle to check for overlaps.
-    positionLabels(tickLabels, lastAngle || tickAngle);
+    positionLabels(tickLabels, (prevAngle + 1) ? prevAngle : tickAngle);
 
     function allLabelsReady() {
         return labelsReady.length && Promise.all(labelsReady);
     }
 
+    var autoangle = null;
+
     function fixLabelOverlaps() {
         positionLabels(tickLabels, tickAngle);
 
-        var autoangle = null;
-
         // check for auto-angling if x labels overlap
         // don't auto-angle at all for log axes with
         // base and digit format
@@ -2584,19 +2589,36 @@ axes.drawLabels = function(gd, ax, opts) {
                 positionLabels(tickLabels, autoangle);
             }
         }
-
-        if(ax._tickAngles) {
-            ax._tickAngles[cls] = autoangle === null ?
-                (isNumeric(tickAngle) ? tickAngle : 0) :
-                autoangle;
-        }
     }
 
     if(ax._selections) {
         ax._selections[cls] = tickLabels;
     }
 
-    var done = Lib.syncOrAsync([allLabelsReady, fixLabelOverlaps]);
+    var seq = [allLabelsReady];
+
+    // N.B. during auto-margin redraws, if the axis fixed its label overlaps
+    // by rotating 90 degrees, do not attempt to re-fix its label overlaps
+    // as this can lead to infinite redraw loops!
+    if(ax.automargin && fullLayout._redrawFromAutoMarginCount && prevAngle === 90) {
+        autoangle = 90;
+        seq.push(function() {
+            positionLabels(tickLabels, prevAngle);
+        });
+    } else {
+        seq.push(fixLabelOverlaps);
+    }
+
+    // save current tick angle for future redraws
+    if(ax._tickAngles) {
+        seq.push(function() {
+            ax._tickAngles[cls] = autoangle === null ?
+                (isNumeric(tickAngle) ? tickAngle : 0) :
+                autoangle;
+        });
+    }
+
+    var done = Lib.syncOrAsync(seq);
     if(done && done.then) gd._promises.push(done);
     return done;
 };

src/plots/plots.js

@@ -1960,7 +1960,19 @@ plots.doAutoMargin = function(gd) {
         } else {
             fullLayout._redrawFromAutoMarginCount = 1;
         }
-        return Registry.call('plot', gd);
+
+        // Always allow at least one redraw and give each margin-push
+        // call 3 loops to converge. Of course, for most cases this way too many,
+        // but let's keep things on the safe side until we fix our
+        // auto-margin pipeline problems:
+        // https://github.com/plotly/plotly.js/issues/2704
+        var maxNumberOfRedraws = 3 * (1 + Object.keys(pushMarginIds).length);
+
+        if(fullLayout._redrawFromAutoMarginCount < maxNumberOfRedraws) {
+            return Registry.call('plot', gd);
+        } else {
+            Lib.warn('Too many auto-margin redraws.');
+        }
     }
 };
 

test/image/baselines/automargin-small-width.png

@@ -0,0 +1,21 @@
+{
+  "data": [
+    {
+      "type": "bar",
+      "x": [ "Montreal", "Tofu Bowl", "Tropical Beaches" ],
+      "y": [ 3, 1, 2 ]
+    }
+  ],
+  "layout": {
+    "margin": {
+      "l": 20,
+      "r": 0,
+      "b": 30,
+      "t": 15
+    },
+    "xaxis": {
+      "automargin": true
+    },
+    "width": 150
+  }
+}