Skip to content

Fix: Optimize Drawing.bBox calls to prevent unnecessary reflows in complex DOMs #7497

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions draftlogs/7497_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- reduce DOM reflows for better performance [[#7497](https://github.com/plotly/plotly.js/issues/7497)]
8 changes: 4 additions & 4 deletions src/components/annotations/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {

var mathjaxGroup = annTextGroupInner.select('.annotation-text-math-group');
var hasMathjax = !mathjaxGroup.empty();
var anntextBB = Drawing.bBox(
(hasMathjax ? mathjaxGroup : annText).node());
var anntextNode = (hasMathjax ? mathjaxGroup : annText).node();
var anntextBB = anntextNode.getBBox();
var textWidth = anntextBB.width;
var textHeight = anntextBB.height;
var annWidth = options.width || textWidth;
Expand Down Expand Up @@ -465,8 +465,8 @@ function drawRaw(gd, options, index, subplotId, xa, ya) {
})
.call(Drawing.setClipUrl, isSizeConstrained ? annClipID : null, gd);
} else {
var texty = borderfull + yShift - anntextBB.top;
var textx = borderfull + xShift - anntextBB.left;
var texty = borderfull + yShift - anntextBB.y;
var textx = borderfull + xShift - anntextBB.x;

annText.call(svgTextUtils.positionText, textx, texty)
.call(Drawing.setClipUrl, isSizeConstrained ? annClipID : null, gd);
Expand Down
8 changes: 4 additions & 4 deletions src/components/colorbar/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ function drawColorBar(g, opts, gd) {

var bb;
if(mathJaxNode) {
bb = Drawing.bBox(mathJaxNode);
bb = mathJaxNode.getBBox();
titleWidth = bb.width;
titleHeight = bb.height;
if(titleHeight > lineSize) {
Expand All @@ -461,7 +461,7 @@ function drawColorBar(g, opts, gd) {
titleTrans[1] -= (titleHeight - lineSize) / 2;
}
} else if(titleText.node() && !titleText.classed(cn.jsPlaceholder)) {
bb = Drawing.bBox(titleText.node());
bb = titleText.node().getBBox();
titleWidth = bb.width;
titleHeight = bb.height;
}
Expand Down Expand Up @@ -606,7 +606,7 @@ function drawColorBar(g, opts, gd) {
var bb;
var innerThickness = thickPx + outlinewidth / 2;
if(ticklabelposition.indexOf('inside') === -1) {
bb = Drawing.bBox(axLayer.node());
bb = axLayer.node().getBBox();
innerThickness += isVertical ? bb.width : bb.height;
}

Expand All @@ -627,7 +627,7 @@ function drawColorBar(g, opts, gd) {
(isVertical && topOrBottom) ||
(!isVertical && !topOrBottom)
)) {
bb = Drawing.bBox(mathJaxNode);
bb = mathJaxNode.getBBox();
titleWidth = bb.width;
_titleHeight = bb.height;
} else {
Expand Down
80 changes: 51 additions & 29 deletions src/components/drawing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1511,38 +1511,25 @@ drawing.bBox = function(node, inTester, hash) {
if(out) return Lib.extendFlat({}, out);
}
}
var testNode, tester;
if(inTester) {
testNode = node;
var bb = {};
if (node.tagName !== "text") {
var nodeBB = node.getBBox();
if (Object.keys(nodeBB).length) {
bb = {
height: nodeBB.height,
width: nodeBB.width,
left: nodeBB.x,
top: nodeBB.y,
right: nodeBB.x + nodeBB.width,
bottom: nodeBB.y + nodeBB.height
};
} else {
bb = testNodeInTester(node, inTester);
}
} else {
tester = drawing.tester.node();

// copy the node to test into the tester
testNode = node.cloneNode(true);
tester.appendChild(testNode);
bb = testNodeInTester(node, inTester);
}

// standardize its position (and newline tspans if any)
d3.select(testNode)
.attr('transform', null)
.call(svgTextUtils.positionText, 0, 0);

var testRect = testNode.getBoundingClientRect();
var refRect = drawing.testref
.node()
.getBoundingClientRect();

if(!inTester) tester.removeChild(testNode);

var bb = {
height: testRect.height,
width: testRect.width,
left: testRect.left - refRect.left,
top: testRect.top - refRect.top,
right: testRect.right - refRect.left,
bottom: testRect.bottom - refRect.top
};

// make sure we don't have too many saved boxes,
// or a long session could overload on memory
// by saving boxes for long-gone elements
Expand All @@ -1558,6 +1545,41 @@ drawing.bBox = function(node, inTester, hash) {
return Lib.extendFlat({}, bb);
};

function testNodeInTester(node, inTester) {
var testNode, tester;
if(inTester) {
testNode = node;
} else {
tester = drawing.tester.node();

// copy the node to test into the tester
testNode = node.cloneNode(true);
tester.appendChild(testNode);
}

// standardize its position (and newline tspans if any)
d3.select(testNode)
.attr('transform', null)
.call(svgTextUtils.positionText, 0, 0);

var testRect = testNode.getBoundingClientRect();
var refRect = drawing.testref
.node()
.getBoundingClientRect();

if(!inTester) tester.removeChild(testNode);

var bb = {
height: testRect.height,
width: testRect.width,
left: testRect.left - refRect.left,
top: testRect.top - refRect.top,
right: testRect.right - refRect.left,
bottom: testRect.bottom - refRect.top
};
return bb;
}

// capture everything about a node (at least in our usage) that
// impacts its bounding box, given that bBox clears x, y, and transform
function nodeHash(node) {
Expand Down
6 changes: 3 additions & 3 deletions src/components/legend/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function horizontalAlignTitle(titleEl, legendObj, bw) {
var titleOffset = 0;
var textNode = titleEl.node();

var width = Drawing.bBox(textNode).width; // width of the title text
var width = textNode.getBBox().width; // width of the title text

if(legendObj.title.side === 'top center') {
titleOffset = 0.5 * (legendObj._width - 2 * bw - 2 * constants.titlePad - width);
Expand Down Expand Up @@ -658,7 +658,7 @@ function computeTextDimensions(g, gd, legendObj, aTitle) {
var height, width;

if(mathjaxNode) {
var mathjaxBB = Drawing.bBox(mathjaxNode);
var mathjaxBB = mathjaxNode.getBBox();

height = mathjaxBB.height;
width = mathjaxBB.width;
Expand All @@ -679,7 +679,7 @@ function computeTextDimensions(g, gd, legendObj, aTitle) {
var textNode = textEl.node();

height = lineHeight * textLines;
width = textNode ? Drawing.bBox(textNode).width : 0;
width = textNode ? textNode.getBBox().width : 0;

// approximation to height offset to center the font
// to avoid getBoundingClientRect
Expand Down
2 changes: 1 addition & 1 deletion src/components/rangeselector/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function reposition(gd, buttons, opts, axName, selector) {
var rect = button.select('.selector-rect');
var text = button.select('.selector-text');

var tWidth = text.node() && Drawing.bBox(text.node()).width;
var tWidth = text.node() && text.node().getBBox().width;
var tHeight = opts.font.size * LINE_SPACING;
var tLines = svgTextUtils.lineCount(text);

Expand Down
8 changes: 4 additions & 4 deletions src/components/shapes/display_labels.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ module.exports = function drawLabel(gd, index, options, shapeGroup) {
svgTextUtils.convertToTspans(s, gd);
return s;
});
var textBB = Drawing.bBox(labelText.node());
var textBBHeight = labelText.node().getBBox().height;

// Calculate correct (x,y) for text
// We also determine true xanchor since xanchor depends on position when set to 'auto'
var textPos = calcTextPosition(shapex0, shapey0, shapex1, shapey1, options, textangle, textBB);
var textPos = calcTextPosition(shapex0, shapey0, shapex1, shapey1, options, textangle, textBBHeight);
var textx = textPos.textx;
var texty = textPos.texty;
var xanchor = textPos.xanchor;
Expand Down Expand Up @@ -160,7 +160,7 @@ function calcTextAngle(shapex0, shapey0, shapex1, shapey1) {
return -180 / Math.PI * Math.atan2(dy, dx);
}

function calcTextPosition(shapex0, shapey0, shapex1, shapey1, shapeOptions, actualTextAngle, textBB) {
function calcTextPosition(shapex0, shapey0, shapex1, shapey1, shapeOptions, actualTextAngle, textBBHeight) {
var textPosition = shapeOptions.label.textposition;
var textAngle = shapeOptions.label.textangle;
var textPadding = shapeOptions.label.padding;
Expand Down Expand Up @@ -270,7 +270,7 @@ function calcTextPosition(shapex0, shapey0, shapex1, shapey1, shapeOptions, actu
var shiftFraction = FROM_TL[yanchor];
// Adjust so that text is anchored at top of first line rather than at baseline of first line
var baselineAdjust = shapeOptions.label.font.size;
var textHeight = textBB.height;
var textHeight = textBBHeight;
var xshift = (textHeight * shiftFraction - baselineAdjust) * sinA;
var yshift = -(textHeight * shiftFraction - baselineAdjust) * cosA;

Expand Down
4 changes: 2 additions & 2 deletions src/components/sliders/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ function findDimensions(gd, sliderOpts) {

var textNode = text.node();
if(textNode) {
var bBox = Drawing.bBox(textNode);
var bBox = textNode.getBBox();
labelHeight = Math.max(labelHeight, bBox.height);
maxLabelWidth = Math.max(maxLabelWidth, bBox.width);
}
Expand Down Expand Up @@ -185,7 +185,7 @@ function findDimensions(gd, sliderOpts) {

sliderLabels.each(function(stepOpts) {
var curValPrefix = drawCurrentValue(dummyGroup, sliderOpts, stepOpts.label);
var curValSize = (curValPrefix.node() && Drawing.bBox(curValPrefix.node())) || {width: 0, height: 0};
var curValSize = (curValPrefix.node() && curValPrefix.node().getBBox()) || {width: 0, height: 0};
var lines = svgTextUtils.lineCount(curValPrefix);
dims.currentValueMaxWidth = Math.max(dims.currentValueMaxWidth, Math.ceil(curValSize.width));
dims.currentValueHeight = Math.max(dims.currentValueHeight, Math.ceil(curValSize.height));
Expand Down
2 changes: 1 addition & 1 deletion src/components/updatemenus/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ function findDimensions(gd, menuOpts) {
var text = button.select('.' + constants.itemTextClassName);

// width is given by max width of all buttons
var tWidth = text.node() && Drawing.bBox(text.node()).width;
var tWidth = text.node() && text.node().getBBox().width;
var wEff = Math.max(tWidth + constants.textPadX, constants.minWidth);

// height is determined by item text
Expand Down
2 changes: 1 addition & 1 deletion src/plot_api/subroutines.js
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ exports.drawMainTitle = function(gd) {

if(title.text && title.automargin) {
var titleObj = d3.selectAll('.gtitle');
var titleHeight = Drawing.bBox(d3.selectAll('.g-gtitle').node()).height;
var titleHeight = d3.selectAll('.g-gtitle').node().getBBox().height;
var pushMargin = needsMarginPush(gd, title, titleHeight);
if(pushMargin > 0) {
applyTitleAutoMargin(gd, y, pushMargin, titleHeight);
Expand Down
10 changes: 5 additions & 5 deletions src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -3677,7 +3677,7 @@ axes.drawLabels = function(gd, ax, opts) {
ax._adjustTickLabelsOverflow();
}
} else {
var mjWidth = Drawing.bBox(mathjaxGroup.node()).width;
var mjWidth = mathjaxGroup.node().getBBox().width;
var mjShift = mjWidth * {end: -0.5, start: 0.5}[anchor];
mathjaxGroup.attr('transform', transform + strTranslate(mjShift, 0));
}
Expand Down Expand Up @@ -3851,18 +3851,18 @@ axes.drawLabels = function(gd, ax, opts) {

var x = ax.l2p(d.x);
var thisLabel = selectTickLabel(this);
var bb = Drawing.bBox(thisLabel.node());
var labelWidth = thisLabel.node().getBBox().width;
maxLines = Math.max(maxLines, svgTextUtils.lineCount(thisLabel));

lbbArray.push({
// ignore about y, just deal with x overlaps
top: 0,
bottom: 10,
height: 10,
left: x - bb.width / 2,
left: x - labelWidth / 2,
// impose a 2px gap
right: x + bb.width / 2 + 2,
width: bb.width + 2
right: x + labelWidth / 2 + 2,
width: labelWidth + 2
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/plots/map/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ exports.toSVG = function(gd) {
'data-unformatted': attributions
});

var bBox = Drawing.bBox(attributionText.node());
var bBox = attributionText.node().getBBox();

// Break into multiple lines twice larger than domain
var maxWidth = size.w * (domain.x[1] - domain.x[0]);
Expand All @@ -135,7 +135,7 @@ exports.toSVG = function(gd) {
.attr('data-unformatted', multilineAttributions)
.call(svgTextUtils.convertToTspans, gd);

bBox = Drawing.bBox(attributionText.node());
bBox = attributionText.node().getBBox();
}
attributionText.attr('transform', strTranslate(-3, -bBox.height + 8));

Expand Down
4 changes: 2 additions & 2 deletions src/plots/mapbox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ exports.toSVG = function(gd) {
'data-unformatted': attributions
});

var bBox = Drawing.bBox(attributionText.node());
var bBox = attributionText.node().getBBox();

// Break into multiple lines twice larger than domain
var maxWidth = size.w * (domain.x[1] - domain.x[0]);
Expand All @@ -193,7 +193,7 @@ exports.toSVG = function(gd) {
.attr('data-unformatted', multilineAttributions)
.call(svgTextUtils.convertToTspans, gd);

bBox = Drawing.bBox(attributionText.node());
bBox = attributionText.node().getBBox();
}
attributionText.attr('transform', strTranslate(-3, -bBox.height + 8));

Expand Down
2 changes: 1 addition & 1 deletion src/plots/polar/polar.js
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ proto.updateRadialAxisTitle = function(fullLayout, polarLayout, _angle) {
// because if plot is editable, pad needs to be calculated anyways
// to properly show placeholder text when title is empty.
if(radialLayout.title) {
var h = Drawing.bBox(_this.layers['radial-axis'].node()).height;
var h = _this.layers['radial-axis'].node().getBBox().height;
var ts = radialLayout.title.font.size;
var side = radialLayout.side;
pad =
Expand Down
2 changes: 1 addition & 1 deletion src/traces/carpet/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ function drawAxisLabels(gd, xaxis, yaxis, trace, t, layer, labels, labelClass) {
.text(label.text)
.call(svgTextUtils.convertToTspans, gd);

var bbox = Drawing.bBox(this);
var bbox = this.getBBox();

labelEl.attr('transform',
// Translate to the correct point:
Expand Down