Skip to content

Fix relayout constrained axes + various multi-subplot perf improvements #2628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 14, 2018
2 changes: 1 addition & 1 deletion src/components/colorbar/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ module.exports = function draw(gd, id) {
// this title call only handles side=right
return Lib.syncOrAsync([
function() {
return Axes.doTicks(gd, cbAxisOut, true);
return Axes.doTicksSingle(gd, cbAxisOut, true);
},
function() {
if(['top', 'bottom'].indexOf(opts.titleside) === -1) {
Expand Down
6 changes: 3 additions & 3 deletions src/components/grid/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,9 @@ function sizeDefaults(layoutIn, layoutOut) {
var dfltGapY = hasSubplotGrid ? 0.3 : 0.1;

var dfltSideX, dfltSideY;
if(isSplomGenerated) {
dfltSideX = 'bottom';
dfltSideY = 'left';
if(isSplomGenerated && layoutOut._splomGridDflt) {
dfltSideX = layoutOut._splomGridDflt.xside;
dfltSideY = layoutOut._splomGridDflt.yside;
}

gridOut._domains = {
Expand Down
35 changes: 15 additions & 20 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1689,19 +1689,9 @@ exports.relayout = function relayout(gd, astr, val) {
// subroutine of its own so that finalDraw always gets
// executed after drawData
seq.push(
// TODO
// no test fail when commenting out doAutoRangeAndConstraints,
// but I think we do need this (maybe just the enforce part?)
// Am I right?
// More info in:
// https://github.com/plotly/plotly.js/issues/2540
subroutines.doAutoRangeAndConstraints,
// TODO
// can target specific axes,
// do not have to redraw all axes here
// See:
// https://github.com/plotly/plotly.js/issues/2547
subroutines.doTicksRelayout,
function(gd) {
return subroutines.doTicksRelayout(gd, specs.rangesAltered);
},
subroutines.drawData,
subroutines.finalDraw
);
Expand All @@ -1728,6 +1718,10 @@ exports.relayout = function relayout(gd, astr, val) {
});
};

var AX_RANGE_RE = /^[xyz]axis[0-9]*\.range(\[[0|1]\])?$/;
var AX_AUTORANGE_RE = /^[xyz]axis[0-9]*\.autorange$/;
var AX_DOMAIN_RE = /^[xyz]axis[0-9]*\.domain(\[[0|1]\])?$/;

function _relayout(gd, aobj) {
var layout = gd.layout,
fullLayout = gd._fullLayout,
Expand Down Expand Up @@ -1806,7 +1800,7 @@ function _relayout(gd, aobj) {
var plen = p.parts.length;
// p.parts may end with an index integer if the property is an array
var pend = plen - 1;
while(pend > 0 && typeof p.parts[plen - 1] !== 'string') { pend--; }
while(pend > 0 && typeof p.parts[pend] !== 'string') pend--;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, nice catch!

// last property in chain (leaf node)
var pleaf = p.parts[pend];
// leaf plus immediate parent
Expand Down Expand Up @@ -1841,11 +1835,11 @@ function _relayout(gd, aobj) {
fullLayout[ai] = gd._initialAutoSize[ai];
}
// check autorange vs range
else if(pleafPlus.match(/^[xyz]axis[0-9]*\.range(\[[0|1]\])?$/)) {
else if(pleafPlus.match(AX_RANGE_RE)) {
recordAlteredAxis(pleafPlus);
Lib.nestedProperty(fullLayout, ptrunk + '._inputRange').set(null);
}
else if(pleafPlus.match(/^[xyz]axis[0-9]*\.autorange$/)) {
else if(pleafPlus.match(AX_AUTORANGE_RE)) {
recordAlteredAxis(pleafPlus);
Lib.nestedProperty(fullLayout, ptrunk + '._inputRange').set(null);
var axFull = Lib.nestedProperty(fullLayout, ptrunk).get();
Expand All @@ -1855,7 +1849,7 @@ function _relayout(gd, aobj) {
axFull._input.domain = axFull._inputDomain.slice();
}
}
else if(pleafPlus.match(/^[xyz]axis[0-9]*\.domain(\[[0|1]\])?$/)) {
else if(pleafPlus.match(AX_DOMAIN_RE)) {
Lib.nestedProperty(fullLayout, ptrunk + '._inputDomain').set(null);
}

Expand Down Expand Up @@ -2061,6 +2055,7 @@ function _relayout(gd, aobj) {

return {
flags: flags,
rangesAltered: rangesAltered,
undoit: undoit,
redoit: redoit,
eventData: Lib.extendDeep({}, redoit)
Expand Down Expand Up @@ -2174,8 +2169,9 @@ exports.update = function update(gd, traceUpdate, layoutUpdate, _traces) {
if(relayoutFlags.layoutstyle) seq.push(subroutines.layoutStyles);
if(relayoutFlags.axrange) {
seq.push(
subroutines.doAutoRangeAndConstraints,
subroutines.doTicksRelayout,
function(gd) {
return subroutines.doTicksRelayout(gd, relayoutSpecs.rangesAltered);
},
subroutines.drawData,
subroutines.finalDraw
);
Expand Down Expand Up @@ -2340,7 +2336,6 @@ exports.react = function(gd, data, layout, config) {
if(relayoutFlags.layoutstyle) seq.push(subroutines.layoutStyles);
if(relayoutFlags.axrange) {
seq.push(
subroutines.doAutoRangeAndConstraints,
subroutines.doTicksRelayout,
subroutines.drawData,
subroutines.finalDraw
Expand Down
57 changes: 54 additions & 3 deletions src/plot_api/subroutines.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ exports.lsInner = function(gd) {
var fullLayout = gd._fullLayout;
var gs = fullLayout._size;
var pad = gs.p;
var axList = Axes.list(gd);
var axList = Axes.list(gd, '', true);

// _has('cartesian') means SVG specifically, not GL2D - but GL2D
// can still get here because it makes some of the SVG structure
Expand Down Expand Up @@ -95,6 +95,11 @@ exports.lsInner = function(gd) {
ax._mainMirrorPosition = (ax.mirror && counterAx) ?
getLinePosition(ax, counterAx,
alignmentConstants.OPPOSITE_SIDE[ax.side]) : null;

// Figure out which subplot to draw ticks, labels, & axis lines on
// do this as a separate loop so we already have all the
// _mainAxis and _anchorAxis links set
ax._mainSubplot = findMainSubplot(ax, fullLayout);
}

fullLayout._paperdiv
Expand Down Expand Up @@ -324,6 +329,48 @@ exports.lsInner = function(gd) {
return gd._promises.length && Promise.all(gd._promises);
};

function findMainSubplot(ax, fullLayout) {
Copy link
Contributor Author

@etpinard etpinard May 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcjohnson I didn't bother making the "find main subplot" algo faster as you proposed in #2549 (comment), I was happy enough with skipping this whole thing except for sploms with showlowerhalf: false with c947b2c

var subplotList = fullLayout._subplots;
var ids = subplotList.cartesian.concat(subplotList.gl2d || []);
var mockGd = {_fullLayout: fullLayout};

var isX = ax._id.charAt(0) === 'x';
var anchorAx = ax._mainAxis._anchorAxis;
var mainSubplotID = '';
var nextBestMainSubplotID = '';
var anchorID = '';

// First try the main ID with the anchor
if(anchorAx) {
anchorID = anchorAx._mainAxis._id;
mainSubplotID = isX ? (ax._id + anchorID) : (anchorID + ax._id);
}

// Then look for a subplot with the counteraxis overlaying the anchor
// If that fails just use the first subplot including this axis
if(!mainSubplotID || !fullLayout._plots[mainSubplotID]) {
mainSubplotID = '';

for(var j = 0; j < ids.length; j++) {
var id = ids[j];
var yIndex = id.indexOf('y');
var idPart = isX ? id.substr(0, yIndex) : id.substr(yIndex);
var counterPart = isX ? id.substr(yIndex) : id.substr(0, yIndex);

if(idPart === ax._id) {
if(!nextBestMainSubplotID) nextBestMainSubplotID = id;
var counterAx = Axes.getFromId(mockGd, counterPart);
if(anchorID && counterAx.overlaying === anchorID) {
mainSubplotID = id;
break;
}
}
}
}

return mainSubplotID || nextBestMainSubplotID;
}

function shouldShowLinesOrTicks(ax, subplot) {
return (ax.ticks || ax.showline) &&
(subplot === ax._mainSubplot || ax.mirror === 'all' || ax.mirror === 'allticks');
Expand Down Expand Up @@ -448,8 +495,12 @@ exports.doLegend = function(gd) {
return Plots.previousPromises(gd);
};

exports.doTicksRelayout = function(gd) {
Axes.doTicks(gd, 'redraw');
exports.doTicksRelayout = function(gd, rangesAltered) {
if(rangesAltered) {
Axes.doTicks(gd, Object.keys(rangesAltered), true);
} else {
Axes.doTicks(gd, 'redraw');
}

if(gd._fullLayout._hasOnlyLargeSploms) {
clearGlCanvases(gd);
Expand Down
127 changes: 84 additions & 43 deletions src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -1456,6 +1456,10 @@ axes.findSubplotsWithAxis = function(subplots, ax) {
// makeClipPaths: prepare clipPaths for all single axes and all possible xy pairings
axes.makeClipPaths = function(gd) {
var fullLayout = gd._fullLayout;

// for more info: https://github.com/plotly/plotly.js/issues/2595
if(fullLayout._hasOnlyLargeSploms) return;

var fullWidth = {_offset: 0, _length: fullLayout.width, _id: ''};
var fullHeight = {_offset: 0, _length: fullLayout.height, _id: ''};
var xaList = axes.list(gd, 'x', true);
Expand Down Expand Up @@ -1494,58 +1498,95 @@ axes.makeClipPaths = function(gd) {
});
};

// doTicks: draw ticks, grids, and tick labels
// axid: 'x', 'y', 'x2' etc,
// blank to do all,
// 'redraw' to force full redraw, and reset:
// ax._r (stored range for use by zoom/pan)
// ax._rl (stored linearized range for use by zoom/pan)
// or can pass in an axis object directly
axes.doTicks = function(gd, axid, skipTitle) {
/** Main multi-axis drawing routine!
*
* @param {DOM element} gd : graph div
* @param {string or array of strings} arg : polymorphic argument
* @param {boolean} skipTitle : optional flag to skip axis title draw/update
*
* Signature 1: Axes.doTicks(gd, 'redraw')
* use this to clear and redraw all axes on graph
*
* Signature 2: Axes.doTicks(gd, '')
* use this to draw all axes on graph w/o the selectAll().remove()
* of the 'redraw' signature
*
* Signature 3: Axes.doTicks(gd, [axId, axId2, ...])
* where the items are axis id string,
* use this to update multiple axes in one call
*
* N.B doTicks updates:
* - ax._r (stored range for use by zoom/pan)
* - ax._rl (stored linearized range for use by zoom/pan)
*/
axes.doTicks = function(gd, arg, skipTitle) {
var fullLayout = gd._fullLayout;
var ax;
var independent = false;

// allow passing an independent axis object instead of id
if(typeof axid === 'object') {
ax = axid;
axid = ax._id;
independent = true;
if(arg === 'redraw') {
fullLayout._paper.selectAll('g.subplot').each(function(subplot) {
var plotinfo = fullLayout._plots[subplot];
var xa = plotinfo.xaxis;
var ya = plotinfo.yaxis;

plotinfo.xaxislayer.selectAll('.' + xa._id + 'tick').remove();
plotinfo.yaxislayer.selectAll('.' + ya._id + 'tick').remove();
if(plotinfo.gridlayer) plotinfo.gridlayer.selectAll('path').remove();
if(plotinfo.zerolinelayer) plotinfo.zerolinelayer.selectAll('path').remove();
fullLayout._infolayer.select('.g-' + xa._id + 'title').remove();
fullLayout._infolayer.select('.g-' + ya._id + 'title').remove();
});
}
else {
ax = axes.getFromId(gd, axid);

if(axid === 'redraw') {
fullLayout._paper.selectAll('g.subplot').each(function(subplot) {
var plotinfo = fullLayout._plots[subplot];
var xa = plotinfo.xaxis;
var ya = plotinfo.yaxis;

plotinfo.xaxislayer.selectAll('.' + xa._id + 'tick').remove();
plotinfo.yaxislayer.selectAll('.' + ya._id + 'tick').remove();
if(plotinfo.gridlayer) plotinfo.gridlayer.selectAll('path').remove();
if(plotinfo.zerolinelayer) plotinfo.zerolinelayer.selectAll('path').remove();
fullLayout._infolayer.select('.g-' + xa._id + 'title').remove();
fullLayout._infolayer.select('.g-' + ya._id + 'title').remove();
});
}

if(!axid || axid === 'redraw') {
return Lib.syncOrAsync(axes.list(gd, '', true).map(function(ax) {
return function() {
if(!ax._id) return;
var axDone = axes.doTicks(gd, ax._id);
ax._r = ax.range.slice();
ax._rl = Lib.simpleMap(ax._r, ax.r2l);
return axDone;
};
}));
}
var axList = (!arg || arg === 'redraw') ? axes.listIds(gd) : arg;

Lib.syncOrAsync(axList.map(function(axid) {
return function() {
if(!axid) return;

var axDone = axes.doTicksSingle(gd, axid, skipTitle);

var ax = axes.getFromId(gd, axid);
ax._r = ax.range.slice();
ax._rl = Lib.simpleMap(ax._r, ax.r2l);

return axDone;
};
}));
};

/** Per axis drawing routine!
*
* This routine draws axis ticks and much more (... grids, labels, title etc.)
* Supports multiple argument signatures.
* N.B. this thing is async in general (because of MathJax rendering)
*
* @param {DOM element} gd : graph div
* @param {string or array of strings} arg : polymorphic argument
* @param {boolean} skipTitle : optional flag to skip axis title draw/update
* @return {promise}
*
* Signature 1: Axes.doTicks(gd, ax)
* where ax is an axis object as in fullLayout
*
* Signature 2: Axes.doTicks(gd, axId)
* where axId is a axis id string
*/
axes.doTicksSingle = function(gd, arg, skipTitle) {
var fullLayout = gd._fullLayout;
var independent = false;
var ax;

if(Lib.isPlainObject(arg)) {
ax = arg;
independent = true;
} else {
ax = axes.getFromId(gd, arg);
}

// set scaling to pixels
ax.setScale();

var axid = ax._id;
var axLetter = axid.charAt(0);
var counterLetter = axes.counterLetter(axid);
var vals = ax._vals = axes.calcTicks(ax);
Expand Down
4 changes: 2 additions & 2 deletions src/plots/cartesian/dragbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var FROM_TL = require('../../constants/alignment').FROM_TL;

var Plots = require('../plots');

var doTicks = require('./axes').doTicks;
var doTicksSingle = require('./axes').doTicksSingle;
var getFromId = require('./axis_ids').getFromId;
var prepSelect = require('./select').prepSelect;
var clearSelect = require('./select').clearSelect;
Expand Down Expand Up @@ -585,7 +585,7 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
updates = {};
for(i = 0; i < activeAxIds.length; i++) {
var axId = activeAxIds[i];
doTicks(gd, axId, true);
doTicksSingle(gd, axId, true);
var ax = getFromId(gd, axId);
updates[ax._name + '.range[0]'] = ax.range[0];
updates[ax._name + '.range[1]'] = ax.range[1];
Expand Down
2 changes: 1 addition & 1 deletion src/plots/cartesian/transition_axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo
activeAxIds = [xa._id, ya._id];

for(i = 0; i < activeAxIds.length; i++) {
Axes.doTicks(gd, activeAxIds[i], true);
Axes.doTicksSingle(gd, activeAxIds[i], true);
}

function redrawObjs(objArray, method, shortCircuit) {
Expand Down
Loading