loopier / animatron

Animatron for Godot 4.x <
15 stars 1 forks source link

Add masking support (let one Actor mask others) #56

Closed totalgee closed 3 weeks ago

totalgee commented 1 month ago

This seems like a simple thing, but is not as simple as it seems. In Godot 3, one could use Light2D masks to do something like this. In Godot 4, there is a feature called clip children that seems like it should be able to do it (also related: CanvasGroup, that similarly uses the backbuffer). I've been able to get clipping to work in a simple test project using clip_children, but when I try it in Animatron I seem to get unreliable results. Sometimes it doesn't work at all as I'd expect, other times it works but doesn't take into account alpha in the child nodes. I don't know if this is related to the fact that we're rendering into a SubViewport, maybe(?).

Here's how I want it to work (it works in a simple test project with a parent "cog" mask and a child "heart" image): clip_children_working

Here's what sometimes happens in Animatron with the same setup (alpha of the child is not erased, instead we see black): clip_children_not_using_child_alpha I thought this might be related to the fact that we're using a custom shader, but if I remove that and use the default shader I still get the same problem.

There is a long and frustratingly unresolved thread related to this in a Godot proposal/issue: https://github.com/godotengine/godot-proposals/issues/4282

totalgee commented 1 month ago

Hmm, doing more testing in my test project (added a SubViewport), and it seems related to the transparent_bg property (oddly, the black alpha happens when this property is on, which is the opposite of what I'd expect).

totalgee commented 1 month ago

If I turn off transparent_bg property on the SubViewport, and also stop using our custom shader, then it seems to work (at least on one specific case I was testing)! So I need to figure out how to fix the shader so it works...progress, at least.

image

totalgee commented 1 month ago

Actually, we just need to stop using our custom shader for Actors that are acting as masks (they aren't displayed anyhow, so we don't need our custom colour-tweaking behaviour).

I got the basic version working now, still need to fine-tune it a bit, but at least it seems to work!

https://github.com/loopier/animatron/assets/306082/f87a3f72-16f6-4b94-bae0-2b78716c9163

totalgee commented 1 month ago

I've also discovered lots of problems with /parent and /parent/free, as well as /front, /behind, /top and /bottom -- all things to do with parenting and reordering nodes. I had to make a few changes:

Actors as children will now always be parented under the "Animation" (AnimatedSprite2D) node of the parent Actor, rather than directly beneath the parent Actor itself (CharacterBody2D aka "MetaNode"). This is required for the way I'm doing masking (using clip_children), and also it is needed to make things work with multi-level Actor hierarchies and node reordering using /front and /behind (and top/bottom), because otherwise you have to contend with the other nodes (not sub-Actors) that also exist under an Actor node (specifically CollisionShape2D, RichTextLabel and AnimationPlayer).

There were also lots of problems with transforms getting messed up when parenting and unparenting, especially when taking into account multi-level hierarchies. Now parenting/unparenting and masking/unmasking should all leave the parent and child with the same (global) transform.

Note that the current transform commands (to move, rotate, scale, etc.) are all working locally (in the parent's space). This is good...but at some point we may want to add global versions of them. For example, when you are masking, you may want to transform the masked Actors globally. (both types of transform are useful to have -- @loopier made a comment that you can work around by unparenting, transforming and reparenting, but I find this a bit "heavy"...would be nicer to expose the global transform commands themselves!)

Also, currently the /mask and /unmask commands also do reparenting. However, it would be possible to just have /mask set up an Actor as a masking node, and then you do the reparenting under it as desired. I could change it to work that way if preferred. And on /unmask, in that case, it wouldn't remove the parenting relationships, just turn off masking...let me know what you think!

totalgee commented 1 month ago

One other thing I noticed (but didn't change)...the /front and /back commands actually move the Actor to be immediately behind or in front of the target. I updated the documentation to reflect this. This may be confusing (to me, it was, a bit)...it could be changed so that if the ordering is already behind or in front as desired, it would not change.

For example, suppose you have Actors drawn in this order: A - B - C (C is "in front" of B and A) If you do /front C A, currently (before, and also after my changes) it will become: A - C - B (that is, C will be set to be immediately in front of A, even though it was already "in front" of A...and also B)

If you want any of those other changes, maybe open new issues for them, so I stop "contaminating" this issue about masking.... ;-)

totalgee commented 1 month ago

My example test code (various permutations of these lines of code, and more):

/load cog
/load red-heart

/create cog cog
/create heart red-heart
/create heart2 red-heart
/color heart2 0.1 0.5 2
/move heart -40 20
/move heart2 40 20
/rotate heart2 -10

/parent heart cog
/parent heart2 heart

/move cog 10 0
/rotate cog 5

/parent/free heart
/parent/free heart2
/parent/free cog

/parent heart2 cog
/mask heart cog
/mask heart2 cog

/unmask cog

/bottom heart
/front heart2 heart

/free cog
totalgee commented 1 month ago

This issue should be resolved by 7e86d12. @loopier please test it to see if it works for your use cases, and report back.

loopier commented 1 month ago

Also, currently the /mask and /unmask commands also do reparenting. However, it would be possible to just have /mask set up an Actor as a masking node, and then you do the reparenting under it as desired. I could change it to work that way if preferred. And on /unmask, in that case, it wouldn't remove the parenting relationships, just turn off masking...let me know what you think!

I think masking should be an opaque operation. When performing, the less thinking the better, so encapsulating the parenting for /mask and /unmask seems the most reasonable thing to me, as you just want to mask without having to know how it works.

For example, suppose you have Actors drawn in this order: A - B - C (C is "in front" of B and A) If you do /front C A, currently (before, and also after my changes) it will become: A - C - B (that is, C will be set to be immediately in front of A, even though it was already "in front" of A...and also B)

I think this wasn't actually that bad, because you can target the front or back of an object, without having to know it's actual position in the overall stack. So in your example, you could send C to be in front of A without having to worry which object is immediately in front of it. Again --for me--, the less thinking the better, but there's no right or wrong here. If you think it makes more sense how you said, let's keep it like that.

totalgee commented 1 month ago

Fixed an issue @loopier found, where it was possible to crash or lose Actors by doing a "cross-reparenting" (A as child of B, and B as child of A). This could also happen with longer chains of ancestry, like A-B-C and trying to parent A under C. I fixed it by detecting the potential cycle in the hierarchy and return an error to the user.

totalgee commented 3 weeks ago

This is not directly related to masking, but I'll mention it here before I close this issue. In 5008021 I exposed color property to appear as though it belongs to the Actor. This means you can easily do things like animate an Actor's colour using:

/tween 1 linear /color actor R G B

See the note on that commit -- there is also a built-in modulate property on the Actor that can be used, but it will multiply the other colour values, and also will apply to the entire Actor hierarchy (Actor plus its descendants, which also means it would affect masking). Meanwhile, color property will only affect this Actor and not its descendants, which is probably more what the user would intend.