Skip to content
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.

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

Refactor scattergl selection #2311

Merged
merged 9 commits into from
Feb 2, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
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
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
138 changes: 80 additions & 58 deletions src/traces/scattergl/index.js
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 @@ -599,6 +599,7 @@ function sceneUpdate(container, subplot) {
scene.error2d.draw(i);
scene.error2d.draw(i + scene.count);
}

if(scene.scatter2d && !scene.selectBatch) {
scene.scatter2d.draw(i);
}
Expand Down Expand Up @@ -740,8 +741,9 @@ 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 i;
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 +784,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 +793,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 @@ -886,17 +883,77 @@ function plot(container, subplot, cdata) {

scene.fill2d.update(scene.fillOptions);
}
}

// 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);
// update selection
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separated code chunks are in single location now

Copy link
Contributor

Choose a reason for hiding this comment

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

On the gl2d_12 mock, calling Plotly.restyle(gd, 'selected.marker.color', 'blue') leads to:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed with 98cddf3

var hasSelectedPoints = false;
for(i = 0; i < cdata.length; i++) {
if(cdata[i][0].trace.selectedpoints) {
hasSelectedPoints = true;
break;
}
}

if(scene.selectBatch || dragmode === 'lasso' || dragmode === 'select' || hasSelectedPoints) {
var newSelectBatch, newUnselectBatch;

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

// regenerate scene batch, if traces number changed during selection
if(scene.selectBatch || hasSelectedPoints) {
if(!scene.selectBatch) scene.selectBatch = [];
if(!scene.unselectBatch) scene.unselectBatch = [];

newSelectBatch = Array(scene.count);
newUnselectBatch = Array(scene.count);

for(var j = 0; j < newSelectBatch.length; j++) {
var trace = cdata[j][0].trace;
var stash = cdata[j][0].t;
var id = stash.index;

// form unselected batch
if(!scene.unselectBatch[id]) {
if(trace.selectedpoints) {
newSelectBatch[id] = trace.selectedpoints;
var selPts = trace.selectedpoints;
var selDict = {};
for(i = 0; i < selPts.length; i++) {
selDict[selPts[i]] = true;
}
var unselPts = [];
for(i = 0; i < stash.count; i++) {
if(!selDict[i]) unselPts.push(i);
}
newUnselectBatch[id] = unselPts;
}
else {
newSelectBatch[id] = [];
newUnselectBatch[id] = arrayRange(stash.count);
}
}
else {
newSelectBatch[id] = scene.selectBatch[id];
newUnselectBatch[id] = scene.unselectBatch[id];
}
}

scene.selectBatch = newSelectBatch;
scene.unselectBatch = newUnselectBatch;

scene.scatter2d.update(scene.unselectedOptions);
}

scene.select2d.update(scene.markerOptions);
scene.select2d.update(scene.selectedOptions);
}
}


// provide viewport and range
var vpRange = cdata.map(function(cdscatter) {
if(!cdscatter || !cdscatter[0] || !cdscatter[0].trace) return;
Expand Down Expand Up @@ -925,41 +982,6 @@ function plot(container, subplot, cdata) {
];

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

// create scatter instance by cloning scatter2d
scene.select2d = createScatter(selectRegl, {clone: scene.scatter2d});
scene.select2d.update(scene.selectedOptions);

// 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);
}

// 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++) {
selDict[selPts[i]] = true;
}
var unselPts = [];
for(i = 0; i < stash.count; i++) {
if(!selDict[i]) unselPts.push(i);
}
scene.unselectBatch[stash.index] = unselPts;
}

// precalculate px coords since we are not going to pan during select
var xpx = Array(stash.count), ypx = Array(stash.count);
for(i = 0; i < stash.count; i++) {
Expand Down Expand Up @@ -1193,18 +1215,18 @@ 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 = [];

// 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