Skip to content

Commit 3574ece

Browse files
etpinardetpinard
committed
move splom scene ref to fullLayout
... and out of the gd.calcdata[i][0].t stash, so that: - scene refs can be relinked properly on updates - scene refs can be destroyed properly when needed - move also visibileDims out of calcdata stash to fullData[i], for convenience.
1 parent 56f2ee9 commit 3574ece

File tree

5 files changed

+95
-78
lines changed

5 files changed

+95
-78
lines changed

Diff for: ‎src/components/fx/hover.js

+4
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,10 @@ function _hover(gd, evt, subplot, noHoverEvent) {
396396
if(fullLayout[subplotId]) {
397397
pointData.subplot = fullLayout[subplotId]._subplot;
398398
}
399+
// add ref to splom scene
400+
if(fullLayout._splomScenes && fullLayout._splomScenes[trace.uid]) {
401+
pointData.scene = fullLayout._splomScenes[trace.uid];
402+
}
399403

400404
closedataPreviousLength = hoverData.length;
401405

Diff for: ‎src/plots/cartesian/select.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,9 @@ function determineSearchTraces(gd, xAxes, yAxes, subplot) {
514514
// FIXME: make sure we don't have more than single axis for splom
515515
trace._xaxes[xAxisIds[0]] && trace._yaxes[yAxisIds[0]]
516516
) {
517-
searchTraces.push(createSearchInfo(trace._module, cd, xAxes[0], yAxes[0]));
517+
var info = createSearchInfo(trace._module, cd, xAxes[0], yAxes[0]);
518+
info.scene = gd._fullLayout._splomScenes[trace.uid];
519+
searchTraces.push(info);
518520
} else {
519521
if(xAxisIds.indexOf(trace.xaxis) === -1) continue;
520522
if(yAxisIds.indexOf(trace.yaxis) === -1) continue;

Diff for: ‎src/traces/splom/base_plot.js

+29-36
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,18 @@ function drag(gd) {
4545
for(var i = 0; i < cd.length; i++) {
4646
var cd0 = cd[i][0];
4747
var trace = cd0.trace;
48-
var stash = cd0.t;
49-
var scene = stash._scene;
48+
var scene = fullLayout._splomScenes[trace.uid];
5049

5150
if(trace.type === 'splom' && scene && scene.matrix) {
52-
dragOne(gd, trace, stash, scene);
51+
dragOne(gd, trace, scene);
5352
}
5453
}
5554
}
5655

57-
function dragOne(gd, trace, stash, scene) {
56+
function dragOne(gd, trace, scene) {
5857
var visibleLength = scene.matrixOptions.data.length;
59-
var visibleDims = stash.visibleDims;
60-
var ranges = new Array(visibleLength);
58+
var visibleDims = trace._visibleDims;
59+
var ranges = scene.viewOpts.ranges = new Array(visibleLength);
6160

6261
for(var k = 0; k < visibleDims.length; k++) {
6362
var i = visibleDims[k];
@@ -168,46 +167,40 @@ function makeGridData(gd) {
168167
return gridBatches;
169168
}
170169

171-
function clean(newFullData, newFullLayout, oldFullData, oldFullLayout, oldCalcdata) {
172-
var oldModules = oldFullLayout._modules || [];
173-
var newModules = newFullLayout._modules || [];
170+
function clean(newFullData, newFullLayout, oldFullData, oldFullLayout) {
171+
oldLoop:
172+
for(var i = 0; i < oldFullData.length; i++) {
173+
var oldTrace = oldFullData[i];
174174

175-
var hadSplom, hasSplom;
176-
var i;
175+
if(oldTrace.type === 'splom') {
176+
for(var j = 0; j < newFullData.length; j++) {
177+
var newTrace = newFullData[j];
177178

178-
for(i = 0; i < oldModules.length; i++) {
179-
if(oldModules[i].name === 'splom') {
180-
hadSplom = true;
181-
break;
182-
}
183-
}
184-
for(i = 0; i < newModules.length; i++) {
185-
if(newModules[i].name === 'splom') {
186-
hasSplom = true;
187-
break;
188-
}
189-
}
179+
if(oldTrace.uid === newTrace.uid && newTrace.type === 'splom') {
180+
continue oldLoop;
181+
}
182+
}
190183

191-
if(hadSplom && !hasSplom) {
192-
for(i = 0; i < oldCalcdata.length; i++) {
193-
var cd0 = oldCalcdata[i][0];
194-
var trace = cd0.trace;
195-
var scene = cd0.t._scene;
196-
197-
if(
198-
trace.type === 'splom' &&
199-
scene && scene.matrix && scene.matrix.destroy
200-
) {
201-
scene.matrix.destroy();
202-
cd0.t._scene = null;
184+
if(oldFullLayout._splomScenes) {
185+
var scene = oldFullLayout._splomScenes[oldTrace.uid];
186+
if(scene && scene.destroy) scene.destroy();
187+
// must first set scene to null in order to get garbage collected
188+
oldFullLayout._splomScenes[oldTrace.uid] = null;
189+
delete oldFullLayout._splomScenes[oldTrace.uid];
203190
}
204191
}
205192
}
206193

194+
if(Object.keys(oldFullLayout._splomScenes || {}).length === 0) {
195+
delete oldFullLayout._splomScenes;
196+
}
197+
207198
if(oldFullLayout._splomGrid &&
208199
(!newFullLayout._hasOnlyLargeSploms && oldFullLayout._hasOnlyLargeSploms)) {
200+
// must first set scene to null in order to get garbage collected
209201
oldFullLayout._splomGrid.destroy();
210202
oldFullLayout._splomGrid = null;
203+
delete oldFullLayout._splomGrid;
211204
}
212205

213206
Cartesian.clean(newFullData, newFullLayout, oldFullData, oldFullLayout);
@@ -228,7 +221,7 @@ function updateFx(gd) {
228221
var trace = cd0.trace;
229222

230223
if(trace.type === 'splom') {
231-
var scene = cd0.t._scene;
224+
var scene = fullLayout._splomScenes[trace.uid];
232225
if(scene.selectBatch === null) {
233226
scene.matrix.update(scene.matrixOptions, null);
234227
}

Diff for: ‎src/traces/splom/index.js

+32-27
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,13 @@ var TOO_MANY_POINTS = require('../scattergl/constants').TOO_MANY_POINTS;
3030
function calc(gd, trace) {
3131
var dimensions = trace.dimensions;
3232
var commonLength = trace._length;
33-
var stash = {};
3433
var opts = {};
3534
// 'c' for calculated, 'l' for linear,
3635
// only differ here for log axes, pass ldata to createMatrix as 'data'
3736
var cdata = opts.cdata = [];
3837
var ldata = opts.data = [];
3938
// keep track of visible dimensions
40-
var visibleDims = stash.visibleDims = [];
39+
var visibleDims = trace._visibleDims = [];
4140
var i, k, dim, xa, ya;
4241

4342
function makeCalcdata(ax, dim) {
@@ -107,22 +106,27 @@ function calc(gd, trace) {
107106
calcAxisExpansion(gd, trace, xa, ya, cdata[k], cdata[k], ppad);
108107
}
109108

110-
var scene = stash._scene = sceneUpdate(gd, stash);
109+
var scene = sceneUpdate(gd, trace);
111110
if(!scene.matrix) scene.matrix = true;
112111
scene.matrixOptions = opts;
113112

114113
scene.selectedOptions = convertMarkerSelection(trace, trace.selected);
115114
scene.unselectedOptions = convertMarkerSelection(trace, trace.unselected);
116115

117-
return [{x: false, y: false, t: stash, trace: trace}];
116+
return [{x: false, y: false, t: {}, trace: trace}];
118117
}
119118

120-
function sceneUpdate(gd, stash) {
121-
var scene = stash._scene;
119+
function sceneUpdate(gd, trace) {
120+
var fullLayout = gd._fullLayout;
121+
var uid = trace.uid;
122122

123-
var reset = {
124-
dirty: true
125-
};
123+
// must place ref to 'scene' in fullLayout, so that:
124+
// - it can be relinked properly on updates
125+
// - it can be destroyed properly when needed
126+
var splomScenes = fullLayout._splomScenes;
127+
if(!splomScenes) splomScenes = fullLayout._splomScenes = {};
128+
129+
var reset = {dirty: true};
126130

127131
var first = {
128132
selectBatch: null,
@@ -131,8 +135,10 @@ function sceneUpdate(gd, stash) {
131135
select: null
132136
};
133137

138+
var scene = splomScenes[trace.uid];
139+
134140
if(!scene) {
135-
scene = stash._scene = Lib.extendFlat({}, reset, first);
141+
scene = splomScenes[uid] = Lib.extendFlat({}, reset, first);
136142

137143
scene.draw = function draw() {
138144
// draw traces in selection mode
@@ -147,13 +153,13 @@ function sceneUpdate(gd, stash) {
147153

148154
// remove scene resources
149155
scene.destroy = function destroy() {
150-
if(scene.matrix) scene.matrix.destroy();
151-
156+
if(scene.matrix && scene.matrix.destroy) {
157+
scene.matrix.destroy();
158+
}
152159
scene.matrixOptions = null;
153160
scene.selectBatch = null;
154161
scene.unselectBatch = null;
155-
156-
stash._scene = null;
162+
scene = null;
157163
};
158164
}
159165

@@ -178,7 +184,7 @@ function plotOne(gd, cd0) {
178184
var gs = fullLayout._size;
179185
var trace = cd0.trace;
180186
var stash = cd0.t;
181-
var scene = stash._scene;
187+
var scene = fullLayout._splomScenes[trace.uid];
182188
var matrixOpts = scene.matrixOptions;
183189
var cdata = matrixOpts.cdata;
184190
var regl = fullLayout._glcanvas.data()[0].regl;
@@ -194,7 +200,7 @@ function plotOne(gd, cd0) {
194200
matrixOpts.upper = trace.showlowerhalf;
195201
matrixOpts.diagonal = trace.diagonal.visible;
196202

197-
var visibleDims = stash.visibleDims;
203+
var visibleDims = trace._visibleDims;
198204
var visibleLength = cdata.length;
199205
var viewOpts = {};
200206
viewOpts.ranges = new Array(visibleLength);
@@ -305,17 +311,16 @@ function plotOne(gd, cd0) {
305311
function hoverPoints(pointData, xval, yval) {
306312
var cd = pointData.cd;
307313
var trace = cd[0].trace;
308-
var stash = cd[0].t;
309-
var scene = stash._scene;
314+
var scene = pointData.scene;
310315
var cdata = scene.matrixOptions.cdata;
311316
var xa = pointData.xa;
312317
var ya = pointData.ya;
313318
var xpx = xa.c2p(xval);
314319
var ypx = ya.c2p(yval);
315320
var maxDistance = pointData.distance;
316321

317-
var xi = getDimIndex(trace, stash, xa);
318-
var yi = getDimIndex(trace, stash, ya);
322+
var xi = getDimIndex(trace, xa);
323+
var yi = getDimIndex(trace, ya);
319324
if(xi === false || yi === false) return [pointData];
320325

321326
var x = cdata[xi];
@@ -352,7 +357,7 @@ function selectPoints(searchInfo, selectionTester) {
352357
var cd = searchInfo.cd;
353358
var trace = cd[0].trace;
354359
var stash = cd[0].t;
355-
var scene = stash._scene;
360+
var scene = searchInfo.scene;
356361
var cdata = scene.matrixOptions.cdata;
357362
var xa = searchInfo.xaxis;
358363
var ya = searchInfo.yaxis;
@@ -364,8 +369,8 @@ function selectPoints(searchInfo, selectionTester) {
364369
var hasOnlyLines = (!subTypes.hasMarkers(trace) && !subTypes.hasText(trace));
365370
if(trace.visible !== true || hasOnlyLines) return selection;
366371

367-
var xi = getDimIndex(trace, stash, xa);
368-
var yi = getDimIndex(trace, stash, ya);
372+
var xi = getDimIndex(trace, xa);
373+
var yi = getDimIndex(trace, ya);
369374
if(xi === false || yi === false) return selection;
370375

371376
var xpx = stash.xpx[xi];
@@ -424,15 +429,15 @@ function style(gd, cds) {
424429

425430
var fullLayout = gd._fullLayout;
426431
var cd0 = cds[0];
427-
var scene0 = cd0[0].t._scene;
432+
var scene0 = fullLayout._splomScenes[cd0[0].trace.uid];
428433
scene0.matrix.regl.clear({color: true, depth: true});
429434

430435
if(fullLayout._splomGrid) {
431436
fullLayout._splomGrid.draw();
432437
}
433438

434439
for(var i = 0; i < cds.length; i++) {
435-
var scene = cds[i][0].t._scene;
440+
var scene = fullLayout._splomScenes[cds[i][0].trace.uid];
436441
scene.draw();
437442
}
438443

@@ -446,11 +451,11 @@ function style(gd, cds) {
446451
}
447452
}
448453

449-
function getDimIndex(trace, stash, ax) {
454+
function getDimIndex(trace, ax) {
450455
var axId = ax._id;
451456
var axLetter = axId.charAt(0);
452457
var ind = {x: 0, y: 1}[axLetter];
453-
var visibleDims = stash.visibleDims;
458+
var visibleDims = trace._visibleDims;
454459

455460
for(var k = 0; k < visibleDims.length; k++) {
456461
var i = visibleDims[k];

Diff for: ‎test/jasmine/tests/splom_test.js

+27-14
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,11 @@ describe('Test splom trace calc step:', function() {
422422
yaxis: {type: 'linear'}
423423
});
424424

425-
var cd = gd.calcdata[0][0];
425+
var trace = gd._fullData[0];
426+
var scene = gd._fullLayout._splomScenes[trace.uid];
426427

427-
expect(cd.t._scene.matrixOptions.data).toBeCloseTo2DArray([[2, 1, 2]]);
428-
expect(cd.t.visibleDims).toEqual([1]);
428+
expect(scene.matrixOptions.data).toBeCloseTo2DArray([[2, 1, 2]]);
429+
expect(trace._visibleDims).toEqual([1]);
429430
expect(Lib.log).toHaveBeenCalledTimes(1);
430431
expect(Lib.log).toHaveBeenCalledWith('Skipping splom dimension 0 with conflicting axis types');
431432
});
@@ -448,13 +449,14 @@ describe('Test splom interactions:', function() {
448449

449450
Plotly.plot(gd, fig).then(function() {
450451
expect(gd._fullLayout._splomGrid).toBeDefined();
451-
expect(gd.calcdata[0][0].t._scene).toBeDefined();
452+
expect(gd._fullLayout._splomScenes).toBeDefined();
453+
expect(Object.keys(gd._fullLayout._splomScenes).length).toBe(1);
452454

453-
return Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout, gd.calcdata);
455+
return Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout);
454456
})
455457
.then(function() {
456-
expect(gd._fullLayout._splomGrid).toBe(null);
457-
expect(gd.calcdata[0][0].t._scene).toBe(null);
458+
expect(gd._fullLayout._splomGrid).toBeUndefined();
459+
expect(gd._fullLayout._splomScenes).toBeUndefined();
458460
})
459461
.catch(failTest)
460462
.then(done);
@@ -658,14 +660,22 @@ describe('Test splom interactions:', function() {
658660
var fig = Lib.extendDeep({}, require('@mocks/splom_iris.json'));
659661

660662
function _assert(msg, exp) {
663+
var splomScenes = gd._fullLayout._splomScenes;
664+
var ids = Object.keys(splomScenes);
665+
661666
for(var i = 0; i < 3; i++) {
662-
expect(Boolean(gd.calcdata[i][0].t._scene))
663-
.toBe(Boolean(exp[i]), msg + ' - trace ' + i);
667+
var drawFn = splomScenes[ids[i]].draw;
668+
expect(drawFn).toHaveBeenCalledTimes(exp[i], msg + ' - trace ' + i);
669+
drawFn.calls.reset();
664670
}
665671
}
666672

667673
Plotly.plot(gd, fig).then(function() {
668-
_assert('base', [1, 1, 1]);
674+
var splomScenes = gd._fullLayout._splomScenes;
675+
for(var k in splomScenes) {
676+
spyOn(splomScenes[k], 'draw').and.callThrough();
677+
}
678+
669679
return Plotly.restyle(gd, 'visible', 'legendonly', [0, 2]);
670680
})
671681
.then(function() {
@@ -890,7 +900,8 @@ describe('Test splom drag:', function() {
890900

891901
Plotly.plot(gd, fig)
892902
.then(function() {
893-
var scene = gd.calcdata[0][0].t._scene;
903+
var uid = gd._fullData[0].uid;
904+
var scene = gd._fullLayout._splomScenes[uid];
894905
spyOn(scene.matrix, 'update');
895906
spyOn(scene.matrix, 'draw');
896907

@@ -906,7 +917,8 @@ describe('Test splom drag:', function() {
906917
})
907918
.then(function() { return _drag([130, 130], [150, 150]); })
908919
.then(function() {
909-
var scene = gd.calcdata[0][0].t._scene;
920+
var uid = gd._fullData[0].uid;
921+
var scene = gd._fullLayout._splomScenes[uid];
910922
// N.B. _drag triggers two updateSubplots call
911923
// - 1 update and 1 draw call per updateSubplot
912924
// - 2 update calls (1 for data, 1 for view opts)
@@ -1107,7 +1119,7 @@ describe('Test splom select:', function() {
11071119
grid: {xgap: 0, ygap: 0}
11081120
};
11091121

1110-
var scene;
1122+
var uid, scene;
11111123

11121124
function _assert(msg, exp) {
11131125
expect(scene.matrix.update).toHaveBeenCalledTimes(exp.updateCnt, 'update cnt');
@@ -1122,7 +1134,8 @@ describe('Test splom select:', function() {
11221134
}
11231135

11241136
Plotly.plot(gd, fig).then(function() {
1125-
scene = gd.calcdata[0][0].t._scene;
1137+
uid = gd._fullData[0].uid;
1138+
scene = gd._fullLayout._splomScenes[uid];
11261139
spyOn(scene.matrix, 'update').and.callThrough();
11271140
spyOn(scene.matrix, 'draw').and.callThrough();
11281141
})

0 commit comments

Comments
 (0)