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

Apply unsupported blending modes in the shader #6070

Merged
merged 11 commits into from
May 27, 2014

Conversation

unknownbrackets
Copy link
Collaborator

This handles unsupported fixed color combinations, alpha doubling, etc.

Where possible, tries to avoid it - it means using a blit (without the framebuffer fetch extension), which can slow things down a lot with tons of drawcalls.

This fixes some effects in games which use doubling blend modes, but I haven't tested all the cases (especially since I don't have all the games.) It also does absdiff blending and mismatched fixed color blending. It will not help GLES2, and requires GLES3+ or OpenGL 3.0+.

I'm worried this might need a setting. Before I made AlphaToColorDoubling() || CanDoubleSrcBlendMode() bail, it made Popolocrois severely slower. However, it does allow more accuracy.

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

Potentially we could also try not blitting as described in #3001 (comment), for certain GPUs.

-[Unknown]

@hrydgard
Copy link
Owner

We could blit on ES2 on Tegra using NV_framebuffer_blit - but of course, on Tegra we don't need it as we can use gl_LastFragColor. Anyway, we could fake blitting the color buffer fairly easily on plain ES2 too by simply rendering a quad with a pass through shader, although a little more expensive. That's for future pull requests though.

I'll merge this once it has received a little bit of testing.

@unknownbrackets
Copy link
Collaborator Author

Right, well, we could replace all the unsupported blits with "BlitFramebuffer_", I guess.

-[Unknown]

@hrydgard
Copy link
Owner

The ones that blit (or care about) only the color buffer, yeah. Can't do depth that way.

@unknownbrackets
Copy link
Collaborator Author

What about a GL_DEPTH24_STENCIL8 texture? Can't those be bound too?

-[Unknown]

@hrydgard
Copy link
Owner

Not as textures, In unextended ES 2.0, there are no depth textures (although you can write to depth in the shader). There are extensions though so it can work on some chips (maybe quite a lot).

return true;

case GE_SRCBLEND_FIXA:
if (funcB == GE_DSTBLEND_FIXB) {
Copy link
Owner

Choose a reason for hiding this comment

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

Here you could also filter out the cases where one of the fix colors is 0xFFFFFF or 0x0, as those are already handled correctly by regular blending by replacing them with GL_ONE/GL_ZERO.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just did it, yeah.

-[Unknown]

@solarmystic
Copy link
Contributor

Do permit me to do the usual and report back later. Hopefully the hit isn't too bad on my system.

@unknownbrackets
Copy link
Collaborator Author

Well, if it gets used a lot the hit will be bad, but these blending modes are only used in certain games and usually not many times per frame. Here are some of them:

http://report.ppsspp.org/logs/kind/353
http://report.ppsspp.org/logs/kind/83 (probably mostly when things are crossfading...)

Hmm, I wonder how this impacts God Eater 2.... demo seems not majorly impacted.

-[Unknown]

solarmystic referenced this pull request May 11, 2014
Otherwise, the frag shader might be generated accounting for different
vertexFullAlpha values, or otherwise won't match up with the id, since we
change flags between the two Apply calls.
@solarmystic
Copy link
Contributor

@unknownbrackets @hrydgard
Performance test results for your perusal. I opted not to publish one for functionality since all games in the test suite remained graphically unaffected as compared to the current master. (Persona 2, which was supposed to be fixed by this pull request had already been resolved by #6072)

capture1

General Observations:-

  1. Gods Eater Burst undergoes a minor, but noticeable performance regression.
  2. The rest of the games in the list are unchanged/or close to it in terms of their performance.

Conclusion:-

Seems like the pull request is good to go. I was expecting a severe performance hit in GEB (since it's a part of the lists http://report.ppsspp.org/logs/kind/353 and http://report.ppsspp.org/logs/kind/83), but it's relatively minor.

@unknownbrackets
Copy link
Collaborator Author

Well, Gods Eater Burst actually loses 4% for me with this (947% -> 910%), but only in areas and conditions where the blend is triggered. It does a blend of 0xfefefe/0x808080, which this currently triggers for (I could probably allow a variance of 0xfefefe though...

Anyway, I'm able now to reproduce (without forcing it) a box in the top left when blending, but I can't figure out why. The framebuffer and fbotex are the right size. It's not strongly noticeable, but i don't get what's causing it. You can reproduce as follows:

  1. Load the demo.
  2. Get to the "den", which is the first area where you can move. There's a reception desk and people sitting/standing around.
  3. Walk upstairs (this triggers the blending.)
  4. Note a slight line in the top left quadrant of the screen.

This occurs for 1x as well as 2x/3x/etc., so it's not something to do with sizing. The buffer is already selected and estimated by the time it hits ApplyDrawState, so it shouldn't be sizing either... I don't get it.

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented May 12, 2014

Persona 2 and Super Robot War Portable A both look good however DBZ Tag got black background in-game
uljs00311_00005

if (funcA != GE_SRCBLEND_DSTCOLOR || funcB != GE_DSTBLEND_SRCCOLOR) {
WARN_LOG_REPORT(G3D, "Using MIN blend equation with odd factors: %d, %d", funcA, funcB);
}
WRITE(p, " v.rgb = min(v.rgb, destColor.rgb);\n", srcFactor, dstFactor);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think MIN/MAX/ABSDIFF don't need the srcFactor and dstFactor (though no %s and being used in equation)

@unknownbrackets
Copy link
Collaborator Author

Dragon Ball Z uses a CLUT on an FBO, which is not supported without the other changes. That's why it's black.

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented May 12, 2014

Arr yes . I almost forget it has to been used with framebuffer-clut branch.

@dbz400
Copy link
Contributor

dbz400 commented May 12, 2014

This is another game which use ABSDIFF blending but not showing propertly .Not too sure if it is related to overlap polygon issue and also happen in clut branch. (btw , how to check ?)

uljs00379_00001

untitled

@dbz400
Copy link
Contributor

dbz400 commented May 12, 2014

texelFetch is not supported on ES 2.0 .Wondering would it be good to fall back to use texture2D though need more work to cope with the screen resolution .

@unknownbrackets
Copy link
Collaborator Author

What is the actual texture it's using absdiff with? Do we have an issue already for it (I don't know the name so can't search)?

-[Unknown]

@daniel229
Copy link
Collaborator

Naruto Shippuden: Narutimate Accel 3 ABSDIFF blending works fine.

master build
01

test build
02

@dbz400
Copy link
Contributor

dbz400 commented May 12, 2014

@unknownbrackets , yep , there is already an issue tracking it though it is closed #1982

I'll check the actual texture when i back home .

On our current implemenation for ABSDIFF blending , we are passing GL_MAX to approximate it .

-softGPU
robot1

-GLES (GL_MAX approximate)
robot2

-this commit
robot3

@daniel229
Copy link
Collaborator

FF zero type ABSDIFF blending,don't know if right.

test build
04

master build
03

softGPU
05

@daniel229
Copy link
Collaborator

God eater 2 has a bug in this area.

test build
06

master build
07

@daniel229
Copy link
Collaborator

demo can reproduce the problem in this mission,go to this area and walk around.
08

@unknownbrackets
Copy link
Collaborator Author

So, that looks similar (but much worse) to the problem in God Eater 1 and Gods Eater Burst. I don't know what's causing it, but it's rendering at the wrong size somehow. There shouldn't be anything to trigger that here? I tried scaling the params to texelFetch, but that just moves the window up and down.. maybe I did it wrong.

Maybe I should not trust abs()?
http://stackoverflow.com/questions/13335507/glsl-abs-broken

Does it help to change this:
WRITE(p, " v.rgb = abs(v.rgb - destColor.rgb);\n");

To:
WRITE(p, " v.r = max(v.r - destColor.r, destColor.r - v.r);\n");
WRITE(p, " v.g = max(v.g - destColor.g, destColor.g - v.g);\n");
WRITE(p, " v.b = max(v.b - destColor.b, destColor.b - v.b);\n");

Also, are any of these games reporting odd blending factors?

-[Unknown]

This handles unsupported fixed color combinations, alpha doubling, etc.
Where possible, tries to avoid it - it means using a blit (without the
framebuffer fetch extension), which can slow things down a lot with tons
of drawcalls.
Just in case there are problems with decimating it or etc.
Seems from reports that they are indeed ignored, just wanted to verify.
@unknownbrackets
Copy link
Collaborator Author

Aha, I think I found it. Scissor affects glBlitFramebuffer()?

"When values are written to the draw buffers, blit operations bypass most of the fragment pipeline. The only fragment operations which affect a blit are the pixel ownership test, the scissor test, and sRGB conversion"

Well, so it does. Mystery solved, fix upcoming.

-[Unknown]

@hrydgard
Copy link
Owner

Ah, nice!!

What does the performance impact of this branch look like now?

@unknownbrackets
Copy link
Collaborator Author

@daniel229 how is God Eater 2? @raven02 how is Dai2Ji Super Robot Taisen Z Hakaihen now?

@hrydgard I don't know, I'm too tired now to test it, before it was not really that bad but it depends on how many blits it does... it will report "Lots of blits needed for obscure blending: %d per frame, blend %d/%d/%d" on a problem. Could use wider testing, mostly of these:
http://report.ppsspp.org/logs/kind/83
http://report.ppsspp.org/logs/kind/353

I have not even tried Tactics Ogre yet...

-[Unknown]

@hrydgard
Copy link
Owner

Alright, I'll wait for more testing before deciding whether we need a setting, and merging.

@dbz400
Copy link
Contributor

dbz400 commented May 27, 2014

Both look good now .
npjh90338_00000
uljs00379_00000

@daniel229
Copy link
Collaborator

@unknownbrackets I think you mean revert this #6067 and test,because that is hiding issue.Revert it,and God Eater 2 is still fine with this branch.

@daniel229
Copy link
Collaborator

Macross Triangle Frontier report this,but I don't where is the problem.
W[G3D]: GLES\StateMapping.cpp:184 Lots of blits needed for obscure blending: 26 per frame, blend 6/3/0

Beats,
FF-Type 0,
Higurashi Daybreak Portable,
Naruto Shippuden: Narutimate Accel 3,
Tenchu 4
are still fine,the others I don't know when they would suing this feature.

@unknownbrackets
Copy link
Collaborator Author

Well, it would mainly be during effects, but yeah, probably hard to find...

That's a GE_SRCBLEND_DOUBLESRCALPHA / GE_DSTBLEND_INVSRCALPHA / GE_BLENDMODE_MUL_AND_ADD blend. What that means is textures/vertices that don't use full alpha, so we can't just flip it to color doubling.

As long as performance was okay, it might be alright. 26 is high but not super bad. Will be worse on slower devices, though...

Yeah, I had to revert that change to test it.

-[Unknown]

@hrydgard
Copy link
Owner

Let's merge and see how it goes. If there's severe slowdown this merits a setting but it seems for most games it works fine so we'll default to on.

hrydgard added a commit that referenced this pull request May 27, 2014
Apply unsupported blending modes in the shader
@hrydgard hrydgard merged commit 8d84c91 into hrydgard:master May 27, 2014
@unknownbrackets unknownbrackets deleted the gpu-blend branch May 28, 2014 06:15
@brujo5
Copy link

brujo5 commented May 30, 2014

#6168

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.

7 participants