Unnecessary clipPaths

[alexcjohnson]

Potential 🐎 issue with many-subplot graphs: seems like we’re making clipPath.axesclip for every possible combination of axes… even though clipPath.plotclip only gets the subplots we use. Dunno if maybe shapes can require other combinations? In any event we should be able to be more discriminating in which of these we make.

[etpinard]

These "unnecessary" clip paths are added to the DOM in Axes.makeClipPaths

plotly.js/src/plots/cartesian/axes.js

Lines 1457 to 1495 in a132b85

	axes.makeClipPaths = function(gd) {
	var fullLayout = gd._fullLayout;
	var fullWidth = {_offset: 0, _length: fullLayout.width, _id: ''};
	var fullHeight = {_offset: 0, _length: fullLayout.height, _id: ''};
	var xaList = axes.list(gd, 'x', true);
	var yaList = axes.list(gd, 'y', true);
	var clipList = [];
	var i, j;
	for(i = 0; i < xaList.length; i++) {
	clipList.push({x: xaList[i], y: fullHeight});
	for(j = 0; j < yaList.length; j++) {
	if(i === 0) clipList.push({x: fullWidth, y: yaList[j]});
	clipList.push({x: xaList[i], y: yaList[j]});
	}
	}
	// selectors don't work right with camelCase tags,
	// have to use class instead
	// https://groups.google.com/forum/#!topic/d3-js/6EpAzQ2gU9I
	var axClips = fullLayout._clips.selectAll('.axesclip')
	.data(clipList, function(d) { return d.x._id + d.y._id; });
	axClips.enter().append('clipPath')
	.classed('axesclip', true)
	.attr('id', function(d) { return 'clip' + fullLayout._uid + d.x._id + d.y._id; })
	.append('rect');
	axClips.exit().remove();
	axClips.each(function(d) {
	d3.select(this).select('rect').attr({
	x: d.x._offset || 0,
	y: d.y._offset || 0,
	width: d.x._length || 1,
	height: d.y._length || 1
	});
	});
	};

Some benchmarks using https://codepen.io/etpinard/pen/wjmqmO:, at 50 dimensions, Axes.makeClipPaths takes around ~50ms, at 20 dimensions we're down to ~8ms - which is non-negligeable but not a major perf hit. Note that makeClipPaths is only called during the lsInner subroutines which is skipped notably during "axrange" and "modebar" editTypes.

That said, as far as I can tell, these per-axes clip paths are only used for shapes and layout images. So, we could easily skip Axes.makeClipPaths when the _hasOnlyLargeSplom flag

plotly.js/src/plots/plots.js

Lines 431 to 440 in a132b85

	// turn on flag to optimize large splom-only graphs
	// mostly by omitting SVG layers during Cartesian.drawFramework
	newFullLayout._hasOnlyLargeSploms = (
	newFullLayout._basePlotModules.length === 1 &&
	newFullLayout._basePlotModules[0].name === 'splom' &&
	splomXa.length > 15 &&
	splomYa.length > 15 &&
	newFullLayout.shapes.length === 0 &&
	newFullLayout.images.length === 0
	);

is turned on. Alternatively, we could call Axes.makeClipPaths only when there are visible shapes or layout images on the graph. This could make some relayout logic trickier though.

[alexcjohnson]

Good to know, thanks for benchmarking it. Sounds like adding a _hasOnlyLargeSplom switch is a no-brainer.

Down the line though we should be able to do even better by only making the ones we need period, by building up the required list during supplyDefaults and using that during makeClipPaths instead of the maximal list we generate there now. Changing axis references for shapes is already editType: 'calc', we could do that with images as well (I don't see much reason to make those operations fast, if we ever autorange images #1111 we'd probably need to do that anyway), that seems like it would solve the relayout concerns.

[etpinard]

Sounds like adding a _hasOnlyLargeSplom switch is a no-brainer.

Done in #2628

[gvwilson]

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson