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

Ensure 2D MSAA resolve is performed when 3D content but no 2D content in scene #84957

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

BastiaanOlij
Copy link
Contributor

When 2D MSAA is enabled we will output our 3D render data directly into the MSAA buffer.
If no 2D elements were being rendered, the 2D MSAA buffer was not being resolved if there were no 2D elements being rendered.
This PR keeps track of a flag that gets set when data is written directly into the 2D MSAA buffer, and gets unset when drawing to this buffer commences which will lead to an automatic resolve.
If the flag is still set at the end of rendering a viewport, a resolve is triggered.

Note that if there are no 2D elements while 2D MSAA is enabled, rendering 3D content into the MSAA buffers and resolving causes unnecessary overhead. It is advisable that a developer disabled 2D MSAA when there is no 2D content. A warning is given in this scenario.

Note while not specifically tested, there is reason to believe that this fix will fail on Adreno hardware due to a driver bug. Adreno will skip empty passes, even if that pass triggers a resolve. This behavior was discovered when using subpasses in the mobile renderer. It is not certain this behavior also applies to normal passes. I do not have hardware to test this. The fix is for the developer to disable 2D MSAA when there are no 2D elements to render, which is better practice anyway.

Fixes #84280

@BastiaanOlij BastiaanOlij added bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Nov 16, 2023
@BastiaanOlij BastiaanOlij added this to the 4.3 milestone Nov 16, 2023
@BastiaanOlij BastiaanOlij self-assigned this Nov 16, 2023
@BastiaanOlij BastiaanOlij requested a review from a team as a code owner November 16, 2023 02:32
Copy link
Member

@clayjohn clayjohn 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 and can confirm that this fixes the reported bug.

The code changes look good.

The actual change is quite low risk, so this should be suitable for merging in rc2.

@clayjohn clayjohn modified the milestones: 4.3, 4.2 Nov 22, 2023
@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Nov 22, 2023
@YuriSizov YuriSizov changed the title Ensure 2D MSAA resolve is performed when 3D content but no 2D content… Ensure 2D MSAA resolve is performed when 3D content but no 2D content in scene Nov 22, 2023
@akien-mga akien-mga merged commit bd74d92 into godotengine:master Nov 22, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@BastiaanOlij BastiaanOlij deleted the fix_msaa2d_when_no_2d branch December 10, 2023 22:58
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.

2D MSAA breaks rendering if there are no 2D nodes to render
3 participants