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

Avoid blitting for a bunch of gpu blending modes #6350

Merged
merged 5 commits into from
Jun 17, 2014

Conversation

unknownbrackets
Copy link
Collaborator

This avoids the copy for a ton of blending modes, which should take care of these situations:
http://report.ppsspp.org/logs/kind/622?status=any
(as you can see some were quite bad...)

Specifically:

  • Uses GL_ONE/GL_CONSTANT_COLOR for FIXED/FIXED and premultiplies src color in the shader.
  • Uses alpha doubling for 1-2_a in addition to 2_a, since GL_ONE_MINUS_SRC_ALPHA will make it work.
  • Uses color doubling as long as the src color is not used in the blend. This is safe regardless of the alpha value, actually, since we're not doubling the alpha value.

Additionally, uses the blit/shader for min/max when unsupported. Of course this might be slower on such devices, but it doesn't seem like one or two blits are too bad (as long as it doesn't get out of control), and this should fix effects.

I haven't tested a ton of games but I tested several that triggered these more unique blending modes.

-[Unknown]

} else {
glstate.blendEquation.set(eqLookupNoMinMax[blendFuncEq]);
void TransformDrawEngine::ApplyDrawState(int prim) {
// TODO: All this setup is soon so expensive that we'll need dirty flags, or simply do it in the command writes where we detect dirty by xoring. Silly to do all this work on every drawcall.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an old comment but it's just getting truer, heh.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh. I tried to keep the changes light as possible, but yeah... this is true.

-[Unknown]

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not singling out this change or anything, it should be fine.

@hrydgard
Copy link
Owner

Nice idea to use src alpha like that for the fix blends.

hrydgard added a commit that referenced this pull request Jun 17, 2014
Avoid blitting for a bunch of gpu blending modes
@hrydgard hrydgard merged commit 565bb98 into hrydgard:master Jun 17, 2014
@unknownbrackets unknownbrackets deleted the gpu-blend branch June 17, 2014 08:05
@daniel229
Copy link
Collaborator

Yes,Gensou Suikoden 1&2 speed back.Are these black boxes blending issue.
01

@hrydgard
Copy link
Owner

Seems likely to be a blending issue, yes. Check the graphics debugger when they are being drawn.

@daniel229
Copy link
Collaborator

02

@daniel229
Copy link
Collaborator

and this
03

@unknownbrackets
Copy link
Collaborator Author

Hmm, alpha test requires that it's ==ff...? Ugh, this is doing that same every other junk.

Did it work before? Which commit broke it?

It seems like bogth cases should use color doubling. I'm worried the dst.a (stencil) may be wrong. What does the stecnil test look like during these draws?

-[Unknown]

@daniel229
Copy link
Collaborator

Tested several builds since 0.9.6,still not works.stecnil test disable.
01
02

@unknownbrackets
Copy link
Collaborator Author

Maybe it never sets the stencil at all. Hexyz Force had a function that cleared buffers right at the start, and set the stencil to 0xFF. We ended up making buffers default to that.

It's not really correct, though. Perhaps this game expects it to default to 0.

Does anything change if you replace these lines in Framebuffer.cpp:

    glstate.stencilFunc.set(GL_ALWAYS, 0xFF, 0xFF);
    glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
    glClearStencil(0xFF);

With:

    glstate.stencilTest.disable();
    glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
    glClearStencil(0);

Or is it still the same?

-[Unknown]

@daniel229
Copy link
Collaborator

Yes,It fixes all graphical issues including #6226
04

@hrydgard
Copy link
Owner

Oh, cool. I think it makes more sense to default buffers to 0 - or technically, we should initialize stencil to whatever is in that channel of memory behind the buffers when they are created. Could use the same trick unknown invented for Star Ocean.

Or maybe hexyz needs a function replacement?

@unknownbrackets
Copy link
Collaborator Author

I think Hexyz just needs function replacement. It actually looked like a function likely to be used by other games (it had an FBO struct and etc.)

One game has the same issue with depth iirc.

-[Unknown]

@daniel229
Copy link
Collaborator

and fixes this #5205
05

@hrydgard
Copy link
Owner

Hm, looks like the blur gets disabled altogether, instead of only being far away. But I guess that's better than a full screen blur, heh.

@daniel229
Copy link
Collaborator

Yes,it's better than full screen blur.another game gets the same thing.
06
07

@unknownbrackets
Copy link
Collaborator Author

What do those scenes look like on a real psp?

-[Unknown]

@daniel229
Copy link
Collaborator

On psp.
201406172043_001

On ppsspp missing blur on the ground
01

@unknownbrackets
Copy link
Collaborator Author

Does it look right in the software renderer at least? Could be something stencil related still...

I do think that uploading memory to FBOs on create makes sense. Even downloading on destroy could, but sounds ultra dangerous (although there may be ways to be more sure.)

Would possibly fix effects especially on savestate/pause.

-[Unknown]

@daniel229
Copy link
Collaborator

software renderer is broken,
02

@daniel229
Copy link
Collaborator

After loading savestate,no difference.

@hrydgard hrydgard mentioned this pull request Feb 17, 2017
12 tasks
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

Successfully merging this pull request may close these issues.

3 participants