Color fonts support #1799

saurtron · 2024-12-04T12:19:06Z

Work done

Add support for color fonts
Add support for rescaling non scalable fonts
Functional, but still needs a bit of work for minor issues

Remarks

It's based on another branch, so here is a direct link to code comparison between lua-fallback-fonts-with-atlasclear and colored-fonts.

For now this is on top of lua-fallback-fonts-with-atlasclear since its a draft, has conflicts with that branch and hopefully that will be merged earlier. Otherwise I'll sort it out.
Presenting this so ppl can voice remarks while I finish some details.
Also intend to add support to force grayscale for specific fonts, and possibly a general switch in case some games just don't want colors ever.
- Also maybe a config option to force colored backend for everything, since that might help for certain brands (ie, AMD).
Will probably need to check for freetype version, but I think 2.10 already supported colors and that's pretty ancient already. Older linux build likely won't have it but newer yes, also depends if the build uses a static lib I guess. (18.04 is 2.8.1 and 20.04 is 2.10.1 according to this)
Need to check how the shadow is working well with colored glyphs.

Implementation

Since font textures are not grayscale, I have to switch textures and atlases on the fly to color as soon as any colored glyph is to be loaded.
Added another shader at fontRenderer so we can draw both with grayscale and colored textures independently.
Could have one shader with two textures instead and have some way to signal which to use for every pixel.
- This would save a bit of memory, but since not all available fonts are expected to include colored glyphs it shouldn't be too bad.
Will need testing/fixes for AMD I guess
Since my test case font is Noto Color, that's also non scalable, so besides color I had to add support for scaling glyphs. There might be a way to signal freetype to do some of the calculations I'm doing but shouldn't be too bad.
Easy to force everything to always use colored textures by setting isColor and needsColor to true at CFontTexture constructor.

Testing

Follow the procedure under "Testing" at Fallback fonts with atlas clearing. #1790
Before actually running the game, edit luaui/Widgets/dbg_test_fallbackfonts.lua and uncomment the color font line.

After

and ClearFallbackFonts.

can still sometimes have it's own priorities.

reason.

This is only safe to do on recent fontconfig versions.

format to RGBA8.

…lear

ReallocAtlases for that since it's aimed at something different.

sprunk · 2024-12-07T14:57:48Z

rts/Rendering/Fonts/CFontTexture.cpp

 	// set render size
-	if ((error = FT_Set_Pixel_Sizes(face, 0, size)) != 0) {
+	if (!FT_IS_SCALABLE(facePtr->face) && facePtr->face->num_fixed_sizes >= 1)
+		size = static_cast<unsigned int>(facePtr->face->available_sizes[0].y_ppem / 64.0);


what does the magic 64 represent, and do we need double precision (i.e. 64.0 instead of 64.0f) if it gets cast to int afterwards? consider static constexpr auto SOME_DESCRIPTIVE_NAME = 64.0f

sprunk · 2024-12-07T15:00:00Z

rts/Rendering/Fonts/CFontTexture.cpp

@@ -575,10 +638,18 @@ CFontTexture::CFontTexture(const std::string& fontfile, int size, int _outlinesi

 	static constexpr int FT_INTERNAL_DPI = 64;


perhaps the magic constant in the review comment above is actually FT_INTERNAL_DPI?

yes it's that same one, i'll move that somewhere else so I can also use it

sprunk · 2024-12-07T15:04:32Z

rts/Rendering/Fonts/CFontTexture.cpp

+	// don't preload for non alphanumeric
+	if (!FT_Get_Char_Index(face, 65))


The comment sounds a bit like we didn't want to preload alphanumeric glyphs, which reverses the meaning (as far as i understand). Also could use an alphanumeric char explicitly:

Suggested change

// don't preload for non alphanumeric

if (!FT_Get_Char_Index(face, 65))

// if given face doesn't contain alphanumerics, don't preload it

if (!FT_Get_Char_Index(face, 'a'))

you're right, wanted to say dont preload for non-alphanumeric fonts, but it can be misintrepreted

also, sorry this sneaked in... didn't really mean to add this part inside this PR, I think I'll move that to a separate PR ... I think that'd be great to have for next release, while color fonts don't think will make it.

in any case I'll follow your suggestion

rts/Rendering/Fonts/CFontTexture.cpp

sprunk · 2024-12-07T15:06:59Z

rts/Rendering/Fonts/CFontTexture.cpp

+		glyphs.clear();
+
+		// clear atlases
+		ClearAtlases(32, 32);


it's the same values initialization uses for CreateTexture

rts/Rendering/Fonts/CFontTexture.cpp

sprunk · 2024-12-07T15:13:08Z

rts/Rendering/Textures/Bitmap.cpp

+		}
+	} else if (src.channels == 1 && channels == 4 && dts == 1 && src.GetDataTypeSize() == 1) {
+		// set rgb to 255 and alpha to src tex value
+		constexpr uint32_t baseColor = ((255<<16)|(255<<8)|(255));


Won't this depend on the color format? e.g. with RGBA vs ARGB you'd have <<24, <<16 and <<8 instead

right, will check for that as well

This should become a generic BitmapAction.

I don't think we need to support anything but RGBA in CBitmap, if it's ARGB or other abracadabra then it's up for a caller to normalize it, or do another Bitmap/BitmapAction routine to shuffle.

@lhog we can introduce more actions for this (like CopyConvertSubImage), and have CopySubImage work only for same format bitmaps. But using several actions when you can use one I'm not so sure it's best, in many cases if you can do it in just one action it's going to be more direct and likely more efficient.

For example when copying RGBA to BGRA or ARGB, you can shuffle first and then CopySubImage, but with the right algo it can be done in one go.

Likewise, in this case we're doing Grayscale to RGBA/BGRA where something similar happens, the Grayscale can be converted first to color, but here it's converted and the result placed directly at destination.

Let me know your final decision and I'll resolve this.

I see what's the problem here, CBitmap doesn't have a pixel format field :P, so it knows nothing of RGBA vs ARGB (or RGBA vs BGRA, but that's not really an issue here).

Also, the CBitmap class has other actions/methods expecting alpha in the same place.

It does support 4 and 1 channels, so imo it should be fine to allow CBitmap(4channels).CopySubimage(cbitmap(1channel)) like I'm doing, at least while a more comprehensive solution for this is developed (I think an action system supporting two bitmaps with specific pixel formats could be designed so we can select the right action when two bitmaps are combined with different pixel formats).

So, supporting ARGB probably atm not worth it since CBitmap is RGBA only, and since destination is usually opengl, the channels can always be swizzled there. Maybe the need arises later on, but for now just RGBA/BGRA can be supported implicitly as is now and call it a day.

Co-authored-by: sprunk <spr.ng@o2.pl>

lhog · 2024-12-07T17:01:43Z

rts/Rendering/Fonts/CFontTexture.cpp

-	if (slot->bitmap.pitch != width) {
-		LOG_L(L_ERROR, "invalid pitch");
+	if (slot->bitmap.pitch != width*channels) {
+		LOG_L(L_ERROR, "invalid pitch %d (width %d channels %d)", slot->bitmap.pitch, width, channels);


Hmm two concerns:

pitch | its absolute value is the number of bytes per bitmap line; it can be either positive or negative depending on the bitmap's vertical orientation;

To speed up memory access, pitch is in most cases a multiple of 16bit, 32bit, or even 64bit. It often happens that the pitch is thus larger than the necessary bits (or bytes) for a bitmap or pixmap row; in such cases, unused bits (or bytes) are at the very right (i.e., the end) of a row.

https://freetype.org/freetype2/docs/glyphs/glyphs-7.html

@lhog, you're right, but also I think the test we're doing here holds, since in those cases it will complain about the non-matching pitch.

Maybe in that case we could bail out instead of just continuing, but just continue is the same logic we had here before, so I just adapted the test to also work on the four channels case similarly, but otherwise didn't change it. As I understood, the test is an alert in case we even encounter that issue, but it's also an issue that didn't pop up yet.

Personally, would rather keep as-is and later work on this to make it bail out in a separate PR if we want to have that, or see that's an issue, or even support the negative pitch or pitch!=width*channels cases (to not complicate this PR further).

Also note the case where pitch!=width*channels is easy to support although maybe not optimal through FT_Bitmap_Convert

lhog · 2024-12-07T17:08:08Z

Will need to test on AMD/Windows in various Recoil games.

saurtron · 2024-12-09T15:40:41Z

Will need to test on AMD/Windows in various Recoil games.

Indeed, also windows in general since it's yet untested there so even with nvidia there could be some unforeseen problem.

always true, as intended.

…lear

saurtron added 16 commits November 18, 2024 11:33

Mechanism to define priority fallback fonts through lua AddFallbackFont

fd9790b

and ClearFallbackFonts.

LDoc documentation for AddFallbackFont and ClearFallbackFonts.

3a65eee

First search game fonts and then system fonts as otherwise fontconfig

a67a9fd

can still sometimes have it's own priorities.

Don't add or clear fallback fonts if fontconfig is disabled for any

1241ccc

reason.

Don't return false on a void function.

a3d0cfa

Merge branch 'master' into lua-fallback-fonts

2e4fb11

Don't try to destroy gameFontSet and fallbackPattern if null.

a1c7135

This is only safe to do on recent fontconfig versions.

Call CanUseFontConfig instead of UseFontConfig inside new methods.

74ff5f4

Refresh glyph atlases after adding or clearing fallback fonts.

b6352a2

Remove some extraneous extra lines.

0fe6786

Clear kerning precached information too.

7e30cae

Remove fast glyph cache clear mode, and all clearMode control from lua.

8be1855

Remove clearGlyphs parameter also from ClearGlyphMode.

9aaee44

return after #endif so HEADLESS doesn't have a warning.

5713e71

FontsUpdated was renamed to FontsChanged, update comments.

51c3aef

Colored and non scalable fonts support.

ec36776

saurtron added the area: fonts label Dec 4, 2024

saurtron changed the title ~~Colored fonts~~ Color fonts support Dec 4, 2024

saurtron added 8 commits December 4, 2024 21:19

Recreate texture at UploadGlyphAtlasTextureImpl and change internal

d7ef8e6

format to RGBA8.

re-add missing y parameter to shadow atlas alloc.

7ab66c9

Merge branch 'master' into lua-fallback-fonts

2d88bf6

Merge branch 'lua-fallback-fonts' into lua-fallback-fonts-with-atlasc…

e31eb5d

…lear

Merge branch 'lua-fallback-fonts-with-atlasclear' into colored-fonts

8ba3987

Refactor atlas clearing into a separate method and don't use

8cf8cc3

ReallocAtlases for that since it's aimed at something different.

Merge branch 'lua-fallback-fonts-with-atlasclear' into colored-fonts

12c432e

Realloc before dispose.

f85dff1

sprunk reviewed Dec 7, 2024

View reviewed changes

also notify channels when pitch doesn't match

48bef3a

Co-authored-by: sprunk <spr.ng@o2.pl>

lhog reviewed Dec 7, 2024

View reviewed changes

saurtron added 2 commits December 9, 2024 09:34

Merge branch 'lua-fallback-fonts-with-atlasclear' into colored-fonts

07758f5

Alloc with the right channel number when doing ClearAtlases.

eb21526

saurtron added 13 commits December 9, 2024 16:51

Make CFontTexture::ClearGlyphs return the changed status instead of

36783c9

always true, as intended.

Merge branch 'lua-fallback-fonts-with-atlasclear' into colored-fonts

5cb9407

Add comment about vfs access use in use by AddFallbackFont.

2a11114

Rename fallbackPattern to basePattern.

6f5da57

Merge branch 'lua-fallback-fonts' into lua-fallback-fonts-with-atlasc…

2fc1dbc

…lear

Merge branch 'lua-fallback-fonts-with-atlasclear' into colored-fonts

c9b82d6

Rename badly named variable.

dfba0b2

Merge branch 'lua-fallback-fonts' into lua-fallback-fonts-with-atlasc…

3fcc1a4

…lear

Merge branch 'lua-fallback-fonts-with-atlasclear' into colored-fonts

d1e7ab4

Merge branch 'master' into lua-fallback-fonts-with-atlasclear

d9ce3a1

Merge branch 'lua-fallback-fonts-with-atlasclear' into colored-fonts

ec3229f

Print pixeldata when invalid pixeldata is detected.

48f935c

Only use FT_LOAD_RENDER for non color fonts.

5d4391e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Color fonts support #1799

Color fonts support #1799

saurtron commented Dec 4, 2024 •

edited

Loading

sprunk Dec 7, 2024

sprunk Dec 7, 2024

saurtron Dec 7, 2024

sprunk Dec 7, 2024

saurtron Dec 7, 2024 •

edited

Loading

sprunk Dec 7, 2024

saurtron Dec 7, 2024 •

edited

Loading

sprunk Dec 7, 2024

saurtron Dec 7, 2024

lhog Dec 7, 2024 •

edited

Loading

saurtron Dec 9, 2024 •

edited

Loading

saurtron Dec 9, 2024 •

edited

Loading

lhog Dec 7, 2024

saurtron Dec 9, 2024 •

edited

Loading

lhog commented Dec 7, 2024

saurtron commented Dec 9, 2024

		@@ -575,10 +638,18 @@ CFontTexture::CFontTexture(const std::string& fontfile, int size, int _outlinesi

		static constexpr int FT_INTERNAL_DPI = 64;

		// don't preload for non alphanumeric
		if (!FT_Get_Char_Index(face, 65))

Color fonts support #1799

Are you sure you want to change the base?

Color fonts support #1799

Conversation

saurtron commented Dec 4, 2024 • edited Loading

Work done

Remarks

Implementation

Testing

After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saurtron Dec 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saurtron Dec 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhog Dec 7, 2024 • edited Loading

Choose a reason for hiding this comment

saurtron Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

saurtron Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saurtron Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

lhog commented Dec 7, 2024

saurtron commented Dec 9, 2024

saurtron commented Dec 4, 2024 •

edited

Loading

saurtron Dec 7, 2024 •

edited

Loading

saurtron Dec 7, 2024 •

edited

Loading

lhog Dec 7, 2024 •

edited

Loading

saurtron Dec 9, 2024 •

edited

Loading

saurtron Dec 9, 2024 •

edited

Loading

saurtron Dec 9, 2024 •

edited

Loading