#29192 closed enhancement (fixed)

MR41: Add option to persist camera state to three.js viewer.

Reported by: galois Owned by:
Priority: major Milestone: sage-9.2
Component: graphics Keywords: threejs camera persist
Cc: ​paulmasson Merged in:
Authors: Paul Masson Reviewers: Joshua Campbell
Report Upstream: N/A Work issues:
Branch: u/paulmasson/mrs/41/threejs-persist-camera Commit: 3b7fc0c1b3c2131cd5a904737e62914ab4b1a540
Dependencies: #28672 #29250 Stopgaps:

Status badges

Description (last modified by gh-jcamp0x2a)

Joshua Campbell (@jcamp0x2a) opened a merge request at https://gitlab.com/sagemath/sage/-/merge_requests/41:


At present, when using interact with three.js plots in the Jupyter notebook, the camera gets reset to its default position each time the plot is regenerated. If the default camera position isn't ideal for whatever is being plotted, it's cumbersome to have to re-adjust the camera after every change.

This MR adds a boolean persist viewing option for three.js that will persist the camera state between viewings of the plot using local storage. In a Jupyter notebook, it's keyed off of the current notebook and cell; otherwise the URL. It defaults to False so as not to interfere with the existing behavior that users may rely upon.

Without persist, if you don't like where you've moved the camera to, it's easy enough to reload the page or regenerate the plot to get back to the default. With persist, however, that becomes more difficult; so this MR also adds a "Reset Camera" feature to the bottom-right pop-up menu . (+ some minor hover highlighting and cursor fixes to the menu)


Change History (26)

comment:1 Changed 20 months ago by gh-jcamp0x2a

  • Component changed from PLEASE CHANGE to graphics
  • Description modified (diff)
  • Keywords threejs camera persist added
  • Priority changed from major to minor
  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 20 months ago by dimpase

  • Cc ​paulmasson added

comment:3 follow-up: Changed 20 months ago by novoselt

I wonder if it would be better to actually change default behaviour to be persistent. If there is an easy to discover way to reset the camera, I don't see any drawbacks.

comment:4 follow-up: Changed 20 months ago by paulmasson

  • Dependencies set to #28672
  • Status changed from needs_review to needs_work

Joshua, while I certainly appreciate all of the effort you put into this feature, there is a much simpler approach. #28672 already makes the camera viewpoint available on the clipboard. This can simply be passed into the plot through a new keyword named viewpoint which will keep the camera at that set position on regeneration. That's how I'm setting the camera in my own JavaScript library that embeds virtually the same viewer in web pages.

While this approach requires a manual step, it avoids the overhead of local storage and the dependence you note on the future structure of Jupyter notebooks. As soon as #28672 is merged, then either you or I can add the new keyword. Please be aware that I handle aspect multipliers a bit differently in my library. I left them more explicit in this viewer so that they don't get forgotten.

comment:5 in reply to: ↑ 4 Changed 20 months ago by gh-jcamp0x2a

Hi Paul!

Yes, a viewpoint option does seem like it would solve the use case I mentioned (keeping a fixed camera with interact).

Another reason I implemented this feature is because I've been experimenting with adding support for three.js-based animation to Sage: discrete keyframes like the existing animate function at present, with the goal of adding some interpolation or image morphing between the frames later on. I'd like to be able to persist the playback state of the animation as well, preferably tied to the same persist option.

Laying the foundation with a persistent camera first seemed the way to go. With the animation state changing on its own every frame, a manual approach similar to viewpoint would be more cumbersome.

I'm open to scrapping persistence for the camera and re-introducing it for the animation. Of course, that presumes that the animation stuff gets accepted later on; there may be better alternatives that I'm unaware of. Or perhaps a hybrid approach: if a viewpoint has been provided, then ignore persist as regards the initial camera state?

Thanks for taking a look at this. :)

comment:6 in reply to: ↑ 3 Changed 20 months ago by gh-jcamp0x2a

Replying to novoselt:

I wonder if it would be better to actually change default behaviour to be persistent. If there is an easy to discover way to reset the camera, I don't see any drawbacks.

I don't speed a lot of time in the Jupyter notebook, preferring the console, except recently in order to use interact, so I wasn't sure what the commons workflows in it might be and whether this change could interfere with them. So, I opted for the cautious route.

comment:7 Changed 20 months ago by paulmasson

Joshua, just took a look at your animation branch on GitLab. Looks like you're deep into it! Are there any active examples online? And don't forget that Three.js has an animation system already, which you don't appear to be using.

Although keyframes are one way to approach the topic, keep in mind that we can manipulate data directly using JavaScript. We haven't yet added a basic spin animation, but that's easily done by updating the rotation parameters of the object. If the object is to be moved along a trajectory, we could pass in a line or function describing the location of its center over time. For more complex alterations, we could pass in a function describing the locations of all vertices over time. Lots of possibilities without thinking in terms of saving a movie frame by frame.

Please also try not to add anything to the viewer that will be dependent on a given environment, in this case a Jupyter notebook. I'd rather the viewer be as portable as possible.

comment:8 Changed 20 months ago by gh-jcamp0x2a

Paul, I gathered the examples I've put in my documentation so far and thrown them up on GitHub pages:

https://jcamp0x2a.github.io/threejs-animation-example/

I opted not to use Three.js's animation system since I wanted to support multiple "times", one per variable being animated. Three.js would allow me to use multiple mixers to blend linearly between several sets of keyframes, but that wouldn't be appropriate for plotting a function f(x,y,...) that's not some linear combination c1*f1(x)+c2*f2(y)+...

Agreed that keyframes aren't necessarily the best approach, but it has two enourmous benefits:

  1. It was quite easy to work into the existing Graphics3d system. No need to, for example, go into the implicit_plot3d code and modify the algorithm (marching cubes I think) to produce a mesh suitable for morphing. Nor worrying about topological changes like holes forming or components pinching off. I love math, but I'm a software developer, not a mathematician! So that kind of stuff is beyond my paygrade. :) Instead, I just plot the keyframes, sum them all up into a single Graphics3d object, attach some metadata as to when each object should be visible, and voila!: animation.
  1. It's very general: it can animate a rigidbody moving along a trajectory up to a vector field representing fluid flow with the same amount of code on both the back- and front-ends.

Perhaps I should create a new ticket for the animation to continue the conversation there? I'll hold off on the persistence stuff then if it's not desired.

Thanks!

comment:9 Changed 20 months ago by paulmasson

Joshua, the animations are great! Definitely new ticket for that. Will study the code more in the near future.

Rather than inspecting the notebook for a unique ID, why not generate a random large number? That's what I've done in my library and haven't had any problems yet, even with pages with lots of embedded content.

comment:10 Changed 19 months ago by paulmasson

  • Dependencies changed from #28672 to #28672 #29250

comment:11 Changed 18 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

comment:12 Changed 16 months ago by paulmasson

  • Branch changed from u/galois/mrs/41/threejs-persist-camera to u/paulmasson/mrs/41/threejs-persist-camera

comment:13 Changed 16 months ago by paulmasson

  • Authors changed from Joshua Campbell to Paul Masson
  • Commit changed from fcb6ed4df57dc61982793484e51312051b9a111a to 66d3471f6dbef8deaa8f396ddef6a15c275333e5
  • Status changed from needs_work to needs_review

Joshua, here at long last is the viewpoint option we discussed above. Use the 'Get Viewpoint' menu option to obtain the value that is passed as a string in the graphic object.


New commits:

66d3471Add viewpoint option

comment:14 Changed 16 months ago by gh-jcamp0x2a

Hi Paul, I've verified that setting the viewpoint option to a string containing the output of Get Viewpoint correctly re-creates that camera orientation in the new plot.

I'm wary of passing structured data like this as a string parameter, though. It works fine for the Get Viewpoint use case, but one may imagine wanting to set the viewpoint to a specific axis-angle or have the axis-angle computed in some way. In the former case a typo (like forgetting a comma) would produce a blank plot instead of a syntax error, and in the latter case you'd have to take the extra step of building up a string.

As Get Viewpoint is currently implemented, we have to surround its output with something to avoid a syntax error, but why not parentheses instead of quotes? ([x,y,z],angle) opposed to "[x,y,z],angle". That way Python can validate the syntax, and we can validate the types/contents.

While testing, I forgot to surround the output with quotes a... non-zero... number of times. Ideally, and I apologize for not thinking of this when reviewing the code that introduced it, Get Viewpoint would do the surrounding for us to avoid this perhaps not uncommon occurrence among some subset of users ;)

Also, do we want the zoom / camera distance to be included as well? Perhaps by letting the length of the axis vector represent the zoom/distance?

Many thanks!

comment:15 Changed 16 months ago by paulmasson

The Get Viewpoint feature was added to support how TikZ interfaces with Sage: see this tutorial for more information. See also the general rotate command in the base class for 3D objects. Adding anything else to the string here might require changes to other interfaces, so I don't think we should do that.

I agree that requiring a structured string for input is not ideal, and I like the idea of using parentheses or square brackets. That is a better way to structure the input for later handling and either can be used in Python. I'll put together a version for testing.

comment:16 Changed 16 months ago by git

  • Commit changed from 66d3471f6dbef8deaa8f396ddef6a15c275333e5 to 37b0c870f371685be673d22d5d6c1526144b4c49

Branch pushed to git repo; I updated commit sha1. New commits:

37b0c87Update viewpoint option

comment:17 Changed 16 months ago by paulmasson

Here's a more stable version, where viewpoint must be a list of the form [[x,y,z],angle] or it will raise a warning. Couple questions:

1) Lots of Sage inputs are allowed to be either lists or tuples, but need to be converted to lists for the JSONing lines. Do we want to allow tuples here?

2) The angle value for TikZ is in degrees, while the angle for the base-class rotate() is in radians. Not sure how to reconcile the two. Maybe just document what's happening?

comment:18 Changed 16 months ago by git

  • Commit changed from 37b0c870f371685be673d22d5d6c1526144b4c49 to f96ccdca5f66ef1f55a84f32d00289e9a39c408a

Branch pushed to git repo; I updated commit sha1. New commits:

f96ccdcFurther refine viewpoint option

comment:19 Changed 16 months ago by paulmasson

Added support for tuples and clarified angle variable in documentation.

Let's add the zoom feature once we get some feedback from users.

comment:20 follow-up: Changed 16 months ago by gh-jcamp0x2a

I checked that the viewpoint is still set correctly, that lists/tuples are both supported, and that a helpful warning message is displayed if the viewpoint option is not of the correct form.

One very minor note: the documentation still states "string of the form...". Otherwise looks good and can give positive review.

Thank you for adding this option. Will make using interact in notebooks with three.js plots much more convenient.

comment:21 Changed 16 months ago by git

  • Commit changed from f96ccdca5f66ef1f55a84f32d00289e9a39c408a to 850c59a8faa3bbc48492dee2b19550244ba9be84

Branch pushed to git repo; I updated commit sha1. New commits:

850c59aUpdate documentation

comment:22 Changed 16 months ago by git

  • Commit changed from 850c59a8faa3bbc48492dee2b19550244ba9be84 to 3b7fc0c1b3c2131cd5a904737e62914ab4b1a540

Branch pushed to git repo; I updated commit sha1. New commits:

3b7fc0cUpdate documentation

comment:23 in reply to: ↑ 20 Changed 16 months ago by paulmasson

Replying to gh-jcamp0x2a:

One very minor note: the documentation still states "string of the form...". Otherwise looks good and can give positive review.

Done!

comment:24 Changed 16 months ago by gh-jcamp0x2a

  • Reviewers set to Joshua Campbell
  • Status changed from needs_review to positive_review

Thanks!

comment:25 Changed 15 months ago by mkoeppe

  • Priority changed from minor to major

comment:26 Changed 15 months ago by mkoeppe

  • Resolution set to fixed
  • Status changed from positive_review to closed

These have been merged into 9.2.beta4

Note: See TracTickets for help on using tickets.