Repository navigation
fix(agc): key a stage without push constants at offset 0 in every pipeline key - #16748
Open
chitters2004 wants to merge 1 commit into
Open
chitters2004 wants to merge 1 commit into
chitters2004 wants to merge 1 commit into
Conversation
…eline key Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
This branch has not been deployed
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
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
With graphics pipeline libraries, registration prebuilds a pixel shader's libraries at its fixed push slot (
FixedPushOffset(Stage::Fragment)inShaderRegistry.cpp), also when the shader has no push constants. A draw places such a stage at offset 0 (Driver::draw,Execution/src/Driver/Draw/Draw.cpp). The push offset is part of the fragment shader library key, so a draw never found the library prebuilt for a pixel shader without push constants and built it again.Graphics::KeyedPushOffsetnow holds the rule (no push constants, offset 0), and every key that hashes a stage's push offset uses it: the pipeline library keys and the full pipeline key inPipeline.cpp, and the draw validation and shape keys inGraphics/src/Draw.cpp. Prebuild and draw both reach the library keys through thePipelineconstructor, so neither caller needs to normalize the offset. Slot and push cursor allocation are unchanged, and a stage with push constants keeps its offset in the key.Every pixel shader without push constants that a title registers is affected when graphics pipeline libraries are in use: its prebuilt fragment library was built and never used, and its first draw built another one. Prebuilt libraries that differ from the draw's in depth or stencil attachment formats still miss; that is a separate change.
Overlap, checked on 2026-11-07 against
main46552cb461withtools/pr_overlap.pyand by hand: one open pull request changes lines this branch changes: #9508 adds descriptor heap helpers toShaders.hppbesideKeyedPushOffsetand editslibraryKeysandValidationKeya few lines from it, adding descriptor placements to the same keys; the rebase is mechanical. No open pull request implements the same thing, and none keys a pipeline by a push offset outside the functions changed here: #9356, #9387 and #15548 build a mesh compute stage fromshaders.front().pushConstantOffset, which goes through the same key functions, and #11551 and #16413 use the offset to patch push data, not to key a pipeline. Rechecked against4977f64052: the tool confirms no conflict and could not compare 49 pull requests because they conflict withmain, among them those named here.Tested
Head
0a25af4ca7onmain4977f64052.Refreshed onto
main4977f64052. Head0a25af4ca7;git range-diffagainst0e323b1bdcshows the same patches (1 commit,=). Local RTX 3080 gate on both, same environment as below (ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation), built one after the other in one build tree, with the focused AGC set of all five pull requests of this series (232 tests on this head, 232 onmain): both 100 % passed, noVUID-line in either log or in replay subprocess logs, and neither log containswas not found but was requested by env var VK_INSTANCE_LAYERS. Fork CI on the rebased head against4977f64052: run 42371889894: Linux 11 of 11 passed; Windows 10 of 11, whereagc_driver_recorderfails onmaintoo. NoVUID-line on either side, conventions check passed. The full runs below were on the previous base46552cb461.Vulkan validation. RTX 3080,
ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ctest --test-dir build -R 'agc|write_tracking|guest|indirect_draw|draw_finisher|depth_bias|draw_pipeline' -Von0e323b1bdc(3199 tests) and onmain46552cb461(3199 tests). Every test failing in either full run was then rerun alone on both builds, one build at a time with nothing else on the GPU, usingctest -Rwith the union of the 16 names and the same environment: on both, 15 of 16 passed and onlyagc_driver_direct_write_completionfailed, which fails onmaintoo (#15547); the other failures of the full runs were timeouts and0xc0000409exits under load from concurrent gates and did not reproduce alone. Both full logs hold the same 18VUID-VkPipelineColorBlendStateCreateInfo-pAttachments-00605lines and no otherVUID-; neither containswas not found but was requested by env var VK_INSTANCE_LAYERS. The change adds no validation error.Fork CI (GitHub runners, Mesa lavapipe with the Khronos validation layer, head against
main): run 42329046283, rebased there onto46552cb461: Linux 11 of 11 passed; Windows 10 of 11, whereagc_driver_recorderfails as it does onmainon the same runner. NoVUID-line on either side, conventions check passed.Tests.
agc_driver_prepared_graphics_prebuild_unset,_0and_1(PreparedGraphics.cpp): when the device has graphics pipeline libraries and prebuilding is on, a pixel stage without push constants is prebuilt at its fixed slot, which builds one fragment library, then drawn at offset 0 throughVulkanDevice::Draw; the draw must not build another fragment library.agc_pipeline_libraryandagc_pipeline_library_fast_only(Recorder.cpp): a pixel shader without push constants looked up at its fixed slot reuses the libraries built at offset 0. The existing fixed-slot check now uses a pixel shader of its own, since the same shader no longer builds a second library.main(e509b29247): withKeyedPushOffsetreturning the raw offset, which is the behaviour ofmain, these tests fail with "a draw built the fragment library its pixel stage had prebuilt" and "a pixel shader without push constants built its libraries again at its fixed slot"; with this change they pass.GTA V. This is a fix with no speed claim. The case it fixes made no measurable difference in GTA V (PPSA04263, content 01.011.001) on the RTX 3080 (driver 621.14, Windows 10.0.26220, before Resizable BAR was turned on), counted with a local counter patch that reports pipeline library part lookups, hits and builds per stage, applied identically to both sides and not part of this PR (diag-base.patch, diag-push.patch). Every boot ran
APS5_PROFILE_DRAW=1, so these are counts, not timings.mainwas7383be1a19; the change wasb115d173d8, this branch before its rebase onto46552cb461(git range-diffshows the same patch).mainboots and one boot of the change: no fragment library part was built at draw time in the window on either side. Boot totals at the window end: fragment parts built at draw time 536 and 540 onmain, 535 on the change; hits 226 and 231, and 228.ANYPS5_NO_SHADER_CACHE=1and an empty driver cache: fragment hit rate 53.6 % onmainand 51.6 % on the change, 0.464 and 0.484 fragment parts built per lookup, 3.62 and 3.70 presents per second. The prebuild at load was identical on both sides (3,104 prebuild calls, 2,584 fragment parts built). The hand-driven routes differ by 11 % in library lookups, so differences below about 15 % are within the noise of this comparison. Free roam on thismainneeds open feat(agc): implement measured single-sample dithered alpha coverage #14561, so both drive builds also carried its diff.Counter lines: standing counters-base.txt, counters-base2.txt, counters-cand.txt; cold drive counters-cold-base.txt, counters-cold-cand.txt.
On an older
main(08a10d2253, committed 2026-10-30) with an earlier form of this fix, cold boots to 150 s of gameplay counted 22 draw-built fragment parts that differed from a prebuilt part only in push offset (0 with the fix) and 271 to 273 fragment parts built by draws onmainagainst 251 with the fix. The currentmainscenes above show no such difference.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code