fix #3003 - fill and layering for multiple stackgroups and unstacked scatter traces

alexcjohnson · alexcjohnson · commit 37ee992cefc2 · 2018-09-12T22:16:52.000-04:00
diff --git a/src/traces/scatter/calc.js b/src/traces/scatter/calc.js
@@ -153,7 +153,7 @@ function calcAxisExpansion(gd, trace, xa, ya, x, y, ppad) {
     var fullLayout = gd._fullLayout;
     var xId = xa._id;
     var yId = ya._id;
-    var firstScatter = fullLayout._firstScatter[xId + yId + trace.type] === trace.uid;
+    var firstScatter = fullLayout._firstScatter[firstScatterGroup(trace)] === trace.uid;
     var stackOrientation = (getStackOpts(trace, fullLayout, xa, ya) || {}).orientation;
     var fill = trace.fill;
 
@@ -257,9 +257,15 @@ function calcMarkerSize(trace, serieslen) {
  * per-trace calc this will get confused.
  */
 function setFirstScatter(fullLayout, trace) {
-    var subplotAndType = trace.xaxis + trace.yaxis + trace.type;
+    var group = firstScatterGroup(trace);
     var firstScatter = fullLayout._firstScatter;
-    if(!firstScatter[subplotAndType]) firstScatter[subplotAndType] = trace.uid;
+    if(!firstScatter[group]) firstScatter[group] = trace.uid;
+}
+
+function firstScatterGroup(trace) {
+    var stackGroup = trace.stackgroup;
+    return trace.xaxis + trace.yaxis + trace.type +
+        (stackGroup ? '-' + stackGroup : '');
 }
 
 function getStackOpts(trace, fullLayout, xa, ya) {
diff --git a/src/traces/scatter/link_traces.js b/src/traces/scatter/link_traces.js
@@ -8,12 +8,56 @@
 
 'use strict';
 
+var LINKEDFILLS = {tonextx: 1, tonexty: 1, tonext: 1};
+
 module.exports = function linkTraces(gd, plotinfo, cdscatter) {
-    var trace, i;
-    var prevtrace = null;
+    var trace, i, group, prevtrace, groupIndex;
 
-    for(i = 0; i < cdscatter.length; ++i) {
+    // first sort traces to keep stacks & filled-together groups together
+    var groupIndices = {};
+    var needsSort = false;
+    var prevGroupIndex = -1;
+    var nextGroupIndex = 0;
+    var prevUnstackedGroupIndex = -1;
+    for(i = 0; i < cdscatter.length; i++) {
         trace = cdscatter[i][0].trace;
+        group = trace.stackgroup || '';
+        if(group) {
+            if(group in groupIndices) {
+                groupIndex = groupIndices[group];
+            }
+            else {
+                groupIndex = groupIndices[group] = nextGroupIndex;
+                nextGroupIndex++;
+            }
+        }
+        else if(trace.fill in LINKEDFILLS && prevUnstackedGroupIndex >= 0) {
+            groupIndex = prevUnstackedGroupIndex;
+        }
+        else {
+            groupIndex = prevUnstackedGroupIndex = nextGroupIndex;
+            nextGroupIndex++;
+        }
+
+        if(groupIndex < prevGroupIndex) needsSort = true;
+        trace._groupIndex = prevGroupIndex = groupIndex;
+    }
+
+    var cdscatterSorted = cdscatter.slice();
+    if(needsSort) {
+        cdscatterSorted.sort(function(a, b) {
+            var traceA = a[0].trace;
+            var traceB = b[0].trace;
+            return (traceA._groupIndex - traceB._groupIndex) ||
+                (traceA.index - traceB.index);
+        });
+    }
+
+    // now link traces to each other
+    var prevtraces = {};
+    for(i = 0; i < cdscatterSorted.length; i++) {
+        trace = cdscatterSorted[i][0].trace;
+        group = trace.stackgroup || '';
 
         // Note: The check which ensures all cdscatter here are for the same axis and
         // are either cartesian or scatterternary has been removed. This code assumes
@@ -22,17 +66,20 @@ module.exports = function linkTraces(gd, plotinfo, cdscatter) {
         if(trace.visible === true) {
             trace._nexttrace = null;
 
-            if(['tonextx', 'tonexty', 'tonext'].indexOf(trace.fill) !== -1) {
-                trace._prevtrace = prevtrace;
+            if(trace.fill in LINKEDFILLS) {
+                prevtrace = prevtraces[group];
+                trace._prevtrace = prevtrace || null;
 
                 if(prevtrace) {
                     prevtrace._nexttrace = trace;
                 }
             }
 
-            prevtrace = trace;
+            prevtraces[group] = trace;
         } else {
             trace._prevtrace = trace._nexttrace = null;
         }
     }
+
+    return cdscatterSorted;
 };
diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js
@@ -23,43 +23,29 @@ var linkTraces = require('./link_traces');
 var polygonTester = require('../../lib/polygon').tester;
 
 module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transitionOpts, makeOnCompleteCallback) {
-    var i, uids, join, onComplete;
+    var join, onComplete;
 
     // If transition config is provided, then it is only a partial replot and traces not
     // updated are removed.
     var isFullReplot = !transitionOpts;
     var hasTransition = !!transitionOpts && transitionOpts.duration > 0;
 
+    // Link traces so the z-order of fill layers is correct
+    var cdscatterSorted = linkTraces(gd, plotinfo, cdscatter);
+
     join = scatterLayer.selectAll('g.trace')
-        .data(cdscatter, function(d) { return d[0].trace.uid; });
+        .data(cdscatterSorted, function(d) { return d[0].trace.uid; });
 
     // Append new traces:
     join.enter().append('g')
         .attr('class', function(d) {
             return 'trace scatter trace' + d[0].trace.uid;
         })
         .style('stroke-miterlimit', 2);
-
-    // After the elements are created but before they've been draw, we have to perform
-    // this extra step of linking the traces. This allows appending of fill layers so that
-    // the z-order of fill layers is correct.
-    linkTraces(gd, plotinfo, cdscatter);
+    join.order();
 
     createFills(gd, join, plotinfo);
 
-    // Sort the traces, once created, so that the ordering is preserved even when traces
-    // are shown and hidden. This is needed since we're not just wiping everything out
-    // and recreating on every update.
-    for(i = 0, uids = {}; i < cdscatter.length; i++) {
-        uids[cdscatter[i][0].trace.uid] = i;
-    }
-
-    join.sort(function(a, b) {
-        var idx1 = uids[a[0].trace.uid];
-        var idx2 = uids[b[0].trace.uid];
-        return idx1 - idx2;
-    });
-
     if(hasTransition) {
         if(makeOnCompleteCallback) {
             // If it was passed a callback to register completion, make a callback. If
@@ -82,12 +68,12 @@ module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transition
             // Must run the selection again since otherwise enters/updates get grouped together
             // and these get executed out of order. Except we need them in order!
             scatterLayer.selectAll('g.trace').each(function(d, i) {
-                plotOne(gd, i, plotinfo, d, cdscatter, this, transitionOpts);
+                plotOne(gd, i, plotinfo, d, cdscatterSorted, this, transitionOpts);
             });
         });
     } else {
         scatterLayer.selectAll('g.trace').each(function(d, i) {
-            plotOne(gd, i, plotinfo, d, cdscatter, this, transitionOpts);
+            plotOne(gd, i, plotinfo, d, cdscatterSorted, this, transitionOpts);
         });
     }
 
diff --git a/test/image/baselines/stacked_area_groups.png b/test/image/baselines/stacked_area_groups.png
diff --git a/test/image/mocks/stacked_area_groups.json b/test/image/mocks/stacked_area_groups.json
@@ -0,0 +1,51 @@
+{
+  "data": [
+    {
+        "x": [0, 2, 4], "y": [6, 1, 6], "line": {"color": "#000"},
+        "name": "bottom", "legendgroup": "l"
+    }, {
+        "x0": -0.5, "y": [1, 2, 3, 4, 5], "name": "a 1",
+        "stackgroup": "a", "legendgroup": "a",
+        "line": {"color": "#f00"}, "fillcolor": "rgba(255,0,0,0.8)"
+    }, {
+        "x0": 0.5, "y": [5, 4, 3, 2, 1], "name": "b 1",
+        "stackgroup": "b", "legendgroup": "b",
+        "line": {"color": "#00f"}, "fillcolor": "rgba(0,0,255,0.8)"
+    }, {
+        "x0": -0.5, "y": [1, 1, 1, 1, 1], "name": "a 2",
+        "stackgroup": "a", "legendgroup": "a",
+        "line": {"color": "#f80"}, "fillcolor": "rgba(255,136,0,0.8)"
+    }, {
+        "x0": 1, "y": [1, 2, 1], "name": "unstacked 1",
+        "fill": "tozeroy", "legendgroup": "u",
+        "line": {"color": "#888"}, "fillcolor": "rgba(136,136,136,0.8)"
+    }, {
+        "x0": -0.5, "y": [1, 1, 1, 1, 1], "name": "a 3",
+        "stackgroup": "a", "legendgroup": "a",
+        "line": {"color": "#ff0"}, "fillcolor": "rgba(255,255,0,0.8)"
+    }, {
+        "x0": 0.5, "y": [1, 1, 1, 1, 1], "name": "b 2",
+        "stackgroup": "b", "legendgroup": "b",
+        "line": {"color": "#80f"}, "fillcolor": "rgba(136,0,255,0.8)"
+    }, {
+        "x0": 1, "y": [5, 8, 5], "name": "unstacked 2",
+        "fill": "tonexty", "legendgroup": "u",
+        "line": {"color": "#ccc"}, "fillcolor": "rgba(204,204,204,0.8)"
+    }, {
+        "x": [0, 2, 4], "y": [7, 3, 7], "line": {"color": "#0c0"},
+        "name": "top", "legendgroup": "l"
+    }, {
+        "x0": 0.5, "y": [1, 1, 1, 1, 1], "name": "b 3",
+        "stackgroup": "b", "legendgroup": "b",
+        "line": {"color": "#f0f"}, "fillcolor": "rgba(255,0,255,0.8)"
+    }
+  ],
+  "layout": {
+    "width": 500,
+    "height": 500,
+    "title": "Stack groups and unstacked filled traces",
+    "xaxis": {"title": "infer zero"},
+    "xaxis2": {"title": "interpolate"},
+    "grid": {"columns": 1, "rows": 2, "pattern": "independent"}
+  }
+}
