Notable graphical errors in Super Robot Wars MX Portable

[aquanull]

Blending seems wrong:

Might be related with the graphical errors in Super Robot Wars A Portable:
#834 (comment)

[unknownbrackets]

Has this always been the case, or did it get worse recently?

Ys 6: The Ark of Napishtim also has these sorts of issues.

-[Unknown]

[aquanull]

I think it has been always like this, except for that screen stretching/aligning problem fixed already (well, mostly) as noted in #834 (comment).

[raven02]

Interesting it only happen on Portable MX but not Portable A

[aquanull]

@raven02: In SRWAP everything except the text is wrongly semi-transparent in battle scenes and I think that issue should be reopened.

[raven02]

I don't own this game .Can you retest it using latest build ? (i think should be fixed)

[hrydgard]

It might be using one of the blend modes that we can't support 100% correctly in OpenGL - when those are encountered we use GL_ONE,GL_ONE instead (additive), which is what it looks like here. See StateMapping.cpp .

Some "impossible" modes can definitely be approximated better.

[aquanull]

@raven02: The graphics are still broken with the latest build as they were with the old builds. Graphics with JPCSP are almost all right without enabling any accuracy fix like SW Rendering mode in it. So I think some modes can be approximated in PPSSPP as hrydgard suggested.

[raven02]

How's the MX right now ? not too sure the double alpha helps

[hrydgard]

JPCSP does some OpenGL trickery that's not possible on OpenGL ES 2.0 though (it reads from the framebuffer and simulates the extra blend modes in the shader).

[raven02]

I see .Thanks @hrydgard

[raven02]

@hrydgard , just wonder from the screenshot below , what would be the likely cause ... blend modes , lighting or alphaTest issue?

[hrydgard]

My first guess would be that the wrong CLUT is being used. Hm.

[unknownbrackets]

Yeah, that does look like wrong clut... it could also be that the wrong mask/offset/etc. is being used, I haven't been able to find many games using those, but I think it should work.

If you disable the texture cache (bool match = false;) does it fix it? If not, it could still be the clut, maybe wrong order of clut load or something...

But it could also be blend/alpha/colortest even or logicop.

-[Unknown]

[raven02]

I see and thanks . Looks like CLUT concluded to be a big offender . I will disable the texture cache CLUT stuff to see if this text issue can be fixed .

(AlphaTest and ColorTest already checked and seems to be not the culprit)

[raven02]

@unknownbrackets , just wonder what does "Texture with unexpected bufw (full=43926)" means? (CLUT related ?)

http://report.ppsspp.org/temp/recent/kind/93

[unknownbrackets]

That's interesting.

Well, the "bufw" is the stride for the buffer that holds the texture. Each line is that many pixels iirc. So, 43926, even with a 4-bit texture, would mean 21963 bytes. That sounds crazy. It might help to add the w/h to that message to see, I guess it could be a texture that is 65536 pixels wide and umm... 1 pixel tall or something? Anyway, it sounds like a bug, it probably should not get that value in the first place.

-[Unknown]

[raven02]

Humm seems to match the above text issue with the this texture with unexpected bufw message

[raven02]

Just tried with bool match = false; however seems not helping .... any other thing you can think of that i can try it

    int dim = gstate.texsize[0] & 0xF0F;
    //bool match = entry->Matches(dim, format, maxLevel);
    bool match = false;
    bool rehash = (entry->status & TexCacheEntry::STATUS_MASK) == TexCacheEntry::STATUS_UNRELIABLE;
    bool doDelete = true;

[aquanull]

@raven02 Still no luck with MX Portable.Mind sharing your fix that (finally!) got Z2 to run?

EDIT: Oh I saw #1922 just now.

[unknownbrackets]

Well, that should make it regenerate the texture each time and not use the cache (it should be slower.)

It is suspicious that it sends such a large bufw. I doubt it'd help but you can try "unlocking" bufw:

static inline u32 GetLevelBufw(int level, u32 texaddr) {
    // Special rules for kernel textures (PPGe):
    if (texaddr < PSP_GetUserMemoryBase())
        return gstate.texbufwidth[level] & 0x1FFF;
    //return gstate.texbufwidth[level] & 0x7FF;
    return gstate.texbufwidth[level] & 0xFFFF;
}

At least, does that change anything?

-[Unknown]

[raven02]

Just tried using "return gstate.texbufwidth[level] & 0xFFFF;" however didn't change anything .....

[unknownbrackets]

Hmm. It might help to determine which type of texture it is. One way is to log all textures and guess. Another is to disable some of the texture decoding by type, and then when it dissapears / becomes garbage, you found it. Maybe it's some DXT texture we don't decode properly...

Or, more likely, is it indexed?

-[Unknown]

[raven02]

Let me disable texture decoding by type to see which disappears or become garbage , would be more easier

[raven02]

Most of the problem text disappears when disable this one (Tested all others like DXT , CLUT are no effect)

    case GE_CMODE_32BIT_ABGR8888:
        {
        tmpTexBuf32.resize(std::max(bufw, w) * h);
        tmpTexBufRearrange.resize(std::max(bufw, w) * h);
        const u32 *clut = GetCurrentClut<u32>() + clutSharingOffset;
        if (!(gstate.texmode & 1)) {
            DeIndexTexture4(tmpTexBuf32.data(), texaddr, bufw * h, clut);
            finalBuf = tmpTexBuf32.data();
        } else {
            UnswizzleFromMem(texaddr, bufw, 0, level);
            // Let's reuse tmpTexBuf16, just need double the space.
            tmpTexBuf16.resize(std::max(bufw, w) * h * 2);
            DeIndexTexture4((u32 *)tmpTexBuf16.data(), (u8 *)tmpTexBuf32.data(), bufw * h, clut);
            finalBuf = tmpTexBuf16.data();
        }
        }

[raven02]

It hits the else part ( or we miss the case if (clutAlphaLinear_ && mipmapShareClut) { ?

        UnswizzleFromMem(texaddr, bufw, 0, level);
        // Let's reuse tmpTexBuf16, just need double the space.
        tmpTexBuf16.resize(std::max(bufw, w) * h * 2);
        DeIndexTexture4((u32 *)tmpTexBuf16.data(), (u8 *)tmpTexBuf32.data(), bufw * h, clut);
        finalBuf = tmpTexBuf16.data();

[unknownbrackets]

Interesting. With a font, CLUTs are usually linear. I wonder why it's not here. That is strange.

I'm not sure about the unswizzling, it seems to work but I don't think I've seen it with a 4 bit texture off the top of my head... Maybe it's not working properly for those. Hmm.

-[Unknown]

[raven02]

I did try to put the clause if (clutAlphaLinear_ && mipmapShareClut) similar to those CMODE_16BIT .However still not able to fix the text issue . It is good that at least narrow down to this GE_CMODE_32BIT_ABGR8888 issue.

I'm wondering anything i can try to change for this GE_CMODE_32BIT_ABGR8888 and test it out again ...............

[unknownbrackets]

Hmm, I'm not sure. What this means is that it's a texture that uses 4-bits per pixel, and a 32-bit clut. And it's swizzled, which means the pixels aren't in the normal order.

I think the next most interesting things are:

1. Log info about the texture (ideally right there before UnswizzleFromMem): NOTICE_LOG(HLE, "Possibly the texture: %dx%d (bufw=%d, fullbufw=%x, fullsize=%x, clut offset=%d, clut format=%x, loaded=%x)", w, h, bufw, gstate.texbufwidth & 0xffff, gstate.texsize, clutSharingOffset, gstate.clutformat, gstate.loadclut);
2. Overwrite the CLUT and see if it helps: for (int i = 0; i < 16; ++i) clut[i] = 0xFF00FFFF | (i << 20); (it will be wrong but I wonder what it will look like...)

-[Unknown]

[raven02]

Thanks for the tips .Let me try all these out in next few hrs (in office now)

[raven02]

I think should be gstate.texbufwidth[0] & 0xffff , right?

33:03:791 Z2_FILE_READ I[HLE]: HLE\sceIo.cpp:1476 Decrypting PGD DRM files
33:03:923 user_main N[HLE]: GLES\TextureCache.cpp:1159 Possibly the texture: 256x256 (bufw=256, fullbufw=256x, fullsize=11337072x, clut offset=0, clut format=c509ff03, loaded=c4000020)
33:03:926 user_main N[HLE]: GLES\TextureCache.cpp:1159 Possibly the texture: 256x256 (bufw=256, fullbufw=256x, fullsize=11337072x, clut offset=0, clut format=c500ff03, loaded=c4000020)
33:14:289 user_main N[HLE]: GLES\TextureCache.cpp:1159 Possibly the texture: 256x256 (bufw=256, fullbufw=256x, fullsize=11337072x, clut offset=0, clut format=c500ff03, loaded=c4000002)
33:14:290 user_main N[HLE]: GLES\TextureCache.cpp:1159 Possibly the texture: 256x256 (bufw=256, fullbufw=256x, fullsize=11337072x, clut offset=0, clut format=c500ff03, loaded=c4000002)
33:14:292 user_main N[HLE]: GLES\TextureCache.cpp:1159 Possibly the texture: 512x512 (bufw=512, fullbufw=512x, fullsize=11337072x, clut offset=0, clut format=c503ff03, loaded=c4000020)
33:14:299 user_main N[HLE]: GLES\TextureCache.cpp:1159 Possibly the texture: 512x512 (bufw=512, fullbufw=512x, fullsize=11337072x, clut offset=0, clut format=c503ff03, loaded=c4000020)

[raven02]

Humm however "1>GLES\TextureCache.cpp(1183): error C3892: 'clut' : you cannot assign to a variable that is const"

    case GE_CMODE_32BIT_ABGR8888:
        {
        tmpTexBuf32.resize(std::max(bufw, w) * h);
        tmpTexBufRearrange.resize(std::max(bufw, w) * h);
        const u32 *clut = GetCurrentClut<u32>() + clutSharingOffset;
        if (!(gstate.texmode & 1)) {
            DeIndexTexture4(tmpTexBuf32.data(), texaddr, bufw * h, clut);
            finalBuf = tmpTexBuf32.data();
        } else {
            for (int i = 0; i < 16; ++i) clut[i] = 0xFF00FFFF | (i << 20);
            NOTICE_LOG(HLE, "Possibly the texture: %dx%d (bufw=%d, fullbufw=%dx, fullsize=%dx, clut offset=%d, clut format=%x, loaded=%x)", w, h, bufw, gstate.texbufwidth[0] & 0xffff, gstate.texsize, clutSharingOffset, gstate.clutformat, gstate.loadclut);
            UnswizzleFromMem(texaddr, bufw, 0, level);
            // Let's reuse tmpTexBuf16, just need double the space.
            tmpTexBuf16.resize(std::max(bufw, w) * h * 2);
            DeIndexTexture4((u32 *)tmpTexBuf16.data(), (u8 *)tmpTexBuf32.data(), bufw * h, clut);
            finalBuf = tmpTexBuf16.data();
        }
        }

[unknownbrackets]

Oh, right. Just change this:

const u32 *clut = GetCurrentClut<u32>() + clutSharingOffset;

To:

u32 *clut = (u32 *)GetCurrentClut<u32>() + clutSharingOffset;

-[Unknown]

[raven02]

Compile okay now . This is the screenshot .

This is original one

[unknownbrackets]

Well, the letters seem clear this way, so it definitely seems like the clut is incorrect.

Hmm, looking at the original, maybe it could sitll be a logic op or blend. It seems like the "Normal" boxes are not intentional...

-[Unknown]

[raven02]

I see. If you have any possible fix want me to try it out , let me know .Thanks.

[raven02]

This is the one i got from jpcsp and i think it looks correct

[unknownbrackets]

Hmm, it seems like it's everywhere it's trying to use a color other than white. Is the black box around the unit/thing to the left of the cursor correct also (the shadow certainly looks smoother)?

Can you paste a link to a frame dump?

-[Unknown]

[raven02]

The black box can be corrected by enabling the dynamic shader generation in jpcsp

This is the frame dump log (rename from .jpg to .log )

[unknownbrackets]

Hmm, it uses z, stencil, color, and alpha tests. I don't see any funny bufw.

I don't see any funny clut loads...

-[Unknown]

[daniel229]

this bug existed in jpcsp older build like r2168,fixed in r2169 or 2170.I can't find r2169 to test.accroding to the Log message,should be 2169.
http://code.google.com/p/jpcsp/source/detail?r=2169

[raven02]

Humm exactly same issue . It try to shift the clutoffset with 4

shaderContext.setClutOffset(context.tex_clut_start << 4);

Many thanks @daniel229 to find out this :)

[unknownbrackets]

Thanks @daniel229, nice find.

@raven02, try this:

static u32 GetClutIndex(u32 index) {
    const u32 clutBase = (gstate.clutformat & 0x1f0000) >> 12;
    const u32 clutMask = (gstate.clutformat >> 8) & 0xff
    const u8 clutShift = (gstate.clutformat >> 2) & 0x1f;
    return ((index >> clutShift) & clutMask) | clutBase;
}

-[Unknown]

[daniel229]

wow~~,fixed,thanks.@unknownbrackets

[raven02]

Prefect :) thanks @unknownbrackets and @daniel229

[daniel229]

same issue as Super Robot Wars MX Portable,but color is black

[unknownbrackets]

That looks a lot more like a blend issue (but I'm not really sure.)

-[Unknown]

[daniel229]

don't know this log can help or not.I dump next frame to log when I saw the black block
https://docs.google.com/file/d/0BzGZGDfFE68zTlpDdXhsWG5MTk0/edit?usp=sharing

[raven02]

@daniel229 , do you this game save ? i would like to test it since i tested them but didn't see those black block

[daniel229]

yes,try this savefile,and attack this monster which you see in the picture.

savefile download
https://docs.google.com/file/d/0BzGZGDfFE68zbVkxcm44WUxBYmM/edit?usp=sharing

[unknownbrackets]

It's using blends like "abs subtract a, a", "add src.a, 1.0 - src.a", and "add src.a, fixed". Theoretically these should work properly but they could be buggy.

-[Unknown]

[aquanull]

@unknownbrackets @raven02: It might seem improved (thanks to #1977) with the auto-builds:

Frame dump (big):
https://docs.google.com/file/d/0B5hoahZqlUL3WUx4OUphTFRWaW8/edit?usp=sharing

But with personal builds:
Debug

Release

[unknownbrackets]

That's without any code changes, and with Visual Studio 2010?

-[Unknown]

[hrydgard]

If so, it smells like an uninitialized variable somewhere.

[aquanull]

@hrydgard @unknownbrackets Yep.

EDIT: It turned out to be indeed uninitialized buffer usage.

[unknownbrackets]

How does this look now?

-[Unknown]

[raven02]

I have tested it on PC and it looks all okay (though not too sure on Android)