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

Fix massive validation errors when enabling TAA + MSAA #81775

Merged
merged 1 commit into from
Sep 24, 2023

Conversation

darksylinc
Copy link
Contributor

@darksylinc darksylinc commented Sep 16, 2023

TAA + MSAA would make Godot request unnecessary flags for an MSAA velocity texture, flags that were not even actually needed.

This was causing:

  1. Unsupported GPUs to fail completely (e.g. Intel Arc 770)
  2. Wrong codepaths to be followed (causing validation errors, possibly crashes or glitches)
  3. Unnecessary performance impact in all GPUs.

See
#71929 (comment)

TAA + MSAA would make Godot request unnecessary flags for an MSAA
velocity texture. flags that were not even actually needed.

This was causing:
 1. Unsupported GPUs to fail completely (e.g. Intel Arc 770)
 2. Wrong codepaths to be followed (causing validation errors, possibly
crashes or glitches)
 3. Unnecessary performance impact in all GPUs.

See
godotengine#71929 (comment)
@Calinou Calinou added the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Sep 16, 2023
@Calinou Calinou added this to the 4.2 milestone Sep 16, 2023
@darksylinc
Copy link
Contributor Author

Note: This only affects (performance & bugs) when both TAA & MSAA are enabled at the same time. This does not affect any issues that arise when TAA is enabled but MSAA is disabled.

@DarioSamo
Copy link
Contributor

Yeah I've been talking with Matias and it was fairly suspect that the MSAA texture requested these attributes. Nothing that uses it afterwards will request it as the MSAA texture will resolve to the version of the texture that has storage and is the version of the texture used in TAA and FSR2.

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 ee15a2e35), it works as expected.

Performance comparison on https://github.com/Calinou/godot-reflection in 3840×2160 on a GeForce RTX 4090:

Antialiasing Before After (this PR)
TAA, no MSAA 271 FPS (3.69 mspf) 269 FPS (3.72 mspf)
TAA, 2× MSAA 236 FPS (4.24 mspf) 249 FPS (4.02 mspf)
TAA, 4× MSAA 206 FPS (4.85 mspf) 226 FPS (4.42 mspf)
TAA, 8× MSAA 171 FPS (5.85 mspf) 182 FPS (5.49 mspf)

Using MSAA with TAA still makes a positive visual difference by making moving edges less aliased, so it's still functional.

@akien-mga akien-mga merged commit 8788b20 into godotengine:master Sep 24, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 24, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

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

Successfully merging this pull request may close these issues.

5 participants