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

GRAPHICS: Improve TTF rendering on transparent surfaces. #2748

Merged
merged 2 commits into from Feb 21, 2021

Conversation

@OMGPizzaGuy
Copy link
Member

@OMGPizzaGuy OMGPizzaGuy commented Jan 29, 2021

Update color blending to match blending found in ManagedSurface. TTF rendering assumed the alpha channel of the surface was completely opaque. ULTIMA8 TTF rendering attempts to use a transparent surface and became near-illegible for text on scrolls. This change will affect TTF rendering on surfaces that are not already painted.

Update color blending to match blending found in ManagedSurface. TTF rendering assumed the alpha channel of the surface was completely opaque. ULTIMA8 TTF rendering attempts to use a transparent surface and became near-illegible for text on scrolls. This change will affect TTF rendering on surfaces that are not already painted.
@mduggan
Copy link
Contributor

@mduggan mduggan commented Feb 3, 2021

This looks good to me. I tried to find other uses of this code to see if there's any potential side-effects but I didn't find any place I thought it could cause any change other than U8. I may have missed something though so if anyone else wants to comment please feel free.

if (sA > dA) {
dA = static_cast<uint8>((sA * alpha) + (dA * (1.0 - alpha)));
}

This comment has been minimized.

@criezy

criezy Feb 18, 2021
Member

Where do those formula come from?
My understanding of color blending is that it should be:

double sAn = (double)sA / 255.0;
double dAn = (double)dA / 255.0;
dR = static_cast<uint8>(sR * sAn + dR * dAn * (1.0 - sAn) / (sAn + dAn * (1.0 - sAn)));
dG = static_cast<uint8>(sG * sAn + dG * dAn * (1.0 - sAn) / (sAn + dAn * (1.0 - sAn)));
dB = static_cast<uint8>(sB * sAn + dB * dAn * (1.0 - sAn) / (sAn + dAn * (1.0 - sAn)));
dA = static_cast<uint8>(sA + dA * (1.0 - sAn));

This also means there is no need for a special case if dA is 0.
And you can of course optimise that code a bit by computing dAn * (1.0 - sAn) only once instead of 6 times.

This comment has been minimized.

@OMGPizzaGuy

OMGPizzaGuy Feb 19, 2021
Author Member

Mostly followed suit with managed_surface.cpp:487-490
Figured if case for dA is 0 might be better to avoid calculations, but perhaps not.
Destination alpha... I just wasn't sure the best call.

This comment has been minimized.

@OMGPizzaGuy

OMGPizzaGuy Feb 19, 2021
Author Member

OK, looking at https://en.wikipedia.org/wiki/Alpha_compositing#Alpha_blending
If I'm understanding it correctly, what I used would be correct for dA being 255, but not complete enough for any other value.
I'll take another swing at this.

This comment has been minimized.

@OMGPizzaGuy

OMGPizzaGuy Feb 19, 2021
Author Member

Yep, the suggested code was great.
I went with calculating a normalized output alpha and converting that to a byte at the end.
I thought it would be a little more clear to anyone reading the code.

@criezy
criezy approved these changes Feb 21, 2021
Copy link
Member

@criezy criezy left a comment

Thank you. It looks good to me now.

@criezy criezy merged commit 286cccd into scummvm:master Feb 21, 2021
3 checks passed
3 checks passed
Codacy Static Code Analysis Codacy Static Code Analysis
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deepcode-ci-bot Well done, no issues found!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants