Add reflection mask to reflection probes

[BastiaanOlij]

The cull_mask property on reflection probes had a double function that did not make sense.

1. It determined which objects were rendered to the probe
2. It determined which objects would get reflections using this probe

Rarely do you want to have an object reflect onto itself so a likely use case is where you'd want to exclude the object that gets reflections from being rendered to the probe. But due to this combined function that was impossible.

The documentation stated that it should be doing 1 and never mentioned 2 however seeing that for all other objects that have a cull_mask (i.e. lights and decals) cull_mask behaves as defined at 2 I've chosen to split off 1 into a separate property.

This PR introduces a new property called reflection_mask that determines which layers are rendered to the probe.

Using the material tester demo I've added a reflection probe to the mirror ball:

Note that the mirror ball itself has been placed into layer 2

The result is proper reflections onto the mirror ball:

Fixes #50195

[clayjohn]

The fact that cull_mask was being used for both jobs in master was a bug. The "layer" property was used in 3.x to determine what objects receive light from the reflection probe and the cull mask was just used to determine which objects get rendered into the reflections. IMO we should keep the behaviour the same as it was in 3.x.

[BastiaanOlij]

The fact that cull_mask was being used for both jobs in master was a bug. The "layer" property was used in 3.x to determine what objects receive light from the reflection probe and the cull mask was just used to determine which objects get rendered into the reflections. IMO we should keep the behaviour the same as it was in 3.x.

Yup, working on a new PR that does just that fix

[BastiaanOlij]

Okay, monkey wrench into this one @clayjohn, turns out we don't have access to the layer mask where we need to.

Basically the two calls to reflection_probe_get_cull_mask in this code:
https://github.com/godotengine/godot/blob/master/servers/rendering/renderer_rd/renderer_scene_render_rd.cpp#L2334
Need to be changed to obtain the layer mask to get the behavior we had in Godot 3.

But as the layer mask is stored on the associated VisualInstance and as a probe in theory can belong to multiple instances (I doubt it ever does) we don't actually have any methods to obtain this mask as we don't know for which instance we're handling the probe.

@reduz promised to have a closer look at this tomorrow.

[akien-mga]

@reduz promised to have a closer look at this tomorrow.

Famous quote :P

Might be worth poking at again soon.

[mrjustaguy]

Any chance this is going to get looked at again in the near future?

[jtnicholl]

The "layer" property was used in 3.x to determine what objects receive light from the reflection probe

I just tested in 3.x, the layers only determine if the probe should render at all depending on if it's part of the camera's cull mask. Is that not what it's intended to do? Or was it broken since you wrote that?

Any chance this is going to get looked at again in the near future?

I wrote a quick and dirty fix over here if you need the 3.x behavior back urgently, but based on the above I'm not sure that's right either.

[jcostello]

What is the status of this?

[jcostello]

Also, maybe related, but I was wondering if it could be implemented a way of priority for a probe to override reflections if 2 or more reflections probes are overlapping instead of blending. There are too many cases where you will have a central reflection probe and other scatter inside for small details and you will want to override this details instead of blending with the central.
If this require a new proposal I will write it

[AThousandShips]

@IPlayKindred Please don't bump without contributing significant new information. Use the 👍 reaction button on the first post instead. Just poking the issue without having something constructive isn't, well, constructive.

[YuriSizov]

Superseded by #86073.