Skip to content

Commit 81367f1

Browse files
etpinardetpinard
committed
refactor scatter(polar)gl scene.(un)selectBatch internal API
- (un)selectBatch are always arrays (no more `null`) - when selectBatch===[] && unselectBatch===[], use 'base' draws - when selectBatch!==[] || unselectBatche!==[], switch into selection draw calls - this helps handling in/out of selection scenarios - adapt tests accordingly
1 parent e7e028c commit 81367f1

File tree

4 files changed

+80
-83
lines changed

4 files changed

+80
-83
lines changed

‎src/traces/scattergl/index.js

+46-46
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ var createScatter = require('regl-scatter2d');
1212
var createLine = require('regl-line2d');
1313
var createError = require('regl-error2d');
1414
var cluster = require('point-cluster');
15-
var arrayRange = require('array-range');
1615
var Text = require('gl-text');
1716

1817
var Registry = require('../../registry');
@@ -126,6 +125,8 @@ function calc(gd, trace) {
126125
scene.textOptions.push(opts.text);
127126
scene.textSelectedOptions.push(opts.textSel);
128127
scene.textUnselectedOptions.push(opts.textUnsel);
128+
scene.selectBatch.push([]);
129+
scene.unselectBatch.push([]);
129130

130131
stash._scene = scene;
131132
stash.index = scene.count;
@@ -209,13 +210,14 @@ function sceneUpdate(gd, subplot) {
209210
errorYOptions: [],
210211
textOptions: [],
211212
textSelectedOptions: [],
212-
textUnselectedOptions: []
213+
textUnselectedOptions: [],
214+
// selection batches
215+
selectBatch: [],
216+
unselectBatch: []
213217
};
214218

215219
// regl- component stubs, initialized in dirty plot call
216220
var initOpts = {
217-
selectBatch: null,
218-
unselectBatch: null,
219221
fill2d: false,
220222
scatter2d: false,
221223
error2d: false,
@@ -272,17 +274,22 @@ function sceneUpdate(gd, subplot) {
272274
if(scene.errorXOptions[i]) error2d.draw(i);
273275
if(scene.errorYOptions[i]) error2d.draw(i + count);
274276
}
275-
if(scatter2d && scene.markerOptions[i] && (!selectBatch || !selectBatch[i])) {
276-
scatter2d.draw(i);
277+
if(scatter2d && scene.markerOptions[i]) {
278+
if(unselectBatch[i].length) {
279+
var arg = Lib.repeat([], scene.count);
280+
arg[i] = unselectBatch[i];
281+
scatter2d.draw(arg);
282+
} else if(!selectBatch[i].length) {
283+
scatter2d.draw(i);
284+
}
277285
}
278286
if(glText[i] && scene.textOptions[i]) {
279287
glText[i].render();
280288
}
281289
}
282290

283-
if(scatter2d && select2d && selectBatch) {
291+
if(select2d) {
284292
select2d.draw(selectBatch);
285-
scatter2d.draw(unselectBatch);
286293
}
287294

288295
scene.dirty = false;
@@ -553,8 +560,6 @@ function plot(gd, subplot, cdata) {
553560
}
554561

555562
// form batch arrays, and check for selected points
556-
scene.selectBatch = null;
557-
scene.unselectBatch = null;
558563
var dragmode = fullLayout.dragmode;
559564
var selectMode = dragmode === 'lasso' || dragmode === 'select';
560565
var clickSelectEnabled = fullLayout.clickmode.indexOf('select') > -1;
@@ -571,11 +576,6 @@ function plot(gd, subplot, cdata) {
571576
if(trace.selectedpoints || selectMode || clickSelectEnabled) {
572577
if(!selectMode) selectMode = true;
573578

574-
if(!scene.selectBatch) {
575-
scene.selectBatch = [];
576-
scene.unselectBatch = [];
577-
}
578-
579579
// regenerate scene batch, if traces number changed during selection
580580
if(trace.selectedpoints) {
581581
var selPts = scene.selectBatch[index] = Lib.selIndices2selPoints(trace);
@@ -613,13 +613,18 @@ function plot(gd, subplot, cdata) {
613613
scene.select2d = createScatter(fullLayout._glcanvas.data()[1].regl);
614614
}
615615

616-
if(scene.scatter2d && scene.selectBatch && scene.selectBatch.length) {
617-
// update only traces with selection
618-
scene.scatter2d.update(scene.markerUnselectedOptions.map(function(opts, i) {
619-
return scene.selectBatch[i] ? opts : null;
620-
}));
616+
// use unselected styles on 'context' canvas
617+
if(scene.scatter2d) {
618+
var unselOpts = new Array(count);
619+
for(i = 0; i < count; i++) {
620+
unselOpts[i] = scene.selectBatch[i].length || scene.unselectBatch[i].length ?
621+
scene.markerUnselectedOptions[i] :
622+
{};
623+
}
624+
scene.scatter2d.update(unselOpts);
621625
}
622626

627+
// use selected style on 'focus' canvas
623628
if(scene.select2d) {
624629
scene.select2d.update(scene.markerOptions);
625630
scene.select2d.update(scene.markerSelectedOptions);
@@ -854,7 +859,6 @@ function calcHover(pointData, x, y, trace) {
854859
return pointData;
855860
}
856861

857-
858862
function selectPoints(searchInfo, selectionTester) {
859863
var cd = searchInfo.cd;
860864
var selection = [];
@@ -864,23 +868,23 @@ function selectPoints(searchInfo, selectionTester) {
864868
var x = stash.x;
865869
var y = stash.y;
866870
var scene = stash._scene;
871+
var index = stash.index;
867872

868873
if(!scene) return selection;
869874

870875
var hasText = subTypes.hasText(trace);
871876
var hasMarkers = subTypes.hasMarkers(trace);
872877
var hasOnlyLines = !hasMarkers && !hasText;
878+
873879
if(trace.visible !== true || hasOnlyLines) return selection;
874880

881+
var els = [];
882+
var unels = [];
883+
875884
// degenerate polygon does not enable selection
876885
// filter out points by visible scatter ones
877-
var els = null;
878-
var unels = null;
879-
// FIXME: clearing selection does not work here
880-
var i;
881886
if(selectionTester !== false && !selectionTester.degenerate) {
882-
els = [], unels = [];
883-
for(i = 0; i < len; i++) {
887+
for(var i = 0; i < len; i++) {
884888
if(selectionTester.contains([stash.xpx[i], stash.ypx[i]], false, i, searchInfo)) {
885889
els.push(i);
886890
selection.push({
@@ -892,30 +896,26 @@ function selectPoints(searchInfo, selectionTester) {
892896
unels.push(i);
893897
}
894898
}
895-
} else {
896-
unels = arrayRange(len);
897899
}
898900

899-
// make sure selectBatch is created
900-
if(!scene.selectBatch) {
901-
scene.selectBatch = [];
902-
scene.unselectBatch = [];
903-
}
901+
if(hasMarkers) {
902+
var scatter2d = scene.scatter2d;
904903

905-
if(!scene.selectBatch[stash.index]) {
906-
// enter every trace select mode
907-
for(i = 0; i < scene.count; i++) {
908-
scene.selectBatch[i] = [];
909-
scene.unselectBatch[i] = [];
910-
}
911-
// we should turn scatter2d into unselected once we have any points selected
912-
if(hasMarkers) {
913-
scene.scatter2d.update(scene.markerUnselectedOptions);
904+
if(!els.length && !unels.length) {
905+
// reset to base styles when clearing
906+
var baseOpts = new Array(scene.count);
907+
baseOpts[index] = scene.markerOptions[index];
908+
scatter2d.update.apply(scatter2d, baseOpts);
909+
} else if(!scene.selectBatch[index].length && !scene.unselectBatch[index].length) {
910+
// set unselected styles on 'context' canvas (if not done already)
911+
var unselOpts = new Array(scene.count);
912+
unselOpts[index] = scene.markerUnselectedOptions[index];
913+
scatter2d.update.apply(scatter2d, unselOpts);
914914
}
915915
}
916916

917-
scene.selectBatch[stash.index] = els;
918-
scene.unselectBatch[stash.index] = unels;
917+
scene.selectBatch[index] = els;
918+
scene.unselectBatch[index] = unels;
919919

920920
if(hasText) {
921921
styleTextSelection(cd);
@@ -938,7 +938,7 @@ function styleTextSelection(cd) {
938938
var opts = Lib.extendFlat({}, baseOpts);
939939
var i, j;
940940

941-
if(els && unels) {
941+
if(els.length || unels.length) {
942942
var stc = selOpts.color;
943943
var utc = unselOpts.color;
944944
var base = baseOpts.color;

‎src/traces/scatterpolargl/index.js

+2
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ function plot(gd, subplot, cdata) {
155155
scene.textOptions.push(opts.text);
156156
scene.textSelectedOptions.push(opts.textSel);
157157
scene.textUnselectedOptions.push(opts.textUnsel);
158+
scene.selectBatch.push([]);
159+
scene.unselectBatch.push([]);
158160

159161
stash.x = x;
160162
stash.y = y;

‎test/jasmine/tests/gl2d_double_click_test.js

+23-28
Original file line numberDiff line numberDiff line change
@@ -454,14 +454,14 @@ describe('Test gl2d lasso/select:', function() {
454454
);
455455
updateCalls.forEach(function(d, i) {
456456
d.args.forEach(function(arg, j) {
457-
if('range' in arg[0]) {
457+
if(Array.isArray(arg) && 'range' in arg[0]) {
458458
// no need to assert range value in detail
459459
expect(exp.updateArgs[i][j]).toBe(
460460
'range',
461461
'scatter.update range update - ' + msg
462462
);
463463
} else {
464-
expect(arg).toBe(
464+
expect(arg).toEqual(
465465
exp.updateArgs[i][j],
466466
'scatter.update call' + i + ' arg' + j + ' - ' + msg
467467
);
@@ -475,7 +475,7 @@ describe('Test gl2d lasso/select:', function() {
475475
);
476476
drawCalls.forEach(function(d, i) {
477477
d.args.forEach(function(arg, j) {
478-
expect(arg).toBe(
478+
expect(arg).toEqual(
479479
exp.drawArgs[i][j],
480480
'scatter.draw call' + i + ' arg' + j + ' - ' + msg
481481
);
@@ -486,9 +486,7 @@ describe('Test gl2d lasso/select:', function() {
486486
scene.scatter2d.draw.calls.reset();
487487
}
488488

489-
var unselectBatchOld;
490-
491-
Plotly.newPlot('graph', [{
489+
Plotly.newPlot(gd, [{
492490
type: 'scattergl',
493491
mode: 'markers',
494492
y: [1, 2, 1],
@@ -507,8 +505,8 @@ describe('Test gl2d lasso/select:', function() {
507505
})
508506
.then(function() {
509507
_assert('base', {
510-
selectBatch: [],
511-
unselectBatch: [],
508+
selectBatch: [[]],
509+
unselectBatch: [[]],
512510
updateArgs: [],
513511
drawArgs: []
514512
});
@@ -521,9 +519,10 @@ describe('Test gl2d lasso/select:', function() {
521519
unselectBatch: [[0, 2]],
522520
updateArgs: [
523521
// N.B. scatter2d now draws unselected options
524-
[scene.markerUnselectedOptions],
522+
scene.markerUnselectedOptions,
525523
],
526524
drawArgs: [
525+
// draw unselectBatch
527526
[scene.unselectBatch]
528527
]
529528
});
@@ -532,45 +531,42 @@ describe('Test gl2d lasso/select:', function() {
532531
.then(function() {
533532
var scene = grabScene();
534533
_assert('after doubleclick', {
535-
selectBatch: [null],
536-
unselectBatch: [[0, 1, 2]],
537-
updateArgs: [],
534+
selectBatch: [[]],
535+
unselectBatch: [[]],
536+
updateArgs: [
537+
// N.B. bring scatter2d back to 'base' marker options
538+
[scene.markerOptions[0]]
539+
],
538540
drawArgs: [
539-
// call in no-selection loop (can we get rid of this?)
540-
[0],
541-
// call with unselectBatch
542-
[scene.unselectBatch]
541+
// call data[0] batch
542+
[0]
543543
]
544544
});
545545
})
546546
.then(function() { return Plotly.relayout(gd, 'dragmode', 'pan'); })
547547
.then(function() {
548548
_assert('after relayout to *pan*', {
549-
selectBatch: [null],
550-
unselectBatch: [[0, 1, 2]],
549+
selectBatch: [[]],
550+
unselectBatch: [[]],
551551
// nothing to do when relayouting to 'pan'
552552
updateArgs: [],
553553
drawArgs: []
554554
});
555-
556-
// keep ref for next _assert()
557-
var scene = grabScene();
558-
unselectBatchOld = scene.unselectBatch;
559555
})
560556
.then(function() { return drag([[200, 200], [250, 250]]); })
561557
.then(function() {
562558
var scene = grabScene();
563559
_assert('after pan', {
564-
selectBatch: null,
565-
unselectBatch: null,
560+
selectBatch: [[]],
561+
unselectBatch: [[]],
566562
// drag triggers:
567563
// - 2 scene.update() calls, which each invoke
568564
// + 1 scatter2d.update (updating viewport)
569-
// + 2 scatter2d.draw (same as after double-click)
565+
// + 1 scatter2d.draw (same as after double-click)
570566
//
571567
// replot on mouseup triggers:
572-
// - 1 scatter2d.update updating viewport
573568
// - 1 scatter2d.update resetting markerOptions
569+
// - 1 scatter2d.update updating viewport
574570
// - 1 (full) scene.draw()
575571
updateArgs: [
576572
['range'],
@@ -580,10 +576,9 @@ describe('Test gl2d lasso/select:', function() {
580576
['range']
581577
],
582578
drawArgs: [
579+
// call data[0] batch
583580
[0],
584-
[unselectBatchOld],
585581
[0],
586-
[unselectBatchOld],
587582
[0]
588583
]
589584
});

‎test/jasmine/tests/gl2d_plot_interact_test.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,7 @@ describe('Test gl2d plots', function() {
884884

885885
it('@gl should restyle opacity', function(done) {
886886
// #2299
887-
spyOn(ScatterGl, 'calc');
887+
spyOn(ScatterGl, 'calc').and.callThrough();
888888

889889
var dat = [{
890890
'x': [1, 2, 3],
@@ -944,8 +944,8 @@ describe('Test gl2d plots', function() {
944944
var scene = gd._fullLayout._plots.xy._scene;
945945

946946
expect(scene.count).toBe(2);
947-
expect(scene.selectBatch).toEqual([[0]]);
948-
expect(scene.unselectBatch).toEqual([[]]);
947+
expect(scene.selectBatch).toEqual([[0], []]);
948+
expect(scene.unselectBatch).toEqual([[], []]);
949949
expect(scene.markerOptions.length).toBe(2);
950950
expect(scene.markerOptions[1].color).toEqual(new Uint8Array([255, 0, 0, 255]));
951951
expect(scene.textOptions.length).toBe(2);
@@ -958,8 +958,8 @@ describe('Test gl2d plots', function() {
958958
var scene = gd._fullLayout._plots.xy._scene;
959959
var msg = 'clearing under dragmode select';
960960

961-
expect(scene.selectBatch).toEqual([], msg);
962-
expect(scene.unselectBatch).toEqual([], msg);
961+
expect(scene.selectBatch).toEqual([[], []], msg);
962+
expect(scene.unselectBatch).toEqual([[], []], msg);
963963

964964
// scattergl uses different pathways for select/lasso & zoom/pan
965965
return Plotly.relayout(gd, 'dragmode', 'pan');
@@ -968,8 +968,8 @@ describe('Test gl2d plots', function() {
968968
var scene = gd._fullLayout._plots.xy._scene;
969969
var msg = 'cleared under dragmode pan';
970970

971-
expect(scene.selectBatch).toEqual([], msg);
972-
expect(scene.unselectBatch).toEqual([], msg);
971+
expect(scene.selectBatch).toEqual([[], []], msg);
972+
expect(scene.unselectBatch).toEqual([[], []], msg);
973973

974974
return Plotly.restyle(gd, 'selectedpoints', [[1, 2], [0]]);
975975
})
@@ -986,8 +986,8 @@ describe('Test gl2d plots', function() {
986986
var scene = gd._fullLayout._plots.xy._scene;
987987
var msg = 'clearing under dragmode pan';
988988

989-
expect(scene.selectBatch).toBe(null, msg);
990-
expect(scene.unselectBatch).toBe(null, msg);
989+
expect(scene.selectBatch).toEqual([[], []], msg);
990+
expect(scene.unselectBatch).toEqual([[], []], msg);
991991
})
992992
.catch(failTest)
993993
.then(done);

0 commit comments

Comments
 (0)