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

CanvasItem light_mask is ignored by LightOccluder2D occluder_light_mask #64939

Closed
h0lley opened this issue Aug 26, 2022 · 19 comments · Fixed by #98835
Closed

CanvasItem light_mask is ignored by LightOccluder2D occluder_light_mask #64939

h0lley opened this issue Aug 26, 2022 · 19 comments · Fixed by #98835

Comments

@h0lley
Copy link

h0lley commented Aug 26, 2022

Godot version

v4.0.alpha14.official [106b680]

System information

Ubuntu, Nvidia

Issue description

By setting CanvasItem light_mask to a different layer than LightOccluder2D light_mask, it was possible to have shadows not being drawn on top of sprites. This does no longer work.

Godot 3:

image

Identical setup in Godot 4:

image

Especially in topdown games you generally need this to work.

Steps to reproduce

  1. Set your PointLight2D's range_item_cull_mask to cover layer 1 and 2
  2. Set your Sprite2D's light_mask to 2 only
  3. Add LightOccluder2D that remains on default for occluder_light_mask and light_mask (layer 1)
  4. have the LightOccluder2D polygon cover (part of) the sprite
  5. hit F6 and note that the shadow is rendered on top of the sprite. the shadow should not be rendered on top of the sprite as is the case in Godot 3.

Minimal reproduction project

projects for both godot 3 and 4 to compare:

light-occluder-sprite-bug.zip

@PoisonousGame
Copy link

In godot3, add a CanvasModulate node, and then a PointLight2d node. Set the CanvasModulate color to black and the light will illuminate things in range, this doesn't work in godot4. The documentation is also not updated and the 2D lights don't seem to be maintained!

@wattsUpJaz
Copy link

In godot3, add a CanvasModulate node, and then a PointLight2d node. Set the CanvasModulate color to black and the light will illuminate things in range, this doesn't work in godot4. The documentation is also not updated and the 2D lights don't seem to be maintained!

Do you have a background set in your Godot 4 scene? I'm able to use the CanvasModulate node fine.

As for documentation, I think that comprehensive Godot 4 docs are still a ways off since the version is still in the early beta phase.

@wattsUpJaz
Copy link

For anyone interested, I'm working around this by :

  1. Following the steps above for the bug.
  2. Adding an extra light with Shadows disabled and the Item Cull Mask only set on 2 (Layer 2 is the layer of the sprite's light mask).
  3. Setting the range_item_cull_mask for the original light to only include 1 (Layer 1 is the layer of the occluder's light masks).

This is what it looks like:
Godot test scene

Scene tree:
Godot scene tree

@hmturnbull
Copy link

Has there been any more attention paid to this issue since 4.0 released? The way light occluders now always draw atop the related sprite has rendered occluders practically unusable in almost every common use case I can think of.

If a tree can't cast a shadow without that shadow obscuring the tree itself, then there's no way to make shadows at all, as far as I can tell.

@eth0net
Copy link

eth0net commented Jun 27, 2023

I'm so glad I found h0lley's video on this! Been fighting with the 2D lighting system for a top-down game and could not get it to work the way I expected... Thanks for the workaround wattsUpJazz!

@Persipaani

This comment was marked as off-topic.

@Calinou
Copy link
Member

Calinou commented Jul 10, 2023

@Persipaani Please don't bump issues without contributing significant new information. Use the 👍 reaction button on the first post instead.

@parthipan51
Copy link

You have to set LightOccluder node's cull mode to counterclockwise/clockwise (depending on where the light should enter) to light faces right under the LightOccluder node. Note: Disabling cull mode turns the node into a closed polygon which is the reason why no light enters into the lightoccluder node's area

@bitbrain
Copy link
Contributor

You have to set LightOccluder node's cull mode to counterclockwise/clockwise

@parthipan51 do you mean this is something that can be done in the Godot UI directly (this ticket is desired behavior and not a bug)? Or is this something that needs to be done in C++ inside Godot source (this ticket is still a bug)?

@parthipan51
Copy link

You have to set LightOccluder node's cull mode to counterclockwise/clockwise

@parthipan51 do you mean this is something that can be done in the Godot UI directly (this ticket is desired behavior and not a bug)? Or is this something that needs to be done in C++ inside Godot source (this ticket is still a bug)?

Yes, it is a desired behavior

theOne

theVideoOne.webm

@h0lley
Copy link
Author

h0lley commented Sep 20, 2023

@parthipan51 cool, although I am not sure that's a solution to this issue.

show_behind_parent doesn't do anything for me (and besides, it may not always be feasible to place the LightOccluder2D as a child node to the sprite(s) it should render underneath to.)

and cull_mode is not sufficient as soon your sprite's outline and light occluder's polygon are not identical (as is probably the case in most actual applications):

image

in case you want to test with that sprite yourself:
godot_4_shadow_occulder_issue.zip

@guilleatm
Copy link
Contributor

Here, the renderer is taking into account what I think is the Light2D light_mask and the LightOccluder2D light_mask but I don't see where the Sprite2D / CanvasItem light_mask is.

if (!co || co->index_array.is_null() || !(p_light_mask & instance->light_mask)) {

I am not sure if this is where the bug is even, just guessing.

As you can see, if the light_mask does not match, it just skips the rendering stuff.

LightOccluderInstance *instance = p_occluders;
while (instance) {
OccluderPolygon *co = occluder_polygon_owner.get_or_null(instance->occluder);
if (!co || co->index_array.is_null() || !(p_light_mask & instance->light_mask)) {
instance = instance->next;
continue;
}

This is the method where the code is.

void RendererCanvasRenderRD::light_update_shadow(RID p_rid, int p_shadow_index, const Transform2D &p_light_xform, int p_light_mask, float p_near, float p_far, LightOccluderInstance *p_occluders) {

@clayjohn
Copy link
Member

clayjohn commented Oct 24, 2024

We discussed this last night in our weekly rendering meeting.

I think the OP is a bit misleading as it mentions the light_mask and occluder_light_mask, but the problem doesn't really have to do with the occluder. The occluder is already properly culled if its mask doesn't match that of the light (as pointed out by Guilleatm: #64939 (comment)). The issue is with the Light2D's shadow_item_cull_mask and the CanvasItem's light_mask. The documentation is also wrong in this regard.

The problem is that in Godot 3.x, CanvasItems additionally took the occluder_light_mask into account. This worked very simply in 3.x because we did a separate draw call for each light for each draw command. We could then easily filter out the shadow at the same time we filtered out the light.

In 4.x, we render all lights in the same draw call. Which means if we want to bring back that behaviour, we need to mask out the shadow in the shader

@dmhagar
Copy link

dmhagar commented Nov 1, 2024

Is there any timeline for fixing this, or is there a simple modification that could be made to Godot's source code so the fix could be built in user-side? This is pretty much game-halting when trying to design an isometric game that needs light occlusion.

@clayjohn
Copy link
Member

clayjohn commented Nov 1, 2024

While waiting for a full fix to this issue, you could merge #93881 locally. It provides a new feature that can be used to work around this problem.

@dmhagar
Copy link

dmhagar commented Nov 1, 2024

Thanks... unfortunately that doesn't help me since I'm building in 2D isometric with y-sorting and I cannot change the z-index of individual tiles without breaking draw order.

Annoyingly, I also can't use the duplicate light trick outlined above by @wattsUpJaz because I programmatically use all of the existing light mask layers already, as my game generates regional maps with up to 20 potential height layers and those are all used to determine where point lights can cast, like so:

func setup_region_tilemaps():
	# Create a TileMap for each additional height layer (1-20) containing only the relevant layer originally from TileMap 0.
	tilemap_layers[0] = self
	var layer_bitmask
	for layer in range(1,max_layer + 1):
		var layer_tilemap = TileMap.new()
		layer_tilemap.tile_set = tile_set.duplicate()
		layer_tilemap.name = str(layer)
		add_child(layer_tilemap)
		var light_layers = []
		for light_layer in range(layer + 1,layer + 4):
			if not light_layer > max_layer + 1:
				light_layers.append(light_layer)
		Global.set_light_mask_for_layers(layer_tilemap,light_layers)

It seems like I'm kind of boned when it comes to using occluders until a fix is provided.

@clayjohn
Copy link
Member

clayjohn commented Nov 4, 2024

Alright, I've figured out a way to bring back the shadow_item_cull_mask I implemented my idea for the RD renderer already and it works! Commit here: c9c3541

It required a bit of a refactor. I just need to carry the same changes over to the Compatibility renderer and then I'll PR it!

Screenshot for Proof ;)
image

@clayjohn clayjohn moved this from In progress / Assigned to Fix pending review in Rendering Issue Triage Nov 5, 2024
@github-project-automation github-project-automation bot moved this from Fix pending review to Done in Rendering Issue Triage Nov 22, 2024
@SlavaSTiwari
Copy link

Will this be added to Godot in the near future or is the fix just modify the files myself?

@h0lley
Copy link
Author

h0lley commented Nov 25, 2024

Will this be added to Godot in the near future or is the fix just modify the files myself?

a fix by clayjohn has been merged into master and will be in the next official build, presumably 4.4.dev6. if you need it earlier you can compile yourself.

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