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

Use render pass uniform set to store viewport samplers. #84637

Merged
merged 1 commit into from Dec 8, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Nov 8, 2023

Follow up to #84317
Fixes: #84083
Fixes: #65690
Fixes: #84084

Basically what this PR does is separate the "default" samplers from the per viewport samplers for the scene shaders. The difference between the sampler types is just LOD bias. However, internally, we almost always want an LOD bias of 0, we only want to apply the LOD bias to decals, projector textures, and user texture fetches with mipmaps enabled.

Accordingly, we need to pass a set of samplers for internal use and external use.

Previously we were updating the render_base_uniform_set each time a viewport change its samplers. The render_base_uniform_set is shared between all viewports. This was leading to all viewports fighting over the same uniform set. It would get cleared anytime any viewport was re-configured, then the first viewport to render would get to pick the samplers used. This behaviour was highly problematic.

Now each viewport can use its own samplers and the viewport's samplers don't conflict with the Godot internal texture reads. In particular this fixes ReflectionProbes and Sky becoming less rough with a lower rendering scale, but it should also fix some of the unexplained aliasing we were getting with FSR2.2 at a low rendering scale.

This fixes a bugs where per-viewport samplers were being used for internal texture fetches (probes, sky, etc.).

This also fixes a bug when using multiple viewports in the same scene.

This also fixes a bug where the texture bias would override the bias from 3D scale.
@clayjohn clayjohn added bug topic:rendering topic:3d cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Nov 8, 2023
@clayjohn clayjohn added this to the 4.3 milestone Nov 8, 2023
@clayjohn clayjohn requested a review from a team as a code owner November 8, 2023 22:33
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased on top of master 1ba920f), it works as expected.

simplescreenrecorder-2023-11-24_15.53.14.mp4

@YuriSizov YuriSizov merged commit ee1bf15 into godotengine:master Dec 8, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@clayjohn clayjohn deleted the RD-sampler-bias branch December 8, 2023 16:16
@clayjohn clayjohn removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 14, 2023
@clayjohn
Copy link
Member Author

This seems to have caused a regression on MacOS. Removing the cherrypick label for now #86155

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