Skip to content

Emit a plotly_relayout' event upon mouseup, wheel #466

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 19 commits into from
May 6, 2016
Merged

Emit a plotly_relayout' event upon mouseup, wheel #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a7177f2
#30a emit an event upon manipulation
monfera Apr 22, 2016
1b19b3a
#30a removing preexisting lint exemption directives and fixing former…
monfera Apr 22, 2016
34d15aa
#30a removing preexisting lint exemption directives and fixing former…
monfera Apr 22, 2016
3d8fe81
#30a covering 2D WebGL plot with the plotly_relayout event on pan / zoom
monfera Apr 28, 2016
a10caec
#30a test case for mouse drag and wheel on gl3d plots
monfera Apr 29, 2016
604d739
#30a proper grouping by interaction type
monfera Apr 29, 2016
ee8ff65
#30a test case for mouse drag on gl2d plots
monfera Apr 29, 2016
da01879
#30a reifying plotly_relayout callback count, as well as layout data …
monfera May 2, 2016
48682a4
#30a adding graphDiv to Scene2d similarly to Scene / thanks @etpinard
monfera May 2, 2016
63ec931
#30a bumping jasmine to 2.4 to enjoy its new facilities such as spy.t…
monfera May 2, 2016
19c478d
#30a panning test on WebGL canvas; adding the DIV update for scene2d
monfera May 3, 2016
e467c88
#30a saving the camera position on the DIV layout when the projection…
monfera May 3, 2016
b212086
#30a PR feedback: not expose scene id and use axis names. Plus: test …
monfera May 3, 2016
e8aa9cb
#30a PR feedback: remove `delay`
monfera May 3, 2016
e02a3f6
#30a adding tests for the 3d callback and DIV layout camera data update
monfera May 3, 2016
4975417
#30a PR feedback: commenting and removing disused binding
monfera May 4, 2016
7c23e2e
#30a PR feedback: adding test case with partially set camera and solv…
monfera May 6, 2016
7eef2e1
#30a removing new test case from the test file that's currently ignor…
monfera May 6, 2016
2a13e92
#30a excluding gl_plot_interact_basic_test.js from the CI run. Benefi…
monfera May 6, 2016
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
"fs-extra": "^0.28.0",
"fuse.js": "^2.2.0",
"glob": "^7.0.0",
"jasmine-core": "^2.3.4",
"jasmine-core": "^2.4.1",
"karma": "^0.13.15",
"karma-browserify": "^5.0.1",
"karma-chrome-launcher": "^0.2.1",
Expand Down
2 changes: 2 additions & 0 deletions src/components/modebar/buttons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ function handleCamera3d(gd, ev) {

if(attr === 'resetDefault') scene.setCameraToDefault();
else if(attr === 'resetLastSave') {
// This handler looks in the un-updated fullLayout.scene.camera object to reset the camera
// to the last saved position.
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks!

scene.setCamera(fullSceneLayout.camera);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/plots/gl2d/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ exports.plot = function plotGl2d(gd) {
// If Scene is not instantiated, create one!
if(scene === undefined) {
scene = new Scene2D({
container: gd.querySelector('.gl-container'),
id: subplotId,
graphDiv: gd,
container: gd.querySelector('.gl-container'),
staticPlot: gd._context.staticPlot,
plotGlPixelRatio: gd._context.plotGlPixelRatio
},
Expand Down
21 changes: 21 additions & 0 deletions src/plots/gl2d/scene2d.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var STATIC_CANVAS, STATIC_CONTEXT;

function Scene2D(options, fullLayout) {
this.container = options.container;
this.graphDiv = options.graphDiv;
this.pixelRatio = options.plotGlPixelRatio || window.devicePixelRatio;
this.id = options.id;
this.staticPlot = !!options.staticPlot;
Expand Down Expand Up @@ -268,6 +269,25 @@ proto.updateFx = function(options) {
fullLayout.hovermode = options.hovermode;
};

var relayoutCallback = function(scene) {

var xrange = scene.xaxis.range,
yrange = scene.yaxis.range;

// Update the layout on the DIV
scene.graphDiv.layout.xaxis.range = xrange.slice(0);
scene.graphDiv.layout.yaxis.range = yrange.slice(0);

// Make a meaningful value to be passed on to the possible 'plotly_relayout' subscriber(s)
var update = { // scene.camera has no many useful projection or scale information
lastInputTime: scene.camera.lastInputTime // helps determine which one is the latest input (if async)
};
Copy link
Contributor

Choose a reason for hiding this comment

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

In gl2d, scene is purely internal. I'd vote for something like:

update = {
  lastInputTime: scene.camera.lastInputTime  // great idea here!
};
update[xaxis._name] = xrange.slice();
update[yaxis._name] = yrange.slice();

where ?axis.name will be xaxis, xaxis2, yaxis, yaxis2 ...

update[scene.xaxis._name] = xrange.slice();
update[scene.yaxis._name] = yrange.slice();
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks!


scene.graphDiv.emit('plotly_relayout', update);
};

proto.cameraChanged = function() {
var camera = this.camera,
xrange = this.xaxis.range,
Expand All @@ -285,6 +305,7 @@ proto.cameraChanged = function() {
this.glplotOptions.ticks = nextTicks;
this.glplotOptions.dataBox = camera.dataBox;
this.glplot.update(this.glplotOptions);
relayoutCallback(this);
}
};

Expand Down
94 changes: 53 additions & 41 deletions src/plots/gl3d/scene.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
*/


/*eslint block-scoped-var: 0*/
/*eslint no-redeclare: 0*/

'use strict';

var createPlot = require('gl-plot3d');
Expand All @@ -34,6 +31,8 @@ var STATIC_CANVAS, STATIC_CONTEXT;

function render(scene) {

var trace;

// update size of svg container
var svgContainer = scene.svgContainer;
var clientRect = scene.container.getBoundingClientRect();
Expand All @@ -50,7 +49,7 @@ function render(scene) {
var lastPicked = null;
var selection = scene.glplot.selection;
for(var i = 0; i < keys.length; ++i) {
var trace = scene.traces[keys[i]];
trace = scene.traces[keys[i]];
if(trace.handlePick(selection)) {
lastPicked = trace;
}
Expand All @@ -68,9 +67,9 @@ function render(scene) {
var oldEventData;

if(lastPicked !== null) {
var pdata = project(scene.glplot.cameraParams, selection.dataCoordinate),
trace = lastPicked.data,
hoverinfo = trace.hoverinfo;
var pdata = project(scene.glplot.cameraParams, selection.dataCoordinate);
trace = lastPicked.data;
var hoverinfo = trace.hoverinfo;

var xVal = formatter('xaxis', selection.traceCoordinate[0]),
yVal = formatter('yaxis', selection.traceCoordinate[1]),
Expand Down Expand Up @@ -172,6 +171,16 @@ function initializeGLPlot(scene, fullLayout, canvas, gl) {
showNoWebGlMsg(scene);
}

var relayoutCallback = function(scene) {
var update = {};
update[scene.id] = getLayoutCamera(scene.camera);
scene.saveCamera(scene.graphDiv.layout);
Copy link
Contributor

@etpinard etpinard May 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'm ok with this here.

BUT we should add a comment in the resetCameraLastSave mode bar button handler.

That handler looks in the un-updated fullLayout.scene.camera object to reset the camera to the last saved position.

Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks if scene.graphDiv.layout is not fully set.

For example, using the gl3d_errorbars_zx mock - which doesn't completly set scene.camera - lead on every mouse up, or scroll to:

image

The saveCamera method needs to be patched.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard thanks for this find! I'll add a test case that covers this circumstance and resolve it. It's late and I'm planning to look at it early morning, but somewhat surprised as the 3d test I already put in, e.g. gl3d_scatter3d-connectgaps.json doesn't set the camera yet it didn't throw on the interaction test, so I'll start from your example above.

Copy link
Contributor

Choose a reason for hiding this comment

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

no rush, the next release will be published next Tuesday or Wednesday.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard may I ask you to check if this single-line modification in saveCamera is all it takes - I infer from the code that the sameness check needs to be prepared for the possibility of a partially set camera.

Also, I refactored the test case under which I put a new test for the partially set camera, and I hope you consider it an improvement over my original test case - it's more promise-oriented and explicitly passes around values rather than relying on globals.

If it's OK with you, I'd try to put some of the test cases into a separate file, because this test file is excluded from the CI run due to unpredictability of some of the tests. I'm uncomfortable with the notion that an ever increasing number of tests is excluded so I'd try to loop some of them in.

Copy link
Contributor

Choose a reason for hiding this comment

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

may I ask you to check if this single-line modification in saveCamera is all it takes

Yes. This is all we need. Thanks!

Also, I refactored the test case under which I put a new test for the partially set camera, and I hope you consider it an improvement over my original test case - it's more promise-oriented and explicitly passes around values rather than relying on globals

Sure. Go for it!

If it's OK with you, I'd try to put some of the test cases into a separate file, because this test file is excluded from the CI run due to unpredictability of some of the tests. I'm uncomfortable with the notion that an ever increasing number of tests is excluded so I'd try to loop some of them in.

+1

scene.graphDiv.emit('plotly_relayout', update);
};

scene.glplot.canvas.addEventListener('mouseup', relayoutCallback.bind(null, scene));
scene.glplot.canvas.addEventListener('wheel', relayoutCallback.bind(null, scene));
Copy link
Contributor

Choose a reason for hiding this comment

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

Great. It would be nice to add a mouse wheel test. See the legend_scroll for inspiration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback!


if(!scene.staticMode) {
scene.glplot.canvas.addEventListener('webglcontextlost', function(ev) {
console.log('lost context');
Expand Down Expand Up @@ -255,7 +264,7 @@ function Scene(options, fullLayout) {

this.contourLevels = [ [], [], [] ];

if(!initializeGLPlot(this, fullLayout)) return;
if(!initializeGLPlot(this, fullLayout)) return; // todo check the necessity for this line
}

var proto = Scene.prototype;
Expand Down Expand Up @@ -283,18 +292,19 @@ proto.recoverContext = function() {
var axisProperties = [ 'xaxis', 'yaxis', 'zaxis' ];

function coordinateBound(axis, coord, d, bounds) {
var x;
for(var i=0; i<coord.length; ++i) {
if(Array.isArray(coord[i])) {
for(var j=0; j<coord[i].length; ++j) {
var x = axis.d2l(coord[i][j]);
x = axis.d2l(coord[i][j]);
if(!isNaN(x) && isFinite(x)) {
bounds[0][d] = Math.min(bounds[0][d], x);
bounds[1][d] = Math.max(bounds[1][d], x);
}
}
}
else {
var x = axis.d2l(coord[i]);
x = axis.d2l(coord[i]);
if(!isNaN(x) && isFinite(x)) {
bounds[0][d] = Math.min(bounds[0][d], x);
bounds[1][d] = Math.max(bounds[1][d], x);
Expand All @@ -317,7 +327,7 @@ proto.plot = function(sceneData, fullLayout, layout) {
if(this.glplot.contextLost) return;

var data, trace;
var i, j;
var i, j, axis, axisType;
var fullSceneLayout = fullLayout[this.id];
var sceneLayout = layout[this.id];

Expand All @@ -341,7 +351,7 @@ proto.plot = function(sceneData, fullLayout, layout) {

// Update axes functions BEFORE updating traces
for(i = 0; i < 3; ++i) {
var axis = fullSceneLayout[axisProperties[i]];
axis = fullSceneLayout[axisProperties[i]];
setConvert(axis);
}

Expand All @@ -354,14 +364,14 @@ proto.plot = function(sceneData, fullLayout, layout) {
[Infinity, Infinity, Infinity],
[-Infinity, -Infinity, -Infinity]
];
for(var i=0; i<sceneData.length; ++i) {
var data = sceneData[i];
for(i=0; i<sceneData.length; ++i) {
data = sceneData[i];
if(data.visible !== true) continue;

computeTraceBounds(this, data, dataBounds);
}
var dataScale = [1,1,1];
for(var j=0; j<3; ++j) {
for(j=0; j<3; ++j) {
if(dataBounds[0][j] > dataBounds[1][j]) {
dataScale[j] = 1.0;
}
Expand All @@ -379,7 +389,7 @@ proto.plot = function(sceneData, fullLayout, layout) {
this.dataScale = dataScale;

//Update traces
for(var i = 0; i < sceneData.length; ++i) {
for(i = 0; i < sceneData.length; ++i) {
data = sceneData[i];
if(data.visible!==true) {
continue;
Expand Down Expand Up @@ -416,8 +426,8 @@ proto.plot = function(sceneData, fullLayout, layout) {
axisTypeRatios = {};

for(i = 0; i < 3; ++i) {
var axis = fullSceneLayout[axisProperties[i]];
var axisType = axis.type;
axis = fullSceneLayout[axisProperties[i]];
axisType = axis.type;

if(axisType in axisTypeRatios) {
axisTypeRatios[axisType].acc *= dataScale[i];
Expand Down Expand Up @@ -471,9 +481,9 @@ proto.plot = function(sceneData, fullLayout, layout) {
var axesScaleRatio = [1, 1, 1];

//Compute axis scale per category
for(var i=0; i<3; ++i) {
var axis = fullSceneLayout[axisProperties[i]];
var axisType = axis.type;
for(i=0; i<3; ++i) {
axis = fullSceneLayout[axisProperties[i]];
axisType = axis.type;
var axisRatio = axisTypeRatios[axisType];
axesScaleRatio[i] = Math.pow(axisRatio.acc, 1.0/axisRatio.count) / dataScale[i];
}
Expand Down Expand Up @@ -567,33 +577,35 @@ proto.setCameraToDefault = function setCameraToDefault() {
});
};

// get camera position in plotly coords from 'orbit-camera' coords
proto.getCamera = function getCamera() {
this.glplot.camera.view.recalcMatrix(this.camera.view.lastT());

var up = this.glplot.camera.up;
var center = this.glplot.camera.center;
var eye = this.glplot.camera.eye;
// getOrbitCamera :: plotly_coords -> orbit_camera_coords
// inverse of getLayoutCamera
function getOrbitCamera(camera) {
return [
[camera.eye.x, camera.eye.y, camera.eye.z],
[camera.center.x, camera.center.y, camera.center.z],
[camera.up.x, camera.up.y, camera.up.z]
];
}

// getLayoutCamera :: orbit_camera_coords -> plotly_coords
// inverse of getOrbitCamera
function getLayoutCamera(camera) {
return {
up: {x: up[0], y: up[1], z: up[2]},
center: {x: center[0], y: center[1], z: center[2]},
eye: {x: eye[0], y: eye[1], z: eye[2]}
up: {x: camera.up[0], y: camera.up[1], z: camera.up[2]},
center: {x: camera.center[0], y: camera.center[1], z: camera.center[2]},
eye: {x: camera.eye[0], y: camera.eye[1], z: camera.eye[2]}
};
}

// get camera position in plotly coords from 'orbit-camera' coords
proto.getCamera = function getCamera() {
this.glplot.camera.view.recalcMatrix(this.camera.view.lastT());
return getLayoutCamera(this.glplot.camera);
};

// set camera position with a set of plotly coords
proto.setCamera = function setCamera(cameraData) {

// getOrbitCamera :: plotly_coords -> orbit_camera_coords
function getOrbitCamera(camera) {
return [
[camera.eye.x, camera.eye.y, camera.eye.z],
[camera.center.x, camera.center.y, camera.center.z],
[camera.up.x, camera.up.y, camera.up.z]
];
}

var update = {};

update[this.id] = cameraData;
Expand All @@ -612,7 +624,7 @@ proto.saveCamera = function saveCamera(layout) {
function same(x, y, i, j) {
var vectors = ['up', 'center', 'eye'],
components = ['x', 'y', 'z'];
return x[vectors[i]][components[j]] === y[vectors[i]][components[j]];
return y[vectors[i]] && (x[vectors[i]][components[j]] === y[vectors[i]][components[j]]);
}

if(cameraDataLastSave === undefined) hasChanged = true;
Expand Down
2 changes: 1 addition & 1 deletion test/jasmine/karma.ciconf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function func(config) {
* exclude them from the CircleCI test bundle.
*
*/
func.defaultConfig.exclude = ['tests/gl_plot_interact_test.js'];
func.defaultConfig.exclude = ['tests/gl_plot_interact_test.js', 'tests/gl_plot_interact_basic_test.js'];

// if true, Karma captures browsers, runs the tests and exits
func.defaultConfig.singleRun = true;
Expand Down
Loading