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

Ghost In The Shell - Stand Alone Complex (ULUS10020) - Black Textures and missing screens. #12519

Closed
piau9000 opened this issue Dec 21, 2019 · 25 comments · Fixed by #12800
Closed
Labels
x86jit x86/x64 JIT bugs
Milestone

Comments

@piau9000
Copy link

What happens?

Some textures are black and some screens are completely missing (also black screen). The missing screens includes the green text intro at the beginning and chapters names. It's happening since 1.6.3-454. It seems #11380 broke it.

ULUS10020_00000
ULUS10020_00001

What should happen?

The proper image should be as follows:

ULUS10020_00000
ULUS10020_00001

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

It happens on any version after 1.6.3-454.
Windows 7 64-bit, GTX 750 ti, 8gb RAM.

@Panderner
Copy link
Contributor

Have you tried every backends?

@piau9000
Copy link
Author

Yes. There's no difference between opengl, vulkan or direct3d. Tried several other graphical settings, but none of them makes a difference.

@hrydgard
Copy link
Owner

Thanks, good report! Especially tracking down the change responsible.

This should just be a matter of handling bad fog parameters a bit differently. Could you get a GE capture of the situation for easy investigation? https://github.com/hrydgard/ppsspp/wiki/How-to-create-a-frame-dump

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Dec 22, 2019
@hrydgard hrydgard added this to the v1.10.0 milestone Dec 22, 2019
@piau9000
Copy link
Author

Here is the dump file:

recording.zip

@iota97
Copy link
Contributor

iota97 commented Dec 23, 2019

The game render with:

  • u_fogcoef = {65535.0f, -65535.0f} // Note the second one is negative
  • u_fogcolor = {0.0f, 0.0f, 0.0f}

Vertex shader does this and get a negative number:

v_fogdepth = (viewPos.z + u_fogcoef.x) * u_fogcoef.y;

Fragment shader then clamp to 0 and just render the fog color (black):

float fogCoef = clamp(v_fogdepth, 0.0, 1.0);
v = mix(vec4(u_fogcolor, v.a), v, fogCoef);
fragColor0 = v;

Frame dump (I hope is dumped correctly):
GhostDump.zip

Set both fog coef to same sign make it render fine.

@hrydgard
Copy link
Owner

Ok, nice debugging! Not quite sure if the game is intending to do something special with those settings or if it's just a game bug that happens to work on the hardware.. so exactly what the correct fix here is I'm not sure..

@iota97
Copy link
Contributor

iota97 commented Dec 23, 2019

Here there is a gameplay on read HW (https://youtu.be/TDlABQogoqw?t=240):

Screenshot_2019-12-23_17-40-42

It seem pretty the same as with no fog.
I wonder the real HW handle it as some kind of overflow skipping it given the max Esp on both value (I have no idea how psp hadle it tho', so just wondering).

@unknownbrackets
Copy link
Collaborator

Hm. So the PSP game itself is using:

Fog End = INF
Fog Density = -NAN

When I use those same parameters in a simple test, I also get black. What's more, when I run that dump on a PSP, I get...

ULUS10020_#12519_ghost_shell_bad_fog

Compare to our rendering (softgpu):

ULUS10020_#12519_ghost_shell_bad_fog_softgpu

I wonder if the actual problem is somewhere else, where it's calculating these values to even send to the display list.

-[Unknown]

@unknownbrackets
Copy link
Collaborator

If you run this game using software rendering (try to use an ingame save if possible), does it render properly?

If yes, it'd really help to have a GE dump while running software rendering too.

-[Unknown]

@iota97
Copy link
Contributor

iota97 commented Mar 23, 2020

Here a dump from 1.9.4 Software rendering, same problem (uploading the dump anyway recording.ppdmp.zip)

This game seem to segfault (on linux at least) at start on master, will bisect later for the right commit.

EDIT: 54e1afd is the first bad commit, guess was expected to be this one.

@hrydgard
Copy link
Owner

@iota97 do you see anything interesting in the log console?

@Panderner
Copy link
Contributor

Panderner commented Apr 4, 2020

This how it looks on a android phone running on OpenGL:
IMG_20200404_153831
IMG_20200404_153903
IMG_20200404_153923
Anybody have you tested any backends on latest build version?

@iota97
Copy link
Contributor

iota97 commented Apr 4, 2020

Mhhh, I still get the black model on my desktop (while is fine on android for me as well), the dump render fine on my desktop as well.

As the previous dump had weird value on the uniform was likely a bug on the CPU side before it get to render, still was is kinda weird is that desktop is broken with every CPU backend (JIT, IR, Interpreter), but Android is fine with all of them. I wonder how it work on desktop ARM like a raspberry TBH, haven't got any to try tho'.

PS: phone is Redmi 4X (Qualcomm Snapdragon 435 MSM8940), desktop is Linux x86_64 (intel i5-6400).

@Panderner
Copy link
Contributor

@iota97 have you tried every backends?

@iota97
Copy link
Contributor

iota97 commented Apr 4, 2020

Both Vulkan and OpenGL produce the same result, DirectX is not available on Linux so that was not tested, but I doubt GPU backend matter tbh.

@Panderner
Copy link
Contributor

Both Vulkan and OpenGL produce the same result, DirectX is not available on Linux so that was not tested, but I doubt GPU backend matter tbh.

Are you using Linux?

@iota97
Copy link
Contributor

iota97 commented Apr 4, 2020

u_fogcoef.y is 65535.0f on the android dump, may ARM just produce a +NaN rather than a negative one somewhere making it display ok...

@unknownbrackets
Copy link
Collaborator

Right, this is either a CPU bug, or some sort of timing bug where some game is not (or is) being executed to fill in some state.

It's probably not ideal that some Android phones are (incorrectly) rendering this as non-black, because we're currently telling them to. So that's actually a separate bug. We should not try to make that bug exist on Android too, because we know that's the wrong behavior.

-[Unknown]

@iota97
Copy link
Contributor

iota97 commented Apr 4, 2020

I don't really get why you say it's incorrect to render them non-black, on android the uniform are different (the android dump render fine on desktop too), doesn't this mean that some value sent to the PSP GPU by the game end up being computed different?

The real PSP render it black with the desktop value as well, while they might be 2 bug cancelling them couldn't also be android being closer to correct?

@unknownbrackets
Copy link
Collaborator

Oh, I thought it was just that the Android device GPU was interpreting differently. If a dump created on Android renders correctly everywhere else, that points to a likely jit bug. Sorry for misunderstanding.

The web debugger can connect to Android: http://ppsspp-debugger.unknownbrackets.org/

I don't have the game, but if you use a data breakpoint to see where it's writing to the fog values in the display list, you can trace it back to where it's generating those values. The trick will be to find where it's calculated differently.

-[Unknown]

@iota97
Copy link
Contributor

iota97 commented Apr 4, 2020

Just to note that it's always broken on desktop and always work on android with all the 3 CPU backend (JIT, IR, interpreter).

About the breakpoint how am I suppose to know where is writing in the display list in first place? Also I can get only the +/-65535.0f from RenderDoc on Linux, anyway to get the exact value the game is passing? I doubt I will be able to find anything in that debugger myself sadly :/

@unknownbrackets
Copy link
Collaborator

Well, I still haven't gotten around to adding the GE debugger to the web debugger (it's technically possible and not all that hard, I should get back to it...)

If you use WINE and run the Windows version, you can use the GE debugger. In some ways, it's like RenderDoc, but it shows you exactly what the game is sending rather than what we translated that to. It also lets you step through the GE display list and set breakpoints on GE registers (such as fog.)

Warning: I've heard the debugger looks kinda not great in WINE without MS fonts.

Once you have the GE debugger up:

  1. Click Step Frame to pause things on the next frame. You can press Step Frame again to update the counter shown to the right if you want.

  2. Click the Settings tab on the lower pane. This is showing you all the PSP GE registers (some are grouped together for easier reading.)

  3. Right click each of Fog 1 and Fog 2 and select Toggle Breakpoint.

  4. Now click Resume in the top right. Emulation should run until the next time a display list alters Fog 1 or Fog 2. You'll see the display list command in the Display List tab.

  5. Copy the address of the display list command, and hit Resume a few more times. You'll end up seeing one of two things:
    a. Easy mode: The display list will cycle through a few different addresses, but ultimately come back to the same address that you saw the first time. This makes things easier.
    b. Hard mode: Each time you hit fog, it'll be a new rolling address. You'll have to use save states during debugging (I'll cover this below.)

  6. After dealing with save states (if necessary), go to Disassembly and set a breakpoint for the address that had the fog command. You can just check Write. This step can be done on Windows or the web debugger.

  7. Run and you'll find the code modifying it, and can inspect what it's trying to write there. You'll have to use breakpoints (potentially with conditions) to work backwards in the function (and maybe also save states.)

From there it's just tracing backwards.

If it's "hard mode", here's how to deal with save states:

  1. First, get to a fresh new frame using Step Frame. The CPU should be unpaused first (showing Break / click Go.)

  2. Now pause the CPU using Break, and then resume the GE using Resume.

  3. Create a save state. The point is to have a save state before the frame with the addresses we are about to know about is rendered.

  4. Now use the breakpointing to find the display list address / etc. Make sure to do all of this before the next frame - if you end up having to go to the next frame, you must load state.

  5. You can pause the CPU, load state, and step forward using breakpoints (this should work in the web debugger too, I think.) The same state will need to be used on both devices so the address matches.

-[Unknown]

@iota97
Copy link
Contributor

iota97 commented Apr 5, 2020

f0 = 0x00000000 (0)
f3 = 0x7F800000 (+inf)

mul.s	f0,f0,f3

After this f0 result in +NaN on android but -NaN on desktop.

Tested a C++ program on x86_64 making that multiplication and result in -NaN indeed (NaN sign is meaningless so I guess is up to the CPU/Architecture what to do?).

If you could test this float operation on the real PSP would be useful I guess, also would be nice to know if -NaN is ever a thing on the PSP FPU (changing the display list conversion to +65535.0f for any NaN might be a workaround in this case).

@unknownbrackets
Copy link
Collaborator

Negative NaN is a thing on the PSP, but mostly with the VFPU (and unfortunately, isn't even meaningless there.)

Will test this in a bit.

-[Unknown]

@unknownbrackets unknownbrackets added the Needs hardware testing Testing on a real device needed to determine correct behavior. label Apr 5, 2020
@unknownbrackets
Copy link
Collaborator

Yep, it's definitely supposed to be +NAN:

 Infinity and zero
-  mul.s 0.000000 * inf = nan / 7fc00000 (rm=00000000)
-  mul.s 0.000000 * inf = nan / 7fc00000 (rm=01000000)
-  mul.s inf * 0.000000 = nan / 7fc00000 (rm=00000000)
-  mul.s inf * 0.000000 = nan / 7fc00000 (rm=01000000)
-  mul.s 0.000000 * -inf = nan / 7fc00000 (rm=00000000)
-  mul.s 0.000000 * -inf = nan / 7fc00000 (rm=01000000)
-  mul.s -inf * 0.000000 = nan / 7fc00000 (rm=00000000)
-  mul.s -inf * 0.000000 = nan / 7fc00000 (rm=01000000)
+  mul.s 0.000000 * inf = nan / ffc00000 (rm=00000000)
+  mul.s 0.000000 * inf = nan / ffc00000 (rm=01000000)
+  mul.s inf * 0.000000 = nan / ffc00000 (rm=00000000)
+  mul.s inf * 0.000000 = nan / ffc00000 (rm=01000000)
+  mul.s 0.000000 * -inf = nan / ffc00000 (rm=00000000)
+  mul.s 0.000000 * -inf = nan / ffc00000 (rm=01000000)
+  mul.s -inf * 0.000000 = nan / ffc00000 (rm=00000000)
+  mul.s -inf * 0.000000 = nan / ffc00000 (rm=01000000)

(the - results are PSP, + are PPSSPP on x86.)

It is correct on ARM for me too, but I'm not sure if it's guaranteed:

(Snapdragon 821)

Infinity and zero
  mul.s 0.000000 * inf = nan / 7fc00000 (rm=00000000)
  mul.s 0.000000 * inf = nan / 7fc00000 (rm=01000000)
  mul.s inf * 0.000000 = nan / 7fc00000 (rm=00000000)
  mul.s inf * 0.000000 = nan / 7fc00000 (rm=01000000)
  mul.s 0.000000 * -inf = nan / 7fc00000 (rm=00000000)
  mul.s 0.000000 * -inf = nan / 7fc00000 (rm=01000000)
  mul.s -inf * 0.000000 = nan / 7fc00000 (rm=00000000)
  mul.s -inf * 0.000000 = nan / 7fc00000 (rm=01000000)

-[Unknown]

@unknownbrackets unknownbrackets removed the Needs hardware testing Testing on a real device needed to determine correct behavior. label Apr 5, 2020
unknownbrackets added a commit to unknownbrackets/ppsspp that referenced this issue Apr 5, 2020
See hrydgard#12519 - this is needed for some graphics to render properly.  Seems
to already happen on ARM, so no change to armjit.
@unknownbrackets unknownbrackets added x86jit x86/x64 JIT bugs and removed GE emulation Backend-independent GPU issues labels Apr 5, 2020
unknownbrackets added a commit to unknownbrackets/ppsspp that referenced this issue Apr 6, 2020
See hrydgard#12519 - this is needed for some graphics to render properly.  Seems
to already happen on ARM, so no change to armjit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x86jit x86/x64 JIT bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants