Add a reflection mask to the reflection probes

[m4rr5]

This is a rebase of PR #50196. Co-authored-by: Bastiaan Olij [email protected]

The use case for this PR is to render an object with a reflective material, in this case a car, in a scene that should be reflected on its body. For that you place a reflection probe in the center of the car. The problem you now run into is that you do not want the (interior of the) car rendered in this probe. The cull_mask that each reflection probe has, does not allow you to solve this. If you put the car on a separate layer and disable that layer, not only does it no longer show up in the reflected map, but it automatically also no longer uses the same map to render the car.

This PR adds a reflection_mask property to reflection probes that allows you to specify which objects are rendered to the probe. The cull_mask still determines which objects are using this reflection probe.

To demonstrate the issue, the image below shows a car in a simple scene. In the center of the car is a reflection probe and to visualize the issue, directly above the car a reflective sphere was added that also shows what gets rendered in the reflection probe. You can clearly see how it renders the interior of the car, making the reflections on the car body look wrong.

The second image is taken with the same setup, but with layer 2 of the reflection_mask disabled. Layer 1 contains everything around the car. Layer 2 is the car. The reflections now look as they should on the car body. The cull_mask can still be used to determine what objects to target with this probe (not done explicitly in this sample screenshot).

[Calinou]

Tested locally, it works as expected in Forward+ and Mobile.

Testing project: test_pr_86073.zip

[AThousandShips]

Please squash your commits into one, see here

[AThousandShips]

Please update your commit message to include the co aurhor properly, like so (exactly, the lines are important):

Add a reflection mask to the reflection probes.


Co-authored-by: Bastiaan Olij <[email protected]>

[BastiaanOlij]

Looking good @m4rr5 !

[BastiaanOlij]

Just some more background info on this fix and the discussion that may need to be had here.

My original PR became stale because at the time we had no clear use case to test the enhancement against, it was based on a simple experiment as I was converting the material demo to Godot 4 at the time. Seeing Godot 4 was years away from release and the rendering team had their hands full, this wasn't exactly and issue that deserved to be at the top of the priority list, and thus became largely forgotten :P

So currently, without this PR, reflection probes have two masks:

• The VisualInstance3D Layers mask, which is the mask that is applied when culling for our camera (the one we render the scene with), what can our camera see?
• The ReflectionProbe Cull Mask, which is the mask that determine which objects the reflection probe is applied to (similar to cull masks on lights, decals, etc).

What is missing was a mask that determines what is rendered on the reflection probe itself, a camera mask for the reflection probe as it were. My old PR, and by extension this reworked PR, solves this by introducing a new mask for this, the Reflection Mask, e.g. "What is reflected in this probe".

There was some debate whether this problem existed in Godot 3 and how Godot 3 solved it, and whether we should ensure Godot 4 works the same. To the best of my memory, if it works in Godot 3 it is because the Layers mask is also used as the Reflection Mask and that would in theory work.

The problem I ran into at the time is that our rendering architecture hides this information where we need it, and there was no real way to access this information. The camera culling system applies the Layers mask and builds a list of objects (including reflection probes) that need to be used to render the viewport, it promptly forgets about the Layers mask at this point in time as it has been applied and is no longer needed. When we get to rendering the reflection probe, all we have access to is the Cull Mask.

There is an equal argument that having this as two separate masks makes sense as you tie two unrelated things together.

This may have changed, this was over 2 years ago that I last worked on it.

[AThousandShips]

@IPlayKindred clayjohn has already been requested for a review and will review this when he has the time, no need to hurry anyone up, it's the holidays, please have patience

[IPlayKindred]

oh yeah didnt think of that at all, I am not christian so it didnt cross my mind at all, happy holidays!

[clayjohn]

I think this looks fine. But I want to take a moment to record the progression of this issue so we can discuss with other contributors and decide if this is what we want.

Godot 3.x

In Godot 3.x the cull_mask controlled which layers were rendered by the reflection probe. Items in the matching layer are shown in the reflection.

Reflections appear on all objects that are visible and are inside the reflection probe's bounds.

Godot 4.2

In Godot 4.2 cull_mask controls both which layers are rendered by the reflection probe and which layers use the reflection from the reflection probe.

This PR

I think this PR makes sense. I am a bit apprehensive at adding yet another mask to ReflectionProbes, but in this case I think it is necessary. Contrast this with Lights. Lights also have a cull_mask which controls which layers they affect. But ReflectionProbes need to both affect and be affected by layers. They are both cameras and a light source so it makes sense to let users control both.

I think the only major decision point we need to pause and think about is whether cull_mask should match the behaviour of 3.x (i.e. operate as a camera cull_mask) or whether it should match the behaviour of other visual instances like lights and decals.

[jcostello]

cull_mask should control with meshes are render intro the reflection

the new layer control should determine what meshes are affected by the reflection

that simple should be really effective

[clayjohn]

Let's wait for @bastiaan to weigh in, but my preference is also for cull_mask to retain its meaning and control what objects are rendered by the reflection probe (i.e. like a camera's cull mask) and for reflection_mask to control which objects receive reflections from the probe.

To do that you can revert the change in renderer_scene_cull.cpp so that cull_mask is used when rendering the reflection probe.

Then just use reflection_mask instead of cull_mask when creating reflection probe instances internally here:

godot/servers/rendering/renderer_rd/storage_rd/light_storage.cpp

Lines 1684 to 1696 in 179dfdc

	rpi->cull_mask = probe->cull_mask;
	reflection_ubo.box_extents[0] = extents.x;
	reflection_ubo.box_extents[1] = extents.y;
	reflection_ubo.box_extents[2] = extents.z;
	reflection_ubo.index = rpi->atlas_index;
	Vector3 origin_offset = probe->origin_offset;
	reflection_ubo.box_offset[0] = origin_offset.x;
	reflection_ubo.box_offset[1] = origin_offset.y;
	reflection_ubo.box_offset[2] = origin_offset.z;
	reflection_ubo.mask = probe->cull_mask;

[BastiaanOlij]

@clayjohn ah, I see my confusion. I was mixing up cull mask on the reflection probe and the layers property on the Visual Instance (which is the one that we loose access to during culling).

Swapping cull mask and reflection layer is indeed easy and makes sense.

[BastiaanOlij]

I think the only major decision point we need to pause and think about is whether cull_mask should match the behaviour of 3.x (i.e. operate as a camera cull_mask) or whether it should match the behaviour of other visual instances like lights and decals.

This change is applied through m4rr5#1
@m4rr5 will have to merge that into his branch for this PR to update.

Looking at this more closely, I agree swapping these two masks around. It makes much more sense.

[m4rr5]

Thanks @clayjohn for the review and fix. I've applied it and retested the build on my machine and it works fine now. The PR is updated and squashed.

[clayjohn]

Looks great!

[YuriSizov]

Thanks for picking it up and carrying it across the finish line! And congrats on your first merged Godot contribution!

[m4rr5]

Thanks for the kind words @YuriSizov and everybody else who helped!

[IPlayKindred]

YESSSSSSSSSS FINALLYYYYY, AWESOME JOB GUYSSS