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

Copy dependency for multisample and non-multisample textures #3382

Merged
merged 3 commits into from
Jun 5, 2022

Conversation

gdkchan
Copy link
Member

@gdkchan gdkchan commented Jun 4, 2022

On the GPU, there are cases where we know that a texture is multisampled, and there are some cases that we don't know, the cases where we don't know, we have an overlap at the same address of a multisample texture, with a width and height that is equal to the multisample width and height multiplied by the amount of samples in X and Y respectively. Since multisample and non-multisample textures are not considered "compatible" in any API, most of those cases are not handled. Currently, there are a few tricks used to make some cases work. 2D engine blit source textures wil get multisample textures from the cache if it can find one at the requested address. Notably, the cases where a texture on the pool accessed from a shader does not have a multisample type on the texture descriptor is not handled right now, which causes problems on a few games using the Vulkan and OpenGL APIs on the Switch.

This change allows non-multisample texture at the same memory region as a multisample one to inherit the multisample texture data, rather than just reading garbage from memory. The main complication doing this is that APIs does not support doing pretty much anything with multisample textures other than rendering or accessing from shaders, so the solution I found here was using blits to "resolve" the multisample texture to a intermmediate texture, then upscale the intermmediate to the final texture. It should be fine if the intention is downscaling it later anyway, even if a bit inefficient. Another possible solution would be doing the copy with a compute shader, using images, but that has other issues too.

This change also allowed the case I added before to allow matching multisample <-> non-multisample textures with different but compatible formats to be removed. Since they are considered copy compatible now, the texture will still correctly inherit the data, but the old approach was slightly more efficient (it creates one view and does one copy, rather than creating a new copy dependant texture and doing one copy (it's actually 2 blits, as explained above) every time it is accessed after modification, plus the actual copy, so I can add it back if desired. I removed it mostly to make the code simpler and since only 1 game seems to trigger this case so far, and it doesn't do it enough for perf to be an issue.

This change fixes black screen on Perky Little Things.
Before:
image
After:
image

It also fixes the 3D sections on Genkai Tokki Moero Crystal H.
Before (from gamedb):
image
After:
image
It is still not rendering correctly due to another issue that is completely unrelated to this, so I will fix in another PR. But at least this change allows it to render at all.

I recommend testing this on games that uses multisampling, to ensure they did not regress.

@gdkchan gdkchan added gpu Related to Ryujinx.Graphics fix Fix something labels Jun 4, 2022
Copy link
Member

@riperiperi riperiperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, seemingly no regression on the games I have with multisampling.

@gdkchan gdkchan merged commit a3e7bb8 into Ryujinx:master Jun 5, 2022
@gdkchan gdkchan deleted the ms-copy-dep branch June 5, 2022 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fix something gpu Related to Ryujinx.Graphics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants