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 masking support (let one Actor mask others) #56

Closed
totalgee opened this issue Jun 6, 2024 · 10 comments
Closed

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

totalgee opened this issue Jun 6, 2024 · 10 comments

Comments

@totalgee
Copy link
Collaborator

totalgee commented Jun 6, 2024

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:
godotengine/godot-proposals#4282

@totalgee
Copy link
Collaborator Author

totalgee commented Jun 6, 2024

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
Copy link
Collaborator Author

totalgee commented Jun 6, 2024

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
Copy link
Collaborator Author

totalgee commented Jun 6, 2024

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!

Recording.2024-06-06.actor.masking.mp4

@totalgee
Copy link
Collaborator Author

totalgee commented Jun 6, 2024

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
Copy link
Collaborator Author

totalgee commented Jun 6, 2024

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
Copy link
Collaborator Author

totalgee commented Jun 6, 2024

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 added a commit that referenced this issue Jun 6, 2024
- masking uses the `clip_children` functionality, which had to be
  made to work in the context of our use of SubViewport and custom
  shaders.
- masking uses reparenting, so masked Actors become children of the
  Actor acting as mask.
- also fixed numerous issues with (un)parenting, transforms getting
  messed up, node ordering. Now, the global transform of an Actor
  should remain constant before and after any kind of reparenting
  or masking.
- also fixed issues related to reordering, including /front, /behind,
  /top and /bottom commands, and also artibrary depth hierarchies of
  Actors.
@totalgee
Copy link
Collaborator Author

totalgee commented Jun 6, 2024

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

@loopier
Copy link
Owner

loopier commented Jun 6, 2024

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 added a commit that referenced this issue Jun 6, 2024
- this can happen if you /parent or /mask A to B and then B to A, it
  would introduce a cycle and also would detach the entire hierarchy
  from the scene graph. Now, if such a cycle is detected, /parent and
  /mask commands will return an error.
- related to #56
@totalgee
Copy link
Collaborator Author

totalgee commented Jun 6, 2024

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
Copy link
Collaborator Author

totalgee commented Jun 9, 2024

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.

@totalgee totalgee closed this as completed Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants