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

God Eater 2 [NPJH50832] crash on subsequent missions (now: blending problems) #4537

Closed
KennethBR opened this issue Nov 14, 2013 · 55 comments
Closed

Comments

@KennethBR
Copy link

Using v0.9.5-389-gd6d54c8 64-bit
Every time you return from a mission, and go on another one, the game hangs at a black screen after loading. Occurs whether you choose to save or not, whether you do the same mission or a new one, and if there's a cutscene before a mission, it'll hang after the cutscene. Doesn't occur if you restart the game before starting a new mission.

Video example: http://www.youtube.com/watch?v=syQ31hT6W5M

Log at hang:
log

Full log: http://www.mediafire.com/view/mlc7bizjjnjw7q3/ppsspp.log

@SmileGate
Copy link

same problem here... i hope this get fixed soon....

@CPkmn
Copy link
Contributor

CPkmn commented Nov 14, 2013

Additionally, there are block allocation failures while we're on the topic of errors in the game (see log).

@hrydgard
Copy link
Owner

Block alloc failures would be very likely to be related to this type of crash/hang.

@Daykod
Copy link

Daykod commented Nov 15, 2013

Yeah, I can confirm that subsequent missions crash when loading. Resetting every mission works.
loading

@Unlimax
Copy link

Unlimax commented Nov 20, 2013

Please Fix The Unbearable brightness !

ge2_ss

@Ritori
Copy link

Ritori commented Nov 20, 2013

Just use Non-Buffered render mode until it get fix :I

@Unlimax
Copy link

Unlimax commented Nov 20, 2013

OMG it works .. thanks ~ <3

@betrayedAngel
Copy link

screen00058

screen00044

To anyone who want to play GE2 with buffered rendering ON, I suggest to use ppsspp ver. ppsspp-v0.9.5-276-gf29d9c0-windows-x86.
Enable buffered rendering is pretty good, so we could use fxaa+coloring shader to enhance graphical quality.

I hope block alloc failure + buffered rendering (over bloom) issues will be fixed soon.
btw, can we disable screenshot message screen?
I think it would be annoying for some good,..

@dbz400
Copy link
Contributor

dbz400 commented Nov 22, 2013

It seems to me that it is getting much brighter in recent build than previous.It would be interesting to find out starting from which build and may somehow help to adjust the brightness accordingly.

@unknownbrackets
Copy link
Collaborator

I suspect framebuffer sizing could be related to the brightness.

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Nov 22, 2013

I think so too .However , i'm wondering how softgpu handle the framebuffer sizing .It seems to me that it can always generate the correct framebuffer size.

@unknownbrackets
Copy link
Collaborator

It doesn't even use framebuffer objects. It just draws pixels to memory (which are the buffers, if you will), like the PSP does.

On a PSP, the coordinate "400,200" means something very obvious:
framebuf address + (framebuf stride * framebuf bitdepth * 200) + (400 * framebuf bitdepth)

But in GLES, we have a buffer, it needs to have a size, it gets complicated.

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Nov 22, 2013

I see.

@unknownbrackets
Copy link
Collaborator

Anyway, specifically what GEB does, and I presume its sequel as well, is this:

  1. Fills the stencil (using a clear) with a non-zero value (must be 1 since it's a 5551 buffer.)
  2. Draws the scene, with an ALWAYS stencil test that KEEPs values on fail/depthfail, and writes ZERO on pass.
  3. Leaves the windows at non-zero stencil (by changing the stencil op to INCREMENT instead of ZERO - note that it's 5551, so INCREMENT means "always 1.")
  4. Draws to several temp buffers, using fixed blend modes, but a != 0 alphatest on the first pass.
  5. Blends the result back to the rendered scene using fixed blend modes.

I think the only impact of the framebuffer size was if it got cut off or something, but not really sure. It uses a lot of small temp buffers.

In comparison, Wipeout Pure does something pretty different. Before I'd thought the misuse of the depth buffer mattered, but I don't think it actually does.

  1. It uses 8888 and clears the stencil to zero first.
  2. As it draws things, it carefully sets stencil - sometimes to 0xff, but usually between 0x0 - 0xf.
  3. Eventually it does a simple alpha blend from the real buffer onto a temp buffer (src.a being the stencil with 0.)

Sword Art Online kinda does something more like GEB but not exactly:

  1. It uses a 5551 buffer (0/1 stencil), but it starts it off at 0.
  2. It uses fail=REPLACE, pass/depthfail=KEEP, pass=REPLACE to draw elements of the sky, etc. with different stencil values. It either uses 0x80 (equivalent to 0xff) or 0.
  3. It again uses several temp buffers, but renders the first with an alphatest (> 0.)
  4. It then blends it in using fixed blending modes.

At least for the purpose of bloom, it seems like the real problem is alpha blend or alpha test when rendering from a texture. I wonder if it'd be possible to READ from the stencil buffer when rendering from a texture, and do alpha blending/testing based on that?

Naturally this would not fix all uses of stencil, but it would theoretically apply bloom properly.

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Nov 22, 2013

I just tried to revert the framebuffer sizing code to the very early one and seems to be no change to the brightness here .Looks like may be it is the alpha blend/test issue .

@dbz400
Copy link
Contributor

dbz400 commented Nov 22, 2013

Seems to be it is blending issue .I tried to disable some blending code and looks better now .Note this is GLES

screen00059

@hrydgard
Copy link
Owner

Great investigations @unknownbrackets !

Reading the stencil buffer directly can only be done with glReadPixels with GL_STENCIL_INDEX which is not supported on mobile except for on Tegra.

However it should be possible to devise a multipass algorithm to effectively copy at least one bit stencil to alpha without reading it directly, and possibly more with clever use of alpha blending. I tried doing this but didn't fully get it working, there's some preliminary code in Framebuffer.cpp line 616 (SetRenderFramebuffer).

Should be something like this, if we for example want to set Alpha to 0xFF where stencil = 0xFF, of course we do this on the render target that will later be used as a texture:

  1. Disable depth test etc
  2. Set stencil test to EQUAL 0xFF
  3. Set color mask to Alpha only
  4. Draw plain color rectangle with ALPHA=0xFF
  5. Set stencil test to NOTEQUAL 0xFF
  6. Draw plain color rectangle with ALPHA=0x0
  7. Reset stencil test and color masks

We might need different algorithms like this for each game, triggering in different conditions so it won't be clean...

@dbz400
Copy link
Contributor

dbz400 commented Nov 22, 2013

I'm wondering amask has something to do with the bloom effect

bool amask = gstate.isStencilTestEnabled() && !gstate.isDepthTestEnabled();

screen00061
screen00063

@hrydgard
Copy link
Owner

Well, that's a hack fix that improves it a bit, sure. The text and UI are not supposed to glow, though. The real root cause here is that on OpenGL, stencil and alpha are not "shared" like they are on the PSP - on the PSP, writing to alpha effectively also updates stencil, and vice versa, because they share bits. I propose above a way to copy one bit of stencil to the alpha buffer to partially mimic this behaviour.

@unknownbrackets
Copy link
Collaborator

I meant reading it via a sampler in the shader, but probably it's still only possible using read pixels...

What if we drew the original polygons twice? For the cases above, the games used REPLACE or INCR on a 5551 buffer, which is in other words REPLACE. So if we can draw the same polygon, but with appropriate masks, before drawing the real polygon and not updating depth, we might be able to set the alpha to the right value.

Also, I think when stencil testing is disabled, we should always mask alpha. And technically, if alpha is masked, that should affect stencil (I haven't tested the actual mechanics of this yet.)

-[Unknown]

@hrydgard
Copy link
Owner

Yeah, samplers can't read stencil unfortunately. Stencil is applied after the shader is executed.

Drawing things twice like that is certainly an option. We did that in Dolphin for a while to simulate a related behaviour (separate alpha used for blending and destination) until the APIs gained support for that.

@unknownbrackets
Copy link
Collaborator

Well, I think you've answered my question, but you keep qualifying it in ways that mean that I'm not communicating properly.

Consider the following:
Framebuffer A: the FBO currently being rendered to
Stencil/Depth A: the FBO currently being rendered to
Framebuffer B: the FBO bound as a texture
Stencil/Depth B: The FBO bound as a texture

With those definitions, I want to essentially do the following:

uniform sampler2D tex;
uniform sampler2D stencil;

vec4 t = texture2D(tex, v_texcoord);
t.a = texture2D(stencil, v_texcoord) / 255.0;

// Proceed with normal drawing and etc.

For Wipeout Pure, for example, that would work - if only we could read Stencil/Depth B. I realize it's probably NOT possible to sample it, as you say, but I'm also pretty sure you CAN'T write to Depth/Stencil B after the shader is executed either, which is why I'm clarifying.

At least for the 3 games with the overbright issue I tested, the only alpha/stencil value that is a problem is the source texture, and this is true of Wild Arms XF as well. Of course, it wouldn't really "fix" all stencil issues anyway...

-[Unknown]

@hrydgard
Copy link
Owner

Exactly, you can't sample from Stencil/Depth B in the shader. You can preprocess B as much as you want before you bind it as a texture though with the multipass trickery I described above, though - my suggestion is to use that method to copy a bit of B's stencil to the alpha channel of B. Then texturing from B should work as expected (except that we only copy 1 bit, not all 8).

As for writing to A's stencil from within a shader, there is no output you can send a stencil value too. All you can do is to use the REPLACE operation so that the shader will write a value you select beforehand where it executes - and in the shader, DISCARD on those pixels you don't want it written to. No idea if this is actually useful though.

@hrydgard
Copy link
Owner

Actually, I just discovered that on some OpenGL implementations (4.3 or 4.4, depending on method) it is indeed possible to sample stencil from a depth/stencil texture. We currently create our depth/stencil surfaces as renderbuffers rather than textures though, but that could be changed.

http://www.opengl.org/registry/specs/ARB/texture_stencil8.txt
http://www.opengl.org/registry/specs/ARB/stencil_texturing.txt

It's a shame this didn't get into OpenGL ES 3.0, that would have allowed them to drop renderbuffers entirely.

@dbz400
Copy link
Contributor

dbz400 commented Nov 23, 2013

I have checked the softgpu rendering for Wipeout Pulese and compare with real PSP , it should be render exactly correct for all scenes.

@dbz400
Copy link
Contributor

dbz400 commented Nov 23, 2013

@unknownbrackets I also tried this ..."when stencil testing is disabled, we should always mask alpha", seems to be partially correct in some scenes or may be need to combine other criteria

bool amask = !gstate.isStencilTestEnabled() ? GL_TRUE : (gstate.pmska & 0xFF) < 128;

@unknownbrackets
Copy link
Collaborator

Well, it may break random things, since alpha should technically only be updated by stencil. If, for example, stencil ops are all GL_KEEP, I guess we should also mask it. But, it might be "saved" right now by alpha updating it.

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Nov 24, 2013

I see.

@kurasa25
Copy link

kurasa25 commented Dec 2, 2013

same here

@dbz400
Copy link
Contributor

dbz400 commented Dec 3, 2013

brightness should be reduced in God Eat burst 2 as well .Just wonder how about Sword Art Online ?

@Ritori
Copy link

Ritori commented Dec 3, 2013

@raven02 it already be fix in latest version thank :D
screen00004

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

Great .

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

BTW, try mess around the alpha blending and accidentaly render this one in God Eater Burst .Note the windows brightness looks correct now as well as those computer screen brightness. They are all not in master build

    if (ReplaceAlphaWithStencilType() == STENCIL_VALUE_ZERO)
        glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, GL_ZERO, GL_ZERO)
    else
        glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, glBlendFuncA, glBlendFuncB);

screen00098
screen00101

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

This is from real PSP from same scenes

1
2

@unknownbrackets
Copy link
Collaborator

In GEB's case, it wants to fill the buffer with 1s. It probably happens that the blending outputs that, but it's not guaranteed.

@hrydgard can we use GL_CONSTANT_ALPHA and blendColor.set() possibly?

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Dec 7, 2013

If you can detect when it expects alpha to be one, then just use GL_ONE?

@unknownbrackets
Copy link
Collaborator

Sure, I meant for the more general case where we know the dest alpha value (REPLACE, etc.) and we're blending. Maybe might help Wipeout...

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Dec 7, 2013

Hm, well, as always these are multipliers so the value you set with GL_CONSTANT will get multiplied with the alpha value used for blending. Only with "dual source blending" can the two alphas be different. So we really need to multipass to set the value to exactly what we want.

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

@hrydgard , just wonder if this correct or not ?

    if (ReplaceAlphaWithStencilType() == STENCIL_VALUE_ZERO)
        glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, GL_ZERO, GL_ZERO);
    else if (ReplaceAlphaWithStencilType() == STENCIL_VALUE_ONE)
        glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, GL_ONE, GL_ONE);
    else
        glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, glBlendFuncA, glBlendFuncB);

@unknownbrackets
Copy link
Collaborator

You need to check stencil enabled, I'm sending a pull.

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Dec 7, 2013

That did the trick?

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

Yep, tested with latest version .God Eater Burst renders prefectly now with correct blending

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

Would it be make sense to do as follow instead of zeroing? This fixes shadow FF Crisis core and Wipeout Pulse

        // For now, let's err at zero.
        case STENCIL_VALUE_UNIFORM:
        case STENCIL_VALUE_UNKNOWN:
            glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, glBlendFuncA, glBlendFuncB);

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

Or at least not GL_ZERO for STENCIL_VALUE_UNIFORM

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

You can see the >>> renders with correct blending now with this change in Wipeout Pulse

screen00105
screen00106
screen00107

@hrydgard
Copy link
Owner

hrydgard commented Dec 7, 2013

Yeah, that might be fine, just check that it doesn't break any of the other games...

@unknownbrackets
Copy link
Collaborator

Well, I think blendFuncA/blendFuncB make no sense, for uniform we could try using constant alpha even if it would be off for some things. Not really sure what would be best though...

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Dec 7, 2013

Something like this right? seems to be working fine as well

        case STENCIL_VALUE_UNIFORM:
            glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, GL_CONSTANT_ALPHA, GL_CONSTANT_ALPHA);
            break;

@unknownbrackets
Copy link
Collaborator

@dbz400
Copy link
Contributor

dbz400 commented Dec 8, 2013

I see.It works pretty well and tested with Wipeout and FF Crisis Core.

@hrydgard
Copy link
Owner

hrydgard commented Dec 8, 2013

I think this one can be closed, for further blending problems let's post in relevant issues or create new ones.

@hrydgard hrydgard closed this as completed Dec 8, 2013
@unknownbrackets
Copy link
Collaborator

I think we actually hijacked this bug about something completely different (oops.)

Does it no longer crash? If it still crashes we should probably create a fresh new bug without all this stencil talk.

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Dec 8, 2013

yeah, I added that to the title a while ago :)

@kurasa25
Copy link

it still crashes

@sumitox
Copy link

sumitox commented Dec 31, 2014

Take a look this version : http://www.emucr.com/2014/12/ppsspp-git-20141231.html
i discouvered the solution (i mean) , It work for me, no crashing after mission

sorry my english!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests