God of war's graphics issue

[arcade1977]

981~1067:

[arcade1977]

I don't know why can not upload pictures:(

Please look it: http://tieba.baidu.com/p/2507244709

[arcade1977]

@unknownbrackets ,can you fix god of war's graphics issue since 981 ( 974 still OK,but 981 is destroyed) ?

[unknownbrackets]

I don't have this game, but I think #3029 may work around it for now.

-[Unknown]

[dbz400]

I have retested that scene and confrim working okay with #3029

[arcade1977]

At 0.81.1167, this problem appeared again:(

[arcade1977]

http://tieba.baidu.com/p/2520154867

[arcade1977]

好像不是 #3082 的问题，1165 战神就有这个问题了，但是1152是正常的，我正在测试到底是哪个版本开始破坏它的

[arcade1977]

OK，找到把战神破坏的版本了，是v0.8.1-1158

[hrydgard]

English in reports please, at least in addition to the Chinese.

[dbz400]

@hrydgard , they're mentioning the broken commit is git 1158

[Rename gpuStats.numFrames to numVBlanks. Switch to using numFlips for things like cache expirations.]

[unknownbrackets]

Please reopen if this isn't fully fixed. Thanks @raven02.

-[Unknown]

[dbz400]

Same issue happened again since i changed from invalid to valid

        } else if ((entry->addr - address) / entry->bufw < framebuffer->height) {
            WARN_LOG_REPORT_ONCE(subarea, HLE, "Render to area containing texture at %08x", address);
            // TODO: Keep track of the y offset.
            AttachFramebufferValid(entry, framebuffer);

[dbz400]

@unknownbrackets , just wonder if MAX_SUBAREA_Y_OFFSET related to (entry->addr - address) / entry->bufw

[unknownbrackets]

@raven02: you have to think of all of this as MEMORY. Everything goes back to memory.

Let's use easy small numbers to make things simple to explain. My memory goes from 0 to 1000. Framebuffers are 200 bytes.

So let's say I do what's normal, and create two framebuffers: one at 0 and the other at 200. What I'm doing is essentially "allocating" the space from 0-199 for fb 1, and 200-399 for fb 2, right? These are the buffers I show on the screen.

Now suppose I draw a texture. I start this texture at 20. 20 is INSIDE 0-199.

If we assume this theoretical display uses a stride/width of 20, then the texture actually starts 1 pixel lower than the start of the framebuffer, right? If I say I want the texture starting at 20, and ending just before 80, then really I'm asking for this:

+-------------------------+
|  fb row 1 (0 - 19)      |
|  fb row 1 (20 - 39)     |   <-- texture starts here
|  fb row 1 (40 - 59)     |   <-- texture includes this line
|  fb row 1 (60 - 79)     |   <-- texture ends after this line
|  fb row 1 (80 - 99)     |
|  fb row 1 (100 - 119)   |
|  fb row 1 (120 - 139)   |
|  fb row 1 (140 - 159)   |
|  fb row 1 (160 - 179)   |
|  fb row 1 (180 - 199)   |
+-------------------------+

In other words, the texture is actually only 3px high, unlike my normal 10px high framebuffers (these are all simple numbers.)

So back to the formula and original question:

(entry->addr - address) / entry->bufw

= (texture address - framebuffer address) / stride

= (20 - 0) / 20
= 20/20
= 1

This tells me how many pixels (rows) below the start of the framebuffer the texture starts at.

-[Unknown]

[dbz400]

Humm it complicated than that i imagaine originally :)

So MAX_SUBAREA_Y_OFFSET = 32 means max 32 pixels below the start of framebuffer ?

[unknownbrackets]

Yes. The reason for it is that if I allowed large numbers (e.g. anything within the buffer), we got glitches in a lot of places. SAO in particular.

-[Unknown]

[dbz400]

Alright . In God of War , it keep tracek its offset like this

            // TODO: Keep track of the y offset.
            INFO_LOG(HLE,"offset=%i", (entry->addr - address) / entry->bufw);

30:09:212 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=16 30:09:212 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=18 30:09:212 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=19 30:09:215 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=2 30:09:215 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=3 30:09:215 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=2 30:09:215 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=3 30:09:216 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=16 30:09:216 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=18 30:09:216 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=19 30:09:226 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=2 30:09:226 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=3 30:09:226 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=2 30:09:226 user_main I[HLE]: GLES\TextureCache.cpp:219 offset=3

[dbz400]

However the max offset above is 19 which is still smaller than MAX_SUBAREA_Y_OFFSET but generate following black texture

[unknownbrackets]

Really strange. What are the texture heights and framebuffer heights for these cases? That's how I compared for other games.

-[Unknown]

[dbz400]

        } else if ((entry->addr - address) / entry->bufw < framebuffer->height) {
            WARN_LOG_REPORT_ONCE(subarea, HLE, "Render to area containing texture at %08x", address);
            // TODO: Keep track of the y offset.
            // Affected game List : God of War Ghost of Sparta , Tactic Orge , Sword Art Online
            INFO_LOG(HLE,"texture height = %i , framebuffer height=%i", entry->bufw , framebuffer->height);
            AttachFramebufferValid(entry, framebuffer);

[dbz400]

46:11:645 user_main I[HLE]: GLES\TextureCache.cpp:226 texture height = 512 , framebuffer height=272 46:11:647 user_main I[HLE]: GLES\TextureCache.cpp:226 texture height = 512 , framebuffer height=272 46:11:647 user_main I[HLE]: GLES\TextureCache.cpp:226 texture height = 512 , framebuffer height=272 46:11:647 user_main I[HLE]: GLES\TextureCache.cpp:226 texture height = 512 , framebuffer height=272

[dbz400]

I checked Tactic Orge and SOA which use subarea offset texture , texture height are 256 and 128 which is smaller than framebuffer height

[unknownbrackets]

Make sure you're checking the initial intro battle against the guy in red. That's where it has a ton of effects.

Just delete / rename savedata to see it.

-[Unknown]

[dbz400]

Yep , just checked it .It is that battle scene against those guy in red.

This one line fixes the God of War issue here while still maintains all stuffs in Tactic Orge and SOA

} else if ((entry->addr - address) / entry->bufw < framebuffer->height && entry->bufw < framebuffer->height) {

[unknownbrackets]

Okay, but that's obviously pure hackery, right? To be fair, limiting the height at all is hackery, but the bufw isn't really related to the height at all...

-[Unknown]

[dbz400]

Yep , however texture height is defintely bigger than that of framebuffer height in GOW so it explains the black textures?

[unknownbrackets]

To be clear (fun ascii art time again):

+------------------------------+
|                              |
|                              |  <-- texture starts here
|                              |  <-- texture ends here
|              FB              |
|                              |
|                              |
|                              |
+------------------------------+
          <-- bufw -->

In this example, we subtract the texture start memory address from the framebuffer address, and now we have how many bytes into the framebuffer the texture starts at (entry->addr - address).

The only reason to divide by bufw is to determine the y value (regardless of width) to compare it to the framebuffer height and make sure it's not outside the framebuffer. That check in other words is verifying that this is not happening:

+------------------------------+
|            SMALL FB          |
+------------------------------+
                                  <-- texture starts here

That said, as I write this, I wonder if it's not taking into account the bits per pixel correctly... bufw is not bytes, it's pixels.

-[Unknown]

[dbz400]

Basically , I did feel strange when i first try to check the value of "(entry->addr - address) / entry->bufw" (at that moment i don't know it is pixel) is always return very small number when compare to framebuffer height

[unknownbrackets]

bitsPerPixel[gstate.getTextureFormat()] should be the bits, so the correct divisor is probably:

u32 bufwBytes = bitsPerPixel[entry->format] * entry->bufw / 8;

Or similar.

-[Unknown]

[dbz400]

Something like this to check ?

} else if ((entry->addr - address) / bufwBytes < framebuffer->height) {

[unknownbrackets]

Yeah... does that help at all?

-[Unknown]

[dbz400]

Humm didn't help ....

    if (framebuffer->fb_stride == entry->bufw && compatFormat) {
        u32 bufwBytes = bitsPerPixel[entry->format] * entry->bufw / 8;
        u32 offset = (entry->addr - address) / bufwBytes;
        if (framebuffer->format != entry->format) {
            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?
            // Affected game List : DBZ VS Tag , 3rd birthday 
            AttachFramebufferValid(entry, framebuffer);
        } else if (offset < framebuffer->height) {
            WARN_LOG_REPORT_ONCE(subarea, HLE, "Render to area containing texture at %08x", address);
            // TODO: Keep track of the y offset.
            // Affected game List : God of War Ghost of Sparta , Tactic Orge , Sword Art Online
            INFO_LOG(HLE,"offset=%i, fbheight=%i", offset, framebuffer->height);
            AttachFramebufferValid(entry, framebuffer);
        }
    }

[dbz400]

I did one observation .For Tactic Orge and SOA , offset is value of 2 and (6,2,8,11,13) respectively . However in , GOW , offset is (0,4) which has 0 offset

[unknownbrackets]

Hmm, well if offset is 0, it should be an exact match. If that's the case, and framebuffer->format == entry->format how is it even getting there?

-[Unknown]

[dbz400]

Just tried something but not too sure if this makes sense but it works for all .

    if (framebuffer->fb_stride == entry->bufw && compatFormat) {
        u32 bufwBytes = bitsPerPixel[entry->format] * entry->bufw / 8;
        u32 fbBytes =  bitsPerPixel[framebuffer->format] * framebuffer->height / 8;
        if (framebuffer->format != entry->format) {
            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?
            // Affected game List : DBZ VS Tag , 3rd birthday 
            AttachFramebufferValid(entry, framebuffer);
        } else if (bufwBytes < fbBytes) {
            WARN_LOG_REPORT_ONCE(subarea, HLE, "Render to area containing texture at %08x", address);
            // TODO: Keep track of the y offset.
            // Affected game List : God of War Ghost of Sparta , Tactic Orge , Sword Art Online
            AttachFramebufferValid(entry, framebuffer);
        }
    }

[unknownbrackets]

Hmm, what that means essentially is that you are disabling it for non-clut types, e.g. these:

            || (framebuffer->format == GE_FORMAT_8888 && entry->format == GE_TFMT_CLUT32)
            || (framebuffer->format != GE_FORMAT_8888 && entry->format == GE_TFMT_CLUT16);

It'd be better to just replace bufwBytes < fbBytes with framebuffer->format == entry->format. But I think the height check still should be there, without it there was some game I was getting a definitely wrong texture pulled in (it may have been SAO.)

-[Unknown]

[dbz400]

This one includes height check and format check as well. All 3 titles are working good as well .

if (framebuffer->fb_stride == entry->bufw && compatFormat) {
    u32 bufwBytes = bitsPerPixel[entry->format] * entry->bufw / 8;
    u32 fbBytes =  bitsPerPixel[framebuffer->format] * framebuffer->height / 8;
    if (framebuffer->format != entry->format) {
        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?
        // Affected game List : DBZ VS Tag , 3rd birthday 
        AttachFramebufferValid(entry, framebuffer);
    } else if (bufwBytes < fbBytes && framebuffer->format == entry->format) {
        WARN_LOG_REPORT_ONCE(subarea, HLE, "Render to area containing texture at %08x", address);
        // TODO: Keep track of the y offset.
        // Affected game List : God of War Ghost of Sparta , Tactic Orge , Sword Art Online
        AttachFramebufferValid(entry, framebuffer);
    }
}

[unknownbrackets]

Well, bufwBytes < fbBytes isn't a height check. It's a width check. Since there's already framebuffer->fb_stride == entry->bufw, it literally ONLY checks that there's no clut being used (or the same depth of clut is being used.)

It'd be better to add back the height check.

-[Unknown]

[dbz400]

To be precise changes as follow (somehow still cannot make GOW happy though)

if ((entry->addr - address) / bufwBytes < framebuffer->height && framebuffer->format == entry->format) {

[dbz400]

I do think GOW here somehow incorrectly fall into the subarea render texture .It should be a exact match .

[unknownbrackets]

I hadn't realized you had height in the "fbBytes", so now I just don't even understand what the logic behind that compare even was, sorry. Can you explain what you're trying to do with an ascii box?

-[Unknown]

[dbz400]

I think my logic mostly get wrong at the beginning.Originally i just want to compare them Bytes by Bytes rather than pixel so derived something wrong below.

u32 bufwBytes = bitsPerPixel[entry->format] * entry->bufw / 8;
u32 fbBytes =  bitsPerPixel[framebuffer->format] * framebuffer->height / 8;

[dbz400]

I think more likely to be this one for a valid width check.

u32 fbBytes = bitsPerPixel[framebuffer->format] * framebuffer->fb_stride / 8;

[unknownbrackets]

So are the graphics okay in this game now?

-[Unknown]

[dbz400]

@unknownbrackets , the graphics all look okay now in-game .I think can be closed :)