Implement MSAA support for desktop GPUs in Vulkan #16458
Conversation
|
Heh, getting the basics up was not so hard, but there's a lot of image layout transition logic in QueueRunner that's kinda bogus or incomplete in multisample mode. All that stuff only works because we don't do copies correctly so never transition away the msaa targets from ATTACHMENT_*_OPTIMAL :) Doing all copies through raster might be the easy way out... Though, won't be perfect for depth buffers where we'll lose valuable resolution (by texturing from a resolved target, drawing to a multisampled one) |
|
Raster copies fixes all the common cases. We do lose depth resolution which might matter in cases like Jeanne D'Arc, but mostly it works just fine. I actually think we could get this in pretty soon. Should probably make a configuration option to select how much sample shading we want - right now we enable full per-sample shading only when DISCARD is used in a shader, which smoothes out fences and stuff very nicely. |
|
Hm, there seems to be some shutdown race condition related to shader destruction, not sure why it appeared here. Doesn't always happen. |
hrydgard
force-pushed
the
desktop-friendly-msaa
branch
from
bffe96c
to
0ef03bf
Compare
|
There's a related race between the pipeline compiler thread and forced recompilation of pipelines with the wrong msaa depth. Think I'll have to purge the compiled pipelines completely on MSAA level switch somehow to get around this.. |
… since we don't texture from stencil, but whatever).
…on't use it while not multisampling.
…rt for sample rate shading.
hrydgard
force-pushed
the
desktop-friendly-msaa
branch
from
588d50a
to
8a3e92a
Compare
|
This seems fairly solid-ish now, and runs validation-clean throughout, switching on and off, and changing MSAA level - although there are things I want to improve (add quality setting, make actual copies work instead of raster copies), everything I throw at it seems to work, and it generally looks absolutely fantastic, at least on GPUs that support sampleRateShading. So even though it's late in the process, would be cool to get in for 1.14. My reluctance to put multiple MSAA levels into the renderpasstype key resulted in the need for some rather ugly invalidation hacks, but I think it's worth it. There's only one additional renderpass key bit I foresee us ever needing now (#16327), while adding 3 additional more bits to represent the various MSAA levels would start to get to the point where we should switch to hashmaps or something instead of plain arrays, and it's nice to avoid that... I think. EDIT: Actually I've found two problems:
Hmm.. |
| caps_.multiSampleLevelsMask = (limits.framebufferColorSampleCounts & limits.framebufferDepthSampleCounts & limits.framebufferStencilSampleCounts); | ||
| // Check for depth stencil resolve. Without it, depth textures won't work, and we don't want that mess | ||
| // of compatibility reports, so we'll just disable stencil in this case for now. | ||
| // There are potential workarounds for devices that don't support it, but those are nearly non-existent now. |
There was a problem hiding this comment.
*MSAA, not stencil. Also, meant to check resolveProperties, like supportedStencilResolveModes?
-[Unknown]
There was a problem hiding this comment.
Oops, yeah.
Not checking resolveModes, we just use "sample0" resolve mode for now which is required to be supported if resolve is supported at all, it's fine. It won't make very much of a difference to use the others, and I think doing average for example could result in artifacts in some casese.
There was a problem hiding this comment.
Although just to be safe, I'll throw in a check for that resolve mode.
|
This SEGFAULT on Linux, will provide a stack trace when I can :/ |
|
@iota97 thanks, please do. Getting this in 1.14 might be ambitious, I may push it to later. |
|
Ok, kinda good news is this actually work on Linux using Vulkan, but crash on OpenGL. You removed quite a few null pointer checks there, does this crash on Windows too with OpenGL? |
|
Oops, I'll fix that up. Meant to always pass a valid pointer in, removing the need for the null checks, but apparently failed to update some of the other backends. |
hrydgard
changed the title
|
It works well enough now. I'm gonna get it in, and deal with any consequences :) |
ghost
mentioned this pull request
Partially solves #6218
This is an implementation of MSAA antialiasing, for desktop GPUs only, and Vulkan only for now.
There are some glitches
(like the sun in Burnout Legends)but it mostly works in a lot of games.It detects available MSAA modes and even allows runtime switching between them.
(In theory this could work on mobile but would be terribly expensive on tiler GPUs because it "stores back" the MSAA framebuffers to memory, so that it can continue rendering to the same render target when needed. On mobile, we should use STORE_OP = DONT_CARE, but then we also need to do a blit from the resolved buffer to the MSAA framebuffer in order to continue rendering, but that's work for another day.)
Remaining work before merge:
Not fully correct yet, but switched to raster copies, which do work, except we lose some sampling resolution.Postponed quality setting until after merge.