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

Onion skinning #12572

Merged
merged 6 commits into from
Nov 26, 2017
Merged

Onion skinning #12572

merged 6 commits into from
Nov 26, 2017

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented Nov 1, 2017

This allows you to see a ghosted view of the animated stuff a numbers of steps to the past or the future (based on the step value of the animation).

Some commits add some infrastructure to some areas of the engine, needed for this to be implemented on top of them.

It works better if the animated hierarchy is edited in a scene by itself because the background color is used as a hint to differentiate frames.

Closes #10425.

Images

Menu in the animation editor

(Nice onion icon by @djrm)

onion-icon onion-menu

Sample with 1-step depth, past & future

onion

Sample with 3-step depth, only past, differences only

onion-only-diff

Current limitations

  • It is always on (as long as the animation editor is visible). No UI to enable/disable or configure yet.
  • It's very slow because I haven't found yet a way of working with viewport captures on VRAM.
  • There isn't a way of seeing in realtime how the past state would be affected according to, e.g., the position of a node as you move it, until you really insert a keyframe or update the current one.
  • The ghost view corresponds to one frame (engine loop frame) earlier. No big issue.

Anyway, please test and report or give suggestions.

Spatial transform and bone transform are handled separately so it'd be nice that testers tried on these kind of properties specifically.

Changelog

UPDATED 11/06

  • Viewport capture achieved and easily extensible to more past and/or future frames.

UPDATED 11/09

  • Menu added to the animation editor. I need a nice onion icon for it. Currently it allows to enable/disable, but the direction and depth settings are ignored.
  • Works for 3D as well.

UPDATED 11/13

  • Now every option in the onion skinning menu work.
  • Editor settings for past/future colors added.
  • Some images added to the post.
  • Description edited.

UPDATED 11/18

  • Better shader.
  • Much less VRAM use.
  • Images updated.

UPDATED 11/20

  • Editor stuff like grids, guides, etc. removed from the onion layer for a less cluttered view.
  • That also applies to environment to keep the viewport clear color as the background as far as possible, since it's the base for the differentiation.
  • New Differences only mode, better described by the second animated sample above. Makes the layering much less noisy if enabled.
  • Images updated.

UPDATED 11/20 (2)

  • Added two more settings: force white modulate to override the colors for past/future in editor settings and include gizmos (3D) to have bones and other stuff in onion layers,
  • Menu image updated.

@blurymind
Copy link

This is very nice!
It would be great if we also get an autokeyframe mode :D

@RandomShaper
Copy link
Member Author

I need suggestions on how to combine full-scene captures of past/future frames with the current.

Rendering past with 50% opacity on top of the current is not good enough since both display as "dimmed". Yhe problem is the background of the captured frame is also overlaid on top of the present time scene.

A solution may be to render non-current frames on a transparent background, but that would just remove the plain color. Maybe not too much a problem as long as animated characters are edited in their separate scene.

Any better ideas?

@HeadClot
Copy link

HeadClot commented Nov 6, 2017

Will this work with 3D?

@reduz
Copy link
Member

reduz commented Nov 6, 2017 via email

@djrm
Copy link
Contributor

djrm commented Nov 6, 2017

@RandomShaper i think you might have already considered this, but just to be sure, usually past and present are represented by a blue and red tint or (green blue).

@RandomShaper
Copy link
Member Author

@HeadClot, I think so.

@reduz, I'll try.

@djrm, that'd be great and colors could be configurable. But I'm not sure yet about the way.

The past/future viewport captures could be rendered on top of the viewport with a screen-reading shader that does some kind of comparison.

A shader pseudocode proposal would help a lot.

Guys, thank you for your feedback. :)

@RandomShaper
Copy link
Member Author

Dear followers, I've pushed some more progress and updated the original comment.

@akien-mga, I need a nice onion icon for the dropdown menu. Where/how can I ask for it?

@djrm
Copy link
Contributor

djrm commented Nov 9, 2017

@RandomShaper dont worry i will do the icon.

@RandomShaper
Copy link
Member Author

@djrm, thank you!

@HummusSamurai
Copy link
Contributor

Pictures please :3

@RandomShaper
Copy link
Member Author

@HummusSamurai, I'll try.

@djrm, I'll need an icon for auto-keyframing as well.

I'll try to move all this forward a lot today.

Auto-keyframing makes onion skinning more useful so I think it's better to add it to this PR, in separate commit(s). Do you agree?

@RandomShaper RandomShaper force-pushed the onion-skinning branch 2 times, most recently from a7e1b39 to eaa15de Compare November 13, 2017 21:38
@RandomShaper
Copy link
Member Author

Code and main post updated! Please check.

@RandomShaper RandomShaper force-pushed the onion-skinning branch 2 times, most recently from 552fdc6 to febffa7 Compare November 18, 2017 01:52
@RandomShaper
Copy link
Member Author

New update! Getting better...

I'd like to ask @reduz and @akien-mga what would they say it's needed before considering this finished.

@blurymind
Copy link

would be cool if onion skin had a mode that includes only keyframed poses and not interpolation

Does this also work for 2d?

@RandomShaper
Copy link
Member Author

@blurymind, yes it works for 2D, too.

@RandomShaper
Copy link
Member Author

Oh, and I'll add that feature soon.

@blurymind
Copy link

@RandomShaper Thank you! Also thank you for looking into autokeyframe mode.
Your additions will make animating in godot so much faster and smoother!

@RandomShaper
Copy link
Member Author

New update! This is getting pretty interesting, I think. As always, please have a look at the updated description.

@djrm, any news about the onion icon? :)

@djrm
Copy link
Contributor

djrm commented Nov 20, 2017

@RandomShaper its ready will do a PR soon, it will be called Onion (if you want to change it in your PR)

@@ -202,6 +259,8 @@ class AnimationPlayerEditorPlugin : public EditorPlugin {
virtual bool handles(Object *p_object) const;
virtual void make_visible(bool p_visible);

virtual void forward_force_draw_over_viewport(Control *p_overlay) { anim_editor->forward_force_draw_over_viewport(p_overlay); }
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand this. Should there not be one for 2D and one for 3D?

Copy link
Member Author

Choose a reason for hiding this comment

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

For this use case, I designed the API dimension-agnostic, but I can split it easily into 2D and 3D if you find it to make more sense to plugin writers.

}
}
}
#endif
Copy link
Member

Choose a reason for hiding this comment

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

technically, you should be able to call MessageQueue::flush() for this to happen, though make sure that you are not called from this function when doing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did it that way because I wasn't sure no unrelated messages that can interfere somehow with the process are in the queue by that point.

Furthermore, this is called during a deferred call to _prepare_onion_layers_2() so we are already there.

@@ -127,7 +127,7 @@ void SceneTree::remove_from_group(const StringName &p_group, Node *p_node) {
group_map.erase(E);
}

void SceneTree::_flush_transform_notifications() {
void SceneTree::flush_transform_notifications() {
Copy link
Member

Choose a reason for hiding this comment

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

This is a good idea, I like it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Glad to read that. :)

@akien-mga
Copy link
Member

It should be fine to merge for 3.0 if it's ready in the coming days :)

@djrm
Copy link
Contributor

djrm commented Nov 20, 2017

@RandomShaper the Onion icon is on master.

@RandomShaper
Copy link
Member Author

@djrm, looks good!

@RandomShaper
Copy link
Member Author

I think I'm feature-freezing this so it can be merged to 3.0.

Key-based steps are pretty hard to implement so they'll have to wait. :(
I'm out of time for auto-keyframing as well.

Aside from code reviews and bug fixing, this is all I can do for 3.0.

@RandomShaper RandomShaper changed the title Onion skinning [WIP] Onion skinning Nov 20, 2017
@blurymind
Copy link

blurymind commented Nov 21, 2017

@RandomShaper autokeyframe for 3.1 ? :)

Also does the differences mode show only onion skin on frames that contain keyframes, or does it also include the tweens?

@RandomShaper
Copy link
Member Author

Yes, I'll try. In the meantime you could open a feature request so we don't forget.

To your other question, it's just a different, les cluttered way of seeing the layers. No stop-at-keys-per-track by now. I studied how it could be done, but it looks pretty hard to do now for 3.0.
Another feat request for that would be nice. :)

@RandomShaper
Copy link
Member Author

I'll fix the CI errors ASAP.

- Now it is usable from both `CanvasItem` and `Spatial` editors.
- `EditorPlugin` API changes:
 - `forward_draw_over_canvas()` becomes `forward_draw_over_viewport()`.
 - `update_canvas()` becomes `update_overlays()`, which now triggers the update of every overlay on top of any 2D or 3D viewports present. Also now it returns the number of such viewports, which is useful whenever you need to know the number of draw-over calls you'll get.
 - New: `[set/is]_force_draw_over_forwarding_enabled()` to force overlaying regardless it handles the current object type, in a similar fashion as `[set/is]_input_event_forwarding_always_enabled`. This kind of overlay is also on top of those for regular handled node types.
 - New: `forward_force_draw_over_canvas()`, which is the callback that gets called for plugins that enable forced overlaying.
@RandomShaper
Copy link
Member Author

Builds fixed!

@reduz, are you satisfied with my replies to your reviews? Would you like me to change something before merging?

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

Successfully merging this pull request may close these issues.

7 participants