Skip to content

Conversation

@davidlamhauge
Copy link
Contributor

With this feature, you can define the camera interpolation you want, from the previous keyframe to the chosen. You can choose between 21 interpolations.
I've made a short video, and I want to excuse for the lag in the video. My Windoze PC is not very good.
About review: Since @candyface has helped a lot in the development of this feature (Thanks!), it would be easy for him to review it, and that might speed up the review process. On the other hand, it could also be used as an argument, to have somebody else to review it. No matter what, I hope this feature will get to our users as soon as possible.
https://youtu.be/ENeYrQTbs7Q

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Mar 8, 2021

billede
I've added a small improvement, so you can see the start and end frame number for the interpolation. As you can see, I am currently on frame 7, but the interpolation is from frame 10 to 33, since I chose keyframe 33.
I also deleted an unused include.

@J5lx
Copy link
Member

J5lx commented Mar 8, 2021

I'm fine with you reviewing the code, to get fresh eyes on it, and it's okay if you don't have the time right now, but I have the hope, that it will be reviewed within the next month...

I’ll keep it in mind. That said, if anyone else wants to review it first, please do, since I don’t know when I’ll be able to do it.

@Jose-Moreno
Copy link
Member

@davidlamhauge Hey David. Great work, I like how you solved the user experience since it's something that makes sense visually. Since animation I also like how you provided a more sensible categorization for the easing levels based in polynomials (e.g sinusoidal, quad, cubic, quartic, quintic, etc)

This will certainly become a foundation for future motion tween work for other layers, and a solid base to expand for more easing types in the future if there is need for them (i.e bounce, sine, back a.k.a overshoot, etc)

I have a few questions and suggestions to review so I can fully understand how this will work for the end user, but I need to gather my thoughts and make a video because otherwise I might end up writing an essay here 😅 💦 Personally I also want to support speeding up PR's so most of what I will mention could be added in a follow-up enhancement PR.

That said, one of the core additions that should be considered right now is having a "no ease" / "hold" option to simulate the exposure hold in the same way Pencil2D does it with other layers, that way we would have a basic set of tools to simulate motion without needing to complicate this PR further. Thank you for your hard work 🙂

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Mar 9, 2021

billede
I've expanded the possibilities (included Sine-based and exponential interpolation), taken 'Circle-based' out of the 'Other'-menu, and grouped them so slow, normal, fast and faster is in one part, and the rest in another.
billede
I also added the option mentioned by @Jose-Moreno , so you can set 'Hold / No interpolation' as an option.
(Edit: I have changed the text for Hold so it, in this case, will show 'Hold from frame 10'. Seems more informative. I will commit it when the PR is reviewed, with eventual other changes.)

@Jose-Moreno
Copy link
Member

@davidlamhauge Looking great! I wanted to ask another thing meanwhile. Is it possible right now to select multiple frames and apply the tweens to all of the selected frames? That'd be very useful regardless of the preset used 😄

I'll be testing this branch a little later today so hopefully I can prepare more informed questions and discussions if there is a need to do so. Thanks again!

@davidlamhauge
Copy link
Contributor Author

@Jose-Moreno, It is only possible to change interpolation if one, and only one, keyframe is selected. This makes it possible to make it easy to use, and more informative.
billede
Here you have selected frame 33, and the menu tells you 'Interpolation frame 10 to 33'. If you select a hold, the menu tells you 'Hold from frame 10'. I think this is very informative and user-friendly, and you couldn't achieve it , if you had selected two or more keyframes. Most often, you would end up with 1-5 keyframes in every scene, so it is not hard work to set each keyframe individually.
Your question gave me another idea though. It could be a good idea, if you could set your preferred interpolation in the Preferences. If I tend to use 'Fast ease-in - ease-out' all the time, it might as well be my default interpolation. That could save time.

@Jose-Moreno
Copy link
Member

@davidlamhauge One thing I noticed yesterday while inspecting Blender and Animate CC is that the "quad" interpolation is considered the default or "normal", while the sinusoidal interpolation would be the slow one, however after I reviewed some sites to clarify this relationship a bit better I got another surprise.

Looking at this site https://sighack.com/post/easing-functions-in-processing#sinusoidal-to-exponential-easing it seems the easing grows from sinusoidal (slowest) up to exponential (fastest); math isn't my strong suit so apologies if this was obvious.

I also looked at this site which has a nice example for most of the easing graphs are laid out horizontally and it solidified the idea https://observablehq.com/@jshernandez15/easing-animations.

Lastly I managed to review this final site which has all the curves visualized in order which helped me conclude that these 6 functions are naturally treated as a set https://easings.net/

Considering these, if we were to use the easing scale from sinusoidal up to exponential instead of quadratic to quintic it would be something like:

  • Slow (sinusoidal / sine)
  • Normal (quadratic)
  • Quick (cubic)
  • Fast (quartic)
  • Faster (quintic)
  • Fastest (exponential)

The terms are just a proposal, but at least this would mean that both the sine and exponential categories could be includedas part of the superlative easing scale you have already setup, so hopefully you can also consider this change as well as it would make it more friendly and easy on the eyes🙂

@Jose-Moreno
Copy link
Member

@Jose-Moreno, It is only possible to change interpolation if one, and only one, keyframe is selected. This makes it possible to make it easy to use, and more informative.
billede
Here you have selected frame 33, and the menu tells you 'Interpolation frame 10 to 33'. If you select a hold, the menu tells you 'Hold from frame 10'. I think this is very informative and user-friendly, and you couldn't achieve it , if you had selected two or more keyframes. Most often, you would end up with 1-5 keyframes in every scene, so it is not hard work to set each keyframe individually.
Your question gave me another idea though. It could be a good idea, if you could set your preferred interpolation in the Preferences. If I tend to use 'Fast ease-in - ease-out' all the time, it might as well be my default interpolation. That could save time.

I see. I think I understand better. What kept me wondering was that If I'm going to hold a frame forward, I'm supposed to be selecting the first frame and expose it forward; selecting the keyframe immediately after the actual held exposure of the timeline is slightly confusing. Similarly when easing, I expected that the first frame you apply ease to will affect everything inbetween itself and the next frame.

But with how I understand your solution and considering your latest description it appears to me that the opposite behavior is happening instead, meaning that the interpolation is affecting not only the current frame but also the one before it, which seems counter intuitive in regards to how time flows in general over the timeline. Maybe this is something that could be reviewed as well in due time.

Note that i'm not saying the current apparent solution is incorrect, but rather that it goes against convention and expectation since even working traditionally you know one can't shoot the film backwards (normally) so telling a frame to be held or interpolated by selecting the keyframe after it seems challenging.

Other than that I don't find any issue with how informative or descriptive the menus are. It certainly is better to be clear in that regard 👍

@davidlamhauge
Copy link
Contributor Author

@Jose-Moreno, I don't agree with your opinion, and I would find it very counter intuitive with your approach.
If you have two keyframes on the camera layer, it is obvious to me, that it is key 2 that tells key 1 where to go and how to get there. If the camera field has changed from key 1 to key 2, then it is key 2 that tells key 1 where to go to, and I can't see any logic, in storing the information on how til get there, in key 1.

@davidlamhauge
Copy link
Contributor Author

@Jose-Moreno, I like your research on the easing curves, but you don't have to go further than https://doc.qt.io/qt-5/qeasingcurve.html#Type-enum, to see fine illustrations.
The reason I've grouped the four together, is that they are based on power-of-2 to power-of-5, but it makes sense to make a group them as you do. Let's hear what the other say.

@davidlamhauge
Copy link
Contributor Author

I've made some changes, so you can either choose interpolation for one keyframe at a time, or you can select one or more keyframes to interpolate. Updated video below:
https://youtu.be/5hluAximXHI

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Mar 20, 2021

After talking to @Jose-Moreno and watching a few tutorials for other software, I've changed the logic, so you always define easing forwards. If you have keyframes on frame 1 and 50, you right click frame 1, to set the easing towards frame 50. It seems to be the prevalent way to do it, even if I still think it is counter-logical
It is still possible to set easing on more than one keyframe at a time, but the more I work with the camera, the more I think it's a bad idea. Camera work is tedious and hard, and you should be fully focused on what you're doing. To save 10 seconds, by manipulating two or more keyframes at a time, is no real gain, compared to the mess you can do if something goes wrong. I vote to reverse it, so you can only work on one keyframe at a time. Any opinions on this?
I can see that an enum for future use is still in the code. I'd say we leave it there, but I'll remove it if you want me to.

@davidlamhauge davidlamhauge changed the title Camera interpolation Camera interpolation. Camera rewrite (1) Mar 22, 2021
davidlamhauge and others added 6 commits April 5, 2021 10:45
At some point I would like to have all camera related logic in its own CameraTimelineCell class so that we don't pollute the general timeline cell class with Camera Layer specific code. This requires some work though as the timelinecells class is not built for that kind of setup currently.
and make sure frame is selected before the menu opens.
…ctoring

Camera interpolation minor refactoring
@MrStevns
Copy link
Member

MrStevns commented Apr 5, 2021

After reviewing and making a PR with minor refactoring, I think it looks good now.

Regarding your comments @davidlamhauge, I don't think working with a camera should be tedious, and if it is, then we should work on making it easier to use (which you are). As for your question about the ability to change interpolation for multiple frames at a time. I still think it's useful and I don't see why you would remove it when it can be convenient.

What do you mean when you say "compared to the mess you can do if something goes wrong", what could go wrong? the application crashes? or do you mean to say it causes a destructive behavior to the frames? As far as I understand, the interpolation functionality doesn't manipulate the content, so it shouldn't be a problem to do it on more than one frame?

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Apr 5, 2021

What do you mean when you say "compared to the mess you can do if something goes wrong",
When I say 'tedious' I mean that it is only slightly creative, and you have to be fully focused. This means that I, in the real world, wouldn't consider working on more than one camera interpolation at a time. You won't ruin content and the program won't crash, but you can accidentally ruin carefully planned interpolations, and I don't think it's worth it, compared to the small amount of time it could save you, to do multiple interpolations at a time.
Edit: I'm also thinking ahead. This PR is the first of a number of PRs, that aim to make the camera easier to use and with more features. When the camera gets more features and possibilities, the use for working on more than one camera interpolation at a time, will diminish or disappear.

Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor changes but I'd like to see them fixed before merging.

I'd say it's good to go when these have been fixed.

@MrStevns
Copy link
Member

MrStevns commented Apr 10, 2021

@J5lx Do you have any other comments regarding this PR, otherwise I plan to merge it after david has fixed the few minor things I had.

Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now 👍

@Jose-Moreno
Copy link
Member

Jose-Moreno commented Apr 11, 2021

@candyface Well I had a few requests I spoke with David during our debugging session, but I figured it would be better to add them later as enhancements rather than delay the PR being merged.

There are some that are tangential to other PR's but the list is focused around bugs and UX issues I found when using the widget and the interpolation presets, but i'll leave the related list here for completeness:

  1. The transform handle for the camera should have rotation handles on all 4 sides
  2. Widget handles should be a little bigger
  3. Wudget Handles should overlap the camera with transparency since after transform they tend to hide below the camera cutout
  4. Instead of using a red point for scaling and blue point for rotation as visual cues it's better to use rectangle borders with the same colors and a caption, similar to the safe area captions.
  5. Motion path looks nice at first, but often times the path steps being drawn look crooked. David explained that this is because for some reason he can only use integers instead of floats, so the line interpolation segments are not properly being laid build. This is most noticeable on straight paths and some curved paths.
  6. We should be able to use the existing discreet snapping with the camera transform widget and pressing the SHIFT key.
  7. I'd like to have the capability to undo the camera transforms (I'll probably add this request to the Undo/Redo PR)
  8. The camera reset is not obvious,. It took me a while to find it so I think It should be part of the camera transform widget as well.
  9. Adding menu actions for the transform widget and motion path would be appreciated.
  10. Motion path manipulation would benefit from being able to add more points to the path like a bezier curve to give it a slightly more complex shape (e.g bounce) manually.
  11. Right click to open the frame menu is not working on top of the playback cursor. It seems this is a Windows 10 issue related to Windows Ink.
  12. The transform widget is showing with other tools, not only the move tool.
  13. When adding a key inbetween an interpolation range the key should automatically take the current value at the current frame position and create a frame with that value stored. We'd need a way to add a keyframe that's part of the interpolation at that time without disrupting the motion.
  14. I think the motion path should be visible on the first key of the interpolation if possible. We looked at this during the debugging session, and we didn't know how to solve limiting the range of frames that should be displayed, however thinking back it could probably be good to always reference it from the frame where the motion tween was applied to and have it work like the onion skins by showing up to a certain amount of keyframes (by default it should be the next and previous keyframes only).

@davidlamhauge
Copy link
Contributor Author

Hi @Jose-Moreno . I haven't forgot your long list of wishes for the camera, but that is another PR. This one has still the old camera, and only makes it possible to define interpolations. No handles, paths etc.

@Jose-Moreno
Copy link
Member

Hi @Jose-Moreno . I haven't forgot your long list of wishes for the camera, but that is another PR. This one has still the old camera, and only makes it possible to define interpolations. No handles, paths etc.

Ah don't worry I wasn't listing those to delay the PR's, though I was interested in having Candyface look into the "integer-only" issue, either way I wanted to leave a hard copy. Unfortunately I did think this was a PR that had both features since the title mentioned Interpolation and camera rewrite and I only noticed afterwards when re-visiting github that this was only the interpolation PR without the widget transform 🤦

Nonetheless I'll be making a new issue for the interpolation requests, another one for the widget requests and a last one for the motion path feature and we'll discuss that afterwards. Here's hoping the camera PR's can get merged soon. Thanks again 👍

@MrStevns
Copy link
Member

I don't have any comments on the "interger-only" issue currently, as I haven't tried out the camera rewrite 2 PR yet, so I can't comment on the UX issues. I will however look into them when I start reviewing the PR.

Without further ado, I will now merge the PR.

In the future at some point I too also have a few wishes:

  • Being able to see which interpolation the camera uses with ease
  • Visually display curves on the timeline.. though this is not ideal when you only have 10 pixels in height, alternatively it could be a separate node editor (which has been discussed before) but that's a topic for another day.
  • Separate the TimelineCells class so that there can be a separate class for each type with their respective logic.

@MrStevns MrStevns merged commit 28ba695 into pencil2d:master Apr 13, 2021
@davidlamhauge
Copy link
Contributor Author

Being able to see which interpolation the camera uses with ease

@candyface This is implemented in the newest push from the camera_transform_camera branch.

@chchwy chchwy added this to the 0.7.0 milestone Jul 15, 2021
@MrStevns MrStevns modified the milestones: 0.7.0, v0.6.7 Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

5 participants