Graphic regression on Legend of Heroes III - Song of the ocean ULUS10144

[MMouta]

What happens?

In PPSSPP ver. 1.7.2
Vulkan - All backrounds display flickering lines even at native psp resolution
- Transparent box appear around sprites over some effects (ie. fire attacks)
OpenGL - Transparent box appear around sprites over some effects (ie. fire attacks)

What should happen?

Graphics were shown correctly in ver. 1.6.3

What hardware, operating system, and PPSSPP version? On desktop, GPU matters for graphical issues.

PPSSPP ver. 1.7.2
Phone: Xiaomi Redmi Note 4
OS: Android 7.0 - MIUI 9.6.3
Chip: Snapdragon 625
GPU: Adreno 506

[unknownbrackets]

On the Vulkan side, it seems like this is caused by writing depth. Even without assume unchanged, it happens. 06340bf is causing this.

Writing to gl_FragDepth in the fragment shader at all, even in a conditional that is always false or using pixel depth rounding, will cause different but bad glitches. Ugh.

Need to look into the box issue, which seems to be a separate issue.

-[Unknown]

[hrydgard]

Argh, these driver bugs again... So terrible and sad.

Need to find a way to get a contact inside Qualcomm as they seem to ignore their forums... Not that it would fix the problem for existing phones anyway.

Or, I guess, write a CTS test...

[hrydgard]

Hm. Maybe the following would tickle the driver into doing what we want (for the StencilBufferVulkan case):

Instead of turning off depth test entirely in VK2DDepthStencilMode::STENCIL_REPLACE_ALWAYS, we could try to leave it on, set the compare op to VK_COMPARE_OP_ALWAYS and turn off depth writes off?

We could also try that in normal rendering..

[hrydgard]

https://github.com/KhronosGroup/VK-GL-CTS/blob/master/external/vulkancts/modules/vulkan/fragment_ops/vktFragmentOperationsEarlyFragmentTests.cpp

Yeah, I can't find any CTS tests for combinations of stencil and discard. Sigh...

[MMouta]

I'm adding a couple of screenshots of the transparent box issue since its probably a different problem also present with OpenGL.
These were taken with the OpenGL backend.

[unknownbrackets]

Enabling depth test without depth write doesn't help the stencil buffer case.

The other is 5ccd3ee, based on a bisect. Looking into the draw.

-[Unknown]

[unknownbrackets]

#11543 fixes the box issue. It's funny, I had just gone through and beaten this game recently with no issues, not long before implementing that optimization. Darn.

-[Unknown]

[MMouta]

Awesome! Thanks for looking into this. I wish I knew more about emulation so I could give you a hand with the code. I think the work you guys are doing is really great!

[unknownbrackets]

Just for reproduction sake, here's a GE dump replicating the problem: ULUS10144_0001.zip

-[Unknown]

[hrydgard]

I don't know if the stencil upload affects this game, but here's a possible alternate path for that on Vulkan which might be useful to avoid this kind of driver bug?

It's possible to use vkCmdCopyBufferToImage to copy data to stencil images. I first thought it might also be possible to Blit to get a stretching behaviour, but that does not seem to be the case. However, we can probably do the following:

• Upload the raw stencil image as R8 to a push buffer
• vkCmdBlitImage to do a hardware stretch to a temporary R8 image of the full rendering resolution.
• vkCmdCopyImageToBuffer from that to a temporary buffer (R8 format)
• vkCmdCopyBufferToImage to the stencil buffer (S8 copy)

When running at 1x resolutions, the two middle steps can be skipped.

S8 and R8 have compatible representations when copied to buffers. It's a bit of seemingly unnecessary copying, but it lets the driver perform the necessary conversions to its internal stencil format.

Alternatively, the stretch could be done by a compute shader which could let us avoid the third step.

[unknownbrackets]

Just to clear something up: StencilBufferVulkan isn't running or related to this bug, afaik. It's related to regular rendering with stencil testing + discard.

To do the above, it would mean a two-pass render, right?

-[Unknown]

[hrydgard]

Oh, ok. Yeah then my method above isn't very relevant (although it might still be a reasonable alternative to the existing stencil upload path).

[unknownbrackets]

It seems like the driver is incapable of doing depth testing (at least properly) after the fragment shader, no matter what. That means discard preventing depth or stencil write just doesn't work, either way (e.g. either depth/stencil boxes around objects, or you get broken depth testing.)

But, affected devices seem to have (in GLES) ARM_shader_framebuffer_fetch_depth_stencil. So presumably the best workaround (assuming a driver that isn't completely broken is off the table) would be to disable the fixed-function depth and stencil tests when using discard, and instead do that within the shader. I'm not sure if there's any complexity to enabling read/write attachments in Vulkan...

I think we should default Adreno devices to GLES. The GLES driver is mostly less broken (except #11355...)

-[Unknown]

[MMouta]

I just wanted to let you know that the transparent box issue is present again in Android store version 1.7.5. I was using version #11543 and the problem was gone in OpenGL but the newest stable version is showing this glitch again.
Thanks for your hard work.

[hrydgard]

1.7.5 does not contain the latest bugfixes from the buildbot builds, only critical fixes on top of 1.7 to avoid regressions. This is fully expected. See #11639

[unknownbrackets]

FWIW, this glitch is also seeming limited to 5xx - not happening on a 6xx with Android 8.0.

-[Unknown]

[hrydgard]

Do we think this is the same bug as the one currently being repro'd in my test? If so that's even more confirmation that that bug is 5xx-only, if not we have another test to write...

[unknownbrackets]

Not exactly, as this one only occurs when writing depth in the fragment shader (would also potentially affect depth rounding.) It's probably a related issue, though. My guess is it is related to the code they tack on to the fragment shader to implement blending / rgba masks / depth write disable / etc.

-[Unknown]