Skip to content

Fix font rendering race condition and menu text loss across video settings and restarts - #174

Closed
jm2 wants to merge 2 commits into
themuffinator:mainfrom
jm2:fix/font-rendering-vid-restart
Closed

jm2 wants to merge 2 commits into
themuffinator:mainfrom
jm2:fix/font-rendering-vid-restart

Conversation

@jm2

@jm2 jm2 commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #168
Addresses #169

Problem

Menu text and HUD fonts could disappear under two distinct video configuration workflows:

  1. Quality Preset Cycling (Bug - Menu #168): Cycling quality presets in Settings -> System -> Video modifies reduction cvars (image_downSize, image_downSizeSpecular, image_downSizeBump), prompting idImageManager::ReloadImages( true ) without a full vid_restart. In idImage::Reload(), a blanket handler purged and reallocated all persistent images with NULL data. Because TTF font atlases are marked persistent, their GPU textures were wiped to blank texels in VRAM while the UI device context never triggered FreeType re-rasterization, causing all menu text to vanish.
  2. Video Mode & Resolution Scaling Changes (vid_restart, Bug - Scale Resolution #169): Window and viewport dimensions in glConfig were only updated in GLimp_SwapBuffers(). During vid_restart, early calls to font registration and image reload ran against uninitialized/stale dimensions, triggering race conditions in atlas size calculations. Furthermore, procedural atlas material definitions were deleted on context shutdown without being properly restored.

Solution

  • src/renderer/Image.h, ImageManager.cpp, Image_load.cpp: Added idImage::IsFontAtlas() and skipped procedural FreeType font atlases during texture reload and reduction passes so they are never purged or overwritten with blank texels.
  • src/renderer/OpenGL/gl_ContextSDL3.cpp: Extracted SDL3_SyncGLConfigWindowDimensions() and called it synchronously inside GLimp_Init() and GLimp_SetScreenParms(), ensuring valid dimensions prior to font allocation and frame presentation.
  • src/renderer/RenderSystem_init.cpp & src/renderer/tr_fontTTF.cpp: Preserved procedural atlas material decls across video restart shutdowns and reasserted them via R_TTFRestoreAtlasMaterials(). Added missing size slot fallback propagation when extreme DPI scaling caps atlas dimensions.
  • src/ui/DeviceContext.cpp: Ensured font slots fallback cleanly to valid active sizes (fontInfoSmall), retry interrupted context reloads, and fallback before dropping characters in DrawText().

Validation

  • Verified on macOS (Vulkan and OpenGL backends) and Windows.
  • Tested cycling through all Quality Presets (Low, Medium, High, Ultra) back and forth multiple times; verified menu text remains 100% visible and crisp (resolves Bug - Menu #168).
  • Tested repeated resolution changes and vid_restart invocations; verified font glyphs reload reliably (addresses Bug - Scale Resolution #169).
  • Passed renderer_picmip_policy.py, renderer_classic_gui_domain.py, macos_renderer_backend_policy.py, and gui_clipping_contract.py.

Repository owner deleted a comment from chatgpt-codex-connector Bot Sep 21, 2026

@themuffinator themuffinator left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The first half of your diagnosis is right, and I measured it before touching anything — thank you, that was the useful part of this and I would not have looked there otherwise.

Confirmed

TTF atlases are created through globalImages->ScratchImage( imageName, &opts, ... ) with opts.isPersistant = true and no generatorFunction (src/renderer/tr_fontTTF.cpp:891 and :1325). So in idImage::Reload they fall into the persistent branch and get DeriveOpts(); AllocImage();, and AllocImage() starts with PurgeImage() — the texture is deleted and recreated with no texels. idImageManager::CheckCvars() reaches that through ReloadImages( true ) exactly when a reduction cvar moves, which is what the Video quality preset does to image_downSize, image_downSizeBump and image_downSizeSpecular.

A/B on a GL client sitting at the menu, developer 1, flipping image_downSize:

build "Texture reduction changed" persistent reallocations font atlases
before 1 42 37
after 1 0 0

The 37 are every _ttfatlas_* and _ttfatlasx_* face/size plus _ttfconsolefont.

I have landed the minimal form of that in 71c3230: ReloadImages takes a fileBackedOnly flag, the reduction path sets it, and it skips anything R_IsMutableRenderImage recognises. That predicate already existed and covers scratch pages and render targets generally, so it needs no name list, and the vid_restart/device-create reloads are untouched — after a real device loss those pages do need reallocating.

Not confirmed, and this is why I have not merged the rest

I could not reproduce the visible symptom. On GL, headless at the menu, the text still rendered after all 37 atlases were reallocated — both from a direct image_downSize flip and from cycling com_performancePreset between ultra and minimum five times. Something is putting those pages back that I have not found, or the loss needs an ingredient I did not have.

So the reallocation is real and worth removing on its own merits, but "this is what makes the menu text disappear" is still an inference. Your description calls it a race condition, which matches AdrielXXO saying it happens sometimes — if you have a deterministic repro, or a developer 1 log from a run where the text actually went, that would settle it and I will finish the job.

The rest of the patch

  • IsFontAtlas() name sniffing is fragile where R_IsMutableRenderImage already answers the same question, and the _consoleFont clause is dead — the constant is Q4_CONSOLE_ATLAS_IMAGE = "_ttfconsolefont" (tr_fontTTF.cpp:97), which your _ttfconsole prefix already covers.
  • Returning early from idImage::Reload for font atlases also skips them on the vid_restart path, where reallocation is correct. That is likely why you then needed the extra restore call.
  • Commenting out ttfAtlasMaterials.DeleteContents( true ) in R_ShutdownTrueTypeFonts leaks the list at shutdown and leaves entries pointing at purged images. Please do not land that.
  • R_TTFRestoreAtlasMaterials() is already called from idRenderSystemLocal::BeginLevelLoad (RenderSystem_init.cpp:5533) for a documented reason. If R_PerformFullVidRestart genuinely needs its own call, say what purge it is answering — as it stands it runs before the GUI fonts have been re-registered, so there is nothing yet to restore for them.
  • The gl_ContextSDL3.cpp extraction of SDL3_SyncGLConfigWindowDimensions() and calling it from GLimp_Init/GLimp_SetScreenParms looks right to me independently of the font question, and is the most interesting thing here for #169. I would take that as its own small PR today.
  • The DeviceContext.cpp font-slot fallbacks are defensible but they are papering over whatever left useFont NULL. Same ask: a repro first.

Leaving this open. Split out the gl_ContextSDL3 change and I will merge that part quickly; for the font half I need the repro before I can tell a fix from a workaround.

@jm2

jm2 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

71c3230 covers the atlas reallocation. The remaining review points stand, so this PR's other changes are withdrawn: the name sniffing, commenting out DeleteContents, the extra restore call and the DeviceContext fallbacks.

Repro for the visible symptom: it reproduces deterministically on macOS with OpenGL (Apple GL 2.1 compatibility path, M1 Max), and 71c3230 fixes it.

At the main menu with developer 1, applyPerformancePreset ultra / minimum was alternated four times, with a screenshot after each switch:

build persistent reallocations of _ttf* atlases menu text
d3edc0a8 (parent of 71c3230) 142 all button and footer text gone, frames and logo intact
71c3230c 0 intact after every switch

On this path the blanked atlases are never refilled. That is probably the ingredient missing from the Windows/GL attempt. It fails every time, not intermittently. Logs and screenshots from both runs are available if useful.

The gl_ContextSDL3.cpp change is split out as #176. Closing this in favour of that PR and 71c3230.

@jm2 jm2 closed this Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug - Menu

2 participants