Implement MSAA for 2D#63003
Conversation
4e9feee to
ce1f1bb
Compare
|
Rebased and added checks for the maximum supported sample count for both 2D and 3D MSAA. This should fix any crashes on Mali when MSAA is above 4x. |
clayjohn
left a comment
There was a problem hiding this comment.
Great work! I am very excited about this PR and I know many users will be too.
I have left a few comments on implementation, mostly just stuff that needs to be fixed up in the GLES3 implementation.
There was a problem hiding this comment.
WebGL2 uses Renderbuffers to support Multisampled Framebuffers. So you can still use it even though GL_TEXTURE_2D_MULTISAMPLE isn't defined
There was a problem hiding this comment.
We need to minimize the number of FBO changes and full screen copies we do in order to keep performance manageable on mobile devices. Accordingly, I would put the resolve inside of render_target_copy_to_back_buffer(). If copying the full rect, you can get away with resolving the Multisampled buffer directly into the backbuffer. If copying a partial rect, you'll have to resolve and then copy still.
There was a problem hiding this comment.
Right now this will break 3D rendering as the same color texture is used in both 2D and 3D. Since the Depth texture does not use Multisample the Framebuffer will fail on creation.
There was a problem hiding this comment.
Currently combined 2D and 3D rendering is broken since the resolve attachment's store op is VK_ATTACHMENT_STORE_OP_DONT_CARE while it should be VK_ATTACHMENT_STORE_OP_STORE. (this took way too long to figure out :))

Just brute forcing all store ops to VK_ATTACHMENT_STORE_OP_STORE resolves the issue:

Now that I know what's wrong I can work on a proper fix, will probably push one tomorrow after I addressed clayjohns review comments. (I also rewrote a few bits to make things cleaner)
de56470 to
b945698
Compare
|
The Vulkan side is should be done now, OpenGL still needs some work, but because of #63600 I can't test my changes - so that might take some time. If it's ok, I would prefer to only have the critical parts in the OpenGL implementation fixed and then implement 2D MSAA properly later (e.g. with webgl2 support) when the OpenGL renderer has matured. This PR was already pretty time-consuming and unfortunately I have not much time currently. |
|
Rebased. Regarding the GLES3 part: I just looked at the code and noticed that 3D MSAA is also not yet implemented. I would rather strip the implementation completely from this PR and add both 2D and 3D MSAA together at a later point in time (and properly, so that you can set both independently from each other like in the Vulkan renderer). |
Fair enough. I am okay with that if you are planning on adding it back at a later time to avoid this getting too complex. |
|
Ok, I removed the GLES3 implementation (a warning is printed when the user tries to enable it). With that I think this PR is complete :) |
clayjohn
left a comment
There was a problem hiding this comment.
Just the one change before it is ready to go
65ef7c7 to
6039b23
Compare
clayjohn
left a comment
There was a problem hiding this comment.
I'm happy with this PR. Should be ready to merge.
CC @BastiaanOlij I think this may conflict with your Framebuffer reorganization PR, but I think we should merge it now as Geometror won't be available for awhile after the end of this week.
|
Does this work properly with backbuffer or clip effects? I think it would be good to test this before merging. |
|
@reduz Yes, this seems to work fine with those effects(MSAA 8x): |
|
@reduz says it's OK so let's merge 👍 |
|
Thanks! |
|
@insomniacUNDERSCORElemon Could you open an issue and provide an reproduction project? :) |
An issue was opened: #66005 |





This PR implements MSAA for the 2D/Canvas renderer. (Implements godotengine/godot-proposals#1297) In addition, this might improve the situation for godotengine/godot-proposals#1126.
Detailed changes:
Without MSAA:




MSAA 2x:
MSAA 4x:
MSAA 8x:
Bugsquad edit: This partially addresses godotengine/godot-proposals#4354. This closes #56233 by adding MSAA sample checks to both 2D and 3D.