Level-load optimization, retail font parity, honest savegame compatibility, and soft particles by default - #147
Closed
themuffinator wants to merge 5 commits into
Closed
Level-load optimization, retail font parity, honest savegame compatibility, and soft particles by default#147themuffinator wants to merge 5 commits into
themuffinator wants to merge 5 commits into
Conversation
The level-load cache manager recorded every opened source by matching it against the whole semantic-hint vector, and each comparison built up to three temporary std::strings. game/airdefense1 records 4,428 hints, so recording a level's media scaled as sources times hints. Hints are now indexed by the three keys the match rules actually use - exact name, compiled-suffix name, and extension-stripped stem - and lookup takes the highest matching hint index, which is the hint the old reverse scan stopped on. A semantic name already resolved in the current generation is also no longer resolved a second time, because the repeat open could only yield the identity the first one learned. On game/airdefense1 with com_levelLoadModernization 1, warm medians of three runs: total load 11,893 -> 9,010 ms, level image loading 4,442 -> 2,209 ms, and first-visit load 21,040 -> 18,044 ms. That path is now the fastest warm configuration measured for this map, 20.1% below the shipping default, but it is still slower on a first visit, so the default stays off. The classic default path is unchanged at 11.3 s. ParseModel now range-checks its file-provided vertex and index counts and every index it reads. ParseShadowModel and the binary render-world cache already applied those predicates; the classic text draw-surface path did not, and FinishSurfaces dereferences every index while deriving tangents and silhouette edges. Verified against game/airdefense1, game/airdefense2, game/storage1, game/medlabs, and game/mcc_landing: no stock surface is rejected, and the .proc parse time is unchanged. LoadLevelImages also bounds its fill loop against the array it pre-sized from CountPendingLevelLoads. The two share a predicate today so the bound cannot trip; without it a future divergence would be a silent heap overflow rather than a dropped image. docs/dev/airdefense1-optimization-evidence.md records the camera-sweep capture, the 360-degree frame-rate results, the load-time breakdown, and the ranked remaining work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SetFontByScale chose its atlas from a viewport-enlarged scale so high-resolution displays stop magnifying the 12-point sheet. Its comment said "text layout and dimensions do not change", but the selected atlas is also what TextWidth, MaxCharHeight and the DrawText advances read, so on a large window the retail .fontdat atlases - drawn by hand at each size, and not agreeing to the pixel once normalised - began reporting different metrics. uiFontParitySelfTest caught it on a 2538x1312 window: the HUD radio marine cases were a pixel out in line height, baseline, glyph y and overhang, the three radio strings two pixels narrow, and the loading title measured 234 px against a retail 223 px, moving its right-aligned x by 11 px. Below roughly 1.5x enlargement nothing moved, which is why 1280x720 and 640x480 both passed. A scalable font rasterises its small/medium/large slots from one face at 12/24/48 point, so their normalised metrics do agree and a larger slot only buys resolution. The enlargement now applies to those fonts only, keyed on the "ttf/" slot-name prefix that the scalable path assigns; the retail atlases select on the authored scale and keep exact retail parity. Three self-test cases pin the distinction, including a font that produced only some scalable slots. renderer_validation_matrix is 36/36 again, from 35/36. Verified directly at the 2538x1312 window that failed: 15 metric mismatches before, zero after. The push validation profile stays green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The note described the uiFontParitySelfTest failure as still needing an owner. It has one now, so record the cause and the fix instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Loading every save on hand showed the compatibility gate admitting payloads it could not read. They failed a sync check or a non-finite window variable roughly 550 KB in, which is after the running map has been unloaded - the outcome this policy exists to prevent. Measured on the current build: v3 build >= 661 restore cleanly (691, 707, 762, 784, 794, 795) v3 build 1 the released v0.10 payload, desyncs mid-restore v2 builds 544/556/614 every tested snapshot desyncs mid-restore, 5 of 5 unstamped legacy already refused correctly Builds 691 and 707 are recovered by the companion change that matches each player liquid field to the build that added it. What remains cannot be read, so it is no longer claimed: v3 payloads below a verified build floor and every v2 payload are refused during preflight, with a message naming the build that wrote them, and the session stays intact. The v2 allowlist is removed rather than corrected. It asserted support that had never been demonstrated against a real save, and every tuple that could be tested was wrong. savegame_v3_contract now forbids reintroducing a tuple allowlist and pins the build floor and the two per-field thresholds instead of the removed snapshot tables. Verified: anderson, airdefense2, and convoy2 refuse with zero map teardowns and zero errors; airdefense1, tram1b, and process1_first still load. push validation and both savegame contracts pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Eligible effect sprites ended in a visible straight edge where they met a floor, wall, or crate. The depth fade that removes it has been available as r_softParticles since the soft-particle path landed, but off by default. Measured on game/airdefense1, the BSE-smoke scene it matters most in, at 2560x1334 through the Vulkan backend with 45-second cooldowns between runs: 429.1 Hz median with the fade on against 437.6 Hz with it off, ranges 313.9-457.1 and 381.2-443.4. About 2%, inside the run-to-run spread on this host. A live capture on the default OpenGL backend renders the plumes, fire, terrain, HUD and view weapon correctly, with no broken effects or black quads. Decals, beams, electricity, trails, model debris, custom GLSL stages, post-process materials, and non-effect translucent materials are unaffected; they were never part of this path. r_softParticleFadeDistance still controls the fade width. The cvar is archived, so an existing configuration keeps whatever value it already recorded and only a fresh one picks the new default up. The release notes say so. push validation and renderer_validation_matrix 36/36 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five commits from one session on
game/airdefense1: a load-time optimization round, two robustness fixes, a font-parity regression fix, a savegame-compatibility correction, and one defaults change.What changed
Level-load recording was quadratic (
1f756624)The level-load cache matched every opened source against the whole recorded semantic-hint vector, building up to three temporary
std::strings per comparison.game/airdefense1records 4,428 hints, so recording scaled as sources x hints. Hints are now indexed by the three keys the match rules actually use, and a semantic name already resolved this generation is not resolved again.With
com_levelLoadModernization 1, warm medians of three runs: total load 11,893 -> 9,010 ms, level image loading 4,442 -> 2,209 ms, first visit 21,040 -> 18,044 ms. The shipping default path is unchanged at 11.3 s — this only speeds up the guarded cache experiment, which is still default off because it remains slower on a first visit.Classic
.procparser hardening (same commit)ParseModeldid not range-check its file-provided vertex/index counts or the indexes it read, whileParseShadowModeland the binary render-world cache both did — andFinishSurfacesdereferences every index. Now bounded. Verified no stock surface is rejected acrossairdefense1,airdefense2,storage1,medlabs,mcc_landing, with.procparse time unchanged.LoadLevelImagesalso bounds its fill loop against the array it pre-sized.Retail font parity (
2afc8ec7)SetFontByScalechose its atlas from a viewport-enlarged scale so high-DPI displays stop magnifying the 12-point sheet. But the selected atlas is also whatTextWidth,MaxCharHeightand theDrawTextadvances read, so above roughly 1.5x enlargement the hand-authored retail.fontdatatlases started reporting different metrics — the loading title measured 234 px against a retail 223 px, moving its right-aligned x by 11 px.Scalable fonts rasterise all three slots from one face, so their normalised metrics agree and a larger slot only adds resolution; the enlargement now applies to those only. This took
renderer_validation_matrixfrom 35/36 back to 36/36, and at the 2538x1312 window that exposed it: 15 metric mismatches before, zero after.Savegame compatibility (
1339033e, plusc4c45c7in openQ4-game)Loading every save on hand showed the gate admitting payloads it could not read; they failed a sync check roughly 550 KB in, after the running map had been unloaded — the outcome the policy exists to prevent.
Root cause:
idPhysics_Player::swimSpeedentered the save on 2026-08-13 andidPlayer::nextLiquidSurfaceSoundTimeon 2026-08-19, but one boolean gated both. Every save written between those builds read a field its file does not contain. Each field now carries the build that introduced it, which recovers builds 691 and 707.What still cannot be read is no longer claimed. The v2 snapshot allowlist is removed rather than corrected — it asserted support never demonstrated against a real save, and all five tuples that could be tested were wrong. v2 payloads and v3 payloads below a verified build floor are refused during preflight, naming the build that wrote them, with the session left intact.
Soft particles on by default (
6cf639e6)Eligible effect sprites ended in a visible straight edge where they met a floor, wall, or crate. Measured at 2560x1334 through Vulkan with cooldowns: 429.1 Hz median with the fade on against 437.6 Hz off, ranges 313.9–457.1 and 381.2–443.4 — about 2%, inside the run-to-run spread.
For the reviewer
1339033eis one change split across repos: the engine refuses what it cannot read, openQ4-game (c4c45c7) does the per-field decoding. Reviewing the engine half alone will look incomplete.2123e02there also addsbenchmarkViewSweep, the deterministic camera pan the frame-rate evidence below depends on.r_softParticlesis a defaults change; it is archived, so existing configs keep their recorded value and only fresh ones pick it up. Both are in the v0.12.0 Upgrade Notes.savegame_v3_contractnow forbids reintroducing a tuple allowlist and pins the build floor plus the two per-field thresholds instead of the removed snapshot tables. If you want the allowlist back, that guard is the thing to argue with.Validation
openq4_validate.py pushpasses.renderer_validation_matrix36/36 (was 35/36 before the font fix).savegame_v3_contractandsavegame_corruption_contractpass.anderson,airdefense2andconvoy2refuse with zero map teardowns and zero errors;airdefense1,tram1b,process1_firststill load.game/airdefense1360-degree camera sweep, 1280x720 windowed, OpenGL, shipping defaults: 282.8–313.1 Hz across six passing captures, P95 6 ms / P99 7 ms, worst single frame 14 ms.docs/dev/airdefense1-optimization-evidence.mdrecords the method, the load-time breakdown, and the ranked remaining work.One measurement caveat recorded in the evidence: this host throttles hard under sustained load — back-to-back runs decay 303 -> 208 -> 183 Hz, and the CPU performance ratio swings 56–131% of nominal. All figures above were taken with cooldowns between runs.