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

Got an error in texture upload: 00000501 in silent hill games and METAL SLUG XX #8371

Closed daniel229 closed 6 years ago

daniel229 commented 8 years ago

Seperated from https://github.com/hrydgard/ppsspp/issues/8173 tracestack 01

2

unknownbrackets commented 8 years ago

What is the value of depal->a_texcoord0? If you comment that line out, does it stop happening?

If so, that means the shader probably doesn't have an a_texcoord0, or doesn't use it. Which would be weird.

Note: this is not a new error, it's just that we started logging this error. It's probably been happening for a while.

-[Unknown]

daniel229 commented 8 years ago

Okay. a_texcoord0 0xffffffff int

comment out glEnableVertexAttribArray(depal->a_texcoord0);?,it crashed.

unknownbrackets commented 8 years ago

Hmm, well, we do set data there later, maybe that's why it crashed.

In GPU/GLES/DepalettizeShader.cpp:

        depal->a_texcoord0 = glGetAttribLocation(program, "a_texcoord0");

If you log after this line, it's here that depal->a_texcoord0 becomes -1, right? Maybe log:

        if (depal->a_texcoord0 == -1) {
            ERROR_LOG(G3D, "Linked program and lost texcoord.\n%s", buf);
        }

And then we can just verify what the shader source is. It should include v_texcoord0, which will mean that it needs a_texcoord0 from the vertex shader.

I'm thinking now... maybe shiftedMask is 0 inside GenerateDepalShader300(). If that were the case, then the shader compiler could definitely optimize out the texcoords entirely. That's probably what's happening. A log of the shader source should verify all of this.

If the game is using a zero mask, I wonder what that is meant to do. It probably does pull the 0th CLUT entry, as we ought to be doing, but it might be worth testing it...

-[Unknown]

daniel229 commented 8 years ago

Get this log

05:21:599 main         E[G3D]: GLES\DepalettizeShader.cpp:270 Linked program and lost texcoord.
#version 330
in vec2 v_texcoord0;
out vec4 fragColor0;
uniform sampler2D tex;
uniform sampler2D pal;
void main() {
  vec4 color = texture(tex, v_texcoord0);
  int r = 0;
  int g = 0;
  int b = 0;
  int index = (b << 11) | (g << 5) | (r);
  index = (int(uint(index) >> 16) & 0xff);
  fragColor0 = texture(pal, vec2((float(index) + 0.5) * (1.0 / 256.000000), 0.0));
}
daniel229 commented 8 years ago

comment out that line still get the errors,the crash is due to GfxDebugOutput = True

unknownbrackets commented 8 years ago

Hm. Mask is 0xff there. shiftedMask should be 0xff0000. But the bitdepth is 565. So it's effectively the same deal.

I wonder what happens in this case. If the tex format is CLUT16, does a shift of 16 zero it, or do nothing (maybe it masks bits)? Or does it even read the next texel (seems unlikely)? Probably it just zeros, and our behavior is correct, but we should really test this on hardware.

-[Unknown]

daniel229 commented 8 years ago

Also happen in METAL SLUG XX In this screen 01

unknownbrackets commented 6 years ago

So, the correct behavior is a rotate at 32 bits, regardless of index size (I say rotate to confirm that the top bit, 0x80, of clutformat is indeed ignored.)

For example, if the clut index value is 8 bit 0x12, then it's shifted based on a zero padded 32 bit value. That is, 0x00000012.

This means that the correct behavior in this case, indeed, is clut entry 0.

-[Unknown]

unknownbrackets commented 6 years ago

I'm going to close this - I suspect the warning is fixed with the GLES refactoring in 1.6.x, since it uses glBindAttribLocation now instead of glGetAttribLocation. We can reopen if not. -[Unknown]