Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Artifact in SOCOM U.S. Navy SEALs games #8973

Closed
daniel229 opened this issue Sep 12, 2016 · 15 comments

Comments

@daniel229
Copy link
Collaborator

commented Sep 12, 2016

Artifact in the middle of the screen,Since #8804
SOCOM U.S. Navy SEALs Fireteam Bravo 3
03

SOCOM U.S. Navy SEALs Tactical Strike
04

@unknownbrackets unknownbrackets added this to the v1.3.0 milestone Sep 12, 2016

@unknownbrackets

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2016

From the log, what FBO is being destroyed when that image goes away? If you set a breakpoint, is anything else writing to that area of memory before the FBO is destroyed?

We're probably not catching a memcpy/memset.

FYI: I'm not gonna be around much this coming week.

-[Unknown]

@daniel229

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 12, 2016

28:15:289 idle0        I[SCEGE]: GLES\Framebuffer.cpp:1938 Destroying FBO for 00000000 : 480 x 272 x 1
28:15:300 idle0        I[SCEGE]: GLES\Framebuffer.cpp:1938 Destroying FBO for 00044000 : 480 x 272 x 1
28:15:301 idle0        I[SCEGE]: GLES\Framebuffer.cpp:1938 Destroying FBO for 000cc000 : 128 x 272 x 3
28:15:302 idle0        I[SCEGE]: GLES\Framebuffer.cpp:1938 Destroying FBO for 000dc000 : 128 x 272 x 3

Is it this
05

@unknownbrackets

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2016

That looks like it could be it, yes. It's doing three lbus and doing some averaging or something? But it seems to ultimately be clearing the memory.

-[Unknown]

@daniel229

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 12, 2016

SOCOM U.S. Navy SEALs Tactical Strike does not break.

06:35:783 idle0        I[SCEGE]: GLES\Framebuffer.cpp:1938 Destroying FBO for 00000000 : 480 x 272 x 1
06:35:784 idle0        I[SCEGE]: GLES\Framebuffer.cpp:1938 Destroying FBO for 00044000 : 480 x 272 x 1
@unknownbrackets

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2016

Ah, I guess what's happening is that it draws the frame initially, but then it draws black, and it relies on memory being updated with the black too.

If we did that, we'd bring back the problems #8804 fixed in other games...

But maybe we could explicitly memset when a clear happens. If you step frame when the original logo is shown, does it ever do a clear (this is just a regular draw, but clear mode is on.) If so, we could use those to update memory when we first see them too.

-[Unknown]

@daniel229

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 12, 2016

This noe?
07

@unknownbrackets

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2016

Try finding this in DrawEngineGLES.cpp:

            int scissorX2 = gstate.getScissorX2() + 1;
            int scissorY2 = gstate.getScissorY2() + 1;
            framebufferManager_->SetSafeSize(scissorX2, scissorY2);

And replace with:

            int scissorX1 = gstate.getScissorX1();
            int scissorY1 = gstate.getScissorY1();
            int scissorX2 = gstate.getScissorX2() + 1;
            int scissorY2 = gstate.getScissorY2() + 1;
            framebufferManager_->SetSafeSize(scissorX2, scissorY2);

            // If easy, immediately clear the RAM too.
            bool singleByteClear = (clearColor >> 16) == (clearColor && 0xFFFF) && (clearColor >> 24) == (clearColor & 0xFF);
            if (g_Config.bBlockTransferGPU && colorMask && alphaMask && singleByteClear) {
                const int bpp = gstate.FrameBufFormat() == GE_FORMAT_8888 ? 4 : 2;
                const int xoff = scissorX1 * bpp;
                const int xlen = (scissorX2 - scissorX1) * bpp;
                const int stride = gstate.FrameBufStride() * bpp;

                u8 *const addr = Memory::GetPointer(gstate.getFrameBufAddress() + xoff);
                for (int y = scissorY1; y < scissorY2; ++y) {
                    memset(addr + y * stride, clearColor, xlen);
                }
            }

This is probably not a perfect fix - there are more cases to handle and maybe it could be more efficient, but if this works it might be a good option. Also, if this breaks other games then an improved version would likely also break other games.

-[Unknown]

@unknownbrackets

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2016

Wait, that won't work here actually... but here's a version that should (definitely could be more optimal...)

            int scissorX1 = gstate.getScissorX1();
            int scissorY1 = gstate.getScissorY1();
            int scissorX2 = gstate.getScissorX2() + 1;
            int scissorY2 = gstate.getScissorY2() + 1;
            framebufferManager_->SetSafeSize(scissorX2, scissorY2);

            // If easy, immediately clear the RAM too.
            if (g_Config.bBlockTransferGPU && colorMask && alphaMask) {
                u8 *addr = Memory::GetPointer(gstate.getFrameBufAddress());
                const bool singleByteClear = (clearColor >> 16) == (clearColor && 0xFFFF) && (clearColor >> 24) == (clearColor & 0xFF);
                if (singleByteClear) {
                    const int bpp = gstate.FrameBufFormat() == GE_FORMAT_8888 ? 4 : 2;
                    const int xoff = scissorX1 * bpp;
                    const int stride = gstate.FrameBufStride() * bpp;
                    const int xlen = (scissorX2 - scissorX1) * bpp;
                    addr += xoff;
                    for (int y = scissorY1; y < scissorY2; ++y) {
                        memset(addr + y * stride, clearColor, xlen);
                    }
                } else {
                    u32 *addr32 = (u32 *)addr;
                    const int stride = gstate.FrameBufStride();
                    for (int y = scissorY1; y < scissorY2; ++y) {
                        for (int x = scissorX1; x < scissorX2; ++x) {
                            addr32[y * stride + x] = clearColor;
                        }
                    }
                }
            }
        }

-[Unknown]

@daniel229

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 12, 2016

I try the second version, still something else on the screen.
08

@unknownbrackets

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2016

Oops, I'm still wrong. Try this one (could really make this more optimal...):

            int scissorX1 = gstate.getScissorX1();
            int scissorY1 = gstate.getScissorY1();
            int scissorX2 = gstate.getScissorX2() + 1;
            int scissorY2 = gstate.getScissorY2() + 1;
            framebufferManager_->SetSafeSize(scissorX2, scissorY2);

            // If easy, immediately clear the RAM too.
            if (g_Config.bBlockTransferGPU && colorMask && alphaMask) {
                u8 *addr = Memory::GetPointer(gstate.getFrameBufAddress());
                const bool singleByteClear = (clearColor >> 16) == (clearColor && 0xFFFF) && (clearColor >> 24) == (clearColor & 0xFF);
                const int bpp = gstate.FrameBufFormat() == GE_FORMAT_8888 ? 4 : 2;
                if (singleByteClear) {
                    const int xoff = scissorX1 * bpp;
                    const int stride = gstate.FrameBufStride() * bpp;
                    const int xlen = (scissorX2 - scissorX1) * bpp;
                    addr += xoff;
                    for (int y = scissorY1; y < scissorY2; ++y) {
                        memset(addr + y * stride, clearColor, xlen);
                    }
                } else if (bpp == 4) {
                    u32 *addr32 = (u32 *)addr;
                    const int stride = gstate.FrameBufStride();
                    for (int y = scissorY1; y < scissorY2; ++y) {
                        for (int x = scissorX1; x < scissorX2; ++x) {
                            addr32[y * stride + x] = clearColor;
                        }
                    }
                } else if (bpp == 2) {
                    u16 *addr32 = (u16 *)addr;
                    const int stride = gstate.FrameBufStride();
                    for (int y = scissorY1; y < scissorY2; ++y) {
                        for (int x = scissorX1; x < scissorX2; ++x) {
                            addr32[y * stride + x] = clearColor;
                        }
                    }
                }
            }

-[Unknown]

@daniel229

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 12, 2016

Look like it's Okay now,same as before missing some framebuffer effect.
09

hrydgard added a commit that referenced this issue Sep 16, 2016

@hrydgard hrydgard modified the milestones: Future, v1.3.0, v1.4.0 Sep 17, 2016

@hrydgard

This comment has been minimized.

Copy link
Owner

commented Sep 17, 2016

By the way it seems that there's likely something else wrong with SOCOM graphics as @daniel229 is saying that a framebuffer effect is missing.

unknownbrackets added a commit to unknownbrackets/ppsspp that referenced this issue Sep 19, 2016
Clear memory when clearing drawing.
This should help synchronize block transfers better.

Should improve hrydgard#8973.
unknownbrackets added a commit to unknownbrackets/ppsspp that referenced this issue Sep 19, 2016
Clear memory when clearing drawing.
This should help synchronize block transfers better.

Should improve hrydgard#8973.
@piau9000

This comment has been minimized.

Copy link

commented Dec 6, 2016

The problem still persists, even with the last commit.

Here is a screeshot showing the problem:

Now, if you press esc and enter ppsspp menu then return to the game, it starts to flicker:

If you look up (to the sky) while it's flickering you can see that there's a ghost image of the screen before the flickering:

Each time you enter the menu and quit the last image on the screen is kinda saved.

@hrydgard

This comment has been minimized.

Copy link
Owner

commented Dec 7, 2016

That commit has not been merged though. I'll look into doing that soon.

@hrydgard hrydgard modified the milestones: v1.5.0, v1.4.0 Mar 17, 2017

@hrydgard

This comment has been minimized.

Copy link
Owner

commented Nov 14, 2017

I think this has been fixed for some time?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.