-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Use default samplers in base uniform set when rendering to reflection probes #84317
Conversation
It seems that we are going to need to implement a base_uniform_set cache. Right now the first Viewport to set a base_uniform set wins and its settings are used for everything else. |
This iteration fixes #83962 and an unreported bug whereby the wrong bias is used. However, there is still an existing This PR splits A perfect solution would be one where we either store the samplers in another location (one that is updated more frequently), or we store the |
Judging by your description, this may also fix #65690 🙂 I'll give it a try.
I assume this is connected to #84083. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, it works as expected.
That's a different issue, ya.
That's right. I hadn't seen #84083 before. It is exactly what I was describing |
Thanks! |
Fixes: #83962
The RenderBuffers used by ReflectionProbes doesn't create and maintain its own set of samplers. So basically what was happening was the Viewport RenderBuffers were invalidating the base_uniform_set (so it would get recreated with the new samplers). But then the ReflectionProbes would get rendered, and they don't have valid samplers, so they would give invalid samplers and the base_uniform_set would be null. Then rendering would fail for a frame. The Viewport would then get rendered and update the base_uniform_set to be valid. Then, in subsequent frames, the ReflectionProbes would just use the updated base_uniform_set.
This PR works, but I think it will bring us back to the state where the sampler gets ignored when changed (due to how the caching works). I need to do a bit more testing before this is ready.