Opened 7 months ago

Closed 8 weeks ago

#29194 closed enhancement (fixed)

Three.js-based Animations

Reported by: gh-jcamp0x2a Owned by:
Priority: minor Milestone: sage-9.2
Component: graphics Keywords: threejs animation
Cc: paulmasson, egourgoulhon, slelievre Merged in:
Authors: Joshua Campbell Reviewers: Paul Masson
Report Upstream: N/A Work issues:
Branch: 8af227d (Commits) Commit: 8af227dc286a4659c213a72b2a5ee69fe9d9e88f
Dependencies: Stopgaps:

Description (last modified by gh-jcamp0x2a)

Add to Sage the ability to produce interactive 3D animations that can be explored by translating, rotating, and zooming as the animation progresses. Previously, an animation would need to be re-generated each time a different viewpoint was desired.

To that end, this ticket implements the following:

  1. an .interactive() method on existing Animation objects that produces a new 3D graphics object containing all of the original frames of animation with additional animation metadata (keyframe assignments) attached that a supported viewer could use to depict the animation.
  1. changes to the Three.js viewer to support keyframe animation of points, lines, texts, and surfaces when animation metadata is present including optional graphical controls allowing the user to play/pause, adjust playback position, control playback speed, and toggle looping.
  1. support for saving a 3D graphics object directly to an HTML file that uses the Three.js viewer, bypassing the need to open it in a browser first and use the "Save as HTML" menu option.
  1. support for saving an animation to an HTML file using features 1 and 3 listed above.

Change History (33)

comment:1 Changed 7 months ago by paulmasson

  • Cc egourgoulhon added

comment:2 Changed 7 months ago by egourgoulhon

Looks very promising!

comment:3 Changed 7 months ago by gh-jcamp0x2a

I opted not to implement persisting the animation state at this time.

Since the template is being used elsewhere, I went ahead and broke the animation javascript and CSS out into separate files in this initial iteration. The changes to the template file are now quite minimal, and I hope that makes porting future changes to it between Sage and other projects easier.

With the feature work complete, I'll move on to documentation and tests. One thing I've encountered so far is that the .. PLOT:: directive doesn't seem to work with Three.js plots. I'm curious if this is a problem in my installation of Sage or if it's a known issue. I noticed that the documentation for the Three.js viewer (src/doc/en/reference/plot3d/threejs.rst) embeds pre-generated HTML files so perhaps that's the direction I should go as well?

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

Joshua, I agree that it is better to separate the animation script as you have done. Have you verified that everything in the threejs folder gets copied during the build process?

The rendering of Three.js graphics requires a browser environment, unlike the PLOT directive. While it would be possible in principle to render WebGL at the command line using Node.js, that is a topic way outside of my interest as an HTML-oriented person. For that reason documentation is currently generated with Jmol as a static image. I am personally much more interested in the live HTML examples I put into the backend documentation and recommend you go in that direction as well. The only drawback is that the images won't appear in PDF versions of the documentation.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 7 months ago by gh-jcamp0x2a

Replying to paulmasson:

Joshua, I agree that it is better to separate the animation script as you have done. Have you verified that everything in the threejs folder gets copied during the build process?

I've verified that animation.js and animation.css are automatically copied after doing a full make build but not after a sage -b. The template behaves similarly. Do we want to try to ensure they get copied during a sage -b as well? I've been copying them all over manually in my own start script in order to avoid a full rebuild when none of the Python had changed.

The rendering of Three.js graphics requires a browser environment, unlike the PLOT directive. While it would be possible in principle to render WebGL at the command line using Node.js, that is a topic way outside of my interest as an HTML-oriented person. For that reason documentation is currently generated with Jmol as a static image. I am personally much more interested in the live HTML examples I put into the backend documentation and recommend you go in that direction as well. The only drawback is that the images won't appear in PDF versions of the documentation.

Ahh, that makes sense. I was forgetting about the PDF version. I will proceed with the separate HTML files for plots in the documentation then. Thank you :)

comment:6 in reply to: ↑ 5 Changed 7 months ago by paulmasson

Replying to gh-jcamp0x2a:

Replying to paulmasson:

Joshua, I agree that it is better to separate the animation script as you have done. Have you verified that everything in the threejs folder gets copied during the build process?

I've verified that animation.js and animation.css are automatically copied after doing a full make build but not after a sage -b. The template behaves similarly. Do we want to try to ensure they get copied during a sage -b as well? I've been copying them all over manually in my own start script in order to avoid a full rebuild when none of the Python had changed.

We won't have any control over the sage -b behavior since our files are not part of the core of Sage. make build or manually copying are the options.

comment:7 Changed 5 months ago by gh-jcamp0x2a

  • Authors set to Joshua Campbell
  • Branch set to u/gh-jcamp0x2a/29194-threejs-animation
  • Commit set to dcd36c9d859729b28df0bb7fdbb90869bcdf6747
  • Status changed from new to needs_review

Hello, I hope this finds you well. I apologize for the delay in getting this ticket up for review, but I just didn't have a good feeling about the branch as it was.

After using it for a couple months, I've found the performance when using multiple animated variables to be quite wanting, as you'd expect from the exponential nature of adding new dimensions to the array of keyframes. Memory use and file size gets very large very quickly, especially when each frame itself is complex or if you've got several of them in the same notebook. Reducing the number of keyframes per variable helps, but having only a handful of frames makes noticing higher frequency stuff going on impossible. I consistently found myself using the interact command in the Jupyter notebook to control those extra variables instead.

I've pushed a new version of the branch for your review that drops support for multiple animated variables. A lot of the complexity of the old branch came from supporting them, so the new one is much simpler. It uses Three.js's built-in animation system now instead of a custom one, and it is invoked from the existing animation functionality users may already be familiar with: instead of calling an animation's gif or apng method, you call interactive. The name interactive was chosen instead of threejs to allow the possibility of other viewers supporting this in the future.

I've updated https://jcamp0x2a.github.io/threejs-animation-example/ with an example animated plot from the new branch.

Please let me know if you'd still prefer the older version. I believe the new one to be superior, but I'll try to keep up with and support either one going forward.

Thanks!

Joshua

comment:8 Changed 5 months ago by git

  • Commit changed from dcd36c9d859729b28df0bb7fdbb90869bcdf6747 to fd6f8a80362fa74a61eae451616e5a24ac57a041

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

1b6d2bfOnly animate when at least two frames present.
fd6f8a8Fix misuse of SEEALSO block

comment:9 follow-up: Changed 5 months ago by paulmasson

Joshua, I’ll take a look at this once we’re into the next beta. Don’t want to make any major changes just before a release.

comment:10 in reply to: ↑ 9 Changed 5 months ago by gh-jcamp0x2a

Replying to paulmasson:

Joshua, I’ll take a look at this once we’re into the next beta. Don’t want to make any major changes just before a release.

Understood. I figured I missed the boat once I started seeing the '9.1rc' tags pop up in the git logs :)

comment:11 Changed 5 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:12 Changed 3 months ago by paulmasson

Joshua, could you rebase this ticket off a current branch? That would save build time in reviewing. Thanks!

comment:13 Changed 3 months ago by git

  • Commit changed from fd6f8a80362fa74a61eae451616e5a24ac57a041 to ee6d7bc7bafefbd7324dd1901735ee281076f5b1

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ee6d7bcAdd support for simple keyframe-based animation to Three.js viewer.

comment:14 Changed 3 months ago by gh-jcamp0x2a

Hi Paul, I've rebased off of the latest in the develop branch. I verified that it still builds fine (hopefully the patch bot agrees) and quickly tested that the animation functionality is working as expected. I'll run a few more tests tonight or tomorrow as time permits to be sure, but it should be in a decent shape for review.

I imagine I'll want to rebase it again once your three.js update gets merged.

Thanks!

comment:15 follow-up: Changed 3 months ago by paulmasson

I've confirmed that the new version builds just fine and runs as expected. There is quite a lot of stuff changing here, so I'll need some time to read through all the code.

My first reaction is about the new bar across the top: although it's convenient for jumping to a frame, not sure if I like it yet. Maybe Eric can take a look and comment on it. The discrete controls would really be enough to control the animation.

Otherwise, it's looking great! Nice addition to Sage.

comment:16 follow-up: Changed 3 months ago by paulmasson

One other thought: shouldn't the decrease play back button be a reverse arrow? Or is this a Google standard icon?

comment:17 in reply to: ↑ 16 Changed 3 months ago by gh-jcamp0x2a

Hi Paul,

I somewhat agree about the slider at the top, but I'm pondering future work where the animations aren't all or shouldn't necessarily all be discrete, in which case a way to navigate continuously would be helpful. For example: when just spinning an object about an axis there's no reason it shouldn't be transformed as continuously as the highest frame-rate the user/browser allows. Ideally, I'm imagining something like:

dodecahedron().rotateX(pi, duration=10).then().rotateY(-pi, duration=10)

The discrete frame-by-frame was a nice starting point, though, since that's what users currently do for 2D animations, and so only a small change is required: animate(frames).interactive() instead of, for example, animate(frames).apng().

Perhaps it's jumping the gun a bit, though it does also serve to show the animation progress. May come in handy for seamlessly looped animations to tell when one period ended and another began without a sharp cut as a visual cue.


Replying to paulmasson:

One other thought: shouldn't the decrease play back button be a reverse arrow? Or is this a Google standard icon?

I used the hollow arrows for decrease speed instead of the reverse arrow since I didn't want the user to think it was a rewind button. I'm not particularly happy with the icons for the speed buttons, but I couldn't find anything in their icon set that seemed to fit better, and I'm not much of a graphics artist I fear.

Do you think chevrons instead of triangle arrows would convey the meaning better? > for slow down and >>> for speed up? That geometry doesn't seem too complicated so I'd probably be able to design it. Not up to Google's standards, I'm sure :)


I understand it's a good bit of code to look through, so no worries.

Thanks!

comment:18 in reply to: ↑ 15 Changed 3 months ago by egourgoulhon

Replying to paulmasson:

My first reaction is about the new bar across the top: although it's convenient for jumping to a frame, not sure if I like it yet. Maybe Eric can take a look and comment on it. The discrete controls would really be enough to control the animation.

I took a look and I don't have a strong opinion on both options, so I leave it to you.

Otherwise, it's looking great! Nice addition to Sage.

Indeed! Thanks for this piece of work!

comment:19 Changed 2 months ago by mkoeppe

needs rebase

comment:20 Changed 2 months ago by git

  • Commit changed from ee6d7bc7bafefbd7324dd1901735ee281076f5b1 to 03c980ff19ca9c17667e3919ea194eb578b22e6b

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

03c980fAdd support for simple keyframe-based animation to Three.js viewer.

comment:21 Changed 2 months ago by paulmasson

One nitpick after reading through code: I don’t think you should change the documentation where you added “specifying an HTML file” since half of the following options aren’t supported. A bit confusing.

comment:22 Changed 2 months ago by paulmasson

  • Reviewers set to Paul Masson

Looks mostly good to me, except for the issue raised about icons. I really think the slow down arrow should point to the left, but you're right that the rewind icon is misleading.

There are several of sets of left/right single chevrons already in the set of icons you're using: keyboard_arrow_left/right, navigation_before/after, chevron_left/right. Another choice would be single filled arrow_left/right.

Or perhaps even better switch_left/right, which shows a difference in direction immediately in the icons. For this choice I would have the filled arrow indicate the direction, which is opposite of the label. I think I like this last choice best.

comment:23 Changed 2 months ago by git

  • Commit changed from 03c980ff19ca9c17667e3919ea194eb578b22e6b to d48807cad635f39a9ad96e08721e44fb58883e89

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

c5666a3Change increase/decrease playback speed icons
d48807cRevert a documentation change in Graphics3d.save

comment:24 Changed 2 months ago by gh-jcamp0x2a

Hi Paul. I agree that that documentation change was confusing, so I've reverted it. I've also changed the icons to switch_left/right as requested, and I think they do indeed convey the meaning better. Here is an example of the new icons in action: https://jcamp0x2a.github.io/threejs-animation-example/simpler.html

comment:25 follow-up: Changed 2 months ago by paulmasson

Almost good to go: you didn't update the example with the new icons...

comment:26 Changed 2 months ago by git

  • Commit changed from d48807cad635f39a9ad96e08721e44fb58883e89 to 8af227dc286a4659c213a72b2a5ee69fe9d9e88f

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

8af227dUpdate animation example in Three.js documentation with new icons

comment:27 in reply to: ↑ 25 Changed 2 months ago by gh-jcamp0x2a

Replying to paulmasson:

Almost good to go: you didn't update the example with the new icons...

Ah! Thank you for catching that.

comment:28 Changed 2 months ago by paulmasson

  • Status changed from needs_review to positive_review

Cool! And you noticed that the examples for documentation should be generated using online=True. I'll add that to the template comment.

Thanks for your patience in the review process and slogging through multiple rebases. Nice work!

comment:29 follow-up: Changed 8 weeks ago by slelievre

  • Cc slelievre added

The ticket description reads

Tasks remaining before I'd be comfortable making a merge request:

If those tasks are now part of the ticket, please update the ticket description to

This ticket implements the following:

comment:30 in reply to: ↑ 29 Changed 8 weeks ago by gh-jcamp0x2a

Replying to slelievre:

The ticket description reads

Tasks remaining before I'd be comfortable making a merge request:

If those tasks are now part of the ticket, please update the ticket description to

This ticket implements the following:

Apologies. The ticket description is indeed out-of-date. Many of the work items mentioned also became irrelevant following a change of course partway through. I will update the description to match the feature as currently implemented.

comment:31 Changed 8 weeks ago by gh-jcamp0x2a

  • Description modified (diff)

comment:32 Changed 8 weeks ago by slelievre

Thanks! Do open a follow-up ticket if some items were not covered here but would make sense.

comment:33 Changed 8 weeks ago by vbraun

  • Branch changed from u/gh-jcamp0x2a/29194-threejs-animation to 8af227dc286a4659c213a72b2a5ee69fe9d9e88f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.