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

AGS: Fix text rendering onto surfaces with a transparent pixel color #2798

Open
wants to merge 1 commit into
base: master
from

Conversation

@dreammaster
Copy link
Member

@dreammaster dreammaster commented Feb 23, 2021

This pull request resolves the outstanding problem with rendering TTF text onto surfaces in the AGS engine.. the problem being that with the surfaces having a designated transparent color, partially transparent pixels in the TTF glyphs were doing an alpha blend of the font color with the pink color AGS uses for transparency. Since this affects the core font code, I figure to make it a pull request for review first.

This change introduces a new virtual method on Graphics::Font called "setTransparentColor", and then overrides that in TTFFont. In the TTFFont glyph drawing code, if it's drawing a partially transparent pixel and detects the transparent color, it forces it to be transparent so it won't be blended.

@@ -597,7 +608,11 @@ static void renderGlyph(uint8 *dstPos, const int dstPitch, const uint8 *srcPos,
sA = *src;

uint8 dA, dR, dG, dB;
dstFormat.colorToARGB(*rDst, dA, dR, dG, dB);

This comment has been minimized.

@lephilousophe

lephilousophe Feb 23, 2021
Member

Shouldn't this be handled in colorToARGB? This would prevent these changes.

This comment has been minimized.

@OMGPizzaGuy

OMGPizzaGuy Feb 23, 2021
Member

Are you suggesting PixelFormat know the transparent color? It might make conceptual sense, but I'd be concerned about performance if always checked there.


if (surf.hasTransparentColor())
fnt->setTransparentColor(surf.getTransparentColor());
fnt->drawString(&surf, text, x, y, bmp->w - x, color);

This comment has been minimized.

@OMGPizzaGuy

OMGPizzaGuy Feb 23, 2021
Member

Would it make sense to change the method signature to take a managed surface? If so, you could avoid setting the transparent color on the font itself.

This comment has been minimized.

@dreammaster

dreammaster Feb 23, 2021
Author Member

That's actually an excellent idea. It would be cleaner than having an explicit setTransparentColor call. Graphics::Font can have a version that simply passes on the internal surface to the existing version, and the TTFFont can overload it to get the transparent color. I'll rework the PR tonight.

@dreammaster dreammaster force-pushed the dreammaster:ags_font branch from 7a624f9 to 31dea7e Feb 24, 2021
@dreammaster
Copy link
Member Author

@dreammaster dreammaster commented Feb 24, 2021

I'd completely forgotten that Font already had a method version that takes in a ManagedSurface. Just internally it simply called the Surface version. So I did a minor rejigging so that the ManagedSurface drawChar version is a virtual method too now, and TTFFont overrides it to get access to the transparent color, if any.

At the same time, I finally got around to moving ManagedSurface's addDirtyRect to be public, and added a method to get access to a ManagedSurface's internal surface if a calling class really needs it. This let me get rid of the "friend class Font" that was previously required just so the Font class could get access to the surface and modify it to write the characters, and then call addDirtyRect itself. So it's all cleaner now.

@OMGPizzaGuy
Copy link
Member

@OMGPizzaGuy OMGPizzaGuy commented Feb 24, 2021

This looks good to me, but I'd feel more comfortable with someone else approving.
One nitpick is the name of this PR as it is more specific to GRAPHICS than AGS at this point

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