Repository navigation
fix(vk): admit only successful samplers across cache layers - #1754
Conversation
10a2f7f to
5fc19b0
Compare
hckrman101
left a comment
There was a problem hiding this comment.
Looks good. Checked the vk_draw.cpp changes against the head: sampler_for only caches non-null handles, sampler_make no longer emplaces a failed/undefined handle into the global map, the thread-local and seen caches are best-effort (the try/catch only drops an acceleration entry), and a failed insert destroys just the new sampler. The per-present failure set is read/written under ParLock, so the retry-on-next-present behaviour is safe across threads. bind_descriptors checks every combined-image-sampler array element before touching the pushed-state cache or allocating a set, and all three callers (graphics, compute, mesh fallback) already handle a false return. Storage images are untouched. The new test is registered in tests/run_all.py and CI is green.
Nit: a draw whose sampler fails is skipped for that present, which is the intended trade-off and is documented, so nothing to change.
Merging order: no direct overlap found with #1654/#1541/#1366; whoever lands second just needs to keep the positive-only admission in any added table.
Preserve retry across registration and thread caches after factory failure, bound attempts per key and present, and roll back an uncommitted resource without losing existing global ownership. Assisted-by: GPT-6 (OpenAI)
Validate every combined-image-sampler array element before pushed-state mutation or pooled-set allocation. Exercise complete binding paths with cold and warm factory failures, recovery and storage-image compatibility. Assisted-by: GPT-6 (OpenAI)
Record observed build, hardware, driver, checkpoint and serial/parallel resize results following the updated contribution guidance. Keep the run tied to its actual pre-rebase build. Assisted-by: GPT-6 (OpenAI)
5fc19b0 to
efe9837
Compare
Failed sampler creation can permanently poison the ID, thread-local and global caches, or return an undefined error-output handle. Returning null alone is insufficient: the final descriptor consumer must also decline the operation instead of passing a failed sampler to Vulkan.
Admit only successful handles through every cache layer and validate every combined-image-sampler array element before either pushed-state mutation or pooled-set allocation. Existing draw, compute and mesh-fallback callers already honor binding failure. Bound failed creation per parameter key/present and retry on later presents without a registration change. Owning insertion failure destroys only the new sampler; acceleration-cache allocation failure preserves registered parameters and globally owned handles. Failed failure-tracking allocation postpones new keys until the next present while valid hits remain available.
Filtering, LOD/compare/border/reduction/anisotropy policies, successful sampler lifetimes, registration checks, replay locking and API12 are unchanged. Storage images still accept their unused null sampler. No replacement sampler silently changes the requested filtering policy.
Validation at
10a2f7f6b25464fdfcc85ef4109ffd7e3f802e9f, based on maincaaee0ba6aef97514132603c9e6fa9e58f99bf57:cbe49ebremain historical; the tested-configuration row names that observed build, hardware, driver, checkpoint and limits.Fresh actual-patch overlap review distinguishes #1654's scene LOD bias/additional biased ID table, #1541's writer/diagnostic refactor, #1366's filter controls, and #1653/#1008's provider-specific resources. This PR implements none of those features. If the sampler signature/table changes first, preserve positive-only admission in every added table and update the source fixture during rebase. No code was copied from those PRs.
The failure models do not establish native GPU OOM/device-loss recovery, image-quality or performance improvements, or campaign coverage. Only authored source, tests and documentation are included; no generated game code, dumps, captures or saves are published.
Current-head fork workflows are pending:
See
docs/sampler-admission.mdfor the contract and model boundaries.Current-head validation
Current head
efe9837, based on main7047756. Earlier local/runtime results describe their recorded source hashes.The standalone contribution patch is unchanged after the audited main refresh; exact-head build/source checks are refreshed below. Earlier runtime evidence remains tied to its recorded hash.
Fork runs are separate from upstream Actions approval; no maintainer approval gate is bypassed.