Group transitions to avoid race conditions

Author: rreusser

src/components/color/index.js

@@ -74,7 +74,10 @@ color.stroke = function(s, c) {
 
 color.fill = function(s, c) {
     var tc = tinycolor(c);
-    s.style({'fill': color.tinyRGB(tc), 'fill-opacity': tc.getAlpha()});
+    s.style({
+        'fill': color.tinyRGB(tc),
+        'fill-opacity': tc.getAlpha()
+    });
 };
 
 // search container for colors with the deprecated rgb(fractions) format

src/components/drawing/index.js

@@ -46,64 +46,26 @@ drawing.setRect = function(s, x, y, w, h) {
     s.call(drawing.setPosition, x, y).call(drawing.setSize, w, h);
 };
 
-drawing.translatePoints = function(s, xa, ya, trace, transitionConfig, joinDirection) {
-    var size;
-
-    var hasTransition = transitionConfig && (transitionConfig || {}).duration > 0;
-
-    if(hasTransition) {
-        size = s.size();
+drawing.translatePoint = function(d, sel, xa, ya, trace, transitionConfig, joinDirection) {
+    // put xp and yp into d if pixel scaling is already done
+    var x = d.xp || xa.c2p(d.x),
+        y = d.yp || ya.c2p(d.y);
+
+    if(isNumeric(x) && isNumeric(y)) {
+        // for multiline text this works better
+        if(this.nodeName === 'text') {
+            sel.node().attr('x', x).attr('y', y);
+        } else {
+            sel.attr('transform', 'translate(' + x + ',' + y + ')');
+        }
     }
+    else sel.remove();
+};
 
+drawing.translatePoints = function(s, xa, ya, trace, transitionConfig, joinDirection) {
     s.each(function(d, i) {
-        // put xp and yp into d if pixel scaling is already done
-        var x = d.xp || xa.c2p(d.x),
-            y = d.yp || ya.c2p(d.y),
-            p = d3.select(this);
-        if(isNumeric(x) && isNumeric(y)) {
-            // for multiline text this works better
-            if(this.nodeName === 'text') {
-                p.attr('x', x).attr('y', y);
-            } else {
-                if(hasTransition) {
-                    var trans;
-                    if(!joinDirection) {
-                        trans = p.transition()
-                        .delay(transitionConfig.delay)
-                        .duration(transitionConfig.duration)
-                        .ease(transitionConfig.ease)
-                        .attr('transform', 'translate(' + x + ',' + y + ')');
-
-                        if(trace) {
-                            trans.call(drawing.pointStyle, trace);
-                        }
-                    } else if(joinDirection === -1) {
-                        trans = p.style('opacity', 1)
-                        .transition()
-                            .duration(transitionConfig.duration)
-                            .ease(transitionConfig.ease)
-                            .style('opacity', 0)
-                            .remove();
-                    } else if(joinDirection === 1) {
-                        trans = p.attr('transform', 'translate(' + x + ',' + y + ')');
-
-                        if(trace) {
-                            trans.call(drawing.pointStyle, trace);
-                        }
-
-                        trans.style('opacity', 0)
-                        .transition()
-                            .duration(transitionConfig.duration)
-                            .ease(transitionConfig.ease)
-                            .style('opacity', 1);
-                    }
-
-                } else {
-                    p.attr('transform', 'translate(' + x + ',' + y + ')');
-                }
-            }
-        }
-        else p.remove();
+        var sel = d3.select(this);
+        drawing.translatePoint(d, sel, xa, ya, trace, transitionConfig, joinDirection);
     });
 };
 
@@ -126,6 +88,16 @@ drawing.crispRound = function(td, lineWidth, dflt) {
     return Math.round(lineWidth);
 };
 
+drawing.singleLineStyle = function(d, s, lw, lc, ld) {
+    s.style('fill', 'none')
+    var line = (((d || [])[0] || {}).trace || {}).line || {},
+        lw1 = lw || line.width||0,
+        dash = ld || line.dash || '';
+
+    Color.stroke(s, lc || line.color);
+    drawing.dashLine(s, dash, lw1);
+};
+
 drawing.lineGroupStyle = function(s, lw, lc, ld) {
     s.style('fill', 'none')
     .each(function(d) {
@@ -221,6 +193,64 @@ drawing.symbolNumber = function(v) {
     return Math.floor(Math.max(v, 0));
 };
 
+function singlePointStyle (d, sel, trace, markerScale, lineScale, marker, markerLine) {
+
+    // 'so' is suspected outliers, for box plots
+    var fillColor,
+        lineColor,
+        lineWidth;
+    if(d.so) {
+        lineWidth = markerLine.outlierwidth;
+        lineColor = markerLine.outliercolor;
+        fillColor = marker.outliercolor;
+    }
+    else {
+        lineWidth = (d.mlw + 1 || markerLine.width + 1 ||
+            // TODO: we need the latter for legends... can we get rid of it?
+            (d.trace ? d.trace.marker.line.width : 0) + 1) - 1;
+
+        if('mlc' in d) lineColor = d.mlcc = lineScale(d.mlc);
+        // weird case: array wasn't long enough to apply to every point
+        else if(Array.isArray(markerLine.color)) lineColor = Color.defaultLine;
+        else lineColor = markerLine.color;
+
+        if('mc' in d) fillColor = d.mcc = markerScale(d.mc);
+        else if(Array.isArray(marker.color)) fillColor = Color.defaultLine;
+        else fillColor = marker.color || 'rgba(0,0,0,0)';
+    }
+
+    if(d.om) {
+        // open markers can't have zero linewidth, default to 1px,
+        // and use fill color as stroke color
+        sel.call(Color.stroke, fillColor)
+            .style({
+                'stroke-width': (lineWidth || 1) + 'px',
+                fill: 'none'
+            });
+    }
+    else {
+        sel.style('stroke-width', lineWidth + 'px')
+            .call(Color.fill, fillColor);
+        if(lineWidth) {
+            sel.call(Color.stroke, lineColor);
+        }
+    }
+};
+
+drawing.singlePointStyle = function(d, sel, trace) {
+    var marker = trace.marker,
+        markerLine = marker.line;
+
+    // allow array marker and marker line colors to be
+    // scaled by given max and min to colorscales
+    var markerIn = (trace._input || {}).marker || {},
+        markerScale = drawing.tryColorscale(marker, markerIn, ''),
+        lineScale = drawing.tryColorscale(marker, markerIn, 'line.');
+
+    singlePointStyle(d, sel, trace, markerScale, lineScale, marker, markerLine);
+
+}
+
 drawing.pointStyle = function(s, trace) {
     if(!s.size()) return;
 
@@ -265,47 +295,7 @@ drawing.pointStyle = function(s, trace) {
         lineScale = drawing.tryColorscale(marker, markerIn, 'line.');
 
     s.each(function(d) {
-        // 'so' is suspected outliers, for box plots
-        var fillColor,
-            lineColor,
-            lineWidth;
-        if(d.so) {
-            lineWidth = markerLine.outlierwidth;
-            lineColor = markerLine.outliercolor;
-            fillColor = marker.outliercolor;
-        }
-        else {
-            lineWidth = (d.mlw + 1 || markerLine.width + 1 ||
-                // TODO: we need the latter for legends... can we get rid of it?
-                (d.trace ? d.trace.marker.line.width : 0) + 1) - 1;
-
-            if('mlc' in d) lineColor = d.mlcc = lineScale(d.mlc);
-            // weird case: array wasn't long enough to apply to every point
-            else if(Array.isArray(markerLine.color)) lineColor = Color.defaultLine;
-            else lineColor = markerLine.color;
-
-            if('mc' in d) fillColor = d.mcc = markerScale(d.mc);
-            else if(Array.isArray(marker.color)) fillColor = Color.defaultLine;
-            else fillColor = marker.color || 'rgba(0,0,0,0)';
-        }
-
-        var p = d3.select(this);
-        if(d.om) {
-            // open markers can't have zero linewidth, default to 1px,
-            // and use fill color as stroke color
-            p.call(Color.stroke, fillColor)
-                .style({
-                    'stroke-width': (lineWidth || 1) + 'px',
-                    fill: 'none'
-                });
-        }
-        else {
-            p.style('stroke-width', lineWidth + 'px')
-                .call(Color.fill, fillColor);
-            if(lineWidth) {
-                p.call(Color.stroke, lineColor);
-            }
-        }
+        drawing.singlePointStyle(d, d3.select(this), trace, markerScale, lineScale)
     });
 };
 

src/plot_api/plot_api.js

@@ -2618,7 +2618,13 @@ Plotly.transition = function(gd, data, layout, traceIndices, transitionConfig) {
         }
     }
 
+    var aborted = false;
+
     function executeTransitions() {
+        gd._transitionData._interruptCallbacks.push(function () {
+            aborted = true;
+        });
+
         var traceTransitionConfig;
         var hasTraceTransition = false;
         var j;
@@ -2648,15 +2654,30 @@ Plotly.transition = function(gd, data, layout, traceIndices, transitionConfig) {
             traceTransitionConfig = transitionConfig;
         }
 
-        for(j = 0; j < basePlotModules.length; j++) {
-            basePlotModules[j].plot(gd, transitionedTraces, traceTransitionConfig);
+        // Construct callbacks that are executed on transition end. This ensures the d3 transitions
+        // are *complete* before anything else is done.
+        var numCallbacks = 0;
+        var numCompleted = 0;
+        function makeCallback () {
+            numCallbacks++;
+            return function () {
+                numCompleted++;
+                // When all are complete, perform a redraw:
+                if (!aborted && numCompleted === numCallbacks) {
+                    completeTransition();
+                }
+            }
         }
 
-        gd._transitionData._completionTimeout = setTimeout(completeTransition, transitionConfig.duration + transitionConfig.delay + 1000);
+        for(j = 0; j < basePlotModules.length; j++) {
+            var _config = Lib.extendFlat({onComplete: makeCallback()}, traceTransitionConfig);
+            basePlotModules[j].plot(gd, transitionedTraces, _config);
+        }
 
         if(!hasAxisTransition && !hasTraceTransition) {
             return false;
         }
+
     }
 
     function completeTransition() {
@@ -2673,14 +2694,7 @@ Plotly.transition = function(gd, data, layout, traceIndices, transitionConfig) {
     }
 
     function interruptPreviousTransitions() {
-        if(gd._transitionData._completionTimeout) {
-            // Prevent the previous completion from occurring:
-            clearTimeout(gd._transitionData._completionTimeout);
-            gd._transitionData._completionTimeout = null;
-
-            // Interrupt an event to indicate that a transition was running:
-            gd.emit('plotly_interrupttransition', []);
-        }
+        gd.emit('plotly_interrupttransition', []);
 
         flushCallbacks(gd._transitionData._cleanupCallbacks);
         return executeCallbacks(gd._transitionData._interruptCallbacks);

src/traces/scatter/plot.js

@@ -29,6 +29,7 @@ module.exports = function plot(gd, plotinfo, cdscatter, transitionConfig) {
     // If transition config is provided, then it is only a partial replot and traces not
     // updated are removed.
     var isFullReplot = !transitionConfig;
+    var hasTransition = !!transitionConfig && transitionConfig.duration > 0;
 
     selection = scatterlayer.selectAll('g.trace');
 
@@ -61,11 +62,27 @@ module.exports = function plot(gd, plotinfo, cdscatter, transitionConfig) {
         return idx1 > idx2 ? 1 : -1;
     });
 
-    // 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, transitionConfig);
-    });
+    if (hasTransition) {
+        var transition = d3.transition()
+            .duration(transitionConfig.duration)
+            .ease(transitionConfig.ease)
+            .delay(transitionConfig.delay)
+            .each('end', function () {
+                transitionConfig.onComplete && transitionConfig.onComplete();
+            });
+
+        transition.each(function () {
+            // 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, transitionConfig);
+            });
+        });
+    } else {
+        scatterlayer.selectAll('g.trace').each(function(d, i) {
+            plotOne(gd, i, plotinfo, d, cdscatter, this, transitionConfig);
+        });
+    }
 
     if(isFullReplot) {
         join.exit().remove();
@@ -127,14 +144,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
     var hasTransition = !!transitionConfig && transitionConfig.duration > 0;
 
     function transition(selection) {
-        if(hasTransition) {
-            return selection.transition()
-                .duration(transitionConfig.duration)
-                .delay(transitionConfig.delay)
-                .ease(transitionConfig.ease);
-        } else {
-            return selection;
-        }
+        return hasTransition ? selection.transition() : selection;
     }
 
     var xa = plotinfo.x(),
@@ -277,8 +287,9 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                             .call(Drawing.lineGroupStyle))
                                 .style('opacity', 1);
                     } else {
-                        transition(el).attr('d', thispath)
-                            .call(Drawing.lineGroupStyle);
+                        var sel = transition(el);
+                        sel.attr('d', thispath);
+                        Drawing.singleLineStyle(cdscatter, sel);
                     }
                 }
             };
@@ -376,14 +387,17 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                 join = selection
                     .data(trace.marker.maxdisplayed ? visFilter : Lib.identity, getKeyFunc(trace));
 
-                join.enter().append('path')
-                    .classed('point', true)
-                    .call(Drawing.pointStyle, trace)
+                var enter = join.enter().append('path')
+                    .classed('point', true);
+
+                enter.call(Drawing.pointStyle, trace)
                     .call(Drawing.translatePoints, xa, ya, trace, transitionConfig, 1);
 
-                join.transition()
-                    .call(Drawing.translatePoints, xa, ya, trace, transitionConfig, 0)
-                    .call(Drawing.pointStyle, trace);
+                join.transition().each(function (d) {
+                    var sel = transition(d3.select(this));
+                    Drawing.translatePoint(d, sel, xa, ya, trace, transitionConfig, 0)
+                    Drawing.singlePointStyle(d, sel, trace);
+                });
 
                 if(hasTransition) {
                     join.exit()

test/jasmine/tests/transition_test.js

@@ -85,5 +85,4 @@ describe('Plotly.transition', function() {
         Plotly.transition(gd, null, {'xaxis.range': [0.2, 0.3]}, null, {duration: 50});
         gd.on('plotly_endtransition', done);
     });
-
 });