godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Add a "Mask2D" node (and/or a "mask_parent" property for Sprite2D) #4282

Open golddotasksquestions opened 2 years ago

golddotasksquestions commented 2 years ago

Describe the project you are working on

This applies to all 2D projects I have worked on in the last 3,5 years, so ever since I started working with Godot.

Describe the problem or limitation you are having in your project

Ever since I started using Godot, 2D masking is one of these fundamental basic areas which has given me the most trouble.

This is something most people who hear the praise for Godots great 2D support would think already exists as an extremely simple and straight forward solution. But very unfortunately, the opposite is the case: The existing solutions are extremely cumbersome and counter-intuitive to use, require a high skill level and often very performance hungry as well.

They are in fact so complicated and hard to explain to a beginner it I actively try to avoid doing so. Even explaining this process is no fun.

Basically you have two options: use Light2D with Mask mode or write a Shader. Neither of them offer the same intuitive simple use as a Sprite with texture. Light2D has it' own crazy collision-layer type system over which Light2D affects which other canvas item. This might make sense for a 2D light, but for a Sprite Mask this is extremely counter productive. Light2D nodes are also really performance hungry, especially on lower end mobile hardware.

Shaders on the other hand are more performance friendly, but are a huge step up in the difficulty curve, especially for those who are not already familiar with a C-like language. Trying to position a shader texture in world coordinates is a nightmare! For programming beginners this makes the shader option a complete nogo. Even after learning 2D shaders more than for 2 years now, I'm still not at the point where I can position a 2D mask texture in a shader as intuitively and freely as I can position Sprite anywhere on Canvas.

Edit: Light2D mask modes does not seem to exit in Godot4 any more. Apparently CanvasGroup is supposed to fill this void, but since it act as a parent of two nodes falls short for this usecase in principle, as well as for many of the same reasons as Light2D does (which I listed in detail in here)

My point is: We really need a proper simple and intuitive solution for this which makes adding a 2D mask and positioning it anywhere as simple, easy and intuitive as adding and positioning a Sprite.

With Godot 4 the "clip_children" property was introduced. This allows 2D parent nodes to mask their children, similarly like it was already possible in Godot 3 using the rect_clip_content property of Control type nodes or the VisualServer canvas_item_set_clip(), but this time on a fragment (per pixel) basis.

This is a useful addition! Unfortunately it has been broken ever since it was first implemented, and still is in the current Godot 4.0 Alpha5 version. https://github.com/godotengine/godot/issues/49198#issuecomment-1050408311

More unfortunately, however, is how even if it would be working, enabling clip_children does not solve the need for simple straight forward intuitive masking and positioning it in an equally simple and straight forward way.

We need a Node2D type of node just like the Sprite with a texture slot, but when it's parent is a 2D canvas item, it will mask this parent with every not transparent pixel (which color this pixel is is irrelevant, only the alpha channel is relevant).

It would look like this: mask2d

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Usage would be intuitive, straightforward and dead-simple:

  1. Add a Mask2D node as child to a Sprite or any other 2D node,
  2. give it a texture and position it where ever you please,
  3. done!

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

mask2d_2 mask2d_3

Edit2: Multiple Mask2Ds: mask2d_4

https://user-images.githubusercontent.com/47016402/160288107-45d29071-6336-4807-99da-c488af87e3ce.mp4

Edit3: difference between the existing clip_children and Mask2D / mask_parent property of this proposal:

mask2d_03

If this enhancement will not be used often, can it be worked around with a few lines of script?

No, right now the opposite is the case. Doing this is very cumbersome and definitely not as straight forward as it should be.

Is there a reason why this should be core and not an add-on in the asset library?

It is desperately needed basic 2D functionality which needs to be equally basic and easy to use.

Calinou commented 2 years ago

For programming beginners this makes the shader option a complete nogo. Even after learning 2D shaders more than for 2 years now, I'm still not at the point where I can position a 2D mask texture in a shader as intuitively and freely as I can position Sprite anywhere on Canvas.

This can surely be made more convenient by an add-on which provides its own editor gizmos for placing masks.

golddotasksquestions commented 2 years ago

@Calinou have you read the proposal? This is feature already exists in Godot, it's just terrible to use. Addons are always additional steps, and their discoverbility is by design worse.

Adding gizmo or tool or anything I have to learn extra is the opposite of what I am proposing here. From a user perspective, with a Mask2D I would need to learn or download nothing extra. If I know how to use a Sprite I know how to use a Mask2D node. They work exactly the same way, only one "adds a texture" and the other "removes from a texture".

This is how masks work in any other software or visual editor.

Trying to solve this by adding more steps (as addon) completely defeats the purpose of this proposal of adding and positioning a mask in an quick and intuitively discoverable way.

If adding a separate node for this is undesired, adding this functionality as property to Sprite2D nodes would be a fix. As I mocked up here: https://user-images.githubusercontent.com/47016402/160154896-65e58e89-2eb9-470a-a0b4-e40868b39fe6.png

But an having this basic 2D functionality as addon makes no sense whatsoever.

fire commented 2 years ago

As far as I know this is implemented by CanvasGroups.

https://godotengine.org/article/godots-2d-engine-gets-several-improvements-upcoming-40

golddotasksquestions commented 2 years ago

@fire I know about CanvasGroup. How is it equal to the Mask2D I am proposing here?

golddotasksquestions commented 2 years ago

I repeat it here: The point of this proposal is to make 2D masking as easy as adding and positioning a Sprite node. If not via a distinct node, give Sprite a mask_parent property. If that parent happens to be a CanvasItem, the parent gets masked.

TheDuriel commented 2 years ago

This is already implemented in Light2D.

golddotasksquestions commented 2 years ago

@TheDuriel

Exactly.

If you read it again you will notice I have stated so. This feature exists, it's just not easy to use or very practical either as shader or the Light2D implementation. Light2D are also really bad on mobile hardware performance. Multiple Light2Ds in mask mode don't work together. You have to use an inverted texture which is super counter intuitive (transparent pixels in the Light2D do the masking) Shaders seem to be much less performant hungy, but such a shader is so much more complicated to implement, I don't manage to write such a shader despite learning Godot shaders now for more than two years. Masking with the shader is not so much the issue here as positioning.

TheDuriel commented 2 years ago

An example for a masking shader is, iirc, given in the docs somewhere. If not, well its only two actual lines of code. All you need is a texture uniform and then use any of its channels as the alpha. Its basic enough in any case that it should likely be noted as a tutorial if it is not already.

The light2D implementation does require fiddling with mask properties, granted, but a developer in need of this feature could extend the class easily enough to provide some exported properties to make it easier.

insomniacUNDERSCORElemon commented 2 years ago

I'm biased, but I like the idea of this being integrated into the clip workflow (working with any canvas items). I believe I've suggested (somewhere) a clip mode option before that would allow some items to be marked as unclippable by parent (to allow for more robust characters to be made with 1 logical tree) as well as clips parent. Unfortunately I don't know if that'd be a viable setup with how the tech works, assuming it can be even fixed to work to its full existing potential that it started with.

Also with clipping you could have a parent polygon and simply alter the shape and have that state saved via an animation player (swapping to it when needed) but I definitely agree that creating a shape to extract (and then just turning visibility on when needed!) is significantly easier and better. Especially when/if you need to edit/swap it out later (even just say, moving up a bit), if you need multiple, as the complexity increases etc etc.

As for the lights thing, I've only tinkered with lights a bit but it seems lights can render incorrectly sometimes when it comes to different modes overlapping. Seems like something that might be an issue if someone has lighting in their game (but I haven't tried it specifically).

TheDuriel commented 2 years ago

How is this going to work without a shader anyways?

Clipping the mesh would be incredibly inefficent and hard to work with.

golddotasksquestions commented 2 years ago

@TheDuriel

You seem to have glanced at the headline of the issue and jump to write your comments without ever reading it properly.

An example for a masking shader is, iirc, given in the docs somewhere. If not, well its only two actual lines of code.

First of, no, such a shader is nowhere. Not in the docs, not anywhere else in the Godot space. I can write a basic masking shader like this since 2 years:

void fragment() {
    vec4 mask_tex = texture(mask, UV );
    vec4 orig_tex = texture(TEXTURE, UV);
    float alpha = clamp(orig_tex.a - mask_tex.a, 0.0, 1.0);
    COLOR.a = alpha;
    COLOR.rgb = orig_tex.rgb;
}

But this is again not what this issue is about. You can't freely drag this mask texture of such a shader around. You can't see, scale it, manipulate it like a Sprite without prior writing all the shader code necessary for these operations. You can't simply add a child and hit a check boy and it will mask the parent with such a simple shader.

I'm sure there is a way to get the texture and the global position of a child and then load the texture into the sampler2D uniform of the parent shader and then somehow (I don't know how), transform the global position of the child to local UV space, ... but ... this is the opposite of intuitive, easy, straight forward or discoverable. It's an hardcore expert user solution for something you don't want to do that often.

We need a masking solution which is as simple and easy to use as a Sprite.

Also you seem to have a strange idea on how Light2D masking works. I suggest you test it.

golddotasksquestions commented 2 years ago

@insomniacUNDERSCORElemon Light2D solution is still the most practical for very basic masks, but has a lot of issues.

it seems lights can render incorrectly sometimes when it comes to different modes overlapping. Seems like something that might be an issue if someone has lighting in their game (but I haven't tried it specifically).

Yeah this is an issue. For example you can't use 2 Light2D in mask mode, masking the same CanvasItem.

TheDuriel commented 2 years ago
shader_type canvas_item;

uniform sampler2D alpha_texture;

void fragment() {
    COLOR.rgb = texture(TEXTURE, UV).rgb; # Or COLOR.rgb = COLOR.rgb, for control nodes.
    COLOR.a = texture(alpha_texture, UV).r;
}

Please do not correct me with incorrect code.

If you want to add an offset, you can add another single uniform.

uniform vec2 offset;

And add this offset to the UV value. You can even multiply it by the pixel resolution if you want it in pixels.

golddotasksquestions commented 2 years ago

You can even multiply it by the pixel resolution if you want it in pixels.

Would you mind adding this in your sample code? I never managed to get global positional values to work when I offset my UVs.

TheDuriel commented 2 years ago

I'm sure there is a way to get the texture and the global position of a child and then load the texture into the sampler2D uniform of the parent shader and then somehow (I don't know how)

Iirc? Proposals are supposed to know how. Suggestions do not. Don't quote me on that.

But I can't see a way to do that. More so, how does that interact with shaders? Do I lose the ability to shade my sprite because your mask sets its own shader? There is no multi pass shading in 2D, and that would be slow.

TheDuriel commented 2 years ago
shader_type canvas_item;

uniform sampler2D alpha_texture;
uniform vec2 offset;

void fragment() {
        # Optional, multiply offset by SCREEN_PIXEL_SIZE.
    COLOR.rgb = texture(TEXTURE, UV + offset).rgb; # Or COLOR.rgb = COLOR.rgb, for control nodes.
    COLOR.a = texture(alpha_texture, UV + offset).r;
}
golddotasksquestions commented 2 years ago

I've tried this many times. Multiplying the offset by SCREEN_PIXEL_SIZE alone won't work. No idea what other things you have to do (please share if you know) to have the mask show up at get_global_mouse_position() for instance.

I gave up on this many times. I will try again many times until I do figure it out. But that's my whole point here. This is not an intuitive user friendly or easy to use solution anyone can do. Masking is a basic task any beginner will want to do.

It should be straight forward as adding a Sprite. Not as cumbersome or difficult or complicated as it is now.

TheDuriel commented 2 years ago

to have the mask show up at get_global_mouse_position() for instance.

You are now talking about specific use cases, rather than generics. At that point I can't think but point to "If this enhancement will not be used often, can it be worked around with a few lines of script?"

It can be, absolutely. Doing space conversions is a fundamental skill you need to master to make games.

Global mouse position > Local sprite position > Feed that to the shader. Convert it to a UV offset. Which is a single multiplication with one of the (I do not recall which.) uniforms provided to you by Godot already.

golddotasksquestions commented 2 years ago

get_global_mouse_position() is of course just a placeholder example for ANY global position I would want the mask to set to.

Telling me something is "a fundamental skill you need to master" is really great. I've only been trying to master this fundamental skill for 3+ years. Like I said I know I have to somehow transform the global coordinates in to the local UV space, but I don't know how. I would still appreciate if you would teach me a lesson on something you consider fundamental or basic. If you don't want to that's fine. But something as top level as "Global mouse position > Local sprite position > Feed that to the shader" apparently does not help me. If you have a code sample that works, again I would really appreciate to try it. I have tried again today for a few hours but can't get it to work (have the mask at the global mouse position for example)

The fact that I've been trying to do this for this long despite learning heaps and bounds about all kinds of fundamental programming skills, alone says a lot about how there needs to be a better solution that works intuitively and easily for everyone imho.

markdibarry commented 2 years ago

To the point of this proposal, I have been working with Godot for years now as well, and this has been a feature that would be greatly appreciated. As someone who spent a good solid two months last year trying to recreate a feature similar to this in Godot, working with masking in 2D is poorly documented, buggy, unintuitive, and difficult. This is a common problem, and while "git gud" is A solution, I'd love for an easy to use built in solution.

insomniacUNDERSCORElemon commented 2 years ago

To further my previous comment... if it were viable, replace the checkbox with options like this:

Clip mode:

Clip receive:

In the improper-fix version (ghosting removed) node2d does have its children clipped by its parent so would be useful for grouping nodes based on their clip options, meaning an unclippable-by-children option isn't needed. You could add that option but you'd probably also want an unclippable (by all) option.

All of this would make in-engine art/characters more viable/powerful/dynamic, but subtract mode on its own would be a powerful tool on-par with the concept of clipping itself.


EDIT: @fire

As far as I know this is implemented by CanvasGroups.

I just tested canvasgroup with a subtract blend-mode material... it works with a transparent polygon. All colors delete all overlapped pixels but the only one that doesn't show up outside of the existing pixels (an intersection) is transparent. Might work if it used masking colors here but it doesn't, unless I'm missing something (underbaked or bug)? Seems like it's completely useless for sprites (unless you want a quadrilateral).

Possibly useful for polygons, but since canvasgroups don't seem to work with clipping (due to similar methods) that makes it useless in most cases there too.

Well, I should say that if you want to use a polygon to subtract from a sprite that will work.

golddotasksquestions commented 2 years ago

Since I've heared now a couple people suggest Light2D or CanvasGroups instead: Light2D does not seem to have a mask mode in Godot4 any more. Like I already explained in the original post, it's also super weird to use in Godot3 already due to the render layer system and the inverted textures and the inability to use multiple Light2D masks together. Also who would look for a "light" node if they would want to "mask" a 2D element in their game?

CanvasGroups have a a lot of the same usability issues are Light2D nodes have in Godot3 for masking:

It's not easily discoverable. "CanvasGroup" says nothing about masking

It's an expert solution: You already need a high understanding of Godots systems to even get the idea that something like this might work and how it might work. Someone needs to tell you the solution, and since this is not very intuitive, it will be hard to remember if you don't do it ever day. This is what the CanvasGroup solution for masking looks like in the Scene Tree: image You then have to set up a CanvasItemMaterial for the mask and either use "substract" or "multiply" mode as contrary to the default mix mode. This is hard to discover even when you know what to do. Impossible to figure it out intuitively!

This again has the same issues as the current Light2D implementation:

The mask works in reverse logic, even worse than the Light2D: The Sprite you assign the "Substract" CanvasItem Material to, is not the one that does the substracting. Which all by itself is confusing as hell. Example: G4_A5_canvasitem_substract01 (here the white circle has the substact material, but the Godot icon without material aka the default mix is doing the substraction)

It's the same for a black-and-white mask btw as you can see here: G4_A5_canvasitem_substract02

If set the CanvasItem Material of the mask to "multiply", the correct node finally does the masking, but just like the crazy current Light2D implementation you will have to use an image with transparency and it's the transparency that does the stancing, not the part of the texture without alpha. Which again is super counter-intuitive. This is what this looks like: G4_A5_canvasitem_multiply01

If you try to use a black-and-white texture, it of course won't work at all: G4_A5_canvasitem_multiply02

Even if these issues would be fixed and or documented, it will never be able to compete with the intuitiveness of simply adding and positioning a Mask2D child node, or enabling a mask_parent property of a Sprite.

SilencedPerson commented 2 years ago

@markdibarry

To the point of this proposal, I have been working with Godot for years now as well, and this has been a feature that would be greatly appreciated. As someone who spent a good solid two months last year trying to recreate a feature similar to this in Godot, working with masking in 2D is poorly documented, buggy, unintuitive, and difficult. This is a common problem, and while "git gud" is A solution, I'd love for an easy to use built in solution.

I agree with your point there, Godot is often presented as easy to use and beginner-friendly, this image starts with the chummy logo, the node system and ends with GDScript's ease of use, it's quite frankly it's one of it's main selling points over engines like Unity and Unreal. Having something used so frequently in 2D games be only truly solvable by shaders is counter-productive to that image. I have small business teaching teens how to make games and it often comes up as a question, for example, just few days ago, one of my students who is making small horror game, needed to make simple textured fog, we tried the proposed approach of using Light2D and we indeed ran into several problems that the @golddotasksquestions correctly identifies.

It really is not covered by Light2D and it is not covered by CanvasGroup either. The user experience is subpar and requires user to reach for somewhat low level solution like shaders (often very daunting for beginners) when this is something so general and used in many different kinds of games. Before Godot, i used to teach Game Maker and while Godot made the overall process and learning curve easier some things are still not exactly frictionless and i see this need for Mask2D node as a symptom of different issue that are Godot's limited custom drawing capabilities.

This proposal can easily coexist with CanvasGroup and actually be very useful in tandem.

dugramen commented 2 years ago

I think a mask property is a bit too cluttered. A Mask2D node sounds good, but the name should be more specific as it sounds like all your masking purposes would be found there, whereas it's only for cutting the parent. Maybe ClipMask2D or something

And in case this doesn't get added, it's currently possible to do this with Canvas Group in a more reliable way than Light2D and more friendly way than a shader. A canvas group with 2 children, the pear and the bite mask. The bite mask has CanvasItemMaterial set to multiply, and the bite texture should be transparent where you want the mask to cut (inverted in the OP's case). (subtract_mode is broken, but it would make this better by not having to invert it)

SilencedPerson commented 2 years ago

I think a mask property is a bit too cluttered. A Mask2D node sounds good, but the name should be more specific as it sounds like all your masking purposes would be found there, whereas it's only for cutting the parent. Maybe ClipMask2D or something

And in case this doesn't get added, it's currently possible to do this with Canvas Group in a more reliable way than Light2D and more friendly way than a shader. A canvas group with 2 children, the pear and the bite mask. The bite mask has CanvasItemMaterial set to multiply, and the bite texture should be transparent where you want the mask to cut (inverted in the OP's case). (subtract_mode is broken, but it would make this better by not having to invert it)

What you describe as possible solution was several times highlighted as a problem in this thread. CanvasGroup is not viable.

atngames commented 2 years ago

Hi, I am really interested to follow this proposal. ClipMask2D is a better name for it, unless it can perform all sorts of masking operations (which would be awesome). Atn

Lielay9 commented 2 years ago

I'd simply like to remind you that in most cases while moving the mask might be more intuitive as a child, the effect is often desired the other way down the scene tree. Look at the following picture for example:

image

Let's say the previous represents my player scene. If I'd want to mask it during gameplay I'd have to go inside the player scene and manually add a mask under each node it has. In this case, the proposal most certainly is not THE fix, as it requires a very specific tree structure. It may even be detrimental for beginners who are led to believe that this is THE solution and who waste time littering their scene with mask nodes. I still think that a mask node might work, but to do that, it must work down the tree and not change the position of its children. As of now, this would only be feasible with CanvasGroups. The least intrusive way would be to document how to do it, and the second-least intrusive would be to introduce the Mask2D as a node specifically to be used with CanvasGroups.

No matter what, I'm more interested in pursuing a solution that works most of the time, preferably all of the time, rather than a case-specific one, which I deem this one to be. That said, with minor changes to the proposal I could see it working and perhaps even being helpful for beginners. However, I am not certain if it would be used often enough to be warranted as a core feature and if it truly is too difficult to implement with the help of proper documentation or as a plugin.

SilencedPerson commented 2 years ago

@Lielay9 in this case, you have to combine it with CanvasGroup that would encapsulate everything and Mask2D as it's child.

golddotasksquestions commented 2 years ago

@Lielay9 I feel like you might have misunderstood this proposal. This is not a proposal to replace the CanvasGroup. The CanvasGroup is a fantastic addition (when it will finally work without bugs that is). Before the CanvasGroup, we had to use VieportContainer+Viewports for the behaviour they fullfil, which is a really bad workflow for a lot of reasons. CanvasGroups are going to be a huge help and simplification in terms of helping usability and making it much easier and more intuitive to "join" multiple textures of separate 2D children nodes into one. They are brilliant!

However what CanvasGroups are not good for is the usecase I was trying to illustrate here with the "bite".

The robot you posted is actually a very good example why such a Mask2D/Clipmask2D or mask_parent property or whatever you want to call is so vital in having: This robot has separate 2D textures for each body part and all body parts are then animated using the cutout animation technique with skeleton/bones.

Now if you were to hit that robot with some heavy ammunition so a chunk from the head, arm or leg texture is missing, the easiest and simplest and imho most intuitive solution would be to simply add another texture node as a child of that particular bodypart node which was hit and position it exactly at the global_position of the impact.

Like in the pear example in the original post above, the robot will now miss a piece of it's armour, but the skeleton animation will still work the same and the chunk the has been "bitten off" animates with the body part that was hit (because the mask is a child of that node)

Everything falls into place.

Try to do this dynamically with CanvasGroups is a nightmare even if you ignore all the things I listed above in my previous comments which make them so counter intuitive and user unfriendly. You would have to dynamically replace the body part of the Robot which was hit with a CanvasGroup, make the original texture a child of that CanvasGroup and then add the mask as separate child, add material to the mask, and set the mode.

If you have not realized how all this node juggling and re-parenting is pointlessly complicated and simply bad from a user and design point of view now, I really don't know what else there is to say anymore.

insomniacUNDERSCORElemon commented 2 years ago

@Lielay9 @SilencedPerson canvasgroups with multiply blend mode (a transparent pixel is 0 and a white pixel is 1) to cut previously rendered pixels already works if canvasgroup works for your workflow (no clipping, no rotation/shearing of the canvasgroup). You can do multiple cuts too, but it depends on draw order (cuts only existing stuff, though if you do -1 z index for a lower node it won't be cut)... I don't know how practical that'll be for complicated designs as I assume multiply goes down the whole tree down to 0.

That works on multiple nodes in the canvasgroup, so the needed thing here is something that work on specific nodes (as OP said with dynamic objects) like integrating it as a clip option as I've stated. Though things here would be significantly more viable if clipping and canvasgroups worked together.

Also if your tree had a sprite or polygon as the root, if you enable clipping on it (even if all items are within it and thus not clipped) if clips parent were added (as a direct child to the tree, first if ordering mattered) then that would mean that it'd clip all clippable children as a result (and this would be significantly easier to create+edit and could be dynamic esp. if a polygon).

Though specific to something dynamic, I'm not sure how often you'd need to mask globally. Especially not when you can use code to hide/delete the nodes themselves if needed. I guess maybe if you want to cut it in half vertically (head, jaw, and torso in this case). Though again you can do that with a canvasgroup and multiply node now (for the shown setup, though don't let arms/legs move because they would re-appear if not under that node)

SilencedPerson commented 2 years ago

@insomniacUNDERSCORElemon please refer to this post. https://github.com/godotengine/godot-proposals/issues/4282#issuecomment-1079676227 This is starting to be frustrating, did anyone even read the proposal/thread before commenting? Please stop raising, it CanvasGroup as a solution, it does exact opposite what the proposals is suggesting and it was suggested multiple times already: https://github.com/godotengine/godot-proposals/issues/4282#issuecomment-1079222085 https://github.com/godotengine/godot-proposals/issues/4282#issuecomment-1079702062 https://github.com/godotengine/godot-proposals/issues/4282#issuecomment-1079676227

Current implementation of CanvasGroup is actually part of the problem. If you still think that CanvasGroup in current state is viable, please, show us a video, I cannot test this feature myself as CanvasGroup is competely broken for me but what i gathered from @golddotasksquestions explanation and what I seen so far, it is not.

@golddotasksquestions It may be necessary to include in the proposal image with big red letters explaining that no, Light2D and CanvasGroup setups are not solutions.

insomniacUNDERSCORElemon commented 2 years ago

@SilencedPerson Here is an image displaying what I was saying in my last comment: wrmpear

All I was saying is that it (multiply) works for some cases. I have repeatedly said things implying that it is not the solution. It's not intuitive and would be clunky using canvasgroups if you frequently use this for art, and currently canvasgroups will render oddly if rotated or sheared (see godot#43543 and godot#55853). And if I weren't enough of a broken record, canvasgroups and clipping don't work together currently so obviously this isn't a solution that works for me (though clipping currently isn't usable right now, and notice I'm the author for that ticket that OP mentioned in the issue itself).

Also see my comment just before the first one you linked. I think it should be done via clipping as I don't think this is something that needs a new node, and it's very similar to clipping (it's reverse clipping) and would often be used together. And clipping applies to all canvasitems, if there's a dedicated node it'd likely just support textures (so no polygon support).

EDIT: Another quick reason why canvasgroup isn't a good solution, if you duplicate the pear example you have to duplicate the entire canvasgroup or make sure they don't get near each other, as otherwise the bite will show up on both pears. Multiply seems to only work on 0 z-index so I was a bit too optimistic with the layering you get, seems to be just the 1 layer of cuts and something above or below that. Well, unless you stack multiple canvasgroups (clunkier).

Lielay9 commented 2 years ago

@golddotasksquestions No, I think I've understood it fairly well. It specifically proposes to add a "Mask2D" node (and/or a "mask_parent" property for Sprite2D), because it claims masking isn't intuitive to do as is. It also, as the title and the contents of the proposal claim, is proposed to work "upwards" the tree, which I disagree it shouldn't. In my opinion, if a Mask2D node were to be implemented, it should work down the tree and mask every child it has instead. In this way, the proposed node would work the same as most others do and it would be easier for beginners to achieve exactly the effect they want. If they want to mask a single node and match its transform, they would add the Mask2D as a child, and e.g. tick "show_behind_parent" true. If they want to mask multiple, they add all the nodes they want to mask as the child of the Mask2D node.

@SilencedPerson I do not claim CanvasGroups to be THE solution. That said, it is A solution to masking specific nodes, even if it isn't a good one. Naturally, in a proposal where you propose a new method to do something, the new method must be compared to methods already available. To add to that, in the scenario I've presented, where I've clearly desired not to mask just a single node, unlike @golddotasksquestions, you suggest using CanvasGroups. That means that this proposal, unmodified, seems to rely on the use of them, in at least my case, most of the time.

I reiterate it once more. The addition of a Mask2D node, in my opinion, could work and be helpful for beginners. Also, in my opinion, if it is added, it should work for multiple nodes out of the box i.e down the tree. However, as @insomniacUNDERSCORElemon proposes, making masking/clipping an innate function of CanvasItems might be in many scenarios better, and hence I hereafter am more in favor of that approach.

golddotasksquestions commented 2 years ago

@Lielay9

No, I think I've understood it fairly well. It specifically proposes to add a "Mask2D" node (and/or a "mask_parent" property for Sprite2D), because it claims masking isn't intuitive to do as is. It also, as the title and the contents of the proposal claim, is proposed to work "upwards" the tree, which I disagree it shouldn't. In my opinion, if a Mask2D node were to be implemented, it should work down the tree and mask every child it has instead. In this way, the proposed node would work the same as most others do and it would be easier for beginners to achieve exactly the effect they want. If they want to mask a single node and match its transform, they would add the Mask2D as a child, and e.g. tick "show_behind_parent" true. If they want to mask multiple, they add all the nodes they want to mask as the child of the Mask2D node.

I'm sorry to repeat, but again, reading this makes it seem as you clearly have not understood the purpose of this proposal. I really don't know how else to say it: CanvasGroups already exist, they are incredibly useful for exactly what you are describing. They are, however, not at all useful for what I am describing in this proposal.

I really don't know what else to add to this other than asking you to read the robot example of my last comment again. Adding a parent to a node you want to mask, having then to reparent all the nodes under the common parent and repositioning them is completely counter productive for userbility. You have to do all this in reverse order too once you want to remove the mask!

What I want here, and apparently a lot of others agree, is something as simple as adding a Node+texture or checking a checkbox of a canvas item and be done with it. The mask has to move with the node it has been attached to, even when you reposition. For that to work it has to be a child of the node it affects and the masking has to act on the parent.

This is a usability proposal. In my opinion usability needs to be prioritized over "beautiful source code". Who cares if you have the most beautiful source code if people flock to alternative software because the usability of this software with beautiful source code just sucks? Masking in Godot sucks. clip_children property and CanvasGroups improves this situation in very particular areas, but not in the incredibly common usecase I have been trying to outline here.

Lielay9 commented 2 years ago

@Lielay9

No, I think I've understood it fairly well. It specifically proposes to add a "Mask2D" node (and/or a "mask_parent" property for Sprite2D), because it claims masking isn't intuitive to do as is. It also, as the title and the contents of the proposal claim, is proposed to work "upwards" the tree, which I disagree it shouldn't. In my opinion, if a Mask2D node were to be implemented, it should work down the tree and mask every child it has instead. In this way, the proposed node would work the same as most others do and it would be easier for beginners to achieve exactly the effect they want. If they want to mask a single node and match its transform, they would add the Mask2D as a child, and e.g. tick "show_behind_parent" true. If they want to mask multiple, they add all the nodes they want to mask as the child of the Mask2D node.

I'm sorry to repeat, but again, reading this makes it seem as you clearly have not understood the purpose of this proposal. I really don't know how else to say it: CanvasGroups already exist, they are incredibly useful for exactly what you are describing. They are, however, not at all useful for what I am describing in this proposal.

I really don't know what else to add to this other than asking you to read the robot example of my last comment again. Adding a parent to a node you want to mask, having then to reparent all the nodes under the common parent and repositioning them is completely counter productive for userbility. You have to do all this in reverse order too once you want to remove the mask!

What I want here, and apparently a lot of others agree, is something as simple as adding a Node+texture or checking a checkbox of a canvas item and be done with it. The mask has to move with the node it has been attached to, even when you reposition. For that to work it has to be a child of the node it affects and the masking has to act on the parent.

This is a usability proposal. In my opinion usability needs to be prioritized over "beautiful source code". Who cares if you have the most beautiful source code if people flock to alternative software because the usability of this software with beautiful source code just sucks? Masking in Godot sucks. clip_children property and CanvasGroups improves this situation in very particular areas, but not in the incredibly common usecase I have been trying to outline here.

Please read the second last sentence of the paragraph you quoted again. That is exacly what you want and that is also what I want. However, I also want to be able mask multiple nodes, which cannot be done with the proposal, unmodified.

SilencedPerson commented 2 years ago

@Lielay9 And that's when i pointed out that it can be done when you use the Mask2D with CanvasGroup. This proposal works within the Godot's ecosystem, it is not a solution that does everything.

Lielay9 commented 2 years ago

@Lielay9 And that's when i pointed out that it can be done when you use the Mask2D with CanvasGroup. This proposal works within the Godot's ecosystem, it is not a solution that does everything.

Indeed it is not, but it could be. If the node would go "downward" the tree I wouldn't need to use a CanvasGroup. I would only need to have the nodes I want to mask as the masks children. If I would want to mask a single parent node i would only need to tick "show_behind_parent" true, which is essentially exactly the same as what you want. Essentially we both get exactly the functionality we are looking for, with no extra complexity or additional nodes.

SilencedPerson commented 2 years ago

image

image

@golddotasksquestions I think I need clarification. Is the Mask2D node required to be child of the sprite it is masking or can live anywhere inside the tree as long as it is on the correct masking layer? The behavior I am looking for is much like the current Light2D but the mask is done with the opaque pixels instead of the transparent.

The Mask2D node is required since the limited and incorrect masking behavior was removed from the Light2D in Godot 4.0.

golddotasksquestions commented 2 years ago

@SilencedPerson I have added an Edit to the original post on your suggestion, clarifying that Light2D and CanvasGroup does not not what is necessisary here and linking to the comment explaining the details.

I think I need clarification. Is the Mask2D node required to be child of the sprite it is masking or can live anywhere inside the tree as long as it is on the correct masking layer? The behavior I am looking for is much like the current Light2D but the mask is done with the opaque pixels instead of the transparent.

The way I would like this Mask2D node or mask_parent to behave is: you need to add it specifically as child to the parent you want to mask. You definitely can't add it anywhere in the scene tree and then assign a node it should mask like the Light2D with it's ItemCullMask and render layers.

The purpose is exactly to simplify the masking process by a lot: The Mask2D has to be a child of a canvasitem (so any 2D node), and you have to give it a texture. But that's all.

@Lielay9 Maybe I have misunderstood you: Are you proposing to make this Mask2D inherit from CanvasGroup? Basically just giving CanvasGroup the additional mask_parent property? If so that's totally fine by me. My interest is only so simplify the process of masking from a user perspective and make the process obvious to new users by making it a distinct node with a destinct name. I don't care all to much if this Mask2D would be an entirely new node inheriting from CanvasItem, or inheriting from CanvasGroup or just from Sprite2D. I'm sure there is someone else more clever than me and with better understanding of the source to say what would be the right inheritance. I really just want a straight forward simple usability of just adding a simple clearly identifiable node or checking a checkbox called mask_parent.

All you should have to do is add the node and add the texture. The mask_parent should be enabled by default. You should not need to add extra child nodes like you currently do with the CanvasGroup (Which again, currently has a totally different purpose that has nothing to do with this proposal)

Lielay9 commented 2 years ago

@golddotasksquestions The default behaviour I propose, i.e. child nodes get masked. Good when you want to mask many nodes at a time. (Selected nodes get masked.)

image Enabling "show_behind_parent" (either a new property/ the one already implemented by CanvasItem)

image

makes the behaviour follow what this proposal originally proposes. Good when you want to mask only one node and retain the transform of the parent (Selected nodes get masked):

image

What I don't want to do is the following i.e use CanvasGroup (Proposed workflow for masking multiple nodes with this proposal) (Selected nodes get masked.): image

I still could do it by enabling "show_behind_parent" in the Mask2D node but I don't want this to be the only way to mask multiple nodes.

We can argue about which should be the default behaviour, but ultimately I want both cases to be supported without the use of CanvasGroups, whether it is through a Mask2D or a CanvasItem-property.

golddotasksquestions commented 2 years ago

I'm really sorry. I really tried to hard to focus and read your comment multiple times. I really don't know what it is, but I don't understand at all the point of what you are saying. Or you don't understand my point, I really don't know anymore. I feel like I've exhausted saying this is about ease of use. Adding children to a mask, for what? What's the usability benefit of that? What's the usability benefit of having yet to enable another property (show_behind_parent) which says nothing about masking?

In my proposal, if you want multiple Mask2Ds, I would expect just to add multiple Mask2Ds, exactly like you would already do with CollisionShape2D node children on physics nodes. Just like CollisionShape2D nodes all Mask2D nodes would mix together and act on the parent as one.

mask2d_4

https://user-images.githubusercontent.com/47016402/160287641-419b6c3d-b1cc-4174-b352-cec86ff38a64.mp4

Lielay9 commented 2 years ago

I'm really sorry. I really tried to hard to focus and read your comment multiple times. I really don't know what it is, but I don't understand at all the point of what you are saying. Or you don't understand my point, I really don't know anymore. I feel like I've exhausted saying this is about ease of use. Adding children to a mask, for what?

No, it is me who should apologize for not being good at explaining. I don't think repeating my proposal is productive as, amusingly, I believe it no longer matters. You opened my eyes to the possibility of using multiple masks which I somehow managed to overlook. In hindsight, multiple masks obviously wouldn't work or would work poorly if the mask was the parent node, in which case CanvasGroups would still be needed. I apologize for being so utterly shortsighted.

That said I still think there is merit in considering whether it is better to provide the masking proposed here as a property of CanvasItem since clipping is. Perhaps a dedicated node would be a better fit at replacing the old Light2D masking workflow i.e mask nodes globally/per layer? Perhaps both could be combined into one node essentially making it the de facto way of masking anything in 2D?

SilencedPerson commented 2 years ago

I would prefer approach where the Mask2D extended Light2D. The functionality is nearly there, just needs to be inverted and one checkbox to affect only the parent would have to be added.

Question, if i wanted to set up bounty for this feature, how would i go about it? This shouldn't be too hard to implement.

atngames commented 2 years ago

I'd say the two big features a light2d offers are occluders and light layers (awesome to mask multiple sprites at once). What if we had a real light2d mask AND the "clip children" transformed to a choice between no_clip / clip_children / clip_parent ?

insomniacUNDERSCORElemon commented 2 years ago

Small note: To me the phrase 'mask' here implies the same sort of feature that clipping is intended to offer. A mask is the areas where it does and doesn't show, while the goal here is just to subtract. Though from what I've seen, some of the context difference is that masking in other software is typically over the entire canvas... and is a bit more compound with shapes/a painted layer that Godot is now.


If it could possibly work, here are some re-thought options:

Clip type: off (default), normal, invisible, inverted (AKA subtract/cut) Clip target: children (default), parent, children&parent, siblings start, sibling end Clippable bit option, allowing multiple choices: by all (default), by parent, by siblings, by children, force clipped by parent. Could use icons (infinity for all, up-arrow for parent, right-arrow for siblings etc).

siblings start would clip all siblings later in the tree (not rendering order!) until the node that has sibling end (last clipped node). If no sibling has siblings end, it ends with the tree itself.

clip type invisible works the same as normal clipping except the original canvasitem is not rendered (similar to inverted or multiply). In concept, its most basic usage is similar to clip content of node2d (except you can use any shape via polygon or sprite). This would allow for more advanced clipping of children, as well as better use of the trick I said (clip parent that clips children). So that's the 3rd way (inverted clip on child/siblings are the other 2) for cutting multiple nodes as @Lielay9 first suggested. A very common usage of this would be using this as bounds, such as for a poster/card art/a TV etc or for a room (roof/wall fade/tear away). EDIT: Also this is a mask (as other programs use the term) in the most literal sense (well, except for colors), especially if the target is set to parent

Some sort of mask bit option (user-defined like light mask or item cull mask) would be better than just having a clippable bit as I've defined it (though they could be part of the same option). This would make sibling clipping more useful (though I don't think it makes sense to group parent/child like that) and thus more complexity.

Remember how I said that canvasgroups and clipping were similar? It turns out that blend modes also work with clipping, so a canvasgroup isn't needed*. I had actually forgotten that this works (I only remembered because of something I made with it) so there is that. Here's an example (with a compiled version, as again clipping is still broken):

ArcoLinux_2022-03-27_19-32-12

Again, one of the ways clipping is currently broken is that clipping can't be recursive (it won't work right if you clip something that also clips) so that will limit some of the tricks you could do (such as a dummy clip layer to enable blending to work everywhere) as well as the current complexity that can be easily achieved with just clipping (in a way that is most logical to do). Though the redundancy here with blend modes might make for potential work-arounds in many cases (other clipping issues being fixed, that is).

*= a parent that clips can use self-modulate, which will flatten the shapes together just like canvasgroup. Which if invisible clipping where added, canvasgroup would actually be redundant (and clipping does not have the transform bug that it has). Though if you already have a node setup that uses clipping (or could), canvasgroup is already unneeded. (not that I'm saying it should be removed)

(I suppose that also makes sense with their similarity, canvasgroup doesn't work right with another canvasgroup just like clipping doesn't work right on things that also clip)


Sidenote: some of the issue here with intuitiveness is just that blend modes don't seem to work in a useful way outside of canvasgroups or clipping (not even working with parent/child nodes), and IIRC it always appeared broken to me in 3.X.

Clipping would also be more useful if there was a better way to get compound shapes (be it of sprites or polygons). Like taking a parent and children and using the final result as a node to use in a different way (be it any sort of clipping, or for a blend mode etc). Though you could also say it'd be better if making complex polygons/meshes with Godot itself were better, though it's still not as great as working with individual components. Maybe some sort of proxy node (load from scene OR from last-in-tree/sibling context), but perhaps that's functionality for another day (add-on or based on usage).


@SilencedPerson I am absolutely on-board with per-node subtraction rather than having only blending (if not clear already). That said, multiply (when it's working right) is absolutely logical (I say that as a not-really-user). Transparent is 0 and white is 1. Other colors allow mixing (seen below) in the same material (think of it like chroma key with white, or a projector screen with a hole cut in it)

ArcoLinux_2022-03-26_17-25-46

It's actually similar to the current clip children, though clipping shouldn't have color blending (it does, and looking at it now it seems that clip children has the same color blending as multiply (except transparency is not transferred) when compiled w/fix).

However Subtract should absolutely use white as delete and black(/transparent) for keep. Or reverse the colors from what I just said if it's instead called mask.

EDIT: As far as usability goes, I could see it being useful to have multiply (or some other blend mode) to have it switchable between transparent/black or even a user-input color key option (for instance, maybe you use a specific color that isn't common in your project). Also how multiply blends colors is ugly, though I don't know what alternative there is especially that'd work with a variable chroma key.

Xrayez commented 2 years ago

The purpose is exactly to simplify the masking process by a lot: The Mask2D has to be a child of a canvasitem (so any 2D node), and you have to give it a texture. But that's all.

So this definitely requires core additions and changes.

If this proposal does not end up being approved (I suppose such functionality won't end up in 3.x either), then the only option I see is creating some kind of self-contained SpriteMask2D/Mask2D that could work together in hierarchical manner just like CSG nodes.

This would definitely require embedding an (uber)shader just like for Light2D I guess.

I have added this proposal to

golddotasksquestions commented 2 years ago

Finally, after more than 2 years of spending countless hours (in total I think I can measure it in days if not weeks) of studying the shader docs, book of shaders, countless shader demos and tutorials, and insistent repeatedly trying, I finally figured out how to solve this with a shader today. Sharing here because I could not find this info anywhere on the internet during all that time:

shader_type canvas_item;

uniform sampler2D mask_sampler;
uniform vec2 mask_offset = vec2(0,0);

void fragment() {
    ivec2 orig_size = textureSize(TEXTURE,0);
    ivec2 mask_size = textureSize(mask_sampler,0);
    vec2 orig_size_to_mask_size_ratio = vec2(orig_size)/vec2(mask_size);
    vec2 mask_offset_pixel_converted = mask_offset * TEXTURE_PIXEL_SIZE * orig_size_to_mask_size_ratio * -1.0;
    vec4 mask_color = texture(mask_sampler, UV * orig_size_to_mask_size_ratio + mask_offset_pixel_converted);
    vec4 orig_color = texture(TEXTURE, UV);
    float alpha = clamp(orig_color.a - mask_color.a, 0.0, 1.0);
    COLOR.a = alpha;
    COLOR.rgb = orig_color.rgb;
}

mask offset is from the top left corner of the original texture

markdibarry commented 2 years ago

Just to chime in here to add to the previously mentioned development that Light2D is now PointLight2D, and does not contain a Mask mode anymore. Another unmentioned issue is that this also makes things much harder for simple classic cases like using masks as artificial light sources. Here is how it was done using light masks in Godot 3.x: G3

The following is what is now required to replicate this: g4 The same result now requires separation of the masks and their sources, needing the overlay AND the light sources to be in the same CanvasGroup. This forces much more scripting (vs none) for parent scenes to manage lighting and add/removal of the "lights" and light sources, additional RemoteTransform2Ds on the sources, and dependency on brittle, carefully crafted scene tree structures.

Along with CanvasGroup not being able to be used more than once per layer, this is a regression in UX and features, making Godot 4.0 a clear downgrade for 2D rendering. It would be a fantastic solution if the proposed Mask2D node could handle clipping children/parent or a masking layer, restoring the feature that was removed from Light2D. An all-in-one functionality to finally separate expensive lighting from masking.

tarragonfly commented 2 years ago

Here's a perspective from a small team scoping out Godot as a potential replacement for our current engine of choice.

Simple to use 2D masking is essential to the games we make. And they have to be designer friendly precisely as described in this proposal. A mask should be as easily defined as a sprite because this is something a level designer will make use of, not a technical artist or a programmer.

The current 2D lighting mask solution is counterintuitive, technically complex and performance intensive, especially for more complex scenes. CanvasGroups in v4 as shown by golddotasksquestions is also counterintuitive, technically limited and far from a user friendly solution. And writing a custom shader for what should be a basic engine feature is not the answer either.

Godot is being touted for its 2D capabilities, but the current masking options are not up to par with the competition. For now, we will leave Godot in the maybe aisle. We'll take another look when v4 comes around to check out the state of masking in 2D.

akien-mga commented 2 years ago

We discussed this briefly in a proposal review meeting and while we see some merit to what is being discussed it in terms of usability, the core issue seems to be that the existing feature for this exact use case has a bug: https://github.com/godotengine/godot/issues/49198

So we want to fix the bug first, and then we can discuss if/how the workflow should be further improved.