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
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/libOpenImageIO/imagebufalgo_draw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,9 @@ ImageBufAlgo::render_text(ImageBuf& R, int x, int y, string_view text,
// If the buffer doesn't have an alpha, but the text color passed
// has 4 values, assume the last value is supposed to be alpha.
textalpha = textcolor[3];
}
alpha_channel = 3;
} else
alpha_channel = -1;

// Convert the UTF to 32 bit unicode
std::vector<uint32_t> utext;
Expand Down Expand Up @@ -1162,8 +1164,14 @@ ImageBufAlgo::render_text(ImageBuf& R, int x, int y, string_view text,
float val = t[0];
float alpha = a[0] * textalpha;
R.getpixel(r.x(), r.y(), pixelcolor);
for (int c = 0; c < nchannels; ++c)
pixelcolor[c] = val * textcolor[c] + (1.0f - alpha) * pixelcolor[c];
if (alpha == 0.0)
continue;
for (int c = 0; c < nchannels; ++c) {
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.

}
R.setpixel(r.x(), r.y(), pixelcolor);
}

Expand Down
Loading