try to remove lsInner dependence in Axes.drawOne

etpinard · etpinard · commit b47aa96f40a2 · 2019-09-23T17:15:41.000-04:00
- add Axes.getAxisLinePosition and wrappers
  getAxisMirrorLinePosition and getAxisSubplotLinePositions
  wrappers and remove ax._mainLinePosition, ax._mainMirrorPosition
  and ax._linepositions stashes
- replace ax._lw by Drawing.crispRound calls
- replace ax._gw by Drawing.crispRound call
- Axes.getAxisLinePosition in graph_interact (!)
diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js
@@ -55,7 +55,6 @@ function lsInner(gd) {
     var gs = fullLayout._size;
     var pad = gs.p;
     var axList = Axes.list(gd, '', true);
-    var i, subplot, plotinfo, ax, xa, ya;
 
     fullLayout._paperdiv.style({
         width: (gd._context.responsive && fullLayout.autosize && !gd._context._hasZeroWidth && !gd.layout.width) ? '100%' : fullLayout.width + 'px',
@@ -74,43 +73,7 @@ function lsInner(gd) {
     if(!fullLayout._has('cartesian')) {
         return Plots.previousPromises(gd);
     }
-
-    function getLinePosition(ax, counterAx, side) {
-        var lwHalf = ax._lw / 2;
-
-        if(ax._id.charAt(0) === 'x') {
-            if(!counterAx) return gs.t + gs.h * (1 - (ax.position || 0)) + (lwHalf % 1);
-            else if(side === 'top') return counterAx._offset - pad - lwHalf;
-            return counterAx._offset + counterAx._length + pad + lwHalf;
-        }
-
-        if(!counterAx) return gs.l + gs.w * (ax.position || 0) + (lwHalf % 1);
-        else if(side === 'right') return counterAx._offset + counterAx._length + pad + lwHalf;
-        return counterAx._offset - pad - lwHalf;
-    }
-
-    // some preparation of axis position info
-    for(i = 0; i < axList.length; i++) {
-        ax = axList[i];
-
-        var counterAx = ax._anchorAxis;
-
-        // clear axis line positions, to be set in the subplot loop below
-        ax._linepositions = {};
-
-        // stash crispRounded linewidth so we don't need to pass gd all over the place
-        ax._lw = Drawing.crispRound(gd, ax.linewidth, 1);
-
-        // figure out the main axis line and main mirror line position.
-        // it's easier to follow the logic if we handle these separately from
-        // ax._linepositions, which are only used by mirror=allticks
-        // for non-main-subplot ticks, and mirror=all(ticks)? for zero line
-        // hiding logic
-        ax._mainLinePosition = getLinePosition(ax, counterAx, ax.side);
-        ax._mainMirrorPosition = (ax.mirror && counterAx) ?
-            getLinePosition(ax, counterAx,
-                alignmentConstants.OPPOSITE_SIDE[ax.side]) : null;
-    }
+    var i, subplot, plotinfo, xa, ya;
 
     // figure out which backgrounds we need to draw,
     // and in which layers to put them
@@ -253,9 +216,10 @@ function lsInner(gd) {
 
     function mainPath(ax, pathFn, pathFnFree) {
         if(!ax.showline || subplot !== ax._mainSubplot) return '';
-        if(!ax._anchorAxis) return pathFnFree(ax._mainLinePosition);
-        var out = pathFn(ax._mainLinePosition);
-        if(ax.mirror) out += pathFn(ax._mainMirrorPosition);
+        var mainLinePosition = Axes.getAxisLinePosition(gd, ax);
+        if(!ax._anchorAxis) return pathFnFree(mainLinePosition);
+        var out = pathFn(mainLinePosition);
+        if(ax.mirror) out += pathFn(Axes.getAxisMirrorLinePosition(gd, ax));
         return out;
     }
 
@@ -280,30 +244,25 @@ function lsInner(gd) {
          */
         var xPath = 'M0,0';
         if(shouldShowLinesOrTicks(xa, subplot)) {
-            leftYLineWidth = findCounterAxisLineWidth(xa, 'left', ya, axList);
+            leftYLineWidth = findCounterAxisLineWidth(gd, xa, 'left', ya, axList);
             xLinesXLeft = xa._offset - (leftYLineWidth ? (pad + leftYLineWidth) : 0);
-            rightYLineWidth = findCounterAxisLineWidth(xa, 'right', ya, axList);
+            rightYLineWidth = findCounterAxisLineWidth(gd, xa, 'right', ya, axList);
             xLinesXRight = xa._offset + xa._length + (rightYLineWidth ? (pad + rightYLineWidth) : 0);
-            xLinesYBottom = getLinePosition(xa, ya, 'bottom');
-            xLinesYTop = getLinePosition(xa, ya, 'top');
+            xLinesYBottom = Axes.getAxisLinePosition(gd, xa, ya, 'bottom');
+            xLinesYTop = Axes.getAxisLinePosition(gd, xa, ya, 'top');
 
             // save axis line positions for extra ticks to reference
             // each subplot that gets ticks from "allticks" gets an entry:
             //    [left or bottom, right or top]
             extraSubplot = (!xa._anchorAxis || subplot !== xa._mainSubplot);
-            if(extraSubplot && (xa.mirror === 'allticks' || xa.mirror === 'all')) {
-                xa._linepositions[subplot] = [xLinesYBottom, xLinesYTop];
-            }
-
             xPath = mainPath(xa, xLinePath, xLinePathFree);
             if(extraSubplot && xa.showline && (xa.mirror === 'all' || xa.mirror === 'allticks')) {
                 xPath += xLinePath(xLinesYBottom) + xLinePath(xLinesYTop);
             }
 
             plotinfo.xlines
-                .style('stroke-width', xa._lw + 'px')
-                .call(Color.stroke, xa.showline ?
-                    xa.linecolor : 'rgba(0,0,0,0)');
+                .style('stroke-width', crispRoundLineWidth(gd, xa) + 'px')
+                .call(Color.stroke, xa.showline ? xa.linecolor : 'rgba(0,0,0,0)');
         }
         plotinfo.xlines.attr('d', xPath);
 
@@ -320,27 +279,22 @@ function lsInner(gd) {
          */
         var yPath = 'M0,0';
         if(shouldShowLinesOrTicks(ya, subplot)) {
-            connectYBottom = findCounterAxisLineWidth(ya, 'bottom', xa, axList);
+            connectYBottom = findCounterAxisLineWidth(gd, ya, 'bottom', xa, axList);
             yLinesYBottom = ya._offset + ya._length + (connectYBottom ? pad : 0);
-            connectYTop = findCounterAxisLineWidth(ya, 'top', xa, axList);
+            connectYTop = findCounterAxisLineWidth(gd, ya, 'top', xa, axList);
             yLinesYTop = ya._offset - (connectYTop ? pad : 0);
-            yLinesXLeft = getLinePosition(ya, xa, 'left');
-            yLinesXRight = getLinePosition(ya, xa, 'right');
+            yLinesXLeft = Axes.getAxisLinePosition(gd, ya, xa, 'left');
+            yLinesXRight = Axes.getAxisLinePosition(gd, ya, xa, 'right');
 
             extraSubplot = (!ya._anchorAxis || subplot !== ya._mainSubplot);
-            if(extraSubplot && (ya.mirror === 'allticks' || ya.mirror === 'all')) {
-                ya._linepositions[subplot] = [yLinesXLeft, yLinesXRight];
-            }
-
             yPath = mainPath(ya, yLinePath, yLinePathFree);
             if(extraSubplot && ya.showline && (ya.mirror === 'all' || ya.mirror === 'allticks')) {
                 yPath += yLinePath(yLinesXLeft) + yLinePath(yLinesXRight);
             }
 
             plotinfo.ylines
-                .style('stroke-width', ya._lw + 'px')
-                .call(Color.stroke, ya.showline ?
-                    ya.linecolor : 'rgba(0,0,0,0)');
+                .style('stroke-width', crispRoundLineWidth(gd, ya) + 'px')
+                .call(Color.stroke, ya.showline ? ya.linecolor : 'rgba(0,0,0,0)');
         }
         plotinfo.ylines.attr('d', yPath);
     }
@@ -360,9 +314,9 @@ function shouldShowLinesOrTicks(ax, subplot) {
  * It's assumed that counterAx is known to overlay the subplot we're working on
  * but it may not be its main axis.
  */
-function shouldShowLineThisSide(ax, side, counterAx) {
+function shouldShowLineThisSide(gd, ax, side, counterAx) {
     // does counterAx get a line at all?
-    if(!counterAx.showline || !counterAx._lw) return false;
+    if(!counterAx.showline || !crispRoundLineWidth(gd, ax)) return false;
 
     // are we drawing *all* lines for counterAx?
     if(counterAx.mirror === 'all' || counterAx.mirror === 'allticks') return true;
@@ -388,19 +342,23 @@ function shouldShowLineThisSide(ax, side, counterAx) {
  * then at all other potential counteraxes on or overlaying this subplot.
  * Take the line width from the first one that has a line.
  */
-function findCounterAxisLineWidth(ax, side, counterAx, axList) {
-    if(shouldShowLineThisSide(ax, side, counterAx)) {
-        return counterAx._lw;
+function findCounterAxisLineWidth(gd, ax, side, counterAx, axList) {
+    if(shouldShowLineThisSide(gd, ax, side, counterAx)) {
+        return crispRoundLineWidth(gd, counterAx);
     }
     for(var i = 0; i < axList.length; i++) {
         var axi = axList[i];
-        if(axi._mainAxis === counterAx._mainAxis && shouldShowLineThisSide(ax, side, axi)) {
-            return axi._lw;
+        if(axi._mainAxis === counterAx._mainAxis && shouldShowLineThisSide(gd, ax, side, axi)) {
+            return crispRoundLineWidth(gd, axi);
         }
     }
     return 0;
 }
 
+function crispRoundLineWidth(gd, ax) {
+    return Drawing.crispRound(gd, ax.linewidth, 1);
+}
+
 exports.drawMainTitle = function(gd) {
     var fullLayout = gd._fullLayout;
 
diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js
@@ -1643,9 +1643,6 @@ axes.draw = function(gd, arg, opts) {
  * - ax._anchorAxis
  * - ax._subplotsWith
  * - ax._counterDomainMin, ax._counterDomainMax (optionally, from linkSubplots)
- * - ax._mainLinePosition (from lsInner)
- * - ax._mainMirrorPosition
- * - ax._linepositions
  *
  * Fills in:
  * - ax._vals:
@@ -1666,10 +1663,11 @@ axes.drawOne = function(gd, ax, opts) {
     var axId = ax._id;
     var axLetter = axId.charAt(0);
     var counterLetter = axes.counterLetter(axId);
-    var mainLinePosition = ax._mainLinePosition;
-    var mainMirrorPosition = ax._mainMirrorPosition;
+    var mainLinePosition = axes.getAxisLinePosition(gd, ax);
+    var mainMirrorPosition = axes.getAxisMirrorLinePosition(gd, ax);
     var mainPlotinfo = fullLayout._plots[ax._mainSubplot];
     var mainAxLayer = mainPlotinfo[axLetter + 'axislayer'];
+    var subplotsWithAx = ax._subplotsWith;
 
     var vals = ax._vals = axes.calcTicks(ax);
 
@@ -1721,8 +1719,6 @@ axes.drawOne = function(gd, ax, opts) {
     var dividerVals = getDividerVals(ax, vals);
 
     if(!fullLayout._hasOnlyLargeSploms) {
-        var subplotsWithAx = ax._subplotsWith;
-
         // keep track of which subplots (by main counter axis) we've already
         // drawn grids for, so we don't overdraw overlaying subplots
         var finishedGrids = {};
@@ -1757,7 +1753,6 @@ axes.drawOne = function(gd, ax, opts) {
     }
 
     var tickSigns = axes.getTickSigns(ax);
-    var tickSubplots = [];
 
     if(ax.ticks) {
         var mainTickPath = axes.makeTickPath(ax, mainLinePosition, tickSigns[2]);
@@ -1790,26 +1785,18 @@ axes.drawOne = function(gd, ax, opts) {
             path: tickPath,
             transFn: transFn
         });
-
-        if(ax.mirror === 'allticks') {
-            tickSubplots = Object.keys(ax._linepositions || {});
-        }
     }
 
-    for(i = 0; i < tickSubplots.length; i++) {
-        sp = tickSubplots[i];
-        plotinfo = fullLayout._plots[sp];
-        // [bottom or left, top or right], free and main are handled above
-        var linepositions = ax._linepositions[sp] || [];
-        var spTickPath = axes.makeTickPath(ax, linepositions[0], tickSigns[0]) +
-            axes.makeTickPath(ax, linepositions[1], tickSigns[1]);
-
-        axes.drawTicks(gd, ax, {
-            vals: tickVals,
-            layer: plotinfo[axLetter + 'axislayer'],
-            path: spTickPath,
-            transFn: transFn
-        });
+    for(i = 0; i < subplotsWithAx.length; i++) {
+        var lp = getAxisSubplotLinePositions(gd, ax, subplotsWithAx[i]);
+        if(lp) {
+            axes.drawTicks(gd, ax, {
+                vals: tickVals,
+                layer: plotinfo[axLetter + 'axislayer'],
+                path: axes.makeTickPath(ax, lp[0], tickSigns[0]) + axes.makeTickPath(ax, lp[1], tickSigns[1]),
+                transFn: transFn
+            });
+        }
     }
 
     var seq = [];
@@ -2333,12 +2320,10 @@ axes.drawGrid = function(gd, ax, opts) {
         .classed(cls, 1)
         .classed('crisp', opts.crisp !== false);
 
-    ax._gw = Drawing.crispRound(gd, ax.gridwidth, 1);
-
     grid.attr('transform', opts.transFn)
         .attr('d', opts.path)
         .call(Color.stroke, ax.gridcolor || '#ddd')
-        .style('stroke-width', ax._gw + 'px');
+        .style('stroke-width', Drawing.crispRound(gd, ax.gridwidth, 1) + 'px');
 
     if(typeof opts.path === 'function') grid.attr('d', opts.path);
 };
@@ -2352,7 +2337,7 @@ axes.drawGrid = function(gd, ax, opts) {
  *  - {boolean} zeroline
  *  - {number} zerolinewidth
  *  - {string} zerolinecolor
- *  - {number (optional)} _gridWidthCrispRound
+ *  - {number} gridwidth
  * @param {object} opts
  * - {d3 selection} layer
  * - {object} counterAxis (full axis object corresponding to counter axis)
@@ -2384,10 +2369,12 @@ axes.drawZeroLine = function(gd, ax, opts) {
             });
         });
 
+    var gw = Drawing.crispRound(gd, ax.gridwidth, 1);
+
     zl.attr('transform', opts.transFn)
         .attr('d', opts.path)
         .call(Color.stroke, ax.zerolinecolor || Color.defaultLine)
-        .style('stroke-width', Drawing.crispRound(gd, ax.zerolinewidth, ax._gw || 1) + 'px');
+        .style('stroke-width', Drawing.crispRound(gd, ax.zerolinewidth, gw || 1) + 'px');
 };
 
 /**
@@ -2636,6 +2623,56 @@ function drawDividers(gd, ax, opts) {
         .attr('d', opts.path);
 }
 
+/**
+ * TODO
+ *
+ * @param {}
+ */
+axes.getAxisLinePosition = function(gd, ax, counterAx, side) {
+    counterAx = counterAx || ax._anchorAxis;
+    side = side || ax.side;
+
+    var fullLayout = gd._fullLayout;
+    var gs = fullLayout._size;
+    var lwHalf = Drawing.crispRound(gd, ax.linewidth, 1) / 2;
+
+    if(ax._id.charAt(0) === 'x') {
+        if(!counterAx) return gs.t + gs.h * (1 - (ax.position || 0)) + (lwHalf % 1);
+        else if(side === 'top') return counterAx._offset - gs.p - lwHalf;
+        return counterAx._offset + counterAx._length + gs.p + lwHalf;
+    }
+
+    if(!counterAx) return gs.l + gs.w * (ax.position || 0) + (lwHalf % 1);
+    else if(side === 'right') return counterAx._offset + counterAx._length + gs.p + lwHalf;
+    return counterAx._offset - gs.p - lwHalf;
+};
+
+/**
+ * TODO
+ *
+ */
+axes.getAxisMirrorLinePosition = function(gd, ax) {
+    return (ax.mirror && ax._anchorAxis) ?
+        axes.getAxisLinePosition(gd, ax, ax._anchorAxis, OPPOSITE_SIDE[ax.side]) :
+        null;
+};
+
+/**
+ * TODO
+ *
+ */
+function getAxisSubplotLinePositions(gd, ax, sp) {
+    if((!ax._anchorAxis || sp !== ax._mainSubplot) &&
+        (ax.mirror === 'allticks' || ax.mirror === 'all')) {
+        var axLetter = ax._id.charAt(0);
+        return [
+            axes.getAxisLinePosition(gd, ax, ax._anchorAxis, {x: 'bottom', y: 'left'}[axLetter]),
+            axes.getAxisLinePosition(gd, ax, ax._anchorAxis, {x: 'top', y: 'right'}[axLetter])
+        ];
+    }
+    return false;
+}
+
 /**
  * Get axis position in px, that is the distance for the graph's
  * top (left) edge for x (y) axes.
@@ -2787,14 +2824,14 @@ function anyCounterAxLineAtZero(gd, ax, counterAxis, rng) {
             return typeof pos2 === 'number' && Math.abs(pos2 - zeroPosition) < tolerance;
         }
 
-        if(closeEnough(ax2._mainLinePosition) || closeEnough(ax2._mainMirrorPosition)) {
-            return true;
-        }
-        var linePositions = ax2._linepositions || {};
-        for(var k in linePositions) {
-            if(closeEnough(linePositions[k][0]) || closeEnough(linePositions[k][1])) {
-                return true;
-            }
+        if(closeEnough(axes.getAxisLinePosition(gd, ax)) ||
+            closeEnough(axes.getAxisMirrorLinePosition(gd, ax))) return true;
+
+        var subplotsWithAx = ax._subplotsWith;
+
+        for(var i = 0; i < subplotsWithAx.length; i++) {
+            var lp = getAxisSubplotLinePositions(gd, ax, subplotsWithAx[i]);
+            if(lp && (closeEnough(lp[0]) || closeEnough(lp[1]))) return true;
         }
     }
 
diff --git a/src/plots/cartesian/graph_interact.js b/src/plots/cartesian/graph_interact.js
@@ -12,6 +12,7 @@
 var d3 = require('d3');
 
 var Fx = require('../../components/fx');
+var Axes = require('../../plots/cartesian/axes');
 var dragElement = require('../../components/dragelement');
 var setCursor = require('../../lib/setcursor');
 
@@ -107,7 +108,7 @@ exports.initInteractions = function initInteractions(gd) {
             // these drag each axis separately
             if(subplot === xa._mainSubplot) {
                 // the y position of the main x axis line
-                var y0 = xa._mainLinePosition;
+                var y0 = Axes.getAxisLinePosition(gd, xa);
                 if(xa.side === 'top') y0 -= DRAGGERSIZE;
                 makeDragBox(gd, plotinfo, xa._offset + xa._length * 0.1, y0,
                     xa._length * 0.8, DRAGGERSIZE, '', 'ew');
@@ -119,7 +120,7 @@ exports.initInteractions = function initInteractions(gd) {
             // y axis draggers
             if(subplot === ya._mainSubplot) {
                 // the x position of the main y axis line
-                var x0 = ya._mainLinePosition;
+                var x0 = Axes.getAxisLinePosition(gd, ya);
                 if(ya.side !== 'right') x0 -= DRAGGERSIZE;
                 makeDragBox(gd, plotinfo, x0, ya._offset + ya._length * 0.1,
                     DRAGGERSIZE, ya._length * 0.8, 'ns', '');
