Improve behaviour of clip_children by clipping to parent alpha value but still retaining parent color

[clayjohn]

Fixes: #49198

This PR turns clip_children into an enum that allows for 3 distinct behaviours

CLIP DISABLED

Child draws over parent and is not clipped

CLIP ONLY

Parent is used for the purposes of clipping only. Child is clipped to the parent's visible area, parent is not drawn.

CLIP AND DRAW

Parent is used for clipping child, but parent is also drawn underneath child as normal before clipping child to its visible area.

I have two points for discussion:

1. Are we happy with these two modes? In the comments to this and the linked issue I have seen demand for both
2. Are we okay breaking compatibility? Right now compatibility isn't totally broken, if clip_children was enabled in the inspector it turns into CLIP_ONLY which is fine, but scripts accessing the setter and getter will break. I can easily adapt this to not break compatibility either by reverting the second commit or by leaving the clip_children bool in place and having a seperate enum that only appears when clip_children is true

Original Description (applies to commit 1)

This brings the behaviour of clip_children more in line with that of Control.clip_contents except the alpha value of the parent node is respected.

Opening as a draft so we can discuss if this is the behaviour we actually want out of clip_children.

An important difference between this and the current master is that in the current master if the parent object is plain white (like a default Label) then the parent disappears in places where the child does not overlap.

Example from blog post

For colored parents they are both visible in master and after applying this PR.

Before:

After:

[markdibarry]

Mentioned on the linked issue, but I like "Clip based on parent alpha only" and "Clip children but keep parent". I can see use-cases for both, but if I had to only pick one, I'd go with (Edit)"Clip children but keep parent". That seems the intended way, most intuitive, and probably most used.

[insomniacUNDERSCORElemon]

I'd go with "Clip based on parent alpha only". That seems the intended way, most intuitive, and probably most used.

A node you've created is now invisible and the created shape/data is lost unless covered by other nodes, why?

I see 2 scenarios where it makes sense, either you're looking to create transparent fadeoff somewhere OR you're trying to make a container of some kind (be it a potion, gummybears... or something like an aquarium or ant-farm where you want more transparency). I expect users will most commonly not want those exact 2 things and will instead need to use a work-around.

OTOH, the method shown in this draft (that I support) is the choice for adding details to characters or to create general art (face, belts/chain, stains/stickers/bandages, shadows/highlights, dirt or some other texture etc on-top-of your original node) that has endless use, including dynamic ones (like an eye, effects like an elevator into the ground, or something screen-like).

Repeat from elsewhere, but relevant:

Testing materials with beta2 and noticed something: the material happens after clipping.... modifying the nodes rendered before-but-not-after it. Thus you can intuitively choose (via render order) what nodes have their alpha changed. For instance, line with 0 alpha and multiply blend:

Via this method, it seems Clip based on parent alpha only makes even less sense. Though I'll admit blend modes are still clunky (the simple addition of subtract_alpha would go a long way IMO). I am not sure how much blend modes are altered by beta2's clip rendering though.

Also: a mask blend mode, though I'm not sure how you'd resolve something like pure white that has 50% alpha. Maybe more blend stuff (like darken-only), especially if it didn't need canvasgroup/clipping to function properly.

[markdibarry]

@insomniacUNDERSCORElemon Now that you mention it, I was thinking more based on what the original proposal was advertising, but I think you're right and most of the use cases would be closer to "Clip children but keep parent".

I think adding separate blend modes to recreate the old Light2D features would also be a nice feature, but that's probably outside the scope of this draft, but would be very beneficial as a proposal if one wasn't already made.

[insomniacUNDERSCORElemon]

I just tested this and it works as expected except for one thing:

the multiply blending mentioned in my previous comment doesn't work (nor can I find a blending configuration that deletes alpha). Purely for investigation if it can be had, the trick still works in a canvasgroup. Related to canvas group mode?

Though I suspect blend modes just need to be rethought/redone so they are actually useful.

Sidenote, it's still an issue if you enable clipping on a node that does not have a (visible) child. In this version it will render as black, although unfortunately this can also make other clip-related nodes render as black (it seems to do this to the node that rendered before it/above it in scene tree).

EDIT: Also now noticing (note: unrelated to above sidenote) that a node with clipping turned on will turn black when it's in the edge of the editor's viewport (first saw this when zooming in closely). Not sure if it happens while running, though. This seems to be dependent on position (very similar to how the issue looks in beta2 and before, likely because it's the same underlying issue). Or maybe it's actually is caused by the clipped child being off-screen and thus not being there to be rendered (though zooming in really close with parent and child both still visible can still cause it).

[clayjohn]

I just tested this and it works as expected except for one thing:

the multiply blending mentioned in my previous comment doesn't work (nor can I find a blending configuration that deletes alpha). Purely for investigation if it can be had, the trick still works in a canvasgroup. Related to canvas group mode?

CanvasGroup has a few differences from clip_children. Notably, CanvasGroup renders to a transparent backbuffer before copying to the front buffer. When rendering to a transparent buffer alpha blending can behave a little differently. So it's expected that tricks you can pull with the multiply blend mode may work in CanvasGroup but may not work with clip_children. At any rate, that is way beyond the scope of this PR

Though I suspect blend modes just need to be rethought/redone so they are actually useful.

Sidenote, it's still an issue if you enable clipping on a node that does not have a (visible) child. In this version it will render as black, although unfortunately this can also make other clip-related nodes render as black (it seems to do this to the node that rendered before it/above it in scene tree).

I think I fixed that issue in #67051

[insomniacUNDERSCORElemon]

So it's expected that tricks you can pull with the multiply blend mode may work in CanvasGroup but may not work with clip_children. At any rate, that is way beyond the scope of this PR

I brought it up because it is relevant to the implementation and because it's possible with clip_children in beta2 and before (see screenshot I posted above, though I do expect it was a side-effect of the problem). If you cannot re-add the capability then I definitely think there should be an option for Clip based on parent alpha only as otherwise I am not aware of how you would get that capability unless the user can write a shader for it.

That aside... the ability to also work subtractively (even if it were restricted to full transparency) is really powerful and fast on its own (and there aren't good work-arounds).

[DrCanvas]

I'm not sure how this feature was meant to be used. But the current state is useless for me:

I personally would use it for animated revealing masks. Something like this:

Unfortunately, this is not possible right now. For that we need to change behavior from "clip children" to "clip parent".

[Zireael07]

@DrCanvas I think there's a proposal already open for what you want?

[insomniacUNDERSCORElemon]

The top one is beta2 or sooner, not the changed version.

Unfortunately, this is not possible right now. For that we need to change behavior from "clip children" to "clip parent".

I think there's a proposal already open for what you want?

This is what I was exactly talking about in my 3 comments above, it was possible with blending modes before (canvasgroup--that still works--in image, but it worked with clipping too):

Hopefully it is viable to have clipping do this while also working for art. More flexibility would be nice too, but I'm not sure how much is possible with the current method.

I'm not sure how this feature was meant to be used.

Similar to clip_contents of control nodes, but via pixels instead of bounding box.

With this pull, icon is parent (yes, the green bit can be animated):

[markdibarry]

Since we can only pick one for the moment, I think the best solution would be to fix the bugs associated with this, and merge the feature as proposed in this draft. Then open up a new PR to expand this as a "clip_mode" dropdown that we can continuously add to, to accommodate other modes. The benefit, as I see it, is we'd have a better chance of getting new features in smaller releases.

[insomniacUNDERSCORElemon]

I've done some percussive coding and have a partial solution that fixes some of the issues I've talked about.

Code (in renderer_canvas_render_rd.cpp):

// Default clip children shader.
shader_type canvas_item;
void fragment() {
	vec4 item = textureLod(SCREEN_TEXTURE, SCREEN_UV, 0.0);
	vec4 original = texture(TEXTURE, UV);
	
	if (item.a == 0.0) { // transparent pixels of child can erase parent via blend modes
		COLOR.a = 0.0;
	} else if (original.a != 0.0 && COLOR.a == 0.0) { // child restores parent opacity only from 0
		COLOR.a = item.a * original.a; // brings opacity back down too
		COLOR.rgb = item.rgb;
	} else {COLOR.rgb = item.rgb;} // only change rgb if item.a is nonzero 

}

Examples:

Description: So if it isn't clear, this is Clip children but keep parent except:

• if the parent has 0.0 alpha value (via selfmodulate or polygon color), it will act as an invisible container (like Clip based on parent alpha only) if multiply mode is enabled
• children with blend modes work like they did in older versions, multiply mode allows deleting pixels if the child has transparent areas/colors (similar to point above)

Problems:

• labels don't really work with this, font rendering and/or SDF screw it up with blending modes (though it's not entirely a problem as it can have a typewriter aesthetic) EDIT: I believe this is caused by how blend modes work (and how they treat overlapping pixels, similar to why Canvasgroup is needed except this is about a singular node output)
• I have not accounted for opacity of (textureless) vertex colors (in the event that someone wants them w/invisible parent), not sure how to get the original (non-selfmodulated) vertex color
• blend modes still

Maybe someone else can be improve it even more? Especially as I don't entirely understand how I did this.

[akien-mga]

You have separation lines here but not in RendererRD, maybe should be harmonized.

[reduz]

Given there is no 100% agreement on how this should work, would clipping being an enum (as in, disable, behavior1, behavior2) make more sense?

If so, what would be the desired possible behaviors?

[clayjohn]

Given there is no 100% agreement on how this should work, would clipping being an enum (as in, disable, behavior1, behavior2) make more sense?

If so, what would be the desired possible behaviors?

Sounds like there are two different behaviours that users would find useful:

1. Clip children based on parent alpha (do not draw parent)

2. Clip children based on parent alpha (still draw parent)

[insomniacUNDERSCORElemon]

@clayjohn have you looked at my code above? Here is a better demonstration of it:

I think it's an intuitive setup, though I could see if some shortcuts were added for usability (for instance if the shader/clipping could internally override a node to use multiply mode if it were needed, such as in this case with 0.0 selfmodulate). Also, does need some documentation (maybe even right in the clip_children tooltip).

The biggest problem is with multiply mode and textures. Users like @golddotasksquestions do not like the workflow of white textures and working directly w/alpha. Sort of like @reduz was saying: if you could add a clip variable alpha_type (normal, subtract, add, and mask*) I imagine that'd make it easy (enough for me if I were there already) to add to my version of the shader. And that would improve the workflow.

*=brightness directly sets alpha (white being full opacity), 0.0 alpha leaves alpha alone

[clayjohn]

@insomniacUNDERSCORElemon I did look at your code above. It sounds like a pretty neat suggestion. I suggest you make a proposal outlining exactly what you would like to see implemented including what options you would like exposed to the user. As it stands now, substantially changing the behaviour of clip_children is well beyond the scope of this PR and the discussion of your new features will get lost once this PR is finalized and merged.

[insomniacUNDERSCORElemon]

@clayjohn at the very minimum you could use the shader I've written, as AFAICT it's just better than not having it. If you agree, I encourage you to add it to this PR.

The changes I've discussed I feel are a natural extension of clip_children, it's the same idea except you're choosing how to use pixels instead of just merging them. Especially as I've said before, even with my version of the shader now it works shockingly smoothly when it comes to render order. I am not sure what else there is to be said, especially if workflow can be made intuitive.

Again, if I am given the setup for the combobox of options (even if it's just a and b if easy to change) with an example of how to reference it in the shader, I will try to make the improvements. (and I could make my own PR if you've already merged this one)

[Zireael07]

@insomniacUNDERSCORElemon This PR is already large enough without adding a new shader. The bigger a PR, the longer it takes to be merged. Improvements should be made in follow-up PRs generally speaking

[golddotasksquestions]

@DrCanvas
The behavior you desire is discussed (as either a boolean property similar to clip_children or as node) in godotengine/godot-proposals#4282 I personally don't have a preference over this being a property or a node, I just want it to be simple and intuitive to use even for beginner Godot users.

@insomniacUNDERSCORElemon
Since you say what I like or dislike I feel like I have to make this correction:

Users like @golddotasksquestions do not like the workflow of white textures and working directly w/alpha

That's definitely not the case. What I and many others (example) find so confusing and counter intuitive is, how transparency is used for what does the masking. Like in your proposed example above. It's like cutting a whole with empty space. Like shielding off sunlight with the open space of a window, instead of the wall. It's like masking your face with the eye slits in your face-mask, instead with the papier mâché.

It's just the total opposite of all natural real life occurrences of masking.

[insomniacUNDERSCORElemon]

Like in your proposed example above. It's like cutting a whole with empty space. Like shielding off sunlight with the open space of a window,

What is wrong with that? You're creating the hole exactly as you want it to appear. It multiplies the alpha and leaves the RGB alone. EDIT: Though light2D calling it mask is an issue, because the name is misleading

I was assuming that you didn't like working directly with alpha as it's harder to work with (like 25% alpha is harder to see than 25% gray, so you need to add a dark layer behind it). Disliking the workflow makes more sense than disliking how multiplication works.

That's totally irrelevant though as I brought that up in context of me wanting to add other methods (similar to what I've already done, if someone can get me started) to make it more intuitive, including the masking that you want (also subtract/add will allow using normal sprites of any color rather than needing specifically-made-mask).

[golddotasksquestions]

Are we happy with these two modes?

Imho this now seem to do what it was originally advertised for, and more. I'd be totally happy to finally being able to use this in the upcoming beta builds. Thanks clayjohn!

Are we okay breaking compatibility?

Functionality wise, noting is broken. It still works with White-to-transparent textures. Yes, maybe scripts need to be adjusted due to it being an enum, but that's a small price to pay for finally have this feature fully working. Due to it's state, I doubt it was used by many people for any production purpose so far.

[insomniacUNDERSCORElemon]

The new commit does not fix the issue: clip_only has ghosting in un-covered areas, as you are not properly handling the transparency. And changing the parent to multiply mode no longer works (and multiply-blend children render transparent as black still)

I am also not convinced 2 shaders are needed here, as aside from the initial condition (parent visibility) the workflow should be exactly the same. My shader could be used for both modes (because it does, the clip_only setup just needs to be done manually), assuming clipping is properly configured to allow transparency.

[clayjohn]

The new commit does not fix the issue: clip_only has ghosting in un-covered areas, as you are not properly handling the transparency. And changing the parent to multiply mode no longer works (and multiply-blend children render transparent as black still)

I'm not sure what you mean by uncovered areas, could you provide an example that highlights the issues you are describing?

I am also not convinced 2 shaders are needed here, as aside from the initial condition (parent visibility) the workflow should be exactly the same. My shader could be used for both modes (because it does, the clip_only setup just needs to be done manually), assuming clipping is properly configured to allow transparency.

2 shaders are needed to avoid forcing the multiply blend mode that the current implementation uses. The shader you have proposed requires users to be aware of the difference between texture alpha, modulate alpha, and color alpha and to utilize each of those in order to get the desired effect. In my opinion, that is way too complex, users should be able to get the appropriate clipping behaviour out of the box. At any rate, if you want more advanced behaviour, you can override the default shader by supplying your own shader. So you can really achieve anything you want with this implementation.

[insomniacUNDERSCORElemon]

I'm not sure what you mean by uncovered areas, could you provide an example that highlights the issues you are describing?

Compare to my previous screenshot, this is after adjusting to your commit:

Ghosting is on the top-right example (text is not updated, I did remove the material from this example... though with it it doesn't render properly either).

2 shaders are needed to avoid forcing the multiply blend mode that the current implementation uses.

Multiply is needed in both cases to handle transparency, and in either case does not cause a blending issue when it's doing so (because in that exact case RGB is not modified). Without multiply, clip_only will immediately look wrong as shown above.

EDIT: And if you're referring to the multiplication in the canvasgroup shader itself, that isn't working. I mean the parent is not rendering, but the blank area is backbuffer ghosting.

clip_and_draw is actually the one that shouldn't have multiply on the parent (not that it even does in your PR!), transparency is handled by the child node that uses multiply.

The shader you have proposed requires users to be aware of <complexities>, users should be able to get the appropriate clipping behavior out of the box

Exactly as I implemented it, yes. But I am suggesting that the above situation is handled automatically outside of the shader (thanks to the enum) so the user does not need to do that.

---

I forgot to say I don't think breaking compatibility is an issue considering this feature has never been in a stable release.

Also: @clayjohn I would like to make a slightly different setup than yours and have the enum working (image below), is there an easy way that I can <reference it in the shader> or <use it in the code instead of CANVAS_GROUP_MODE>?

(note I am not sure about the last 2, and especially last 1, but it's an idea I had)

I did get self-modulation for a clip_only effect (in canvas_item.cpp):

void CanvasItem::set_clip_effect(ClipEffect p_clip_effect) {

	if (clip_effect == p_clip_effect) {
		return;
	}
	clip_effect = p_clip_effect;
	if (clip_effect == CLIP_EFFECT_CLIP_ONLY) {
		set_self_modulate(Color( 0, 0, 0, 0 ));
	}	else {set_self_modulate(Color( 1, 1, 1, 1 ));}
}

(setting blending the same way is not that easy)

[clayjohn]

@insomniacUNDERSCORElemon Again, I am not supportive of making this feature so complex that users have to carefully manage transparency and blend modes in order to get the basic functionality. This should be something that users can turn on have it just work.

Something I should have noted above, when you assign a custom material it replaces the default_canvas_group_material and you become responsible for blending the backbuffer yourself in the shader. It appears that in your images you have replaced the default shader with your own, so you aren't even testing the shader in this PR. If you post the text of your shader I can offer tips on how to handle blending to help you achieve what you are trying to do. But again, that is advanced usage of this feature, what we are concerned with right now is whether the basic behaviour is correct and will work for the typical use case.

Also: @clayjohn I would like to make a slightly different setup than yours and have the enum working (image below), is there an easy way that I can or ?

It needs to be a property of the CanvasItem as the renderer needs to change the order it renders objects in so that this works (children have to be rendered before their parent).

As for the additional modes you propose in your image, again, please make a proposal with a detailed explanation of what additional modes you want, how you expect them to work, and why you need them for your project. Considering new modes is way beyond the scope of this PR.

[insomniacUNDERSCORElemon]

I am not supportive of making this feature so complex that users have to carefully manage transparency and blend modes in order to get the basic functionality. This should be something that users can turn on have it just work.

I am not sure why you keep saying this when I am trying to make it clear that it should be handled by the engine.

If I could figure out how to fix the backbuffer ghosting issue automatically the basic functionality of what my shader does would already be there. And yes, good enough for basic usage.

It appears that in your images you have replaced the default shader with your own, so you aren't even testing the shader in this PR.

The newer image was your 2nd commit of this PR. Ghosting can be easy to miss if you don't have anything that will show up.

Redownloaded, recompiled, new project that has just 2 nodes with clip only (plus a background to make it obvious):

MRP: ghost_mrp.zip

[clayjohn]

Redownloaded, recompiled, new project that has just 2 nodes with clip only (plus a background to make it obvious):

Thanks for the MRP! I was able to reproduce your issue when running on the RenderingDevice renderer, but not in the OpenGL3 renderer. I tracked down the issue to an existing bug with our CopyEffects class and I have pushed a fix to this PR!

[insomniacUNDERSCORElemon]

I tracked down the issue to an existing bug with our CopyEffects class and I have pushed a fix to this PR!

Thank you, this fixed my issues with this almost entirely!

Multiply is still broken, but can be fixed by in the clip shader, add after setting rgb: COLOR.a *= c.a;

But that makes me ask again, is there actually a need for an opaque and transparent version of the shader? Is multiplying just the alpha not enough to work in both cases? Pasting the shader to be the same as multiplied alpha clip shader and I am not seeing a difference.

---

Sidenote: I am still getting the pop-outs (or shaded black w/o fix above) w/clipping when zooming in the editor. Or has that not been pushed yet?

[mhilbrunner]

PR meeting: Approved, feature is nice, the 2 modes make sense and are useful, and breaking compat. a bit at this point is better than the uglier alternative.

Still needs a final review + approval by reduz.

[clayjohn]

Two approvals and reviewed in the PR review meeting, so I will go ahead and merge this.