Skip to content

tr_font: no need for file extension - #1803

Merged
illwieckz merged 3 commits into
masterfrom
illwieckz/font-texture
Sep 27, 2025
Merged

illwieckz merged 3 commits into
masterfrom
illwieckz/font-texture

Conversation

@illwieckz

@illwieckz illwieckz commented Sep 13, 2025 •

Copy link
Copy Markdown
Member
  • tr_font: no need for file extension
    I did it for log æsthetics, but it fixes testshader fonts/unifont_0_0_32 as an unexpected side effect.
  • tr_image: use GL_RGBA8 explicitly for glyphes
    We always use an explicit format as internalFormat everywhere else.

@illwieckz illwieckz added T-Improvement Improvement for an existing feature A-Renderer labels Sep 13, 2025
@slipher

slipher commented Sep 13, 2025

Copy link
Copy Markdown
Member

It might be good to use a name that can't conflict with / is distinct from normal file names. Like for the RmlUi font textures we have $generatedTexture6 or whatever.

@illwieckz illwieckz changed the title Illwieckz/font texture tr_font: no need for file extension Sep 14, 2025
@illwieckz

Copy link
Copy Markdown
Member Author

That's doable. I guess it was named like this because in the past it could be a generated image in the pk3. There are comments about it. But it was tied to some limitations we don't have if I'm right.

@illwieckz

Copy link
Copy Markdown
Member Author

I discovered that testshader can't find and then display an image whose name starts with $.

@slipher

slipher commented Sep 15, 2025

Copy link
Copy Markdown
Member

That's doable. I guess it was named like this because in the past it could be a generated image in the pk3. There are comments about it.

OK, that makes some sense, it's fine as is.

Comment thread src/engine/renderer/tr_image.cpp Resolved
@slipher

slipher commented Sep 15, 2025 •

Copy link
Copy Markdown
Member

That's doable. I guess it was named like this because in the past it could be a generated image in the pk3. There are comments about it.

OK, that makes some sense, it's fine as is.

Actually no, the code is confusing but it doesn't look like there would be any way to use an image from a pak instead of rendering using Freetype. RE_RegisterShaderFromImage creates a shader directly from an image_t without doing lookups in the shader or image namespaces. So using a name that has no overlap with normal shaders would make more sense.

I think I changed that behavior recently; before it might have been possible to do an image lookup for that function. But that could only happen after the glyph was already rendered with Freetype. It seems clearly wrong to render something and then discard it so I prevented that lookup from happening.

@illwieckz

Copy link
Copy Markdown
Member Author

I believe we have no reason to load such file from pk3/dpk anymore, so I'm OK with making the name using some internal naming scheme, but then we must clean-up anything about loading such file from the VFS.

@slipher

slipher commented Sep 15, 2025

Copy link
Copy Markdown
Member

I believe we have no reason to load such file from pk3/dpk anymore, so I'm OK with making the name using some internal naming scheme, but then we must clean-up anything about loading such file from the VFS.

OK, if you find anything to clean up go ahead. Regarding pre-rasterized fonts I'm only aware of the code loading gfx/2d/bigchars but that's a single image that can't be split into blocks.

@illwieckz

Copy link
Copy Markdown
Member Author

I discovered that testshader can't find and then display an image whose name starts with $.

Not only our builtin image detector doesn't check for paths starting with $, but our builtin image detector is actually checking for the image name to not contain any /, and font names contain one.

@illwieckz

Copy link
Copy Markdown
Member Author

But is the image generated by RE_StoreImage() actually used? The image object is local to the function…

@slipher

slipher commented Sep 16, 2025

Copy link
Copy Markdown
Member

But is the image generated by RE_StoreImage() actually used? The image object is local to the function…

A shader is created using the image and the shader handle is stored.

@illwieckz
illwieckz force-pushed the illwieckz/font-texture branch from 34fbd0b to 18bb78f Compare September 26, 2025 20:13
@illwieckz

Copy link
Copy Markdown
Member Author

So now, with latest patch, instead of being named fonts/unifont_0_0_16.tga it is named *fonts_unifont_0_0_16.

Comment thread src/engine/renderer/tr_font.cpp Outdated

Com_sprintf( fileName, sizeof( fileName ), "%s_%i_%i_%i", font->name, chunk, page, font->pointSize );
std::string fontName = font->name;
std::replace( fontName.begin(), fontName.end(), '/', '_');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Replacing characters seems pointless

@illwieckz illwieckz Sep 26, 2025 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's true we can now use / in those names (even if that looks ugly), so I removed the character substitution.

@illwieckz
illwieckz force-pushed the illwieckz/font-texture branch from 18bb78f to 4ef54b2 Compare September 26, 2025 21:40
@slipher

slipher commented Sep 27, 2025

Copy link
Copy Markdown
Member

LGTM

@illwieckz
illwieckz merged commit 5c2f6b4 into master Sep 27, 2025
1 check passed
@illwieckz
illwieckz deleted the illwieckz/font-texture branch September 27, 2025 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Renderer T-Improvement Improvement for an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants