renderer: idTech5-modernization ownership milestones + cinData_t init fix - #128
Merged
Conversation
Document the complete compatibility-safe roadmap and dated implementation state; land the P0 security, provenance, stock-evidence, PBR authoring/resource, packaging, renderer-readback, and multiplayer validation foundations. Companion game-code commit: 834ed9f28817e9d01355ea3aeecb0f144e1ce69b.
The struct was returned with cd.image uninitialized; consumers (RB_BindVariableStageImage, the Vulkan GUI executor) test cin.image and upload from it, so stack garbage triggered an UploadScratch from a wild pointer with 0x0 dimensions via any "waveform" videomap material. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # docs/dev/release-completion.md # docs/dev/releases/v0.12.0.md
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab67d4fe8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
game_type_module_selection.py was updated to require GAME_API_VERSION 45 but both workflows still fetched openQ4-game at the GAME_API 44 companion revision, failing Validation Script Smoke on every push. The 45-bearing companion commits are now published on openQ4-game's matching branch; pin its tip (80cbf476, "game: optimize presentation sampling and screen effects"), which the full local smoke sweep passes against. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
stage_fast_install.py now stages by the host platform runtime policy, so the guard/copy validation cannot pin Windows .exe/.dll artifact names: on Linux the policy excludes them and the staged-file assertions fail. Parametrize the fixture and assertions by sys.platform; exact per-platform policy output stays pinned by validate_fast_stage_cross_platform_policy. Verified via the full smoke sweep on Windows and WSL Ubuntu (ext4). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
draw_arb2.cpp and draw_common.cpp call std::memcmp/memset/memcpy without including <cstring>, which breaks the Linux ARM64 cross build (GCC libstdc++ does not provide them transitively there). vk_GuiExecutor.cpp has the same latent gap and is compiled by the Linux renderer-vk build, which has been skipped on this branch while script-smoke was red. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
uiFontParitySelfTest forces the retail bitmap path (the probe sets r_useTrueTypeFonts 0) and then requires fonts/marine to register, but an assetless validation startup has no retail .fontdat atlases, so registration cannot succeed anywhere CI runs the probe. Windows dev machines never showed this because fs_basepath auto-discovery silently mounts the installed retail Quake 4 assets. Skip the marine registration and atlas cases only when the TrueType path is off, registration failed, and no marine fontdat exists to load - the same say-so-instead-of-failing treatment the test already gives the TrueType path. With retail assets installed, or TTF enabled, the cases still run and still fail on a real regression. Verified on a CI-parity Linux x64 build (assetless): the parity failure is gone and the skip line is reported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RendererContractsTest deliberately stores out-of-range values (99) to prove the validators reject corrupt passes, but loading such a value from an unscoped enum without a fixed underlying type is undefined behavior, and the Linux ASan+UBSan job fails on the validator load in RendererContracts_ValidateMaterialPass. Give every contract enum ": int" - layout-identical to what compilers already chose, so the shared ABI is unchanged but now guaranteed, and any representable value round-trips with defined behavior. Verified with the contracts test under -fsanitize=address,undefined: old header reproduces the CI error, fixed header runs clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rning fixes Shadow-map corrections across classic GL, direct Vulkan, and the shared interaction paths: automatic culling stores the near shell for sealed geometry and renders open/uncertain stock geometry two-sided; large point-light bias is world-bounded; balanced filter footprints are tighter and cheaper; PCSS blocker search is bias-consistent; map/alpha/dynamic cache ownership fails closed; projected and point resources carry exact world/pass/storage provenance through bind time; partial mapped ownerships receive exact stencil supplements or return to full stencil; modern GL refuses resources it cannot consume completely; atlas placement follows physical limits; nested GL elapsed queries no longer raise validation errors. Stock startup warnings resolved without replacement content: the three brown-fluid images and the large water splash gain fail-after-primary shipped-media fallbacks; generated TrueType atlas materials bind their already-uploaded intrinsic images on first parse and survive renderer restart; wide loading backgrounds keep atomic publication under long Windows save paths via a root-relative 128-bit CSPRNG staging identity; generated image caches fall back from the legacy qpath to a versioned SHA-256/128 compact identity without bypassing VFS or pure-server policy; immediately clamped or rested rigid-body states become developer diagnostics while unrecovered physics faults keep warning severity; binary-image header, mip, and payload writes reject short output.
21 verified behavior-neutral optimization groups across the renderer and asset pipeline, found by a fan-out/adversarial-verify sweep: - modern GL: per-program shadow-uniform location memo; GPU-driven submit memset elision (~1.4 MB/frame); submit-plan permutation sort; shadow-planner fairness high-water mark; clustered UBO-fallback prefix uploads; GUI-domain index maps - classic GL/ARB2: GLSL uniform dirty-tracking and program-bind record cache; ClassicInteractionDomain hash-index lookups; decal sealed-command O(records) lookup with CSR scratch/seal swap-publish - frontend: AddActiveInteraction loop-invariant hoist and per-frame r_actualRenderApi memo; drawSurf area-resolve memo; scene-packet temporal history hashing; DM_CACHED bytewise-identical entity-update elide with a c_entityUpdatesElided counter under r_showUpdates; persistent-upload bind elision - models: generated-cache hits no longer re-run R_BuildDeformInfo or R_IdentifySilEdges (new R_AllocDeformInfo); array-codec model cache serialization - images: DDS-replacement probe memo; BinaryImage load copy elision - cinematics: frameSerial dirty-tracking (cinData_t gains frameSerial; header mirrored to GameLibs) - Vulkan: batched image upload replacing per-mip fence waits; triangle memo sized 4096; pipeline MRU and depth-fill rebind guards; interaction descriptor suffix-rebind - materials: custom-GLSL chain-fact static flag Validated: full MSVC build, 31 static pin tests, renderer validation matrix, gameplay benchmark smoke on game/storage1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…' into codex/idtech5-modernization-plan
The companion gains the lockstep cinData_t frameSerial / material GLSL chain-fact header mirror and the recovered rigid-body diagnostic severity change; GAME_API stays 45. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The release-docs generator rewrote every relative .md link to .html even when the target is not part of the generated site, leaving dead links; README's new .github/CONTRIBUTING.md reference broke the offline site's link integrity. Resolve each .md link against the rendered source set and point unrendered targets at the repository blob URL instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An extensive find -> adversarial-verify -> implement sweep across the renderer and asset pipeline. Every change was verified against the current tree and the token-pinning tests; behavior is unchanged for stock content except where a latent bug is corrected. Robustness (untrusted file parsing and error paths): - ROQ cinematic decoder: reject VQ frames before QUAD_INFO (was a wild heap write via uninitialized qStatus), bound the cel-stream walk to the chunk payload, and validate motion-compensation source blocks against the decode image; JPEG loader gains the TGA-style dimension overflow guard and a short-read check; MD5 mesh weight spans are now fatal-on-out-of-range instead of an OOB read/write. - Render-world .proc parsing: bounds-check inter-area portal areas and point counts, validate the BSP node graph before it is recursed, and validate shadow-model counts/indices (FinishSurfaces is skipped there). - Null/oob guards: ModelTrace deform+customShader surfaces, GetPortal area range, R_RemoteRender entity-less surfaces, zero-stage referenceShader, polytope degenerate windings, R_FindARBProgram table scan, R_FrameAlloc negative size, UpdateEntity/LightDef interaction table resize. - Vulkan: recover the frame fence on submit failure and on a device-lost BeginFrame/screenshot-resume instead of wedging or deadlocking; fail closed when a swapchain reports more than 8 images; latch the frame ring overflow warning so one oversized scene cannot storm the console. Correctness: - Beam and sprite dynamic models freed their persistent vertex cache so parm-driven geometry/color changes reach the GPU again (the beam's reuse path was dead code); the modern light-image atlas re-uploads when a reload changes source dimensions under a recycled GL name. Optimization (behavior-neutral): - Frontend: per-entity draw-surf-area memo invalidation and a memo clear in FreeDefs; cached point-light emitter-caster verdict; Trace loop hoist and allocation-free name compares. - Modern GL: viewLight->descriptor hash; mapped-light selection sorted once instead of rescanned; in-place shadow descriptor build; single memset for index/shadow staging; cluster bin bounds cached across the count/fill passes; light-atlas count refresh once per frame; debug-name formatting gated on the diagnostics that read it. - Vulkan: one ring flush per frame instead of per allocation; depth capture via a single flipped blit when supported; interaction pipeline bind de-duplicated across a receiver chain. - Classic GL: drop the redundant per-draw point-shadow-far upload; retain the scene-scale state across views; skip unit-1 combiner teardown when it was never set up; set the current-depth sampler parameters once per storage generation. Validated: full MSVC build (client, dedicated, both renderer modules); executed CI test suite; renderer validation matrix 36/36; gameplay benchmark smoke on game/storage1 for both OpenGL and Vulkan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
What changed
This branch carries the idTech5-modernization renderer line (25 commits):
ab67d4fe):idSoundSystemLocal::ImageForTimereturned acinData_twithcd.imageuninitialized. Consumers (RB_BindVariableStageImagein the GL backend, and the Vulkan GUI executor) testcin.imageand callUploadScratchfrom it, so stack garbage caused an upload from a wild pointer with 0x0 dimensions — reachable through any material using a"waveform"videomap. The struct is now zeroed beforestatus = FMV_IDLEis set, matching the pattern the otherImageForTimeimplementations already use.Why
The modernization milestones move renderer frame ownership onto explicit, sealed contracts so the modern GL/Vulkan paths and the classic ARB2 path can coexist without implicit shared state. The
cinData_tfix closes a latent wild-pointer upload found while auditing that consumer chain.Reviewer notes
cinData_tfix needs no consumer-side changes: both the GL and Vulkan paths already take a safe fallback branch whencin.imageis NULL.snd_system.cpp(tools/tests/level_load_cache.py,tools/tests/fatal_error_startup_safety.py) pass locally.🤖 Generated with Claude Code