Skip to content

Commit

Permalink
Merge pull request #1690 from plotly/drawing-perf-take2
Browse files Browse the repository at this point in the history
Drawing perf take2
  • Loading branch information
etpinard authored May 16, 2017
2 parents 3fdb308 + a8fba03 commit 66259ed
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 24 deletions.
25 changes: 14 additions & 11 deletions src/components/drawing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,11 +542,11 @@ drawing.steps = function(shape) {
};

// off-screen svg render testing element, shared by the whole page
// uses the id 'js-plotly-tester' and stores it in gd._tester
// uses the id 'js-plotly-tester' and stores it in drawing.tester
// makes a hash of cached text items in tester.node()._cache
// so we can add references to rendered text (including all info
// needed to fully determine its bounding rect)
drawing.makeTester = function(gd) {
drawing.makeTester = function() {
var tester = d3.select('body')
.selectAll('#js-plotly-tester')
.data([0]);
Expand Down Expand Up @@ -579,16 +579,17 @@ drawing.makeTester = function(gd) {
tester.node()._cache = {};
}

gd._tester = tester;
gd._testref = testref;
drawing.tester = tester;
drawing.testref = testref;
};

// use our offscreen tester to get a clientRect for an element,
// in a reference frame where it isn't translated and its anchor
// point is at (0,0)
// always returns a copy of the bbox, so the caller can modify it safely
var savedBBoxes = [],
maxSavedBBoxes = 10000;
var savedBBoxes = [];
var maxSavedBBoxes = 10000;

drawing.bBox = function(node) {
// cache elements we've already measured so we don't have to
// remeasure the same thing many times
Expand All @@ -597,22 +598,24 @@ drawing.bBox = function(node) {
return Lib.extendFlat({}, savedBBoxes[saveNum.value]);
}

var test3 = d3.select('#js-plotly-tester'),
tester = test3.node();
var tester3 = drawing.tester;
var tester = tester3.node();

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

// standardize its position... do we really want to do this?
d3.select(testNode).attr({
x: 0,
y: 0,
transform: ''
});

var testRect = testNode.getBoundingClientRect(),
refRect = test3.select('.js-reference-point')
.node().getBoundingClientRect();
var testRect = testNode.getBoundingClientRect();
var refRect = drawing.testref
.node()
.getBoundingClientRect();

tester.removeChild(testNode);

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 @@ -117,7 +117,7 @@ function keyFunction(opts) {

// Compute the dimensions (mutates sliderOpts):
function findDimensions(gd, sliderOpts) {
var sliderLabels = gd._tester.selectAll('g.' + constants.labelGroupClass)
var sliderLabels = Drawing.tester.selectAll('g.' + constants.labelGroupClass)
.data(sliderOpts.steps);

sliderLabels.enter().append('g')
Expand Down Expand Up @@ -154,7 +154,7 @@ function findDimensions(gd, sliderOpts) {

if(sliderOpts.currentvalue.visible) {
// Get the dimensions of the current value label:
var dummyGroup = gd._tester.append('g');
var dummyGroup = Drawing.tester.append('g');

sliderLabels.each(function(stepOpts) {
var curValPrefix = drawCurrentValue(dummyGroup, sliderOpts, stepOpts.label);
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 @@ -504,7 +504,7 @@ function findDimensions(gd, menuOpts) {
menuOpts.lx = 0;
menuOpts.ly = 0;

var fakeButtons = gd._tester.selectAll('g.' + constants.dropdownButtonClassName)
var fakeButtons = Drawing.tester.selectAll('g.' + constants.dropdownButtonClassName)
.data(menuOpts.buttons);

fakeButtons.enter().append('g')
Expand Down
18 changes: 16 additions & 2 deletions src/lib/svg_text_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ var Lib = require('../lib');
var xmlnsNamespaces = require('../constants/xmlns_namespaces');
var stringMappings = require('../constants/string_mappings');

var DOM_PARSER;

exports.getDOMParser = function() {
if(DOM_PARSER) {
return DOM_PARSER;
} else if(window.DOMParser) {
DOM_PARSER = new window.DOMParser();
return DOM_PARSER;
} else {
throw new Error('Cannot initialize DOMParser');
}
};

// Append SVG

d3.selection.prototype.appendSVG = function(_svgString) {
Expand All @@ -27,8 +40,9 @@ d3.selection.prototype.appendSVG = function(_svgString) {
'</svg>'
].join('');

var dom = new DOMParser().parseFromString(skeleton, 'application/xml'),
childNode = dom.documentElement.firstChild;
var domParser = exports.getDOMParser();
var dom = domParser.parseFromString(skeleton, 'application/xml');
var childNode = dom.documentElement.firstChild;

while(childNode) {
this.node().appendChild(this.node().ownerDocument.importNode(childNode, true));
Expand Down
4 changes: 2 additions & 2 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ Plotly.plot = function(gd, data, layout, config) {
d3.select(gd).classed('js-plotly-plot', true);

// off-screen getBoundingClientRect testing space,
// in #js-plotly-tester (and stored as gd._tester)
// in #js-plotly-tester (and stored as Drawing.tester)
// so we can share cached text across tabs
Drawing.makeTester(gd);
Drawing.makeTester();

// collect promises for any async actions during plotting
// any part of the plotting code can push to gd._promises, then
Expand Down
2 changes: 0 additions & 2 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -1173,8 +1173,6 @@ plots.purge = function(gd) {

// these get recreated on Plotly.plot anyway, but just to be safe
// (and to have a record of them...)
delete gd._tester;
delete gd._testref;
delete gd._promises;
delete gd._redrawTimer;
delete gd.firstscatter;
Expand Down
4 changes: 2 additions & 2 deletions src/traces/carpet/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ function plotOne(gd, plotinfo, cd) {
drawGridLines(xa, ya, boundaryLayer, aax, 'a-boundary', aax._boundarylines);
drawGridLines(xa, ya, boundaryLayer, bax, 'b-boundary', bax._boundarylines);

var maxAExtent = drawAxisLabels(gd._tester, xa, ya, trace, t, labelLayer, aax._labels, 'a-label');
var maxBExtent = drawAxisLabels(gd._tester, xa, ya, trace, t, labelLayer, bax._labels, 'b-label');
var maxAExtent = drawAxisLabels(Drawing.tester, xa, ya, trace, t, labelLayer, aax._labels, 'a-label');
var maxBExtent = drawAxisLabels(Drawing.tester, xa, ya, trace, t, labelLayer, bax._labels, 'b-label');

drawAxisTitles(labelLayer, trace, t, xa, ya, maxAExtent, maxBExtent);

Expand Down
2 changes: 0 additions & 2 deletions test/jasmine/tests/plots_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,6 @@ describe('Test Plots', function() {
expect(gd.undonum).toBeUndefined();
expect(gd.autoplay).toBeUndefined();
expect(gd.changed).toBeUndefined();
expect(gd._tester).toBeUndefined();
expect(gd._testref).toBeUndefined();
expect(gd._promises).toBeUndefined();
expect(gd._redrawTimer).toBeUndefined();
expect(gd.firstscatter).toBeUndefined();
Expand Down

0 comments on commit 66259ed

Please sign in to comment.