hrydgard / ppsspp

A PSP emulator for Android, Windows, Mac and Linux, written in C++. Want to contribute? Join us on Discord at https://discord.gg/5NJB6dD or just send pull requests / issues. For discussion use the forums at forums.ppsspp.org.
https://www.ppsspp.org
Other
11.19k stars 2.17k forks source link

Harvest Moon Graphic Issue on Adreno #10421

Closed Lotv closed 5 years ago

Lotv commented 6 years ago

On Vulkan Renderer I habe graphical Issues (Black Pixel Boxes around the Clouds) with OpenGL everything is fine

Axon 7 Android 7.1.1 Adreno 530

Lotv commented 6 years ago

screenshot_2017-12-18-23-22-52

ppmeis commented 6 years ago

Tested with latest build, Windows version default settings under Vulkan API, everything is fine: image

unknownbrackets commented 6 years ago

Another device lying about dualSrcBlend support?

Maybe it'd be worth seeing if we can get the conformance tests improved...

-[Unknown]

hrydgard commented 6 years ago

That would surprise me...

http://vulkan.gpuinfo.org/listreports.php?feature=dualSrcBlend

No adrenos there.

unknownbrackets commented 6 years ago

Maybe a rounding issue with alpha testing, then? Like in Popolocrois with GL...

-[Unknown]

hrydgard commented 6 years ago

Or it's a depth issue..

mrcmunir commented 6 years ago

screenshot_2018-10-06-18-38-03-336_org ppsspp ppsspp screenshot_2018-10-06-18-49-26-278_org ppsspp ppsspp screenshot_2018-10-06-18-37-58-168_org ppsspp ppsspp screenshot_2018-10-06-18-49-55-582_org ppsspp ppsspp

I can confirm this only happen in vulkan backend in Xiaomi mi 8 (adreno 630) the driver are supported dualsrcblend but no helped sigh adreno specific bug

unknownbrackets commented 6 years ago

Could you try exporting a GE debugger dump on PC?

To do this, open the game and select Debug -> GE debugger..., then when it's displaying the scene, press Record in the top right. After a second or so, it'll finish and save a trace of the drawing activity.

After that, check the memstick/PSP/SYSTEM/DUMP folder and it'll have created a file named something like "ULES12345_0000.ppdmp". You can zip that and then drag and drop it into a reply here.

-[Unknown]

mrcmunir commented 6 years ago

@unknownbrackets on PC however works well tested with an Intel GPU

Here the ppdmp files I made of several scenes on the mobile looks bad but on the PC it works fine.

DUMPULUS10458.zip

hrydgard commented 6 years ago

Did an experimental commit disabling dual src blend on Adreno, so you'll get a build to try soon.

mrcmunir commented 6 years ago

@hrydgard Thanks I try and no detected correct blacklist srcblend always enabled.

unknownbrackets commented 6 years ago

I lost you a bit in that message (maybe autocorrect), but do you mean that even in the latest git build, you're still seeing the issue and dualSrcBlend is still marked enabled: 1?

-[Unknown]

mrcmunir commented 6 years ago

@unknownbrackets Correct I said dualSrcBlend is still marked enabled:1 in the latest git build 1.6.3.548-geba6c00a8 :+1:

unknownbrackets commented 6 years ago

The change won't prevent it from being enabled, it just won't use it. Hmm, but if it still displays wrong maybe even enabling the feature causes problems with the driver...

-[Unknown]

mrcmunir commented 6 years ago

@unknownbrackets Ah I see . I could see that the last change "solves" some crashes in some games where the game was previously closed, now you can go further with in (forgetting the graphic defects)

hrydgard commented 6 years ago

Just for the sake of experiment, I committed a change where we don't enable the feature at all on Adreno.

mrcmunir commented 6 years ago

https://github.com/hrydgard/ppsspp/commit/6fd1c0e3d9c7c537d11d6466d1ec1d80ff6bd0e8 now detected correct DualsrcBlend:0 but not helped same glitch error.

So it's not related to DualsrcBlend issue?

Regards

hrydgard commented 6 years ago

Seems not, fortunately or unfortunately, depending on point of view...

unknownbrackets commented 6 years ago

I guess we should revert the dualSrcBlend change.

TL;DR: Driver bug involving depth testing.

This is actually happening on my Pixel (using dump 3.) I had to make a small hack because of an issue with us dumping render-to-texture.

The clouds are drawn using simple src.a + (1.0 - src.a), stencil test disabled, alpha test disabled. There is a depth test, src > dst. Depth write disabled. It's a pretty simple draw. It does reuse state from a previous draw.

Blend state ends up:

The texture is 128x128, and has 0/0/0/0 for transparent pixels. It's ultimately a 4-bit swizzled texture using a CLUT (8888.) Modulate, no doubling, pretty standard.

Things I've tried:

But then... on a lark, I disabled depth testing. Great success, issue fixed. Clouds look fine. src > dst depth testing is just too hard for Adreno, it turns out.

Adreno trying to prove that our hopes, dreams, and wishes that Vulkan drivers couldn't be as bad/worse than GL drivers were silly...

-[Unknown]

hrydgard commented 6 years ago

That's just... awful. And sad. darn.

hrydgard commented 6 years ago

Don't see any hope of working this around for 1.7.0 so bumping to 1.8.0, or maybe should close as driver bug...

unknownbrackets commented 5 years ago

If you can create a new ppdmp with the latest version of PPSSPP, it would help. It will dump this correctly so people can reproduce without any workarounds (sorry, there was a bug in the version you initially created the dump with.)

It works for Android now too: https://github.com/hrydgard/ppsspp/wiki/How-to-create-a-frame-dump

Anyway - as noted in #11620, it seems the full issue is that Adreno Vulkan drivers are simply unable to do depth testing properly (as outlined above) when not doing early fragment tests. Maybe something about the draw is causing it to skip early fragment tests (despite any shader hint)?

As noted above, this is apparently not affected by early_fragment_tests / force writing to depth to prevent the OpKill bug, and isn't using alpha or color testing. So #11620 is not likely to have helped.

-[Unknown]

mrcmunir commented 5 years ago

@unknownbrackets Okay, see if that new ppdmp help you now : DUMPULUS10458

unknownbrackets commented 5 years ago

Turns out, this is ultimately caused by draw 96 from 0x088f1e60 specifically.

I think it's a driver bug with colorWriteMask @hrydgard. This works around it:

            if ((gstate.pmskc & 0x00FFFFFF) == 0x00FFFFFF) {
                key.colorWriteMask = VK_COLOR_COMPONENT_R_BIT | VK_COLOR_COMPONENT_G_BIT | VK_COLOR_COMPONENT_B_BIT | VK_COLOR_COMPONENT_A_BIT;
                key.blendEnable = true;
                key.blendOpColor = VK_BLEND_OP_ADD;
                key.blendOpAlpha = VK_BLEND_OP_ADD;
                key.srcColor = VK_BLEND_FACTOR_ZERO;
                key.srcAlpha = VK_BLEND_FACTOR_ZERO;
                key.destColor = VK_BLEND_FACTOR_ONE;
                key.destAlpha = VK_BLEND_FACTOR_ONE;
            }

It seems like when the depth test fails, it incorrectly ignores the color write mask. Not altogether different from the stencil driver bug... but I was mistaken on how depth testing was broken.

This draw is NOT the initial clouds draw, but a later one redrawing cloud depth (not sure why.) It is:

The goal is presumably only to write depth. Stencil testing is off so stencil is not written either. It seems to draw the same positions as the clouds, but is a much later draw.

-[Unknown]

hrydgard commented 5 years ago

Wow, nasty. Thanks for investigating that. I guess I'll need to write another test...

unknownbrackets commented 5 years ago

Hm, proving difficult to create a test that replicates. Doing depth testing + color write mask alone isn't reproducing, but the draw is very simple...

-[Unknown]

hrydgard commented 5 years ago

Are you making sure that the depth test actually fails?

unknownbrackets commented 5 years ago

Yes, I turned on color write to verify that part was working. Tried making it the same depth, greater, and less to make sure it wasn't one of those states...

-[Unknown]

unknownbrackets commented 5 years ago

Also - confirmed not happening on Adreno 6xx / Android 8.0. Probably only 5xx...

-[Unknown]

hrydgard commented 5 years ago

Strange, wonder what other state could be involved - or, possibly, it needs to be a more complex shader, with something particular in it. Maybe has something to do with that early_test flag?

unknownbrackets commented 5 years ago

No, as noted above it happens regardless of that, even before the commit that added any of that.

-[Unknown]

unknownbrackets commented 5 years ago

I'm going to mark this as fixed since we applied the workaround in #11691. Hopefully the workaround prevents other similar bugs.

-[Unknown]

unknownbrackets commented 4 years ago

This driver bug still seems present on latest GPUs. I cut down a dump a lot to make a more minimal reproduction case, because it seems to require multiple draws to trigger it.

ULUS10458_#10421_harvest_moon_edit.zip

This is an edited dump. If the clouds look ugly, but you only see white and blue: no bug. If the clouds have black boxes around them: bug.

It's not really minimal now, but I got it down to 5 draw commands and 15 KB of GE dump from n original 495 KB and 112 draw commands, so it's much reduced and still reproduces.

-[Unknown]