Skip to content

Conversation

@manuele-bonanno
Copy link
Contributor

@manuele-bonanno manuele-bonanno commented Nov 1, 2021

Purpose of this PR

The way RenderTextures handle MSAA fallback when an unsupported sample count of 2 is requested (falling back to numSamples = 1), differs fom the way the fallback is handled when setting up the Vulkan swapchain (rounding up numSamples to 4, if supported). This caused an issue on Mali GPUs which don't support 2x MSAA.
This PR makes sure that on Vulkan the MSAA unsupported fallback behaviour is consistent between RenderTextures and Swapchain.

We are also making sure that depth copy is always disabled on GLES, since it doesn't work on that platform.


Testing status

Tested the repro project of this case.


Comments to reviewers

we should review how all backends handle MSAA fallbacks and move these implementation details in engine code.

check the implementation of SelectSupportedSampleCount(const VkSampleCountFlags supportedSamplesCounts, const VkSampleCountFlagBits requested) in GfxDeviceVK.cpp for more info

@github-actions
Copy link

github-actions bot commented Nov 1, 2021

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

URP
/jobDefinition/.yamato%252Fall-urp.yml%2523PR_URP_trunk
With changes to URP packages, you should also run
/jobDefinition/.yamato%2Fall-lightmapping.yml%23PR_Lightmapping_trunk

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@manuele-bonanno manuele-bonanno marked this pull request as ready for review November 1, 2021 13:05
@manuele-bonanno manuele-bonanno requested review from a team as code owners November 1, 2021 13:05
@manuele-bonanno manuele-bonanno changed the title [URP] make the Vulkan MSAA support fallback work in the same way as the swapchain backend implementation [URP] make the Vulkan RenderTexture MSAA support fallback work in the same way as the swapchain backend implementation Nov 1, 2021
@phi-lira phi-lira changed the base branch from master to universal/staging November 10, 2021 09:08
@pushmatrix
Copy link

Are there any details you can share about We are also making sure that depth copy is always disabled on GLES, since it doesn't work on that platform. ?

My webgl project worked with depthtexture copy in 2020, and updating to 2021 still works if I uncomment the part where you return false inside CanCopyDepth. I'm curious about what's not working about depth in GLES

@manuele-bonanno
Copy link
Contributor Author

I'm curious about what's not working about depth in GLES

It doesn't work when MSAA is enabled. We noticed that we could still allow it when MSAA is off, so we are pushing a fix for that

while waiting for the fix you can do this locally:

if (IsGLESDevice() && msaaDepthResolve)
return false;

@pushmatrix
Copy link

That makes sense. Thanks for the update on this!

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