Skip to content

Commit

Permalink
[PoC] refactor auto-margin pipeline
Browse files Browse the repository at this point in the history 
- Plots.autoMargin and Plots.doAutoMargin no longer
  call Plotly.plot redraw !!!
- 🔪 `fullLayout._replotting` flag
- 🔪 now useless `_redrawFromAutoMarginCount` counter
- call component `pushMargin` before `draw`,
  call Plots.doAutoMargin once after all `pushMargin` are done
- merge `lsInner` into `layoutStyles`
- 🔪 now obsolete `Axes.allowAutoMargin` and `_pushmarginIds` stash
- adapt Plotly.plot pipeline
- interplay between pushMargin and doAutoRangeAndConstraints is still a WIP
  • Loading branch information
@etpinard
etpinard committed Sep 23, 2019
1 parent 06d7abf commit e606c7b
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 161 deletions.
135 changes: 53 additions & 82 deletions src/plot_api/plot_api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,6 @@ function plot(gd, data, layout, config) {
'but this container doesn\'t yet have a plot.', gd);
}

function addFrames() {
if(frames) {
return exports.addFrames(gd, frames);
}
}

// transfer configuration options to gd until we move over to
// a more OO like model
setPlotContext(gd, config);
Expand Down Expand Up @@ -147,10 +141,6 @@ function plot(gd, data, layout, config) {
return plotLegacyPolar(gd, data, layout);
}

// so we don't try to re-call Plotly.plot from inside
// legend and colorbar, if margins changed
fullLayout._replotting = true;

// make or remake the framework if we need to
if(graphWasEmpty) makePlotFramework(gd);

Expand All @@ -163,11 +153,6 @@ function plot(gd, data, layout, config) {
// clear gradient defs on each .plot call, because we know we'll loop through all traces
Drawing.initGradients(gd);

// save initial show spikes once per graph
if(graphWasEmpty) Axes.saveShowSpikeInitial(gd);

// prepare the data and find the autorange

// generate calcdata, if we need to
// to force redoing calcdata, just delete it before calling Plotly.plot
var recalc = !gd.calcdata || gd.calcdata.length !== (gd._fullData || []).length;
Expand All @@ -182,8 +167,9 @@ function plot(gd, data, layout, config) {
if(gd._context.responsive) {
if(!gd._responsiveChartHandler) {
// Keep a reference to the resize handler to purge it down the road
gd._responsiveChartHandler = function() { if(!Lib.isHidden(gd)) Plots.resize(gd); };

gd._responsiveChartHandler = function() {
if(!Lib.isHidden(gd)) Plots.resize(gd);
};
// Listen to window resize
window.addEventListener('resize', gd._responsiveChartHandler);
}
Expand All @@ -195,7 +181,11 @@ function plot(gd, data, layout, config) {
* start async-friendly code - now we're actually drawing things
*/

var oldMargins = Lib.extendFlat({}, fullLayout._size);
function addFrames() {
if(frames) {
return exports.addFrames(gd, frames);
}
}

// draw framework first so that margin-pushing
// components can position themselves correctly
Expand All @@ -210,19 +200,11 @@ function plot(gd, data, layout, config) {
}

if(!fullLayout._glcanvas && fullLayout._has('gl')) {
fullLayout._glcanvas = fullLayout._glcontainer.selectAll('.gl-canvas').data([{
key: 'contextLayer',
context: true,
pick: false
}, {
key: 'focusLayer',
context: false,
pick: false
}, {
key: 'pickLayer',
context: false,
pick: true
}], function(d) { return d.key; });
fullLayout._glcanvas = fullLayout._glcontainer.selectAll('.gl-canvas').data([
{ key: 'contextLayer', context: true, pick: false },
{ key: 'focusLayer', context: false, pick: false },
{ key: 'pickLayer', context: false, pick: true }
], function(d) { return d.key; });

fullLayout._glcanvas.enter().append('canvas')
.attr('class', function(d) {
Expand Down Expand Up @@ -278,37 +260,31 @@ function plot(gd, data, layout, config) {
return Plots.previousPromises(gd);
}

// draw anything that can affect margins.
function marginPushers() {
// First reset the list of things that are allowed to change the margins
// So any deleted traces or components will be wiped out of the
// automargin calculation.
// This means *every* margin pusher must be listed here, even if it
// doesn't actually try to push the margins until later.
Plots.clearAutoMarginIds(gd);
var oldMargins = Lib.extendFlat({}, fullLayout._size);

subroutines.drawMarginPushers(gd);
Axes.allowAutoMargin(gd);
function pushMargin() {
Registry.getComponentMethod('rangeselector', 'pushMargin')(gd);
Registry.getComponentMethod('sliders', 'pushMargin')(gd);
Registry.getComponentMethod('updatemenus', 'pushMargin')(gd);
Registry.getComponentMethod('legend', 'pushMargin')(gd);
Registry.getComponentMethod('colorbar', 'pushMargin')(gd);
if(hasCartesian) Axes.pushMargin(gd);

Plots.doAutoMargin(gd);
return Plots.previousPromises(gd);
return Lib.syncOrAsync([Plots.previousPromises, Plots.doAutoMargin], gd);
}

// in case the margins changed, draw margin pushers again
function marginPushersAgain() {
if(!Plots.didMarginChange(oldMargins, fullLayout._size)) return;
function pushMarginAgain() {

return Lib.syncOrAsync([
marginPushers,
subroutines.layoutStyles
], gd);
if(Plots.didMarginChange(oldMargins, fullLayout._size)) {
oldMargins = Lib.extendFlat({}, fullLayout._size);
return pushMargin();
}
}

function positionAndAutorange() {
if(!recalc) {
doAutoRangeAndConstraints();
return;
}
if(!hasCartesian) return;
if(!recalc) return doAutoRangeAndConstraints();

// TODO: autosize extra for text markers and images
// see https://github.com/plotly/plotly.js/issues/1111
Expand All @@ -324,48 +300,49 @@ function plot(gd, data, layout, config) {

subroutines.doAutoRangeAndConstraints(gd);

// store initial ranges *after* enforcing constraints, otherwise
// we will never look like we're at the initial ranges
if(graphWasEmpty) Axes.saveRangeInitial(gd);

// this one is different from shapes/annotations calcAutorange
// the others incorporate those components into ax._extremes,
// this one actually sets the ranges in rangesliders.
Registry.getComponentMethod('rangeslider', 'calcAutorange')(gd);
}

// draw ticks, titles, and calculate axis scaling (._b, ._m)
function saveInitial() {
if(graphWasEmpty && hasCartesian) {
// store initial ranges *after* enforcing constraints, otherwise
// we will never look like we're at the initial ranges
Axes.saveRangeInitial(gd);
// save initial show spikes once per graph
Axes.saveShowSpikeInitial(gd);
}
}

function drawAxes() {
return Axes.draw(gd, graphWasEmpty ? '' : 'redraw');
if(hasCartesian) {
return Axes.draw(gd, graphWasEmpty ? '' : 'redraw');
}
}

var seq = [
Plots.previousPromises,
addFrames,
drawFramework,
marginPushers,
marginPushersAgain
];

if(hasCartesian) seq.push(positionAndAutorange);

seq.push(subroutines.layoutStyles);
if(hasCartesian) seq.push(drawAxes);

seq.push(
positionAndAutorange,
pushMargin,
pushMarginAgain,
pushMarginAgain,
positionAndAutorange,
saveInitial,
subroutines.layoutStyles,
subroutines.drawData,
subroutines.drawMarginPushers,
drawAxes,
subroutines.finalDraw,
initInteractions,
Plots.addLinks,
Plots.rehover,
Plots.redrag,
// TODO: doAutoMargin is only needed here for axis automargin, which
// happens outside of marginPushers where all the other automargins are
// calculated. Would be much better to separate margin calculations from
// component drawing - see https://github.com/plotly/plotly.js/issues/2704
Plots.doAutoMargin,
Plots.previousPromises
);
];

// even if everything we did was synchronous, return a promise
// so that the caller doesn't care which route we took
Expand All @@ -379,13 +356,7 @@ function plot(gd, data, layout, config) {
}

function emitAfterPlot(gd) {
var fullLayout = gd._fullLayout;

if(fullLayout._redrawFromAutoMarginCount) {
fullLayout._redrawFromAutoMarginCount--;
} else {
gd.emit('plotly_afterplot');
}
gd.emit('plotly_afterplot');
}

function setPlotConfig(obj) {
Expand Down
14 changes: 2 additions & 12 deletions src/plot_api/subroutines.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ var SVG_TEXT_ANCHOR_START = 'start';
var SVG_TEXT_ANCHOR_MIDDLE = 'middle';
var SVG_TEXT_ANCHOR_END = 'end';

exports.layoutStyles = function(gd) {
return Lib.syncOrAsync([Plots.doAutoMargin, lsInner], gd);
};

function overlappingDomain(xDomain, yDomain, domains) {
for(var i = 0; i < domains.length; i++) {
var existingX = domains[i][0];
Expand All @@ -50,7 +46,7 @@ function overlappingDomain(xDomain, yDomain, domains) {
return false;
}

function lsInner(gd) {
exports.layoutStyles = function(gd) {
var fullLayout = gd._fullLayout;
var gs = fullLayout._size;
var pad = gs.p;
Expand Down Expand Up @@ -302,7 +298,7 @@ function lsInner(gd) {
Axes.makeClipPaths(gd);

return Plots.previousPromises(gd);
}
};

function shouldShowLinesOrTicks(ax, subplot) {
return (ax.ticks || ax.showline) &&
Expand Down Expand Up @@ -563,9 +559,6 @@ exports.drawData = function(gd) {
Registry.getComponentMethod('annotations', 'draw')(gd);
Registry.getComponentMethod('images', 'draw')(gd);

// Mark the first render as complete
fullLayout._replotting = false;

return Plots.previousPromises(gd);
};

Expand Down Expand Up @@ -673,9 +666,6 @@ exports.doAutoRangeAndConstraints = function(gd) {
}
};

// An initial paint must be completed before these components can be
// correctly sized and the whole plot re-margined. fullLayout._replotting must
// be set to false before these will work properly.
exports.finalDraw = function(gd) {
// TODO: rangesliders really belong in marginPushers but they need to be
// drawn after data - can we at least get the margin pushing part separated
Expand Down
29 changes: 0 additions & 29 deletions src/plots/cartesian/axes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2975,35 +2975,6 @@ function selectTickLabel(gTick) {
return mj.empty() ? s.select('text') : mj;
}

/**
* Find all margin pushers for 2D axes and reserve them for later use
* Both label and rangeslider automargin calculations happen later so
* we need to explicitly allow their ids in order to not delete them.
*
* TODO: can we pull the actual automargin calls forward to avoid this hack?
* We're probably also doing multiple redraws in this case, would be faster
* if we can just do the whole calculation ahead of time and draw once.
*/
axes.allowAutoMargin = function(gd) {
var axList = axes.list(gd, '', true);
for(var i = 0; i < axList.length; i++) {
var ax = axList[i];
if(ax.automargin) {
Plots.allowAutoMargin(gd, axAutoMarginID(ax));
if(ax.mirror) {
Plots.allowAutoMargin(gd, axMirrorAutoMarginID(ax));
}
}
if(Registry.getComponentMethod('rangeslider', 'isVisible')(ax)) {
Plots.allowAutoMargin(gd, rangeSliderAutoMarginID(ax));
}
}
};

function axAutoMarginID(ax) { return ax._id + '.automargin'; }
function axMirrorAutoMarginID(ax) { return axAutoMarginID(ax) + '.mirror'; }
function rangeSliderAutoMarginID(ax) { return ax._id + '.rangeslider'; }

// swap all the presentation attributes of the axes showing these traces
axes.swap = function(gd, traces) {
var axGroups = makeAxisGroups(gd, traces);
Expand Down
4 changes: 0 additions & 4 deletions src/plots/cartesian/dragbox.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,9 +539,6 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
return;
}

// prevent axis drawing from monkeying with margins until we're done
gd._fullLayout._replotting = true;

if(xActive === 'ew' || yActive === 'ns') {
if(xActive) {
dragAxList(xaxes, dx);
Expand Down Expand Up @@ -778,7 +775,6 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
Lib.syncOrAsync([
Plots.previousPromises,
function() {
gd._fullLayout._replotting = false;
Registry.call('_guiRelayout', gd, updates);
}
], gd);
Expand Down
1 change: 0 additions & 1 deletion src/plots/cartesian/set_convert.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,6 @@ module.exports = function setConvert(ax, fullLayout) {
}

if(!isFinite(ax._m) || !isFinite(ax._b) || ax._length < 0) {
fullLayout._replotting = false;
throw new Error('Something went wrong with axis scaling');
}
};
Expand Down
Loading

0 comments on commit e606c7b

Please sign in to comment.