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

fix: text rendering did not work correctly with alpha #3934

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

progweb
Copy link
Contributor

@progweb progweb commented Jul 31, 2023

As render text with shadow with channel alpha, I don't get the same result that without channel alpha.

Without channel alpha, shadow option permit to draw a dilated text in black, whereas with channel alpha, OIIO makes a gap...

Working with dev-2.2 branch, I can see the same issue on master branch.

In using this patch, shadow option works as expected on image with alpha buffer.

Ref: #3932

[Note]: OpenImageIO is used in the gpx2video project.

@progweb
Copy link
Contributor Author

progweb commented Jul 31, 2023

Before:

orig

@progweb
Copy link
Contributor Author

progweb commented Jul 31, 2023

After:

patch

Signed-off-by: Nicolas VIVIEN <nicolas@vivien.fr>
@lgritz lgritz changed the title Fix/shadow fix: text rendering did not work correctly with alpha Aug 11, 2023
if (c == alpha_channel)
pixelcolor[c] = alpha + (1.0f - alpha) * pixelcolor[c];
else
pixelcolor[c] = (val * alpha * textcolor[c]) + (1.0f - alpha) * pixelcolor[c];
Copy link
Collaborator

Choose a reason for hiding this comment

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

You may have noticed CI failures. Basically, every test with text in it is failing with this PR.

I think this line is the part that's responsible. Compared to the old code, you have an extra multiplication of the foreground color by alpha. I believe (but admit that I am not 100% sure) that it was correct before because freetype is supplying color results that are already scaled by the glyph's alpha coverage, so now you are doubly accounting for alpha coverage in the foreground.

@lgritz
Copy link
Collaborator

lgritz commented Aug 21, 2023

@progweb Any comments on the last concerned I raised about the CI failure?

@progweb
Copy link
Contributor Author

progweb commented Aug 22, 2023

Not available, I’ll fix this in few days for the next week.

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.

2 participants