Dragon Ball Z Budokai Tenkaichi Tag Team (dbz) dark with artifacts

[dbz400]

Since git-981 , DBZ Vs Tag broken as follow

-Non-buffered mode

-Buffered mode

[vsub]

Same here...the problem is somewhere between 974 and 981

[brujo5]

yeah,please fix as soon posible

[unknownbrackets]

Seems to be rendering a texture using CLUT:
http://report.ppsspp.org/logs/game/ULUS10537_1.00

That means it was drawing random memory data before, most likely.

-[Unknown]

[brujo5]

for now aviod updates my ppsspp emu.lol

stay in v.8.974

this is the most enjoyable fighting game for me:(

[unknownbrackets]

It's hitting these:
Render to texture with different formats 7 != 3 at 00088000
Render to texture with different formats 6 != 0 at 00158000

7 and 3 are CLUT and non-CLUT 32-bit. 6 and 0 are CLUT 16-bit and 565. So, this probably needs the palette translation to be correct. (this: // TODO: Use an FBO to translate the palette?)

Temporarily commenting out only this case would fix it, but would break 3rd Birthday again and possibly that other game which has a similar issue:

                    WARN_LOG_REPORT_ONCE(diffFormat2, HLE, "Render to texture with different formats %d != %d at %08x", entry->format, framebuffer->format, address);
                    // TODO: Use an FBO to translate the palette?
                    entry->framebuffer = framebuffer;

I doubt there's a way to have both happy without implementing the palette translation, since they use similar methods. Here's 3rd Birthday:

Render to texture with different formats 7 != 3 at 00000000
Render to texture with different formats 6 != 2 at 04154000

-[Unknown]

[dbz400]

Wondering if same format , we should be safe to bind to FBO ?

[unknownbrackets]

If it's the same format, then it will have been a direct match and follow the old logic. In fact, the old logic (for a direct match) did not care about formats which may even be wrong and could be causing the Kingdom Hearts issue:

        // If they match exactly, it's non-CLUT and from the top left.
        if (it->first == cacheKey) {
            DEBUG_LOG(HLE, "Render to texture detected at %08x!", address);
            if (!entry->framebuffer) {
                if (entry->format != framebuffer->format) {
                    WARN_LOG_REPORT_ONCE(diffFormat1, HLE, "Render to texture with different formats %d != %d", entry->format, framebuffer->format);
                }
                entry->framebuffer = framebuffer;
                // TODO: Delete the original non-fbo texture too.
            }

Assuming it's NOT a direct match, there are only two possibilities:

• It's not a direct match because there's a CLUT.
• It's not a direct match because the address is slightly different.

So those are the two cases that are checked for. There's really no need to consider the impossible case where the formats and addresses match exactly inside the else. Just like we don't need to check that the texture isn't a completely different address than the FBO either. Those are knowns.

-[Unknown]

[dbz400]

Thanks @unknownbrackets .I think i bit more understanding how this logic works

[dbz400]

Meanwhile , git-981 also breaks God of War series as reported from other forum . Black texture.

[brujo5]

so is possible revert commit v8.981 until mplement the palette translation?

thx in advance.

ps..sorry for my stupid question.

[dbz400]

@unknownbrackets , just wonder what would be the idea behind to use an FBO to translate the palette? I would like to try it .

so for example in DBZ Tag VS

Render to texture with different formats 7 != 3 at 00088000

7 is GE_TFMT_CLUT32 and 3 is GE_FORMAT_8888

so we need to translate framebuffer format from 3 to 7?

[unknownbrackets]

Well, the idea would be to use a shader. I don't know if it will work but here's a brief rundown of what I was thinking:

1. Bind the source framebuffer's color texture as a source texture when encountering a render-to-framebuffer with a CLUT format.
2. Create a new temporary framebuffer (maybe store this in a map? not sure.) Probably want this to live as long as the texture does.
3. Create a "texture" for the palette, probably just 1px wide and as tall as there are palette entries.
4. The shader will use both textures as samplers, I think.
5. Possibly could use an unsigned format for the texture to improve accuracy? Probably slow. Float may be fine. Maybe increase the size of the palette "entries" if it helps?
6. Make sure to disable linear filtering and enable clamping when translating.
7. Make sure that there's an even ratio between the source framebuffer pixels and the output texture pixels.
8. Perform the translation by running the shader.
9. When binding the texture, bind the output texture, not the framebuffer.
10. Use the normal clamping / filter options when binding the output texture (just not during translation.)
11. Potentially detect changes in the framebuffer (using is dirty / etc.?) to "cache" it, if the above is slow which it probably will be.
12. Make sure to still decimate the additional textures/framebuffers.

-[Unknown]

[dbz400]

Humm seems to be complicated and understood previously why you would like to find other way to make that 2 games happy without implementing the palette translation

[hrydgard]

There is one alternative way that doesn't require you to create a new framebuffer when sampling from a paletted texture: Simply generate different shaders that incorporate the palette lookup, when a "paletted FBO" is bound. (add an extra bit or two to the fragment shader ID, conditionally generate the corresponding shader code, and do a small amount of setup code creating the palette texture etc in ApplyState).

[unknownbrackets]

But then you have to deal with linear filtering and stuff, right? Well, it would probably be a lot faster, so may be worth it...

-[Unknown]

[hrydgard]

Yes, although I would guess that many uses would be for full screen effects and similar, in which case filtering doesn't matter.

[unknownbrackets]

Interestingly, this game uses an unsupported blending mode, absdiff.

-[Unknown]

[dbz400]

Humm wondering this type of blending mode we can implement for desktop OpenGL ?

46:16:714 I[SCEGE]: GLES\Framebuffer.cpp:603 Creating FBO for 00158000 : 480 x 272 x 0
46:16:714 W[G3D]: GLES\TextureCache.cpp:207 Render to texture with different formats 7 != 3 at 04000000
46:16:714 I[G3D]: GLES\ShaderManager.cpp:104 Linked shader: vs 49 fs 50
46:16:714 W[G3D]: GLES\StateMapping.cpp:230 Unsupported absdiff blend mode
46:16:715 I[G3D]: GLES\ShaderManager.cpp:104 Linked shader: vs 52 fs 23

[dbz400]

Just curious , any other games use this GE_BLENDMODE_ABSDIFF blending mode from your report?

[unknownbrackets]

http://report.ppsspp.org/logs/kind/353

I'm not sure that GL can do it, but I wonder if GL_FUNC_SUBTRACT would be a better approximation. By the way, this seems like a nice way to visualize the blending:
http://www.andersriggelsen.dk/glblendfunc.php

-[Unknown]

[dbz400]

If we want to test it using GL_FUNC_SUBTRACT , which of the following is better?

     GL_FUNC_ADD, // should be abs(diff)

change to

    GL_FUNC_SUBTRACT, // should be abs(diff)

[dbz400]

or simply set blendFunEq to GE_BLENDMODE_MUL_AND_SUBTRACT

    if (blendFuncEq == GE_BLENDMODE_ABSDIFF) {
        blendFunEq = GE_BLENDMODE_MUL_AND_SUBTRACT;
        WARN_LOG_REPORT_ONCE(blendAbsdiff, G3D, "Unsupported absdiff blend mode");
    }

[dbz400]

Unfortunately , I cannot test game right now .If anybody able to compile and own this game , feel free to test it .

[daniel229]

I tested these codes.black background with Buffered Rendering.

[dbz400]

Thanks @daniel229 .Or may be if we use GL_FUNC_SUBTRACT, it involves dstfactor and srcfactor so not correct .

[unknownbrackets]

So, this game does self-texturing, but I'm not sure that's the only thing wrong.

It looks okay in softgpu, but is unbearably slow without generous helpings of frameskip.

I've attempted to implement a (possibly very slow) self-texturing fix (which does fix Brave Story's artifacts.) However, it does not fix this game (at least not alone.) I think the problem is rendering using a CLUT.

So, to test this, I also tried a (probably very very slow) method of forcing CLUT'd framebuffers to read the framebuffer first, and the demo looks okay now. So, at least, we can confirm that fixing that will help this game (as well as 3rd Birthday.)

-[Unknown]

[dbz400]

That's great .If demo works , this retail version should work as well.

[unknownbrackets]

This screenshot from softgpu illustrates what the game seems to be attempting to accomplish:

It uses this to draw the outlines.

Currently, GLES looks like this with #5767 (for the texture on the left above):

-[Unknown]

[hrydgard]

Ah very cool.

Is the bottom image at the same "step" as the top one? If so I wonder what's going wrong.. it's not using a "logic op" drawing mode, is it?

[unknownbrackets]

Yes, unfortunately the GE debugger doesn't work entirely right for CLUT textures (since it's a different thread/context), so things turn grey if I use it, but names are shared (I'm not sure where it's getting confused, maybe RebindFramebuffer...) and I'm pretty sure this step is right.

Oh, that's weird. I just ran it and only the characters are black, but there's no code difference... the bg looks more correct now, although the character's hair is wrong. How did that happen? Ran it a second time and it's all dark now.... hmmm....

It's only using alpha blend.

-[Unknown]

[unknownbrackets]

Here's a picture of the CLUT (I think I did it right, just using v_texcoord0.x as the index):

-[Unknown]

[unknownbrackets]

So, this first framebuffer is built using alpha values. It extracts the alpha component and & 0x7fs it.

It initially clears the stencil to 0xff, afaict. It doesn't bother to clear color, only stencil/depth.

When drawing character 1, it uses ALWAYS KEEP/KEEP/REPLACE to draw 0x03-0x0e, 0x12-0x1e for the next character, etc. Basically each one gets a range of 0-0x0f for its body parts, I guess.

That feeds directly into this texture. Since we're using REPLACE, we should theoretically get an exact value. That means we're either off in our stencil->alpha, or we're off in the palette index.

Adding a fixed (1.0 / 512.0) to the index makes it select the right colors for the characters, at least. The bg is a farther-off violet, though.

-[Unknown]

[hrydgard]

Hm. We can easily be off-by-one in stencil->alpha as you say - just as with color values in general, we convert the fixed stencil value to a floating point shader output which then gets converted back to an effective integer by the rasterizer. That round trip must preserve the value exactly. Or we could of course also be off in the palette lookup indeed...

As for the bg color being differently wrong, maybe we are failing to write the correct stencil value to the alpha channel at all there, depending on how it's drawn?

[unknownbrackets]

Using:

WRITE(p, "  if (a == 0xff) fragColor0.r = 1.0; else fragColor0.r = 0.0;\n");

(conveniently the red value is ignored in the next clut stage...)

Shows that the alpha of the background is exactly 0xff. Doing the same thing with the hair (0xe) shows that it is indeed 0xe, and that should be translating to green but is instead cyan (index 0xd). I'm suspicious of the - 0.5.

Anyway, it still doesn't work with that. It's using abs subtract dst,src blending, which is probably why...

-[Unknown]

[unknownbrackets]

Well, I think the - 0.5 is incorrect. For example, this:

WRITE(p, "  fragColor0 = texture2D(pal, vec2((floor(float(color))) * (1.0 / %f), 0.0));\n", texturePixels);

Would scale a source value from [0, 255] to [0, 0.9961]. But this might be inaccurate if the float errors downward, since it's at the "bottom" of each pixel. 127 gets 0.4961 and 128 gets 0.5.

WRITE(p, "  fragColor0 = texture2D(pal, vec2((floor(float(color))) * (1.0 / %f), 0.0));\n", texturePixels - 1.0f);

Would scale most naturally to [0, 1.0], but I'm not sure if that's the correct scaling. If there are 256 texels, which texel is 0.5 closest to? This will output 0.498 and 0.502 for 127/128.

WRITE(p, "  fragColor0 = texture2D(pal, vec2((floor(float(color)) + 0.5) * (1.0 / %f), 0.0));\n", texturePixels);

Would scale to [0.00195, 0.99805]. 127 gets 0.49805 and 128 gets 0.50195, so it's obviously very close to the above in the middle.

WRITE(p, "  fragColor0 = texture2D(pal, vec2((floor(float(color)) - 0.5) * (1.0 / %f), 0.0));\n", texturePixels);

The current logic, which has got to be wrong I think. This would scale to [-0.5, 0.99414]. 127 gets 0.49414 and 128 gets 0.49805.

This indicates that the + is probably right:
http://stackoverflow.com/questions/5879403/opengl-texture-coordinates-in-pixel-space/5879551#5879551

-[Unknown]

[unknownbrackets]

Fixing that, and doing this atrocity, gives me the edges just fine (by the way, I don't see how the NV_shader_framebuffer_fetch stuff was at all working...):

diff --git a/GPU/GLES/FragmentShaderGenerator.cpp b/GPU/GLES/FragmentShaderGenerator.cpp
index a5f6638..74bdd2d 100644
--- a/GPU/GLES/FragmentShaderGenerator.cpp
+++ b/GPU/GLES/FragmentShaderGenerator.cpp
@@ -424,6 +424,9 @@ void GenerateFragmentShader(char *buffer) {

    if (doTexture)
        WRITE(p, "uniform sampler2D tex;\n");
+   if (computeAbsdiff && !gl_extensions.NV_shader_framebuffer_fetch) {
+       WRITE(p, "uniform sampler2D fbotex;\n");
+   }

    if (enableAlphaTest || enableColorTest) {
        WRITE(p, "uniform vec4 u_alphacolorref;\n");
@@ -605,7 +608,10 @@ void GenerateFragmentShader(char *buffer) {
    // Handle ABSDIFF blending mode using NV_shader_framebuffer_fetch
    if (computeAbsdiff && gl_extensions.NV_shader_framebuffer_fetch) {
        WRITE(p, "  lowp vec4 destColor = gl_LastFragData[0];\n");
-       WRITE(p, "  gl_FragColor = abs(destColor - v);\n");
+       WRITE(p, "  v = abs(destColor - v);\n");
+   } else if (computeAbsdiff) {
+       WRITE(p, "  lowp vec4 destColor = %s(fbotex, vec2(gl_FragCoord.x / 480.0, gl_FragCoord.y / 272.0));\n", texture);
+       WRITE(p, "  v = abs(destColor - v);\n");
    }

    switch (stencilToAlpha) {
diff --git a/GPU/GLES/Framebuffer.cpp b/GPU/GLES/Framebuffer.cpp
index 4db0c33..d33745c 100644
--- a/GPU/GLES/Framebuffer.cpp
+++ b/GPU/GLES/Framebuffer.cpp
@@ -984,6 +984,10 @@ void FramebufferManager::BindFramebufferDepth(VirtualFramebuffer *sourceframebuf
 }

 void FramebufferManager::BindFramebufferColor(VirtualFramebuffer *framebuffer) {
+   if (framebuffer == NULL) {
+       framebuffer = currentRenderVfb_;
+   }
+
    if (!framebuffer->fbo || !useBufferedRendering_) {
        glBindTexture(GL_TEXTURE_2D, 0);
        gstate_c.skipDrawReason |= SKIPDRAW_BAD_FB_TEXTURE;
diff --git a/GPU/GLES/ShaderManager.cpp b/GPU/GLES/ShaderManager.cpp
index 08c4d41..4b0e9b9 100644
--- a/GPU/GLES/ShaderManager.cpp
+++ b/GPU/GLES/ShaderManager.cpp
@@ -146,6 +146,7 @@ LinkedShader::LinkedShader(Shader *vs, Shader *fs, u32 vertType, bool useHWTrans
    INFO_LOG(G3D, "Linked shader: vs %i fs %i", (int)vs->shader, (int)fs->shader);

    u_tex = glGetUniformLocation(program, "tex");
+   u_fbotex = glGetUniformLocation(program, "fbotex");
    u_proj = glGetUniformLocation(program, "u_proj");
    u_proj_through = glGetUniformLocation(program, "u_proj_through");
    u_texenv = glGetUniformLocation(program, "u_texenv");
@@ -247,6 +248,7 @@ LinkedShader::LinkedShader(Shader *vs, Shader *fs, u32 vertType, bool useHWTrans

    // Default uniform values
    glUniform1i(u_tex, 0);
+   glUniform1i(u_fbotex, 1);
    // The rest, use the "dirty" mechanism.
    dirtyUniforms = DIRTY_ALL;
    use(vertType, previous);
diff --git a/GPU/GLES/ShaderManager.h b/GPU/GLES/ShaderManager.h
index b589b66..1e1eeb2 100644
--- a/GPU/GLES/ShaderManager.h
+++ b/GPU/GLES/ShaderManager.h
@@ -60,6 +60,7 @@ public:

    int u_stencilReplaceValue;
    int u_tex;
+   int u_fbotex;
    int u_proj;
    int u_proj_through;
    int u_texenv;
diff --git a/GPU/GLES/StateMapping.cpp b/GPU/GLES/StateMapping.cpp
index 130ee16..e5bf9cd 100644
--- a/GPU/GLES/StateMapping.cpp
+++ b/GPU/GLES/StateMapping.cpp
@@ -176,6 +176,12 @@ void TransformDrawEngine::ApplyDrawState(int prim) {

    // Set blend
    bool wantBlend = !gstate.isModeClear() && gstate.isAlphaBlendEnabled();
+   if (wantBlend && gstate.getBlendEq() == GE_BLENDMODE_ABSDIFF && !gl_extensions.NV_shader_framebuffer_fetch) {
+       glActiveTexture(GL_TEXTURE1);
+       framebufferManager_->BindFramebufferColor(NULL);
+       glActiveTexture(GL_TEXTURE0);
+       wantBlend = false;
+   }
    glstate.blend.set(wantBlend);
    if (wantBlend) {
        // This can't be done exactly as there are several PSP blend modes that are impossible to do on OpenGL ES 2.0, and some even on regular OpenGL for desktop.

Probably not a good idea for performance, though, and I hacked in the framebuffer size due to exccessive laziness. Obviously this would not work if there was polygon overlap in the combined drawcall.

-[Unknown]

[dbz400]

Very nice .Previous i tried the texelFetch #5917 (i think do similar thing) but not working :(

[dbz400]

TexelFetch() is working okay now :)

  WRITE(p, "  lowp vec4 destColor = texelFetch(fbotex, ivec2(gl_FragCoord.xy), 0);\n");

[unknownbrackets]

I don't have texelFetch, I'm on OpenGL 3. That or I didn't specify the version correctly.

-[Unknown]

[dbz400]

I see. And texelFetch also only availabe starting from ES 3.0

EDIT: Not too sure , i seen the support matrix , it is saying that it supports since OpenGL 1.3 on desktop

https://www.opengl.org/sdk/docs/man4/html/texelFetch.xhtml

[dbz400]

Probably we can do those 2x blending mode as well natively and support for all platforms and get rid of that GL_NV_framebuffer_fetch.

[unknownbrackets]

No wait, I just didn't have the third parameter.

The shadows are still wrong...

-[Unknown]

[dbz400]

The shadow probably related to the framebuffer size. I remember Read to framebuffer mode did show the shadow correct .

[dbz400]

Looks like it only work when apply to framebuffer-clut branch only but not master.....

[unknownbrackets]

Right. Specifically what this game is doing is pretty neat, although I can't imagine it's very fast.

1. Draw the scene. For different cells, use different stencil values.
2. Use a CLUT to map the stencil values to colors. This gives the green/blue blocky version.
3. Redraw this same texture inset by a couple pixels, with absdiff blending. This is the neat trick: it nulls out all the same colors, and the inset allows it to create borders.
4. Use another CLUT to turn the borders into gray.
5. Blend or something (didn't look at this closely) those greys to get the nice black borders.

It's a good effect (although it looks to me like it would involve a lot of stalling.) It requires both absdiff and framebuffer-clut to work.

-[Unknown]

[brujo5]

This fix work for all plataforms?

[dbz400]

Probably you can try out when merged .Desktop should be working at all .Mobile probably ES 3.0 only.

(On ES 2.0 , possibly use texture2D above and setup uniform for framebuffer size)

[brujo5]

thanks for replying

I hope it works also on iphone 5S, hopefully someone can fix the bad ES 3.0 initialization,as henrik says.

[hrydgard]

The "atrocity" solution (rendering to the active texture to simulate custom blends), while cool, is "supposed" (hinted by GPU manufacturers) to work reliably on most desktop chips (non-tiled renderers) only if:

 * The same pixel is being drawn to as is read as a texel
 * The polygons drawn do not self intersect

Which may be the case here? (although the 1-pixel offset would contradict this, hm)

However it's unlikely to work on tiled GPUs like PowerVR so it won't be a universal solution, but we can hide that path behind a feature flag that we set during the following conditions:

* !defined(MOBILE_DEVICE)
* gpuVendor one of [NVIDIA, ATI, INTEL]

[unknownbrackets]

Well, that function automatically makes a copy by blitting (which will be slow) of the framebuffer if it is the current one, which it will be.

I'm mostly concerned about it's potentially debilitating slowness.

-[Unknown]

[hrydgard]

Oh, right, I didn't look very carefully. Then it's fine of course. GPUs are pretty fast at blitting so won't be much of an issue for desktop chips at least.

Also, I don't see where the "stalls" would be in the method the game is using. There's no readback to the CPU anywhere, it's just a lot of pretty cheap passes...

[unknownbrackets]

Oh, I assumed that binding a framebuffer as a texture needed to wait for it to draw.

-[Unknown]

[hrydgard]

It does but current GPUs aren't parallel in the way you might imagine - it can't start drawing a new thing beforce the last draw is completed anyway. Switching render targets does involve flushing some caches in the GPU, and maybe something extra needs to be flushed if you use it as a texture, but it's not a huge deal. Should avoid switching render targets per draw call in general though obviously but a bunch of switches per frame is okay.

[unknownbrackets]

So, this should now be working on GLES 2 as well. I'm going to close it, but if it's still a problem, please comment/reopen.

I think we have a dupe anyway...

-[Unknown]