Skip to content

Refactor scattergl selection #2311

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Feb 2, 2018
+136 −56
Merged

Refactor scattergl selection #2311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c331266
Unify selection mode
dy Jan 30, 2018
c30e3f3
Decompatibilitize
dy Jan 31, 2018
a9d387a
Force plot opacity change
dy Jan 31, 2018
53c548f
Add opacity change test
dy Jan 31, 2018
1da7796
Add selected points test
dy Jan 31, 2018
4e66ff5
Add #2298 test
dy Jan 31, 2018
ab5f6f0
Make selection per-trace instead of per-plot
dy Feb 1, 2018
98cddf3
Make scattergl selection persistent with scatter
dy Feb 1, 2018
0ad735c
Unsingle test
dy Feb 1, 2018
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
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"3d-view": "^2.0.0",
"@plotly/d3-sankey": "^0.5.0",
"alpha-shape": "^1.0.0",
"array-range": "^1.0.1",
"bubleify": "^1.0.0",
"canvas-fit": "^1.5.0",
"color-normalize": "^1.0.3",
Expand Down Expand Up @@ -98,7 +99,7 @@
"regl": "^1.3.1",
"regl-error2d": "^2.0.3",
"regl-line2d": "^2.1.2",
"regl-scatter2d": "^2.1.12",
"regl-scatter2d": "^2.1.13",
"right-now": "^1.0.0",
"robust-orientation": "^1.1.3",
"sane-topojson": "^2.0.0",
Expand Down
11 changes: 8 additions & 3 deletions src/traces/scattergl/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

'use strict';

var plotAttrs = require('../../plots/attributes');
var scatterAttrs = require('../scatter/attributes');
var colorAttributes = require('../../components/colorscale/color_attributes');
var colorAttrs = require('../../components/colorscale/color_attributes');

var DASHES = require('../../constants/gl2d_dashes');
var extendFlat = require('../../lib/extend').extendFlat;
Expand Down Expand Up @@ -56,7 +57,7 @@ var attrs = module.exports = overrideAll({
description: 'Sets the style of the lines.'
}
},
marker: extendFlat({}, colorAttributes('marker'), {
marker: extendFlat({}, colorAttrs('marker'), {
symbol: scatterMarkerAttrs.symbol,
size: scatterMarkerAttrs.size,
sizeref: scatterMarkerAttrs.sizeref,
Expand All @@ -65,7 +66,7 @@ var attrs = module.exports = overrideAll({
opacity: scatterMarkerAttrs.opacity,
showscale: scatterMarkerAttrs.showscale,
colorbar: scatterMarkerAttrs.colorbar,
line: extendFlat({}, colorAttributes('marker.line'), {
line: extendFlat({}, colorAttrs('marker.line'), {
width: scatterMarkerLineAttrs.width
})
}),
Expand All @@ -82,6 +83,10 @@ var attrs = module.exports = overrideAll({
marker: scatterAttrs.unselected.marker
},

opacity: extendFlat({}, plotAttrs.opacity, {
editType: 'calc'
}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

I think the best way to test this would be to spy on ScatterGl.calc checking that Plotly.relayout(gd, 'opacity', /**/) does invoke it - similar to this suite.


error_y: scatterAttrs.error_y,
error_x: scatterAttrs.error_x
}, 'calc', 'nested');
Expand Down
104 changes: 53 additions & 51 deletions src/traces/scattergl/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var createError = require('regl-error2d');
var rgba = require('color-normalize');
var svgSdf = require('svg-path-sdf');
var createRegl = require('regl');
var arrayRange = require('array-range');
var fillHoverText = require('../scatter/fill_hover_text');
var isNumeric = require('fast-isnumeric');

Expand Down Expand Up @@ -122,7 +123,6 @@ function calc(container, trace) {
}
}


calcColorscales(trace);

var options = sceneOptions(container, subplot, trace, positions);
Expand Down Expand Up @@ -396,7 +396,9 @@ function sceneOptions(container, subplot, trace, positions) {
function makeSelectedOptions(selected, markerOpts) {
var options = {};

if(selected.marker.symbol) {
if(!selected) return options;

if(selected.marker && selected.marker.symbol) {
options = makeMarkerOptions(extend({}, markerOpts, selected.marker));
}

Expand Down Expand Up @@ -599,12 +601,15 @@ function sceneUpdate(container, subplot) {
scene.error2d.draw(i);
scene.error2d.draw(i + scene.count);
}
if(scene.scatter2d && !scene.selectBatch) {
scene.scatter2d.draw(i);
if(scene.scatter2d) {
// traces in no-selection mode
if(!scene.selectBatch || !scene.selectBatch[i]) {
scene.scatter2d.draw(i);
}
}
}

// persistent selection draw
// draw traces in selection mode
if(scene.select2d && scene.selectBatch) {
scene.select2d.draw(scene.selectBatch);
scene.scatter2d.draw(scene.unselectBatch);
Expand Down Expand Up @@ -740,8 +745,8 @@ function plot(container, subplot, cdata) {
if(!cdata.length) return;

var layout = container._fullLayout;
var stash = cdata[0][0].t;
var scene = stash.scene;
var scene = cdata[0][0].t.scene;
var dragmode = layout.dragmode;

// we may have more subplots than initialized data due to Axes.getSubplots method
if(!scene) return;
Expand Down Expand Up @@ -782,6 +787,7 @@ function plot(container, subplot, cdata) {
scene.fill2d = createLine(regl);
}

// update main marker options
if(scene.line2d) {
scene.line2d.update(scene.lineOptions);
}
Expand All @@ -790,13 +796,7 @@ function plot(container, subplot, cdata) {
scene.error2d.update(errorBatch);
}
if(scene.scatter2d) {
if(!scene.selectBatch) {
scene.scatter2d.update(scene.markerOptions);
}
else {
scene.scatter2d.update(scene.unselectedOptions);
scene.select2d.update(scene.selectedOptions);
}
scene.scatter2d.update(scene.markerOptions);
}
// fill requires linked traces, so we generate it's positions here
if(scene.fill2d) {
Expand Down Expand Up @@ -888,21 +888,15 @@ function plot(container, subplot, cdata) {
}
}

// make sure selection layer is initialized if we require selection
var dragmode = layout.dragmode;

if(dragmode === 'lasso' || dragmode === 'select') {
if(scene.select2d && scene.selectBatch) {
scene.scatter2d.update(scene.unselectedOptions);
}
}
var selectMode = dragmode === 'lasso' || dragmode === 'select';

// provide viewport and range
var vpRange = cdata.map(function(cdscatter) {
if(!cdscatter || !cdscatter[0] || !cdscatter[0].trace) return;
var cd = cdscatter[0];
var trace = cd.trace;
var stash = cd.t;
var id = stash.index;
var x = stash.rawx,
y = stash.rawy;

Expand All @@ -924,30 +918,16 @@ function plot(container, subplot, cdata) {
(height - vpSize.t) - (1 - yaxis.domain[1]) * vpSize.h
];

if(trace.selectedpoints || dragmode === 'lasso' || dragmode === 'select') {
// create select2d
if(!scene.select2d && scene.scatter2d) {
var selectRegl = layout._glcanvas.data()[1].regl;
if(trace.selectedpoints || selectMode) {
if(!selectMode) selectMode = true;

// create scatter instance by cloning scatter2d
scene.select2d = createScatter(selectRegl, {clone: scene.scatter2d});
scene.select2d.update(scene.selectedOptions);
if(!scene.selectBatch) scene.selectBatch = [];
if(!scene.unselectBatch) scene.unselectBatch = [];

// create selection style once we have something selected
if(trace.selectedpoints && !scene.selectBatch) {
scene.selectBatch = Array(scene.count);
scene.unselectBatch = Array(scene.count);
scene.scatter2d.update(scene.unselectedOptions);
}
}
else {
// update selection positions, since they may have changed by panning or alike
scene.select2d.update(scene.selectedOptions);
}
// regenerate scene batch, if traces number changed during selection
if(trace.selectedpoints) {
scene.selectBatch[id] = trace.selectedpoints;

// form unselected batch
if(trace.selectedpoints && !scene.unselectBatch[stash.index]) {
scene.selectBatch[stash.index] = trace.selectedpoints;
var selPts = trace.selectedpoints;
var selDict = {};
for(i = 0; i < selPts.length; i++) {
Expand All @@ -957,7 +937,7 @@ function plot(container, subplot, cdata) {
for(i = 0; i < stash.count; i++) {
if(!selDict[i]) unselPts.push(i);
}
scene.unselectBatch[stash.index] = unselPts;
scene.unselectBatch[id] = unselPts;
}

// precalculate px coords since we are not going to pan during select
Expand All @@ -979,6 +959,21 @@ function plot(container, subplot, cdata) {
} : null;
});

if(selectMode) {
// create select2d
if(!scene.select2d) {
// create scatter instance by cloning scatter2d
scene.select2d = createScatter(layout._glcanvas.data()[1].regl, {clone: scene.scatter2d});
}

// update only traces with selection
scene.scatter2d.update(scene.unselectedOptions.map(function(opts, i) {
return scene.selectBatch[i] ? opts : null;
}));
scene.select2d.update(scene.markerOptions);
scene.select2d.update(scene.selectedOptions);
}

// uploat viewport/range data to GPU
if(scene.fill2d) {
scene.fill2d.update(vpRange);
Expand Down Expand Up @@ -1193,18 +1188,25 @@ function selectPoints(searchInfo, polygon) {
}
}
else {
unels = Array(stash.count);
for(i = 0; i < stash.count; i++) {
unels[i] = i;
}
unels = arrayRange(stash.count);
}

// create selection style once we have something selected
// make sure selectBatch is created
if(!scene.selectBatch) {
scene.selectBatch = Array(scene.count);
scene.unselectBatch = Array(scene.count);
scene.selectBatch = [];
scene.unselectBatch = [];
}

if(!scene.selectBatch[stash.index]) {
// enter every trace select mode
for(i = 0; i < scene.count; i++) {
scene.selectBatch[i] = [];
scene.unselectBatch[i] = [];
}
// we should turn scatter2d into unselected once we have any points selected
scene.scatter2d.update(scene.unselectedOptions);
}

scene.selectBatch[stash.index] = els;
scene.unselectBatch[stash.index] = unels;

Expand Down
74 changes: 73 additions & 1 deletion test/jasmine/tests/gl2d_plot_interact_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var Plotly = require('@lib/index');
var Plots = require('@src/plots/plots');
var Lib = require('@src/lib');
var Drawing = require('@src/components/drawing');
var ScatterGl = require('@src/traces/scattergl');

var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
Expand All @@ -14,6 +15,7 @@ var selectButton = require('../assets/modebar_button');
var delay = require('../assets/delay');
var readPixel = require('../assets/read_pixel');


function countCanvases() {
return d3.selectAll('canvas').size();
}
Expand Down Expand Up @@ -406,7 +408,6 @@ describe('Test gl2d plots', function() {
.then(done);
});


it('@noCI should display selection of big number of miscellaneous points', function(done) {
var colorList = [
'#006385', '#F06E75', '#90ed7d', '#f7a35c', '#8085e9',
Expand Down Expand Up @@ -645,4 +646,75 @@ describe('Test gl2d plots', function() {
.catch(fail)
.then(done);
});

it('should restyle opacity', function(done) {
// #2299
spyOn(ScatterGl, 'calc');

var dat = [{
'x': [1, 2, 3],
'y': [1, 2, 3],
'type': 'scattergl',
'mode': 'markers'
}];

Plotly.plot(gd, dat, {width: 500, height: 500})
.then(function() {
expect(ScatterGl.calc).toHaveBeenCalledTimes(1);

return Plotly.restyle(gd, {'opacity': 0.1});
})
.then(function() {
expect(ScatterGl.calc).toHaveBeenCalledTimes(2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant test ✨

})
.catch(fail)
.then(done);
});

it('should update selected points', function(done) {
// #2298
var dat = [{
'x': [1],
'y': [1],
'type': 'scattergl',
'mode': 'markers',
'selectedpoints': [0]
}];

Plotly.plot(gd, dat, {
width: 500,
height: 500,
dragmode: 'select'
})
.then(function() {
var scene = gd._fullLayout._plots.xy._scene;

expect(scene.count).toBe(1);
expect(scene.selectBatch).toEqual([[0]]);
expect(scene.unselectBatch).toEqual([[]]);
spyOn(scene.scatter2d, 'draw');

var trace = {
x: [2],
y: [1],
type: 'scattergl',
mode: 'markers',
marker: {color: 'red'}
};

return Plotly.addTraces(gd, trace);
})
.then(function() {
var scene = gd._fullLayout._plots.xy._scene;

expect(scene.count).toBe(2);
expect(scene.selectBatch).toBeDefined();
expect(scene.unselectBatch).toBeDefined();
expect(scene.markerOptions.length).toBe(2);
expect(scene.markerOptions[1].color).toEqual(new Uint8Array([255, 0, 0, 255]));
expect(scene.scatter2d.draw).toHaveBeenCalled();
})
.catch(fail)
.then(done);
});
});