Repository navigation
Conversation
…uest bytes The largest-accessible rule compared guest bytes only. Linear pitch aligns to 256 bytes, so a 32-wide and a 64-wide variant both take 1024 bytes, and the first one in the table won. The narrow variant then served both entries.
A borrowing entry took a texture slot of its own behind the other entries, and the full walk of a cached set looked it up by its own extent. That walk could never pass: the slot order failed it, or the lookup created the texture that substitution avoids and then failed it. A borrowing entry now shares the slot of its representative. The walk repeats the choice, fails when it moved, and skips the borrowers.
Contributor
|
Windows check of head Setup: Windows 11 Enterprise 22621, WinLibs GCC 15.2 UCRT, Release, Radeon RX 6700 XT, AMD driver 26.10.1. Run with
AI-assisted: yes. |
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (9 IDs in 595 tests)Skipped (56)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
A borrowing entry shares the texture slot of its representative, so the slot check in refreshOwnObjects failed on the second entry of every such set and sent it to the full walk.
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
Two bugs in the dynamic-resolution variant substitution from my #10256. A table entry can borrow the texture of another variant of its surface. Neither fix changes the rule that keeps such a table out of the reuse slots.
RunLargestlists the wide variant first, so it never reached the tie.revalidateFulllooked it up by its own extent. That walk could never pass. With the borrower ahead of another entry, the slot order failed it. With the borrower last, the lookup created the texture that substitution avoids and then failed it. A borrower now shares the slot of its representative. The walk repeats the choice, fails when it moved, and skips the borrowers. The own-image refresh (refreshOwnObjects) skips them too. Without that, a set with a borrower answeredUncachedthere at once, because two entries share one slot, and always took the full walk.Left as it is:
mainhas the same gap, and the debt entry now states it.This PR claims no change in speed. I found both bugs in the code and with tests, not from a wrong picture on a title. In 13 saved Wolverine runs, 130 surfaces are logged (122 largest accessible, 6 newest write, 2 active depth surface). The tie rule changes the chosen extent on none of them. The
refreshOwnObjectsskip has no test of its own: a scenario that reaches it needs a tiled surface with a pending alias, likeOwnRefreshRepackAlias, and I could not build one cheaply. A hand-built table without a borrower also failed to reach it.tools/pr_overlap.pyreports #15563 (the walk loop and therepresentativeSourcesline) and #15710 (debt entry, line 341). It could not compare the other PRs it lists, so I searched the diffs of all 62 reported PRs for the variant code and the walk line. Only #15563 and #15710 touch the variant code. #10894, #11444 and #15563 also edit the walk's lookup line. None of them has the tie rule or skips borrowers in the walk. I will rebase on whichever lands first.Tested
Rocky Linux 10.2 (kernel 6.12), i7-14700K, RTX 4070 SUPER, NVIDIA 615.71.10, Release.
mainc36ab99 and this branch (head c692bfd) on it, from the same machine and GPU. "Validation" meansctest -VwithVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1.main-R '^agc_driver_image_table', validation-R '^agc_driver', validationANYPS5_REQUIRE_VULKAN=1only, run before the last commit (809d1f3)Both sides fail the same tests:
agc_driver_division,agc_driver_f32_denormal_flush,agc_driver_image_table,agc_driver_mullit_f32_denormal_modes.agc_driver_image_tablefails only because its regex catchesVUID-VkDeviceCreateInfo-pNext-pNext, and the layer here is older than the driver.agc_driver_division,agc_driver_f32_denormal_flush,agc_driver_mullit_f32_denormal_modes,avplayer_http_patched_modules,ngs2_filter,vector_constant_factor_tininess.The
VUID-counts in the two^agc_driverlogs are identical:Neither log reports
was not found but was requested by env var VK_INSTANCE_LAYERS, apart from ctest printing its own regex.agc_driver_image_table_variantsalso passes under the same validation settings on Intel RPL-S and on llvmpipe, both Mesa 25.2.7, selected withVK_DRIVER_FILES.Negative controls of
agc_driver_image_table_variants, run on the previous base 3a2cd3d with the final test:mainwith the same guest bytes, the narrow variant ahead of the wide one did not borrow the wide onea set over two surfaces and a borrowing entry holds 3 texturesthe full walk looked up the own extent of a borrowing entrya set whose borrowing entry moved to another representative proved currentwith the same guest bytes, the narrow variant ahead of the wide one did not borrow the wide oneThe walk on
main, with the tie rule added and the slot count check taken out of the test:the full walk over a table with a borrowing entry failedthe full walk looked up the own extent of a borrowing entryThis branch has not run on a title. A wider local branch with the same tie rule and slot sharing ran Marvel's Wolverine (PPSA03671) twice for about 125 presents on the RTX 4070 SUPER, with variant tables active, and logged no refusal or crash.
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)agc_driver_image_table_variantsonly)