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

[f588ca1e-f96a-4dea-804d-c55eab873dab]

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

---

At present, when using [interact](https://wiki.sagemath.org/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)

Depends on #28672 Depends on #29250

CC: @paulmasson

Component: graphics

Keywords: threejs camera persist

Author: Paul Masson

Branch/Commit: u/paulmasson/mrs/41/threejs-persist-camera @ 3b7fc0c

Reviewer: Joshua Campbell

Issue created by migration from https://trac.sagemath.org/ticket/29192

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

Description changed:

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

Changed keywords from none to threejs camera persist

[novoselt]

comment:3

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.

[53959995-fd20-47f5-85e6-5e769b863d1f]

Dependencies: #28672

[53959995-fd20-47f5-85e6-5e769b863d1f]

comment:4

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.

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

comment:5

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. :)

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

comment:6

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.

[53959995-fd20-47f5-85e6-5e769b863d1f]

comment:7

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.

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

comment:8

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 c1f1(x)+c2f2(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.

2. 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!

[53959995-fd20-47f5-85e6-5e769b863d1f]

comment:9

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.

[53959995-fd20-47f5-85e6-5e769b863d1f]

Changed dependencies from #28672 to #28672 #29250

[mkoeppe]

comment:11

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.

[53959995-fd20-47f5-85e6-5e769b863d1f]

Changed branch from u/galois/mrs/41/threejs-persist-camera to u/paulmasson/mrs/41/threejs-persist-camera

[53959995-fd20-47f5-85e6-5e769b863d1f]

Changed author from Joshua Campbell to Paul Masson

[53959995-fd20-47f5-85e6-5e769b863d1f]

comment:13

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:

66d3471

Add viewpoint option

[53959995-fd20-47f5-85e6-5e769b863d1f]

Changed commit from fcb6ed4 to 66d3471

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

comment:14

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!

[53959995-fd20-47f5-85e6-5e769b863d1f]

comment:15

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.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 66d3471 to 37b0c87

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

37b0c87

Update viewpoint option

[53959995-fd20-47f5-85e6-5e769b863d1f]

comment:17

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?

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

f96ccdc

Further refine viewpoint option

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 37b0c87 to f96ccdc

[53959995-fd20-47f5-85e6-5e769b863d1f]

comment:19

Added support for tuples and clarified angle variable in documentation.

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

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

comment:20

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.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from f96ccdc to 850c59a

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

850c59a

Update documentation

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

3b7fc0c

Update documentation

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 850c59a to 3b7fc0c

[53959995-fd20-47f5-85e6-5e769b863d1f]

comment:23

Replying to @jcamp0x2a:

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

Done!

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

comment:24

Thanks!

[21c3cf6e-6c50-4e8b-9831-5a03f9a599c1]

Reviewer: Joshua Campbell

[mkoeppe]

comment:26

These have been merged into 9.2.beta4