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.15k stars 2.16k forks source link

Sora no kiseki SC / HD light problem #8403

Closed daniel229 closed 5 years ago

daniel229 commented 8 years ago

HD version capture from https://youtu.be/n94plorFBXE?t=14m40s schd1

PPSSPP OpenGL just black behind the box schd2

PPSSPP software redering looks fine. schd3

Non-HD version PSP Looks darker psp1

PPSSPP OpenGL less darkness psp2

software rendering looks like the HD version psp3

daniel229 commented 8 years ago

GE debugger ,don't know if this one,next prim still not draw the boxes.

HD version hd https://gist.github.com/daniel229/c24d3f143c47bb08c055

Non-HD non-hd https://gist.github.com/daniel229/26d2c8c8bcdbe13503c5

unknownbrackets commented 8 years ago

I think that's the right place. If you go there, and then double click one of the lights (like light 3), does it look any closer to the PSP?

I wonder if this could be influenced at all by the normals. Maybe it's just a precision thing.

-[Unknown]

daniel229 commented 8 years ago

Yes,double click lights3,it looks closer to the PSP.

unknownbrackets commented 5 years ago

Here's a dump (just hurried to the same place, since it's early): NPUH10191_#8403_trails_lighting.zip

Should look like this per a PSP (as noted above, just showing this dump's frame): npuh10191_ 8403_trails_lighting

Though, of course, it looks much better the way PPSSPP displays it. I'd ideally prefer it looked like the HD version.

Draw uses texture 0x04100000, and point light as above. It also uses color doubling.

To make software renderer use the PSP behavior (correct?) I can make this change:

-       if (gstate.isUsingSpecularLight(light)) {
+       if (gstate.isUsingSpecularLight(light) && diffuse_factor > 0.f) {

That is, ignore specular if diffuse factor was <= 0.

To make the Vulkan/backend renderer use the HD/software behavior (better looking - I think the PC version looks this way too):


                WRITE(p, "  dot%i = dot(normalize(toLight + vec3(0.0, 0.0, 1.0)), worldnormal);\n", i);
+               WRITE(p, "  if (dot%i <= 0.0 && light.matspecular.a == 0.0) {\n", i);
+               WRITE(p, "    dot%i = 1.0;\n", i, i);
+               WRITE(p, "  } else {\n");
+               WRITE(p, "    dot%i = pow(dot%i, light.matspecular.a);\n", i, i);
+               WRITE(p, "  }\n");
+               WRITE(p, "  if (dot%i > 0.0)\n", i);
+               WRITE(p, "    lightSum1 += light.specular[%i] * %s * (dot%i %s);\n", i, specularStr, i, timesLightScale);
-               WRITE(p, "    lightSum1 += light.specular[%i] * %s * (pow(dot%i, light.matspecular.a) %s);\n", i, specularStr, i, timesLightScale);

Note the logic - similar to #2424. Importantly, this includes <= 0.0 not just == 0.0, which might be interesting to try in the powered diffuse case (not sure...) Maybe we should just simplify the check to light.matspecular.a == 0.0 and ignore dot%i.

Anyway, I wonder if this difference affects lighting in other cases, such as other lighting bugs we're aware of...

-[Unknown]

hrydgard commented 5 years ago

I think if we do this, the if (gstate.isUsingSpecularLight(light) && diffuse_factor > 0.f) { should check the diffuse factor from before it gets changed by the powered diffuse - that is, just use the dot product directly. It seems very plausible that the PSP would use that check to skip the specular processing if possible.