-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 using constant color when blending #7520
Conversation
If it's just ONE/ZERO or etc., we can keep those constants. May be faster, and apparently less buggy in some drivers.
@@ -384,11 +388,14 @@ void TransformDrawEngine::ApplyBlendState() { | |||
default: | |||
break; | |||
} | |||
needsConstantAlpha = constantAlpha < 1.0f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&& constantAlpha != 0.0f ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, currently it's not smart enough to optimize out 0, but yeah, that should be doable...
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, b5126f7 avoids GL_CONSTANT_ALPHA where safe too. Probably ZERO is pretty common.
-[Unknown]
(see #7510, just for the record) |
Cool. I tried all the games I have with problematic alpha/stencil and nothing seems to break, really tempted to merge before 1.0.1 .. hm |
glBlendFuncA = GL_CONSTANT_COLOR; | ||
} else if (glBlendFuncA != GL_INVALID_ENUM && glBlendFuncB == GL_INVALID_ENUM) { | ||
// Can use blendcolor trivially. | ||
const float blendColor[4] = {fixB.x, fixB.y, fixB.z, constantAlpha}; | ||
glstate.blendColor.set(blendColor); | ||
setBlendColorv(fixB); | ||
glBlendFuncB = GL_CONSTANT_COLOR; | ||
} else if (glBlendFuncA == GL_INVALID_ENUM && glBlendFuncB == GL_INVALID_ENUM) { | ||
if (blendColorSimilar(fixA, Vec3f::AssignToAll(constantAlpha) - fixB)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this line. Why is it constantAlpha - fixB
? Should it just be 1.0f, or is there some special handling of alpha for GL_ONE_MINUS_CONSTANT_COLOR
? I think it was just a mistake I made search/replacing. Normally we will premultiply in the shader anyway.
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's intended for the case where the blend factors are say 0x111111 and 0xEEEEEE, in which case we can set one as constant color, and the other as GL_ONE_MINUS_CONSTANT_COLOR, to simulate that we have two constant colors even though we don't.
Uh, and yes, it should be 1.0f - ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, just wasn't sure why I had constantAlpha
there instead of 1.0f
for the check to see if we can use that.
-[Unknown]
I'm pretty sure this was not intentional.
Yeah, that should be right. |
Avoid using constant color when blending
If it's just ONE/ZERO or etc., we can keep those constants (without setting one explicitly.) May be faster, and apparently less buggy in some drivers.
-[Unknown]