Skip to content

Commit

Permalink
Group transitions to avoid race conditions
Browse files Browse the repository at this point in the history
  • Loading branch information
@rreusser
rreusser committed Aug 23, 2016
1 parent 62277c2 commit 0946987
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 130 deletions.
5 changes: 4 additions & 1 deletion src/components/color/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
182 changes: 86 additions & 96 deletions src/components/drawing/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
};

Expand All @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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)
});
};

Expand Down
36 changes: 25 additions & 11 deletions src/plot_api/plot_api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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);
Expand Down
56 changes: 35 additions & 21 deletions src/traces/scatter/plot.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
}
}
};
Expand Down Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion test/jasmine/tests/transition_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

});

0 comments on commit 0946987

Please sign in to comment.