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

Animation Path Support #151

Open
wants to merge 39 commits into
base: master
from

Conversation

Projects
None yet
@tisho
Contributor

tisho commented Jan 11, 2015

DO NOT MERGE. FOR DISCUSSION ONLY.

Hey folks, I've been working on adding support for animation paths to Framer and wanted your help with the design of the API, testing and overall comments.

I prepared a build that's up to date with this PR that you can use to check out the stuff I've been working on. You can download it from here.

Demos

Here are a few demos using that build to get you started:

API Overview

The current implementation is based on SVG. SVG paths (http://www.w3.org/TR/SVG/paths.html) are super versatile, supported by most major browsers and generally a pleasure to work with. I'd like to hear your opinion on this particular choice, though. It informed a lot of the API, as you'll see.

I've added a path option to the .animate() method. You use it like this:

myLayer.animate
  path: [your path object here]
  pathOptions:
    autoRotate: true # will make the layer's Z rotation correspond to the angle of the curve as the animation progresses
  debug: true # will show the path used right before the animation starts and hide it after it ends
  curve: 'ease-in'

There are a number of different ways to create a Path object.

  1. Using the Path constructor and the basic building blocks of an SVG path (the methods are moveTo, lineTo, hlineTo, vlineTo, curve, curveTo, qcurveTo, smoothCurveTo, qsmoothCurveTo, arc:
Path.curve(to: { x: 400, y: 400}, control1: { x: 200, y: 200 }, control2: { x: 300, y: 300})

The above will create a new cubic bezier curve from 0,0 to 400,400 with control points at 200,200 and 300,300. All path methods return the path object, so you can chain them easily:

Path.curve(to: { x: 400, y: 400}, control1: { x: 200, y: 200 }, control2: { x: 300, y: 300})
  .curve(to: { x: 500, y: 500 }, control1: { x: 350, y: 350 }) # a curve with just one control point is quadratic, instead of cubic
  .lineTo(x: 600, y: 500)
  .vlineTo(400)
  .arc(to: { x: 700, y: 700 }, rx: 200, ry: 250)

Notice that none of the paths specify an origin point. This is because the origin of a path is assumed to be the layer's origin point at the start of the animation.

  1. By specifying points to draw a curve through:
point1 = { x: 200, y: 100 }
point2 = { x: 300, y: 120 }
point3 = { x: 400, y: 180 }

myLayer.animate
  path: Path.thru([point1, point2, point3], curviness: 8)
  debug: true

The curviness parameter specifies how "curvy" the path will be as it passes through the points you've specified. A value of 0 will cause the path to be made up of straight lines between the points. A value of 10 will make a pretty curvy line. The default is 5.

  1. By using an SVG Path string:
myLayer.animate
  path: Path.fromString('M0,0 C200,200 300,300')
  debug: true

Refer to the SVG spec for the syntax: http://www.w3.org/TR/SVG/paths.html. It's very terse and extremely powerful, but without visual aids it's kind of a pain to work with quickly.

  1. By loading the path from an SVG file.
myLayer.animate
  path: Path.loadPath('path.svg')
  debug: true
  ...

In this case, the file will be loaded synchronously, the first path from it will be extracted and then used as an animation path. Came in handy when I wanted to draw more complex paths in Sketch and then just loaded them for the animation.

Implementation Details

Here's what happens when you hand an animation path to animation:

  1. An invisible SVG path is created behind the scenes.
  2. If debug is true, a debug layer with a copy of the SVG path and extra elements like control points is created and shown.
  3. x and y properties of the animation (if they were there) are ignored, because we'll be using the path to inform position. The same goes for rotationZ if autoRotate is true (it is by default).
  4. The browser's native getPointAtLength implementation for SVG paths is used to get the x/y coordinates of the layer at a specific point along the progress of the animation.
  5. The layer's position (and rotation) is changed according to the results of the previous operation.
  6. The last two operations are repeated on every tick until the animation ends.

Open Questions

  1. Do we want this to be SVG based?
  2. Is it fast enough? I haven't really done extensive performance testing, yet. Please test out the examples on all the devices you can get your hands on and let me know what the results are.
  3. Is the API easy to use?
  4. Is the API complete?
  5. Does it work on every browser it needs to be supported on?

Known Issues

  1. No tests (yet)
  2. If you use a cubic-bezier or spring animation curve that is supposed to overshoot, things will be buggy, because the browser has no idea what the path looks like beyond its end.

Let me know what you think.

In the meantime, I'm going to use a lot of this code to build support for using splines as animation curves (so more than just cubic beziers), which will give us very fine-grained animation control.

tisho and others added some commits Jan 5, 2015

@tisho tisho changed the title from [Proposal] Support for animation Paths to [Proposal] Support for Animation Paths Jan 11, 2015

@jordandobson

This comment has been minimized.

Show comment
Hide comment
@jordandobson

jordandobson Jan 11, 2015

Contributor

A video of your examples would be nice to see what the expected speed is. Hard to tell if the speed is correct or not.

Contributor

jordandobson commented Jan 11, 2015

A video of your examples would be nice to see what the expected speed is. Hard to tell if the speed is correct or not.

@tisho

This comment has been minimized.

Show comment
Hide comment
@tisho

tisho Jan 11, 2015

Contributor

@jordandobson The speed is as always determined by your curve (animation curve, not path) settings. So if you specify curve: 'linear', time: 2 along with the path parameter, your layer should move along the animation path in 2 seconds. Not more, not less. What's important is there's no stuttering or artifacts during animation. A video for reference would not be all that useful in this case.

Contributor

tisho commented Jan 11, 2015

@jordandobson The speed is as always determined by your curve (animation curve, not path) settings. So if you specify curve: 'linear', time: 2 along with the path parameter, your layer should move along the animation path in 2 seconds. Not more, not less. What's important is there's no stuttering or artifacts during animation. A video for reference would not be all that useful in this case.

@jordandobson

This comment has been minimized.

Show comment
Hide comment
@jordandobson

jordandobson Jan 11, 2015

Contributor

@tisho - It seemed fine then. Some seemed slow but I didn't notice any stuttering on the few devices I tested on. I'll dig into this more later but I love this idea... I used path animations a lot back in the flash days and somewhat miss it.I've also had to deal with flower style animations lately and this approach would make it sooo much easier.

Nice work!

Contributor

jordandobson commented Jan 11, 2015

@tisho - It seemed fine then. Some seemed slow but I didn't notice any stuttering on the few devices I tested on. I'll dig into this more later but I love this idea... I used path animations a lot back in the flash days and somewhat miss it.I've also had to deal with flower style animations lately and this approach would make it sooo much easier.

Nice work!

@cleercode

This comment has been minimized.

Show comment
Hide comment
@cleercode

cleercode Jan 11, 2015

This is awesome! No comments about the API, but I'm really excited for the functionality.

cleercode commented Jan 11, 2015

This is awesome! No comments about the API, but I'm really excited for the functionality.

@borryshasian

This comment has been minimized.

Show comment
Hide comment
@borryshasian

borryshasian Jan 12, 2015

@tisho , I haven't tried it out, but I can't wait to say you're awesome!

borryshasian commented Jan 12, 2015

@tisho , I haven't tried it out, but I can't wait to say you're awesome!

@stakes

This comment has been minimized.

Show comment
Hide comment
@stakes

stakes Jan 14, 2015

Contributor

👍 looks amazing. Path.thru looks great as an accessible entry point. I'm excited.

Contributor

stakes commented Jan 14, 2015

👍 looks amazing. Path.thru looks great as an accessible entry point. I'm excited.

@mrrocks

This comment has been minimized.

Show comment
Hide comment
@mrrocks

mrrocks Mar 3, 2015

This looks super useful 👍

mrrocks commented Mar 3, 2015

This looks super useful 👍

@koenbok

This comment has been minimized.

Show comment
Hide comment
@koenbok

koenbok Mar 3, 2015

Owner

I'll start merging this as soon as I finished custom scrolling.

Owner

koenbok commented Mar 3, 2015

I'll start merging this as soon as I finished custom scrolling.

@NoamElbaz

This comment has been minimized.

Show comment
Hide comment
@NoamElbaz

NoamElbaz Apr 29, 2015

Any chance that this implementation will allow a user to control the speed of motion. So the user could - for example - slide a circular slider to 50% ?

NoamElbaz commented Apr 29, 2015

Any chance that this implementation will allow a user to control the speed of motion. So the user could - for example - slide a circular slider to 50% ?

@joshpuckett

This comment has been minimized.

Show comment
Hide comment
@joshpuckett

joshpuckett Nov 19, 2015

Wanted to bring this back up. Tisho, I think in general this is 💯👌. I think SVG is definitely the way to go, given it's broad support. I think Path should take a string, that way we can either use Sketch plugins (https://github.com/joshpuckett/FramerModules/blob/master/SVGLayer/pathToSVG.sketchplugin), or better yet build that in to the importer.

Proposal: name sketch layers "paths.pathName" (where "path." is protected). Once you import, you can access all your path layers in the paths object, and load them in the API you've proposed:

myLayer.animate
    path: path.flowerCurve

Would be nice to merge this in, leave it undocumented for a while so we can test...

joshpuckett commented Nov 19, 2015

Wanted to bring this back up. Tisho, I think in general this is 💯👌. I think SVG is definitely the way to go, given it's broad support. I think Path should take a string, that way we can either use Sketch plugins (https://github.com/joshpuckett/FramerModules/blob/master/SVGLayer/pathToSVG.sketchplugin), or better yet build that in to the importer.

Proposal: name sketch layers "paths.pathName" (where "path." is protected). Once you import, you can access all your path layers in the paths object, and load them in the API you've proposed:

myLayer.animate
    path: path.flowerCurve

Would be nice to merge this in, leave it undocumented for a while so we can test...

@tisho

This comment has been minimized.

Show comment
Hide comment
@tisho

tisho Nov 19, 2015

Contributor

Thanks for resurrecting this @joshpuckett. I'll start fixing up the merge conflicts.

Path does take a string (Path.fromString(string)), but I'll add support for the full stringified path element representation from Sketch. Thanks for mentioning that.

👍 wrt importing paths from Sketch. Should there be some sort of naming convention for paths you actually want to use for animation? Otherwise we'd be importing all kinds of tiny detail paths that would have otherwise gotten flattened at import. Thoughts?

Contributor

tisho commented Nov 19, 2015

Thanks for resurrecting this @joshpuckett. I'll start fixing up the merge conflicts.

Path does take a string (Path.fromString(string)), but I'll add support for the full stringified path element representation from Sketch. Thanks for mentioning that.

👍 wrt importing paths from Sketch. Should there be some sort of naming convention for paths you actually want to use for animation? Otherwise we'd be importing all kinds of tiny detail paths that would have otherwise gotten flattened at import. Thoughts?

@joshpuckett

This comment has been minimized.

Show comment
Hide comment
@joshpuckett

joshpuckett Nov 19, 2015

Yea sorry, for naming convention you'd have to name your layer something like

paths.logoLine

Default paths are named "Path 1" in Sketch, so we'd just add paths to the imported paths object that started with "paths.", make sense?

joshpuckett commented Nov 19, 2015

Yea sorry, for naming convention you'd have to name your layer something like

paths.logoLine

Default paths are named "Path 1" in Sketch, so we'd just add paths to the imported paths object that started with "paths.", make sense?

@koenbok

This comment has been minimized.

Show comment
Hide comment
@koenbok

koenbok Dec 14, 2015

Owner

Update. We're still looking to land this.

Owner

koenbok commented Dec 14, 2015

Update. We're still looking to land this.

@koenbok koenbok changed the title from [Proposal] Support for Animation Paths to Animation Path Support Dec 14, 2015

@koenbok koenbok added the proposal label Dec 14, 2015

@tisho

This comment has been minimized.

Show comment
Hide comment
@tisho

tisho Dec 14, 2015

Contributor

I've got a lighter week ahead of me. Should be able to finish it up.

Contributor

tisho commented Dec 14, 2015

I've got a lighter week ahead of me. Should be able to finish it up.

@koenbok

This comment has been minimized.

Show comment
Hide comment
@koenbok

koenbok Dec 15, 2015

Owner

What are the biggest todo's? Maybe I can help?

Also make sure you re-merge master. We did a ton of cleanup.

Owner

koenbok commented Dec 15, 2015

What are the biggest todo's? Maybe I can help?

Also make sure you re-merge master. We did a ton of cleanup.

@tisho

This comment has been minimized.

Show comment
Hide comment
@tisho

tisho Dec 15, 2015

Contributor

Merging with master is one of them, and I've already started on that. Also, tests and all of the stuff that I've put under "Open Questions".

Contributor

tisho commented Dec 15, 2015

Merging with master is one of them, and I've already started on that. Also, tests and all of the stuff that I've put under "Open Questions".

@jornvandijk

This comment has been minimized.

Show comment
Hide comment
@jornvandijk

jornvandijk Dec 15, 2015

Collaborator

@tisho You're a legend. Can't wait for this to land.

Collaborator

jornvandijk commented Dec 15, 2015

@tisho You're a legend. Can't wait for this to land.

Fixed call to setDefaultProperties function, which no longer exists. …
…Fixed an issue with debug method which would incorrectly render anchor points with a zero coordinate. Added PathAnimation studio project to use in testing.

@nvh nvh referenced this pull request Aug 3, 2016

Closed

SVG Support? #48

@tisho

This comment has been minimized.

Show comment
Hide comment
@tisho

tisho Aug 29, 2016

Contributor

@nvh Can you take another look at this PR? I made some changes:

  • It's now compatible with the new animateTo syntax of the Animation API.
  • path now accepts just an SVG path description string, instead of a Path instance. I've added an SVGPathProxy class to help with the length/position calculations and act as a proxy to the underlying SVG node, and I've completely removed Path and the procedural path building API. It can be made available as a standalone module, or under extras/.
  • Removed the additional easing equations. They're out of scope for the PR.
  • Updated to latest master. Tests all pass.
Contributor

tisho commented Aug 29, 2016

@nvh Can you take another look at this PR? I made some changes:

  • It's now compatible with the new animateTo syntax of the Animation API.
  • path now accepts just an SVG path description string, instead of a Path instance. I've added an SVGPathProxy class to help with the length/position calculations and act as a proxy to the underlying SVG node, and I've completely removed Path and the procedural path building API. It can be made available as a standalone module, or under extras/.
  • Removed the additional easing equations. They're out of scope for the PR.
  • Updated to latest master. Tests all pass.
@ServusJon

This comment has been minimized.

Show comment
Hide comment
@ServusJon

ServusJon Sep 14, 2016

I am looking forward to use this!

ServusJon commented Sep 14, 2016

I am looking forward to use this!

Show outdated Hide outdated framer/Animation.coffee Outdated
Show outdated Hide outdated framer/Animation.coffee Outdated
Show outdated Hide outdated framer/Animation.coffee Outdated
@nvh

This comment has been minimized.

Show comment
Hide comment
@nvh

nvh Sep 14, 2016

Collaborator

It looks great now, but as the animation API changes are still a bit in flux, so I'm still hesitant to merge this in before that one is merged, to avoid merge conflicts later on.
I have still a couple minor remarks:

  • I'm not sure, but maybe it makes sense to make the path part of the animation options?
  • Same goes for the autoRotate option.
  • I understand the scoping of Utils.SVG functions, but we don't do this anywhere else in the library. Would suggest renaming them to getSVGContext and createSVGElement and remove the scoping.
  • These Utils methods could use some tests too.
  • We might need a warning when you try to animate both x or y and path
  • I don't like the amount of debugging-code that this PR adds, is there any way to confine that more?
  • I added some style remarks to the code between the lines

@koenbok what do you think?

Collaborator

nvh commented Sep 14, 2016

It looks great now, but as the animation API changes are still a bit in flux, so I'm still hesitant to merge this in before that one is merged, to avoid merge conflicts later on.
I have still a couple minor remarks:

  • I'm not sure, but maybe it makes sense to make the path part of the animation options?
  • Same goes for the autoRotate option.
  • I understand the scoping of Utils.SVG functions, but we don't do this anywhere else in the library. Would suggest renaming them to getSVGContext and createSVGElement and remove the scoping.
  • These Utils methods could use some tests too.
  • We might need a warning when you try to animate both x or y and path
  • I don't like the amount of debugging-code that this PR adds, is there any way to confine that more?
  • I added some style remarks to the code between the lines

@koenbok what do you think?

@tisho

This comment has been minimized.

Show comment
Hide comment
@tisho

tisho Sep 15, 2016

Contributor

Thanks for the comments again @nvh. I've updated the PR with the following:

  • Tidied up the debugging code by moving pretty much all of it into its own class (SVGPathDebugLayer). It covers creating the debug representation for a path, aligning it to a layer, and animating it as the path animation progresses. After this refactor, there are 3 lines total that mention the debugging layer in Animation.coffee and all debugging code has been taken out of SVGPathProxy.coffee
  • Extracted the two functions under Utils.SVG like you suggested and added tests for them (which it turns out were necessary, because some of the code was buggy).
  • Added a warning when trying to animate x or y while animating on a path.
  • Addressed the style comments.

Now, re: making path part of options, I've considered it before and it was in fact how it was implemented initially, but I've changed my mind since. There are a couple of reasons for that:

  1. Path is essentially a compound property (x + y) that happens to not change linearly. Since it overrides any x or y properties you pass in, it should be considered a special case of a property.

  2. Very practically, it would be cumbersome to always have to write animateTo(options: path: path) vs animateTo(path: path).

Re: autoRotate, it's already an option, not a top-level property. You could argue that since it changes rotation, it can also be a property, but my rationale for keeping it under options is that it changes the animation by turning an automatic behavior on/off, instead of specifying concrete waypoints like path.

As for when this should be merged, I agree that it can probably wait until the new animation API is done, but that's your call.

Contributor

tisho commented Sep 15, 2016

Thanks for the comments again @nvh. I've updated the PR with the following:

  • Tidied up the debugging code by moving pretty much all of it into its own class (SVGPathDebugLayer). It covers creating the debug representation for a path, aligning it to a layer, and animating it as the path animation progresses. After this refactor, there are 3 lines total that mention the debugging layer in Animation.coffee and all debugging code has been taken out of SVGPathProxy.coffee
  • Extracted the two functions under Utils.SVG like you suggested and added tests for them (which it turns out were necessary, because some of the code was buggy).
  • Added a warning when trying to animate x or y while animating on a path.
  • Addressed the style comments.

Now, re: making path part of options, I've considered it before and it was in fact how it was implemented initially, but I've changed my mind since. There are a couple of reasons for that:

  1. Path is essentially a compound property (x + y) that happens to not change linearly. Since it overrides any x or y properties you pass in, it should be considered a special case of a property.

  2. Very practically, it would be cumbersome to always have to write animateTo(options: path: path) vs animateTo(path: path).

Re: autoRotate, it's already an option, not a top-level property. You could argue that since it changes rotation, it can also be a property, but my rationale for keeping it under options is that it changes the animation by turning an automatic behavior on/off, instead of specifying concrete waypoints like path.

As for when this should be merged, I agree that it can probably wait until the new animation API is done, but that's your call.

@nvh

This comment has been minimized.

Show comment
Hide comment
@nvh

nvh Sep 15, 2016

Collaborator

Thanks for the quick response, @tisho! I agree with your argumentation for keeping path a property.

It's good to merge now, so will do so as soon as the animation API is merged, which shouldn't take long.

FYI: We changed our minds about animateTo, and will keep the animate function with both functionalities. But this won't be a problem, as it will probably by fine after we merge the animation API.

Collaborator

nvh commented Sep 15, 2016

Thanks for the quick response, @tisho! I agree with your argumentation for keeping path a property.

It's good to merge now, so will do so as soon as the animation API is merged, which shouldn't take long.

FYI: We changed our minds about animateTo, and will keep the animate function with both functionalities. But this won't be a problem, as it will probably by fine after we merge the animation API.

@nvh nvh referenced this pull request Sep 15, 2016

Closed

Animation Paths #418

@nvh

This comment has been minimized.

Show comment
Hide comment
@nvh

nvh Sep 15, 2016

Collaborator

So I've merged the animation stuff into master, and then merged master in your branch and fixed the conflicts.

@tisho: could you check if this looks alright to you? #418

Collaborator

nvh commented Sep 15, 2016

So I've merged the animation stuff into master, and then merged master in your branch and fixed the conflicts.

@tisho: could you check if this looks alright to you? #418

@koenbok

This comment has been minimized.

Show comment
Hide comment
@koenbok

koenbok Sep 16, 2016

Owner

We're going to wait a little bit until the new animation api completely settles and circle back. Should be a matter of a few weeks.

Owner

koenbok commented Sep 16, 2016

We're going to wait a little bit until the new animation api completely settles and circle back. Should be a matter of a few weeks.

@noAlvaro

This comment has been minimized.

Show comment
Hide comment
@noAlvaro

noAlvaro Jan 10, 2017

What's the current status of this?

noAlvaro commented Jan 10, 2017

What's the current status of this?

@iconmaster

This comment has been minimized.

Show comment
Hide comment
@iconmaster

iconmaster Jul 25, 2017

I could really use some animation paths right now. 😘

iconmaster commented Jul 25, 2017

I could really use some animation paths right now. 😘

@nvh nvh referenced this pull request Dec 8, 2017

Merged

Path animation #555

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