Skip to content

Commit

Permalink
When blending (and we thus can't use stencil-to-alpha in frag shader)…
Browse files Browse the repository at this point in the history
…, always write zero to alpha.

It seems the PSP doesn't blend in the alpha channel.

Reduces glow problems in Gods Eater Burst and Wipeout - although Wipeout loses some glow
that should be there.
  • Loading branch information
hrydgard committed Dec 3, 2013
1 parent a6150db commit 33efffe
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions GPU/GLES/StateMapping.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,12 @@ void TransformDrawEngine::ApplyDrawState(int prim) {
#endif

// At this point, through all paths above, glBlendFuncA and glBlendFuncB will be set right somehow.
if (!gstate.isStencilTestEnabled() && gstate.isDepthWriteEnabled()) {
// Fixes some Persona 2 issues, may be correct? (that is, don't change dest alpha at all if blending)
// If this doesn't break anything else, it's likely to be right.
// I guess an alternative solution would be to simply disable alpha writes if alpha blending is enabled.
glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, GL_ZERO, GL_ZERO);
} else {
glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, glBlendFuncA, glBlendFuncB);
}

// The stencil-to-alpha in fragment shader doesn't apply here (blending is enabled), and we shouldn't
// do any blending in the alpha channel as that doesn't seem to happen on PSP. So lacking a better option,
// the only value we can set alpha to here without multipass and dual source alpha is zero (by setting
// the factors to zero). So let's do that.
glstate.blendFuncSeparate.set(glBlendFuncA, glBlendFuncB, GL_ZERO, GL_ZERO);

// Don't report on Android device (why?)
#if !defined(USING_GLES2)
Expand Down

10 comments on commit 33efffe

@unknownbrackets
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we could do some tricks based on REPLACE/etc. here. Definitely sometimes it shouldn't be 0... at least in the case of 5551, we can know when it should be GL_ONE maybe? Or maybe that wouldn't work...

-[Unknown]

@hrydgard
Copy link
Owner Author

Choose a reason for hiding this comment

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

GL_ONE and GL_ZERO here are multipliers, not the value that will be written. Anything multiplied by ZERO is 0, while anything (the alpha from the texture or whatever) multiplied by ONE is , well, anything.

We could put the stencil ref value in the .A of the blend color and use GL_CONSTANT_COLOR, then we'd get stencilref * texalpha instead of texalpha, which might be better at least...

@thedax
Copy link
Collaborator

@thedax thedax commented on 33efffe Dec 3, 2013

Choose a reason for hiding this comment

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

This seems to have fixed Pangya's black alpha issue (#3993). Nice work.

@solarmystic
Copy link
Contributor

Choose a reason for hiding this comment

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

@hrydgard

On a hunch, I decided to retest FFVII Crisis Core again.

Unfortunately, the missing shadows on Zack outside the Shinra building issue #2815 is back again after this commit was merged to master:

838

Was last working in v0.9.5-837-ga6150db:

837

@hrydgard
Copy link
Owner Author

Choose a reason for hiding this comment

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

Given how much we've fixed, that's almost an acceptable tradeoff :P

But yeah, need to really figure out what's going on there.

@solarmystic
Copy link
Contributor

Choose a reason for hiding this comment

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

@hrydgard

More good news to offset the singular regression in Crisis Core. It seems like the 3rd Birthday's persistent graphical garbage (blue flecks onscreen) that was often rendered at lower rendering resolutions in certain scenes is now resolved thanks to this commit:

838

Previous behaviour in v0.9.5-837-ga6150db:-

837

It does seem like this is the right way to go for the majority of games.

@unknownbrackets
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, in some games, it's probably incorrectly killing stencilalpha (for render-to-texture with blend), but it's better than filling it with the alpha value. But probably bloom is incorrectly missing in some games, as well as the shadows. Even so, no glow is better than everything glowing like a thousand stadium lights.

-[Unknown]

@raven02
Copy link
Contributor

@raven02 raven02 commented on 33efffe Dec 4, 2013

Choose a reason for hiding this comment

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

Just for record .FF Type-0 character shadow also missing as well

@solarmystic
Copy link
Contributor

Choose a reason for hiding this comment

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

@raven02

The FF Type 0 missing chracter shadow issue is caused by a previous related @hrydgard commit however, not this one. However, all builds after that one have the issue, including the latest build at the moment, v0.9.5-850-g0bdf17b

v0.9.5-826-g39b632b 39b632b is the first responsible commit for that issue.

826ff0

Last working revision for FF Type 0's chracter shadows is v0.9.5-825-g198e230 198e230

825ff0

@raven02
Copy link
Contributor

@raven02 raven02 commented on 33efffe Dec 4, 2013

Choose a reason for hiding this comment

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

Thanks @solarmystic . It is good to narrow down .

Please sign in to comment.