Skip to content

Font cleanup #1847

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 8, 2025
Merged

Font cleanup #1847

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
076dc0c
RE_RegisterFont: fix possibly uninitialized pointer
slipher Oct 5, 2025
c593352
Remove unused font things
slipher Oct 5, 2025
7ecdc7a
NUKE .dat pre-rasterized font file format
slipher Oct 5, 2025
2436d6d
NUKE font handle table
slipher Oct 5, 2025
3451be2
Move font stuff out of q_shared.h
slipher Oct 5, 2025
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
5 changes: 2 additions & 3 deletions src/engine/client/cl_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1440,8 +1440,7 @@ void CL_Vid_Restart_f()
Audio::StopAllSounds();
// shutdown the CGame
CL_ShutdownCGame();
// clear the font cache
re.UnregisterFont( nullptr );
re.UnregisterFont( cls.consoleFont );
cls.consoleFont = nullptr;
// shutdown the renderer and clear the renderer interface
CL_ShutdownRef();
Expand Down Expand Up @@ -2461,7 +2460,7 @@ void CL_Shutdown()

if ( re.UnregisterFont )
{
re.UnregisterFont( nullptr );
re.UnregisterFont( cls.consoleFont );
cls.consoleFont = nullptr;
}

Expand Down
3 changes: 0 additions & 3 deletions src/engine/qcommon/q_shared.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2171,9 +2171,6 @@ struct glyphInfo_t
char shaderName[ 32 ];
};

// Unlike with many other handle types, 0 is valid, not an error or default return value.
using fontHandle_t = int;

using glyphBlock_t = glyphInfo_t[256];

struct fontInfo_t
Expand Down
166 changes: 23 additions & 143 deletions src/engine/renderer/tr_font.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,6 @@ FT_Library ftLibrary = nullptr;

static const int FONT_SIZE = 512;

static const int MAX_FONTS = 16;
static const int MAX_FILES = ( MAX_FONTS );
static fontInfo_t registeredFont[ MAX_FONTS ];
static unsigned int fontUsage[ MAX_FONTS ];

static struct {
void *data;
int length;
int count;
char name[ MAX_QPATH ];
} fontData[ MAX_FILES ];

void RE_RenderChunk( fontInfo_t *font, const int chunk );


Expand Down Expand Up @@ -427,79 +415,33 @@ void RE_RenderChunk( fontInfo_t *font, const int chunk )

static int RE_LoadFontFile( const char *name, void **buffer )
{
int i;
void *tmp;
int length = ri.FS_ReadFile( name, &tmp );

// if we already have this file, return it
for ( i = 0; i < MAX_FILES; ++i )
if ( length <= 0 )
{
if ( !fontData[ i ].count || Q_stricmp( name, fontData[ i ].name ) )
{
continue;
}

++fontData[ i ].count;

*buffer = fontData[ i ].data;
return fontData[ i ].length;
return 0;
}

// otherwise, find a free entry and load the file
for ( i = 0; i < MAX_FILES; ++i )
{
if ( !fontData [ i ].count )
{
void *tmp;
int length = ri.FS_ReadFile( name, &tmp );
void *data = Z_AllocUninit( length );
*buffer = data;

if ( length <= 0 )
{
return 0;
}
memcpy( data, tmp, length );
ri.FS_FreeFile( tmp );

fontData[ i ].data = Z_AllocUninit( length );
fontData[ i ].length = length;
fontData[ i ].count = 1;
*buffer = fontData[ i ].data;

memcpy( fontData[ i ].data, tmp, length );
ri.FS_FreeFile( tmp );

Q_strncpyz( fontData[ i ].name, name, sizeof( fontData[ i ].name ) );

return length;
}
}

return 0;
return length;
}

static void RE_FreeFontFile( void *data )
{
int i;

if ( !data )
{
return;
}

for ( i = 0; i < MAX_FILES; ++i )
{
if ( fontData[ i ].data == data )
{
if ( !--fontData[ i ].count )
{
Z_Free( fontData[ i ].data );
}
break;
}
}
Z_Free( data );
}

fontInfo_t* RE_RegisterFont( const char *fontName, int pointSize )
{
FT_Face face;
void *faceData = nullptr;
int i, len, fontNo;
int len;
char strippedName[ MAX_QPATH ];

if ( pointSize <= 0 )
Expand All @@ -512,41 +454,12 @@ fontInfo_t* RE_RegisterFont( const char *fontName, int pointSize )

COM_StripExtension2( fontName, strippedName, sizeof( strippedName ) );

fontNo = -1;

for ( i = 0; i < MAX_FONTS; i++ )
{
if ( !fontUsage[ i ] )
{
if ( fontNo < 0 )
{
fontNo = i;
}
}
else if ( pointSize == registeredFont[ i ].pointSize && Q_stricmp( strippedName, registeredFont[ i ].name ) == 0 )
{
++fontUsage[ i ];
return &registeredFont[ i ];
}
}

if ( fontNo < 0 )
{
Log::Warn("RE_RegisterFont: Too many fonts registered already." );
return nullptr;
}

fontInfo_t* font = &registeredFont[ fontNo ];
ResetStruct( *font );

if ( ftLibrary == nullptr )
{
Log::Warn("RE_RegisterFont: FreeType not initialized." );
return nullptr;
}

Q_strncpyz( font->name, strippedName, sizeof( font->name ) );

len = RE_LoadFontFile( fontName, &faceData );

if ( len <= 0 )
Expand All @@ -572,13 +485,14 @@ fontInfo_t* RE_RegisterFont( const char *fontName, int pointSize )
return nullptr;
}

auto *font = new fontInfo_t{};
Q_strncpyz( font->name, strippedName, sizeof( font->name ) );
font->face = face;
font->faceData = faceData;
font->pointSize = pointSize;

RE_RenderChunk( font, 0 );

++fontUsage[ fontNo ];
return font;
}

Expand All @@ -590,69 +504,35 @@ void R_InitFreeType()
}
}

void RE_UnregisterFont_Internal( fontHandle_t handle )
void RE_UnregisterFont( fontInfo_t *font )
Copy link
Contributor

@necessarily-equal necessarily-equal Oct 7, 2025

Choose a reason for hiding this comment

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

I would kind of suggest having a

Suggested change
void RE_UnregisterFont( fontInfo_t *font )
void RE_UnregisterFont( fontInfo_t **font )

That then sets *font = nullptr; — I think the current code is correct and unlikely to change, so this extra safety doesn't do much difference. Feel free to ignore.

Copy link
Member Author

@slipher slipher Oct 8, 2025

Choose a reason for hiding this comment

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

I'll pass on that - this style of freeing function seems a little wonky since most deallocation functions like free or delete don't do that. And I caused a crashing bug one time by implementing this style of deleting function in the particle code... If we want to make it any safer probably RAII with unique_ptr is the way to go.

{
int i;

if ( !fontUsage[ handle ] )
{
return;
}

if ( --fontUsage[ handle ] )
if ( !font )
{
return;
}


if ( registeredFont[ handle ].face )
if ( font->face )
{
FT_Done_Face( (FT_Face) registeredFont[ handle ].face );
RE_FreeFontFile( registeredFont[ handle ].faceData );
FT_Done_Face( (FT_Face) font->face );
RE_FreeFontFile( font->faceData );
}

for ( i = 0; i < 0x1100; ++i )
for ( int i = 0; i < 0x1100; ++i )
{
if ( registeredFont[ handle ].glyphBlock[ i ] && registeredFont[ handle ].glyphBlock[ i ] != nullGlyphs )
if ( font->glyphBlock[ i ] && font->glyphBlock[ i ] != nullGlyphs )
{
Z_Free( registeredFont[ handle ].glyphBlock[ i ] );
registeredFont[ handle ].glyphBlock[ i ] = nullptr;
Z_Free( font->glyphBlock[ i ] );
font->glyphBlock[ i ] = nullptr;
}
}

ResetStruct( registeredFont[ handle ] );
}

void RE_UnregisterFont( fontInfo_t *font )
{
int i;

for ( i = 0; i < MAX_FONTS; ++i )
{
if ( !fontUsage[ i ] )
{
continue;
}

if ( font && font != &registeredFont[ i ] )
{
continue; // name & size don't match
}

RE_UnregisterFont_Internal( i );

if ( font )
{
break;
}
}
delete font;
}

void R_DoneFreeType()
{
if ( ftLibrary )
{
RE_UnregisterFont( nullptr );
FT_Done_FreeType( ftLibrary );
ftLibrary = nullptr;
}
Expand Down