Skip to content
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

Add animation when switching between "graph sets" #95

Closed
amanda-phet opened this issue Nov 11, 2022 · 19 comments
Closed

Add animation when switching between "graph sets" #95

amanda-phet opened this issue Nov 11, 2022 · 19 comments

Comments

@amanda-phet
Copy link
Contributor

Related to #63

When the user choose which graphs are in view, I'd like the graphs to smoothly animate up or down (so it looks like they are sliding) to reveal the new visible graph.

As for the zoom and eyeball buttons, those probably need to animate with the graphs, since f(x) doesn't have zoom buttons.

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 11, 2022

@veillette You and I had talked about this a few weeks ago. This will involve a twixt animation, similar to what I did in hookes-law IntroAnimator. To demonstrate, run Hooke's Law, go to the Intro screen, and use the radio buttons to switch between 1-spring and 2-spring systems.

@amanda-phet A couple of questions:

(1) Do you want the "newly visible" (and newly invisible) graphs to fade in/out, like the springs do in the Intro screen of Hooke's Law? Or do you want those graphs to just instantly become visible?

(2) Does the animation need to be stateful for PhET-iO? This is identical to the question that I asked in phetsims/equality-explorer#197.

@amanda-phet
Copy link
Contributor Author

Ooh, I love what you did in Hooke's Law!

(1) yes, let's use fade in/out for the graph that is newly visible/invisible, if you think that will work. It seems like the sequence will be: fade out --> slide up --> fade in in the case of removing an integral graph and adding a derivative graph. Does that sound right? I hope that isn't too slow of an animation.

(2) The answer will be identical, I would assume. As we discussed, there isn't a good way to handle this with PhET-iO, and we'll need to communicate to clients that they should not save the state of a sim while an animation is in progress. I assigned that other issue to @kathy-phet so she is aware that a decision is needed.

@veillette
Copy link
Contributor

Thanks @amanda-phet .
I'll take a look at IntroAnimator in HookesLaw and get familiar with twixt.

@veillette
Copy link
Contributor

I was able to understand most of what is going on in IntroAnimator of HookesLaw. It is nicely setup to toggle between two given scenery Nodes and the transition between the two is triggered by an axon property.

This approach is orthogonal to how we build the graphs in GraphsNode .
In GraphsNode we have an axon Property graphsSelectedProperty, that is a property of <GraphChoice> .

type GraphType = 'original' | 'integral' | 'derivative' | 'secondDerivative';
type GraphChoice = GraphType[];
type GraphChoices = GraphChoice[];

With the knowledge of GraphChoice, we can pick the appropriate graphNodes based on the GraphType , assemble them into a container and them lay them out, the new container is then added to the scene graph.

In principle there can be many GraphChoices. In practice, the derivative and integral have only one GraphChoice, whereas the Lab and Advanced Screens have two possible GraphChoices. I'm not sure how to handle the animation in the common view, given that the animation should only happened when there are two GraphChoices.

We either need to change significantly the approach used in Hooke's law or we need to change the approach in GraphsNode .

I'll bring it up to @pixelzoom during our assigned meeting.

@veillette
Copy link
Contributor

veillette commented Nov 15, 2022

After some consideration, I think the animations should not be done at the level of the container of the graphs but the graphs themselves.

Considering the advanced graph

we start with

  • integral graph
  • f(x) graph

and press the derivative button. we want to end up

  • f(x) graph
  • derivative graph

A possible sequence of animations would be

  • make the derivative graph appears (by changing the opacity) below the f(x) , out of dev bounds
  • move the three graphs up ( derivative graph moves within the dev bounds, while the integral graph moves out of it)
  • change opacity of integral graph to make it disappear (although it was already out of dev bounds by then)

Pushing the integral button , the sequence of animations would be

  • make the integral graph appears (by changing the opacity) above the f(x) , out of dev bounds
  • move the three graphs down ( integral graph moves within the dev bounds, while the derivative graph moves out of it)
  • change opacity of derivative graph to make it disappear (although it was already out of dev bounds by then)

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 15, 2022

We either need to change significantly the approach used in Hooke's law or we need to change the approach in GraphsNode.

Indeed, I didn't think we'd be able to use the same code as Hooke's Law, sorry for not being clear about that. I referred to Hooke's Law mainly as an exemplar of how twixt can be used to animate things, and a demonstration of how animation of translation and opacity in particular might be appropriate for Calculus Grapher. The actual animation sequence that we use should be based on what's need for Calculus Grapher.

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 15, 2022

@veillette and I discussed in our 11/15 meeting.

  • type GraphChoices has served us well so far, and the flexibility of specifying other configurations of graphs is likely to serve us well in the future. So let's stick with it.

  • The most general approach to animation is something like:
    (1) compare newGraphChoices to oldGraphChoices
    (2) fade out any graphs are only in oldGraphChoices
    (3) translated all graphs that are in both newGraphChoices and oldGraphChoices
    (4) fade in any graphs that are only in newGraphChoices

  • Graphs would need to be vertically shrunk/expanded based on newGraphChoices.length. This does not currently happen in this sim. So when writing the animation code, start by making this assumption:

assert && assert( newGraphChoices.length === oldGraphChoices.length );

  • This feature is relatively low-priority until the sim is feature complete.

@veillette
Copy link
Contributor

This feature is relatively low-priority until the sim is feature complete. Set as deferred for the time being

@pixelzoom
Copy link
Contributor

While we can wait on implementing this until the sim is feature complete, we should probably start gathering designer feedback. How we implement this is dependent on the desired animation behavior. And it will be diffucult to change that behavior without starting over.

@amanda-phet I've proposed animation in #95 (comment):

(1) compare newGraphChoices to oldGraphChoices
(2) fade out any graphs are only in oldGraphChoices
(3) translated all graphs that are in both newGraphChoices and oldGraphChoices
(4) fade in any graphs that are only in newGraphChoices

... where newGraphChoices and oldGraphChoices are the sets of graphs that are visible.

Does this behavior sound acceptable? If not, what did you have in mind?

@amanda-phet
Copy link
Contributor Author

This sounds like what I had in mind in a previous comment. Just to clarify, does "translated all graphs that are in both newGraphChoices and oldGraphChoices" essentially mean a vertical shift?

@veillette
Copy link
Contributor

veillette commented Dec 7, 2022

Yes the common graphs would be simply moved up or down.

@amanda-phet amanda-phet removed their assignment Dec 7, 2022
@pixelzoom pixelzoom self-assigned this Jan 25, 2023
@pixelzoom
Copy link
Contributor

I volunteered for this, self assigning.

@pixelzoom pixelzoom changed the title Add animation when switching between graphs in view Add animation when switching between "graph sets" Jan 26, 2023
@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 31, 2023

Notes to self:

See hookes-law IntroAnimator for inspiration.

This will involve revising this listener in GraphsNode.ts:

model.graphSetProperty.link( graphSet => {
  ...
} );

@pixelzoom
Copy link
Contributor

pixelzoom commented Feb 14, 2023

99% of the animation work was completed in the above commits, and it seems to be working nicely. I would encourage @amanda-phet and @catherinecarter to have a look. Switch between graph sets (radio buttons) in the Advanced and Lab screens to demonstrate.

Note to self about remaining loose ends:

  • Interrupt in-progress interactions before the animation begins.
  • The highlight around the original graph causes a minor layout problem. After animation finishes in the Advanced screen, the reference line tool moves up/down a bit. What we really need to know is the bounds of the top/bottom ChartRectangles, not the top/bottom GraphNodes, in order to properly size tools that involve vertical lines.
  • Should GraphSetsAnimator be instantiated only if there is > 1 GraphSet? Instrumentation is currently conditional. Conditional instantiation night result in some duplicate code, so it might be better to leave "as is".
  • Do not animate when restoring PhET-iO state of graphSetProperty.

@amanda-phet
Copy link
Contributor Author

This is looking lovely to me.

One request. Can you reduce the time it currently takes for one graph to fade out? It might be too fast if we do that, but I'd like to give it a shot. If you try it and it's way too fast, then keep it as-is :)

@pixelzoom
Copy link
Contributor

pixelzoom commented Feb 14, 2023

All of the animations are currently pretty fast. The whole sequence is 1.5 sec. The breakdown is as follows, in this order:

  • fade out: 0.5 sec
  • move to new positions: 0.5 sec
  • fade in: 0.5 sec

@amanda-phet Let me know specifically what values you'd like to see.

EDIT: I should also mention that that there is precedent for the specific time values that I used for the animations. They are identical to the fade and move times that we used for similar animation in the Hooke's Law Intro screen. In that sim, we started with fade=0.1 sec, and move=0.15 sec. We found that we needed to slow them down them to 0.5 sec to make them look nice on iPad and lower-performance platforms. See "slow down tween animations, so that they work on ipad" in phetsims/hookes-law@55dc3f9a from 5/3/2015.

@amanda-phet
Copy link
Contributor Author

Let's leave it as it is. Thanks for sharing those values and sharing that extra context.

@pixelzoom
Copy link
Contributor

Alll of the remaining work that I listed in #95 (comment) has been addressed.

I'll assign this to @veillette in case he wants to review the implementation. Animation is handled by GraphSetsAnimator. It's called by graphSetProperty listener in GraphsNode. Feel free to close if OK.

@veillette
Copy link
Contributor

Well done @pixelzoom . I reviewed the implementation (mostly for my own sake, to better understand animations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants