Add transparency support for LightmapGI #90109

guerro323 · 2024-04-01T16:55:58Z

Scene from the original issue

Current `master`	This PR `size=0`	This PR `size=0.1`

Test Scenes

The culling logic was updated in the Lightmapper to correctly support Back and Disabled cull modes.
To preserve the current behavior and to not have any breaking change, the first direct light rays will retain the old behavior where light is blocked even if it should be culled.
This behavior is still needed otherwise there would be holes in the shadows of penetrated meshes (which can be quite common) and would need the material to be updated to Disabled cull mode, which may not be intuitive in most cases.

TL;DR: There should be no breaking changes on existing scenes. This do need some more testing but all my scenes looked correct.

note: I've split the colored shadows into another PR (#90132)

doc/classes/ProjectSettings.xml

EzraT · 2024-04-04T13:09:23Z

Thanks!

Calinou

Tested locally, I can't get transparent shadows to show up for a static object regardless of its transparency mode. Shadows are always fully opaque. This is with Alpha, but the same happens with Alpha Scissor:

The DirectionalLight3D in the scene has the Static bake mode. I'm using the Compatibility rendering method in the test project (it bakes via a local RenderingDevice that uses Vulkan), but this also happens with Forward+. I've also tried disabling Use Texture for Bounces in LightmapGI (then baking lightmaps again) to no avail.

Testing project: test_transparent_shadows.zip

guerro323 · 2024-04-04T14:19:28Z

@Calinou I'm not able to repro it, even from the artifact builds.

No changes Forward+	Increased the alpha range from the noise texture¹	Alpha scissor¹

¹ If the shadows don't match it's because the bottom of the cube has a different view of the noise texture.

I noticed that alpha scissor didn't correctly work on the current commit (it only discarded alpha under the threshold but kept the same alpha if not, now it's binary) so it's fixed.
Also compatibility seems to not work (it just totally discard the alpha so it's completely transparent), it is using a custom RD specialized for it? I thought it would have used either the Forward+ or Mobile one.

Calinou · 2024-04-04T14:21:03Z

Also compatibility seems to not work (it just totally discard the alpha so it's completely transparent), it is using a custom RD specialized for it? I thought it would have used either the Forward+ or Mobile one.

Baking lightmaps while using Compatibility was implemented in #87386.

guerro323 · 2024-04-04T14:50:31Z

Baking on compatibility works now

No changes Compatibility	Increased the alpha range from the noise texture¹	Alpha scissor¹

¹ If the shadows don't match it's because the bottom of the cube has a different view of the noise texture.

patwork · 2024-04-10T14:12:18Z

The culling logic was updated in the Lightmapper to correctly support Back and Disabled cull modes.

Will it fix #89402 ?

guerro323 · 2024-04-16T13:55:36Z

Sorry for the delay in my response.

The culling logic was updated in the Lightmapper to correctly support Back and Disabled cull modes.

Will it fix #89402 ?

No, it only apply when a ray hit a surface.
For the receiver, aka the texel shooting the rays:

Front culling is possible¹, as we just flip the normal in the texture.
Disabled is not possible, as we can't determinate what is the direction for shooting rays (it's not 'back' neither 'front' but both at the same time) in this case it's better to have 2 planes with Back culling for both direction.

¹ This should be done for another PR as it's out of scope for this one (and can be a breaking change in some scenario)

lawnjelly · 2024-04-17T04:09:42Z

I had limited time, but after the rendering meeting yesterday I had a look through and this looked fairly sensible from what I could see. It is also possible to do translucent colors in a similar way, but maybe for a later PR.

I think @clayjohn mentioned wanting to double check some of the changes didn't affect some of the other GI paths.

jcostello · 2024-04-17T13:24:34Z

I had limited time, but after the rendering meeting yesterday I had a look through and this looked fairly sensible from what I could see. It is also possible to do translucent colors in a similar way, but maybe for a later PR.

I think @clayjohn mentioned wanting to double check some of the changes didn't affect some of the other GI paths.

There is a PR for translucent colors

clayjohn

I don't know why, but when baking with this PR in the Forward+ backend the resulting lightmap is completely black. When baking the MRP, with 4.3 dev 5 I get the same result that Calinou did, but with this PR I get a totally black lightmap.

Have you done something to the MRP to make it work with this PR?

modules/lightmapper_rd/lightmapper_rd.cpp

guerro323 · 2024-04-18T00:06:52Z

Have you done something to the MRP to make it work with this PR?

No, except for the second screenshot where I needed to tweak the point lights size.

Calinou · 2024-04-18T01:13:30Z

but with this PR I get a totally black lightmap.

This might be due to #90627.

clayjohn · 2024-04-18T01:20:27Z

but with this PR I get a totally black lightmap.

This might be due to #90627.

Just tested, you are right. Fixing #90627 makes this PR work perfectly in the Forward+ renderer!

clayjohn · 2024-04-22T21:40:35Z

servers/rendering/renderer_rd/storage_rd/particles_storage.h

@@ -360,6 +360,7 @@ class ParticlesStorage : public RendererParticlesStorage {
 		virtual void set_code(const String &p_Code);
 		virtual bool is_animated() const;
 		virtual bool casts_shadows() const;
+		virtual RendererRD::MaterialStorage::ShaderData::CullMode get_cull_mode() const;


We shouldn't expose this, instead we should read the cull mode from the shader directly.

Mickeon

Documentation is fine. It's gonna be really cool to implement this in due time.

Calinou · 2024-08-15T15:02:20Z

@guerro323 Are you available to rebase this PR to fix merge conflicts? If not, no worries – let me know and I can label this as salvageable 🙂

guerro323 · 2024-08-15T15:20:09Z

@guerro323 Are you available to rebase this PR to fix merge conflicts? If not, no worries – let me know and I can label this as salvageable 🙂

Yes, I've just done an oopsie when trying to use the git's stash function to see if the missing code for compatibility was here (spoiler: it wasn't) and broke my branch.

I'll get it done for this week along with some perf improvements I found while making my own raytracer.

spookto · 2024-10-14T12:40:39Z

Any update on this?

I needed this feature since my game relies on baked lighting for visuals, so I rebased against 4.3.x (4.3.1rc1 as of writing), and the only conflict was renaming alpha_scissor to alpha_scissor_threshold which was easily resolved.

Rebasing against 4.4 proved to be more confusing, as #95828 also improved the lightmapper's shader and logic. I am still looking into it and wouldn't mind helping.

guerro323 · 2024-10-14T13:08:28Z

I got busy with life and personal projects so I'll not be able to work on it for an undefined amount of time.
If anyone also want to continue working on it go ahead.

jcostello · 2024-10-14T15:33:06Z

@guerro323 understandable. If its not much to ask, can you rebase the branch?

Co-authored-by: Guerro323 <kaltobattle@gmail.com>

clayjohn · 2024-12-17T20:53:04Z

Closing as superseded by #99538. We are aiming to merge #99538 soon to include it in the 4.4 release.

Thank you everyone here for your help and a special thank you to guerro323 for getting this PR so far along!

guerro323 requested review from a team as code owners April 1, 2024 16:55

AThousandShips added bug topic:rendering topic:3d labels Apr 1, 2024

AThousandShips added this to the 4.3 milestone Apr 1, 2024

guerro323 force-pushed the lightmap-gi-transparent-surface branch from c509bef to e6faa54 Compare April 1, 2024 17:55

AThousandShips reviewed Apr 1, 2024

View reviewed changes

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved

guerro323 force-pushed the lightmap-gi-transparent-surface branch 2 times, most recently from 1132cab to 715275c Compare April 1, 2024 19:46

guerro323 marked this pull request as draft April 1, 2024 21:16

guerro323 force-pushed the lightmap-gi-transparent-surface branch 2 times, most recently from 88f5148 to eccdae5 Compare April 2, 2024 09:44

guerro323 marked this pull request as ready for review April 2, 2024 10:05

guerro323 force-pushed the lightmap-gi-transparent-surface branch from eccdae5 to 0f3bfa5 Compare April 2, 2024 12:39

guerro323 mentioned this pull request Apr 2, 2024

Add colored transparent shadows to LightmapGI #90132

Closed

Calinou reviewed Apr 4, 2024

View reviewed changes

guerro323 force-pushed the lightmap-gi-transparent-surface branch from 0f3bfa5 to 3c906c0 Compare April 4, 2024 14:19

guerro323 force-pushed the lightmap-gi-transparent-surface branch from 3c906c0 to 37db35d Compare April 4, 2024 14:50

clayjohn reviewed Apr 17, 2024

View reviewed changes

modules/lightmapper_rd/lightmapper_rd.cpp Outdated Show resolved Hide resolved

clayjohn reviewed Apr 22, 2024

View reviewed changes

clayjohn modified the milestones: 4.3, 4.4 May 2, 2024

Mickeon approved these changes Jul 5, 2024

View reviewed changes

Calinou mentioned this pull request Jul 11, 2024

[TRACKER] LightmapGI (GPU lightmapper) issues #56033

Open

sievaxx mentioned this pull request Sep 3, 2024

Meshes without UV2 cannot block light / contribute to LightmapGI #96538

Open

jcostello mentioned this pull request Sep 5, 2024

Support baking Light3D Projector textures for lights with Static bake mode in LightmapGI godotengine/godot-proposals#10664

Open

atirut-w mentioned this pull request Oct 1, 2024

LightmapGI does not take transparency into account when baking shadows. #77590

Closed

clayjohn added the salvageable label Oct 17, 2024

Geometror force-pushed the lightmap-gi-transparent-surface branch from 37db35d to cb7de2e Compare November 22, 2024 12:56

Geometror mentioned this pull request Nov 22, 2024

Add transparency support for LightmapGI #99538

Merged

Geometror force-pushed the lightmap-gi-transparent-surface branch 2 times, most recently from a270bdb to 45e6c7e Compare December 16, 2024 11:40

Add transparency support for LightmapGI

a5a4005

Co-authored-by: Guerro323 <kaltobattle@gmail.com>

Geometror force-pushed the lightmap-gi-transparent-surface branch from 45e6c7e to a5a4005 Compare December 16, 2024 12:05

akien-mga requested a review from clayjohn December 17, 2024 20:33

clayjohn removed this from the 4.4 milestone Dec 17, 2024

clayjohn closed this Dec 17, 2024

AThousandShips added the archived label Dec 17, 2024

akien-mga removed the salvageable label Dec 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add transparency support for LightmapGI #90109

Add transparency support for LightmapGI #90109

guerro323 commented Apr 1, 2024 •

edited

Loading

EzraT commented Apr 4, 2024

Calinou left a comment •

edited

Loading

guerro323 commented Apr 4, 2024 •

edited

Loading

Calinou commented Apr 4, 2024

guerro323 commented Apr 4, 2024

patwork commented Apr 10, 2024

guerro323 commented Apr 16, 2024 •

edited

Loading

lawnjelly commented Apr 17, 2024 •

edited

Loading

jcostello commented Apr 17, 2024

clayjohn left a comment

guerro323 commented Apr 18, 2024 •

edited

Loading

Calinou commented Apr 18, 2024

clayjohn commented Apr 18, 2024

clayjohn Apr 22, 2024

Mickeon left a comment

Calinou commented Aug 15, 2024 •

edited

Loading

guerro323 commented Aug 15, 2024 •

edited

Loading

spookto commented Oct 14, 2024

guerro323 commented Oct 14, 2024 •

edited

Loading

jcostello commented Oct 14, 2024

clayjohn commented Dec 17, 2024

Add transparency support for LightmapGI #90109

Add transparency support for LightmapGI #90109

Conversation

guerro323 commented Apr 1, 2024 • edited Loading

Test Scenes

EzraT commented Apr 4, 2024

Calinou left a comment • edited Loading

Choose a reason for hiding this comment

guerro323 commented Apr 4, 2024 • edited Loading

Calinou commented Apr 4, 2024

guerro323 commented Apr 4, 2024

patwork commented Apr 10, 2024

guerro323 commented Apr 16, 2024 • edited Loading

lawnjelly commented Apr 17, 2024 • edited Loading

jcostello commented Apr 17, 2024

clayjohn left a comment

Choose a reason for hiding this comment

guerro323 commented Apr 18, 2024 • edited Loading

Calinou commented Apr 18, 2024

clayjohn commented Apr 18, 2024

clayjohn Apr 22, 2024

Choose a reason for hiding this comment

Mickeon left a comment

Choose a reason for hiding this comment

Calinou commented Aug 15, 2024 • edited Loading

guerro323 commented Aug 15, 2024 • edited Loading

spookto commented Oct 14, 2024

guerro323 commented Oct 14, 2024 • edited Loading

jcostello commented Oct 14, 2024

clayjohn commented Dec 17, 2024

guerro323 commented Apr 1, 2024 •

edited

Loading

Calinou left a comment •

edited

Loading

guerro323 commented Apr 4, 2024 •

edited

Loading

guerro323 commented Apr 16, 2024 •

edited

Loading

lawnjelly commented Apr 17, 2024 •

edited

Loading

guerro323 commented Apr 18, 2024 •

edited

Loading

Calinou commented Aug 15, 2024 •

edited

Loading

guerro323 commented Aug 15, 2024 •

edited

Loading

guerro323 commented Oct 14, 2024 •

edited

Loading