Skip to content

Make relayout work for scene.camera #1364

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

Merged
merged 6 commits into from
Feb 10, 2017
Merged

Make relayout work for scene.camera #1364

merged 6 commits into from
Feb 10, 2017

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Feb 8, 2017

fixes #934

so that http://codepen.io/monfera/pen/KMmPPV can now use Plotly.relayout as

function rotate(id, angle) {
  var scene = gd._fullLayout[id];
  var camera = scene.camera;

  var rtz = xyz2rtz(camera.eye);
  rtz.t += angle;
  camera.eye = rtz2xyz(rtz);
  
  Plotly.relayout(gd, id + '.camera', camera);
}

instead of digging into the scene object internals.

gifrecord_2017-02-08_155327


In addition,

  • by making relayout work for scene.camera we can now rewrite the 3d camera modebar handlers in a much cleaner way 604f7a0
  • to ensure that camera relayout are as fast as they can be, a new relayout path docamera is added in 99ad3cb

- setting paperdiv style is done in the
  layoutStyles plot_api subroutine
- which calls scene.setCamera in relayout/update
- instead of hacky setCamera call
- save 'initial' camera setting during scene creation
- use that + relayout to set camera to last save
- no need to emit plotly_relayout in setCamera no more!
@etpinard etpinard added status: reviewable bug something broken labels Feb 8, 2017
@jackparmer
Copy link
Contributor

ping @thejackluo , who was working a on remake of this NYT classic:
https://www.nytimes.com/interactive/2015/03/19/upshot/3d-yield-curve-economic-growth.html

mouseEvent('mousemove', start[0], start[1]);
mouseEvent('mousedown', start[0], start[1], { buttons: 1 });
mouseEvent('mousemove', end[0], end[1], { buttons: 1 });
mouseEvent('mouseup', end[0], end[1]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this commit in a PR about 3D?

Copy link
Contributor Author

@etpinard etpinard Feb 10, 2017

Choose a reason for hiding this comment

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

5f36071 broke the test w/o me noticing. As test cases that require a gl context are skipped on CI, this went unnoticed until now.

Copy link
Contributor Author

@etpinard etpinard Feb 10, 2017

Choose a reason for hiding this comment

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

Full disclosure, I actually noticed this broken test case twice before (when releasing 1.22.0 and 1.23.0), but I was too lazy and didn't feel like making a PR just for that.

@alexcjohnson
Copy link
Collaborator

Very nice, much cleaner. 💃

@etpinard etpinard merged commit 53ba120 into master Feb 10, 2017
@etpinard etpinard deleted the relayout-scene-camera branch February 10, 2017 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Relayout with scatter3d (and other 3d traces, probably?)
3 participants