Skip to content

Commit 33e5370

Browse files
committed
fix #2797 - clear full canvas and use redrawReglTraces on selections
- no longer need _module.styleOnSelect! - selections on overlaid subplots now work! - remove clearViewport (for now), we could bring it back again to optimize selections on disjoint subplots.
1 parent f9a3b05 commit 33e5370

File tree

6 files changed

+67
-99
lines changed

6 files changed

+67
-99
lines changed

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

+15-32
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ var polygon = require('../../lib/polygon');
1919
var throttle = require('../../lib/throttle');
2020
var makeEventData = require('../../components/fx/helpers').makeEventData;
2121
var getFromId = require('./axis_ids').getFromId;
22-
var sortModules = require('../sort_modules').sortModules;
22+
var clearGlCanvases = require('../../lib/clear_gl_canvases');
23+
var redrawReglTraces = require('../../plot_api/subroutines').redrawReglTraces;
2324

2425
var constants = require('./constants');
2526
var MINSELECT = constants.MINSELECT;
@@ -663,7 +664,7 @@ function isOnlyOnePointSelected(searchTraces) {
663664
}
664665

665666
function updateSelectedState(gd, searchTraces, eventData) {
666-
var i, j, searchInfo, trace;
667+
var i, searchInfo, cd, trace;
667668

668669
if(eventData) {
669670
var pts = eventData.points || [];
@@ -696,43 +697,25 @@ function updateSelectedState(gd, searchTraces, eventData) {
696697
}
697698
}
698699

699-
// group searchInfo traces by trace modules
700-
var lookup = {};
700+
var hasRegl = false;
701701

702702
for(i = 0; i < searchTraces.length; i++) {
703703
searchInfo = searchTraces[i];
704+
cd = searchInfo.cd;
705+
trace = cd[0].trace;
704706

705-
var name = searchInfo._module.name;
706-
if(lookup[name]) {
707-
lookup[name].push(searchInfo);
708-
} else {
709-
lookup[name] = [searchInfo];
707+
if(Registry.traceIs(trace, 'regl')) {
708+
hasRegl = true;
710709
}
710+
711+
var _module = searchInfo._module;
712+
var fn = _module.styleOnSelect || _module.style;
713+
if(fn) fn(gd, cd);
711714
}
712715

713-
var keys = Object.keys(lookup).sort(sortModules);
714-
715-
for(i = 0; i < keys.length; i++) {
716-
var items = lookup[keys[i]];
717-
var len = items.length;
718-
var item0 = items[0];
719-
var trace0 = item0.cd[0].trace;
720-
var _module = item0._module;
721-
var styleSelection = _module.styleOnSelect || _module.style;
722-
723-
if(Registry.traceIs(trace0, 'regl')) {
724-
// plot regl traces per module
725-
var cds = new Array(len);
726-
for(j = 0; j < len; j++) {
727-
cds[j] = items[j].cd;
728-
}
729-
styleSelection(gd, cds);
730-
} else {
731-
// plot svg trace per trace
732-
for(j = 0; j < len; j++) {
733-
styleSelection(gd, items[j].cd);
734-
}
735-
}
716+
if(hasRegl) {
717+
clearGlCanvases(gd);
718+
redrawReglTraces(gd);
736719
}
737720
}
738721

Diff for: src/traces/scattergl/index.js

-34
Original file line numberDiff line numberDiff line change
@@ -294,18 +294,6 @@ function sceneUpdate(gd, subplot) {
294294
scene.dirty = false;
295295
};
296296

297-
scene.clear = function clear() {
298-
var vp = getViewport(gd._fullLayout, subplot.xaxis, subplot.yaxis);
299-
300-
if(scene.select2d) {
301-
clearViewport(scene.select2d, vp);
302-
}
303-
304-
var anyComponent = scene.scatter2d || scene.line2d ||
305-
(scene.glText || [])[0] || scene.fill2d || scene.error2d;
306-
if(anyComponent) clearViewport(anyComponent, vp);
307-
};
308-
309297
// remove scene resources
310298
scene.destroy = function destroy() {
311299
if(scene.fill2d) scene.fill2d.destroy();
@@ -357,14 +345,6 @@ function getViewport(fullLayout, xaxis, yaxis) {
357345
];
358346
}
359347

360-
function clearViewport(comp, vp) {
361-
var gl = comp.regl._gl;
362-
gl.enable(gl.SCISSOR_TEST);
363-
gl.scissor(vp[0], vp[1], vp[2] - vp[0], vp[3] - vp[1]);
364-
gl.clearColor(0, 0, 0, 0);
365-
gl.clear(gl.COLOR_BUFFER_BIT);
366-
}
367-
368348
function plot(gd, subplot, cdata) {
369349
if(!cdata.length) return;
370350

@@ -889,19 +869,6 @@ function selectPoints(searchInfo, selectionTester) {
889869
return selection;
890870
}
891871

892-
function styleOnSelect(gd, cds) {
893-
var stash = cds[0][0].t;
894-
var scene = stash._scene;
895-
896-
// don't clear the subplot if there are splom traces
897-
// on the graph
898-
if(!gd._fullLayout._has('splom')) {
899-
scene.clear();
900-
}
901-
902-
scene.draw();
903-
}
904-
905872
function styleTextSelection(cd) {
906873
var cd0 = cd[0];
907874
var stash = cd0.t;
@@ -951,7 +918,6 @@ module.exports = {
951918
calc: calc,
952919
plot: plot,
953920
hoverPoints: hoverPoints,
954-
styleOnSelect: styleOnSelect,
955921
selectPoints: selectPoints,
956922

957923
sceneOptions: sceneOptions,

Diff for: src/traces/scatterpolargl/index.js

-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ module.exports = {
183183
calc: calc,
184184
plot: plot,
185185
hoverPoints: hoverPoints,
186-
styleOnSelect: ScatterGl.styleOnSelect,
187186
selectPoints: ScatterGl.selectPoints,
188187

189188
meta: {

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

-26
Original file line numberDiff line numberDiff line change
@@ -437,31 +437,6 @@ function selectPoints(searchInfo, selectionTester) {
437437
return selection;
438438
}
439439

440-
function styleOnSelect(gd, cds) {
441-
var fullLayout = gd._fullLayout;
442-
var cd0 = cds[0];
443-
var scene0 = fullLayout._splomScenes[cd0[0].trace.uid];
444-
scene0.matrix.regl.clear({color: true, depth: true});
445-
446-
if(fullLayout._splomGrid) {
447-
fullLayout._splomGrid.draw();
448-
}
449-
450-
for(var i = 0; i < cds.length; i++) {
451-
var scene = fullLayout._splomScenes[cds[i][0].trace.uid];
452-
scene.draw();
453-
}
454-
455-
// redraw all subplot with scattergl traces,
456-
// as we cleared the whole canvas above
457-
if(fullLayout._has('cartesian')) {
458-
for(var k in fullLayout._plots) {
459-
var sp = fullLayout._plots[k];
460-
if(sp._scene) sp._scene.draw();
461-
}
462-
}
463-
}
464-
465440
function getDimIndex(trace, ax) {
466441
var axId = ax._id;
467442
var axLetter = axId.charAt(0);
@@ -490,7 +465,6 @@ module.exports = {
490465
plot: plot,
491466
hoverPoints: hoverPoints,
492467
selectPoints: selectPoints,
493-
styleOnSelect: styleOnSelect,
494468
editStyle: editStyle,
495469

496470
meta: {

Diff for: test/jasmine/tests/gl2d_click_test.js

+43
Original file line numberDiff line numberDiff line change
@@ -1135,4 +1135,47 @@ describe('@noCI Test gl2d lasso/select:', function() {
11351135
.catch(failTest)
11361136
.then(done);
11371137
});
1138+
1139+
it('should work on overlaid subplots', function(done) {
1140+
gd = createGraphDiv();
1141+
1142+
var scene, scene2;
1143+
1144+
Plotly.plot(gd, [{
1145+
x: [1, 2, 3],
1146+
y: [40, 50, 60],
1147+
type: 'scattergl',
1148+
mode: 'markers'
1149+
}, {
1150+
x: [2, 3, 4],
1151+
y: [4, 5, 6],
1152+
yaxis: 'y2',
1153+
type: 'scattergl',
1154+
mode: 'markers'
1155+
}], {
1156+
xaxis: {domain: [0.2, 1]},
1157+
yaxis2: {overlaying: 'y', side: 'left', position: 0},
1158+
showlegend: false,
1159+
margin: {l: 0, t: 0, b: 0, r: 0},
1160+
width: 400,
1161+
height: 400,
1162+
dragmode: 'select'
1163+
})
1164+
.then(delay(100))
1165+
.then(function() {
1166+
scene = gd._fullLayout._plots.xy._scene;
1167+
scene2 = gd._fullLayout._plots.xy2._scene;
1168+
1169+
spyOn(scene.scatter2d, 'draw');
1170+
spyOn(scene2.scatter2d, 'draw');
1171+
})
1172+
.then(function() { return select([[20, 20], [380, 250]]); })
1173+
.then(function() {
1174+
expect(scene.scatter2d.draw).toHaveBeenCalledTimes(1);
1175+
expect(scene2.scatter2d.draw).toHaveBeenCalledTimes(1);
1176+
})
1177+
.catch(failTest)
1178+
.then(done);
1179+
1180+
});
11381181
});

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

+9-6
Original file line numberDiff line numberDiff line change
@@ -1298,23 +1298,26 @@ describe('Test splom select:', function() {
12981298
var cnt = 0;
12991299
var scatterGlCnt = 0;
13001300
var splomCnt = 0;
1301+
var scatterglScene, splomScene;
13011302

13021303
Plotly.newPlot(gd, fig).then(function() {
1303-
// 'scattergl' trace module
1304-
spyOn(gd._fullLayout._modules[0], 'styleOnSelect').and.callFake(function() {
1304+
var fullLayout = gd._fullLayout;
1305+
scatterglScene = fullLayout._plots.xy._scene;
1306+
splomScene = fullLayout._splomScenes[gd._fullData[1].uid];
1307+
1308+
spyOn(scatterglScene, 'draw').and.callFake(function() {
13051309
cnt++;
13061310
scatterGlCnt = cnt;
13071311
});
1308-
// 'splom' trace module
1309-
spyOn(gd._fullLayout._modules[1], 'styleOnSelect').and.callFake(function() {
1312+
spyOn(splomScene, 'draw').and.callFake(function() {
13101313
cnt++;
13111314
splomCnt = cnt;
13121315
});
13131316
})
13141317
.then(function() { return _select([[20, 395], [195, 205]]); })
13151318
.then(function() {
1316-
expect(gd._fullLayout._modules[0].styleOnSelect).toHaveBeenCalledTimes(1);
1317-
expect(gd._fullLayout._modules[1].styleOnSelect).toHaveBeenCalledTimes(1);
1319+
expect(scatterglScene.draw).toHaveBeenCalledTimes(1);
1320+
expect(splomScene.draw).toHaveBeenCalledTimes(1);
13181321

13191322
expect(cnt).toBe(2);
13201323
expect(splomCnt).toBe(1, 'splom redraw before scattergl');

0 commit comments

Comments
 (0)