Repository navigation
test(agc): cover the DWORD rounding of guest buffer view ranges - #6695
1 commit merged into
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@adriadam10 a short request for review of this test-only PR ( AI-assisted (Claude Code). |
|
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.
|
jotitadev
left a comment
There was a problem hiding this comment.
The new cases pass, but they only cover the case where both branches of ViewBytes are right. Every byte count in the test is a multiple of four (4, 16, 4096). Off a DWORD boundary, the function returns 4 * (adjustment / 4 + bytes / 4 + 1). That truncates bytes / 4 on its own, so when adjustment % 4 + bytes % 4 > 4 the range stops before the view's last bytes:
| bytes | adjustment | ViewBytes |
bytes the view spans from the aligned start |
|---|---|---|---|
| 7 | 2 | 8 | 9 |
| 6 | 3 | 8 | 9 |
| 3 | 3 | 4 | 6 |
| 2 | 3 | 4 | 5 |
| 4098 | 3 | 4100 | 4101 |
Those byte counts can happen: the size comes from ShaderBufferResource::GetSize(), which is Stride() * NumRecords(), or NumRecords() when the stride is 0, with no rounding (ShaderResources.cpp, the guest-buffer path that pushes {address, size, true, ...}). So a byte buffer with an odd record count, or a stride of 2 or 6, gets a descriptor range one to three bytes short of the bytes the shader may read. The same value also sizes the snapshot and overlap checks at the two ViewBytes(size, item.adjustment) call sites.
For the unaligned branch, the covering range is 4 * ((adjustment + bytes + 3) / 4). It equals the test's bytes + 4 * ((adjustment + 3) / 4) whenever bytes is a multiple of four. With that expectation and 7ull added to the outer list, adjustments 2, 3 and 7 fail on current main. Whether the fix belongs in this PR or a separate one is up to you. As it stands, the test only checks the formula where it agrees with the intended one.
Checked with a standalone copy of ViewBytes from this head, built with clang++ -std=c++20 on macOS: the PR's 27 cases pass, and the rows above come out as shown.
Written with AI assistance (Claude Code). I ran the checks above on my machine.
|
The #6999 report confirms a real under-coverage in the off-DWORD branch. I am preparing a separate follow-up with the production formula correction and regression sizes 2, 3, 6, and 7. Since it touches the same test file, I plan to base it on your test commit and list |
|
The follow-up is now open as #7001, based on your test commit unchanged and declaring |
660cf7b
What
GuestBufferMemory::ViewBytesgives the descriptor range of a guest buffer view that startsadjustmentbytes after a storage-buffer offset boundary (added in 38e89df). No test calls it directly;BufferUnalignedBasereaches it only through a whole draw. AddguestBufferViewBytesTeststoagc_driver_graphics_tests:4 * ceil(adjustment / 4)), the case 38e89df describes as "the range covers the dwords around it".Byte counts are multiples of four (4, 16, 4096). The expected values follow from that commit message and the function; the test does not change behaviour and passes on
main.Tested
Release build, MinGW-w64, Windows 10 (19045),
main5d9dcc42dplus this commit (edb74cb58; merge-tree againstmainis clean).ctest -R '^agc_driver_graphics$': passed (7.4 s). With the first expectation changed tobytes + adjustment + 1the same test fails with "a view whose adjustment is a whole number of DWORDs did not add the adjustment", so the new cases run.tools/check_conventions.py --base upstream/main: all checks passed. No Vulkan device is needed by the new function; the executable itself ran against the NVIDIA GeForce RTX 4080 SUPER driver. Linux/lavapipe not tested.Checklist
main; no other open PR implements the same tests (open PRs that edittests/Graphics.cpp, fix(agc): keep the guest's sample order where pixel (0,0)'s locations stand in for a 2x2 grid #6683, fix(agc): read a multisampled target's CMASK by its MSAA codes #6669 and fix(agc): bind image and sampler tables through update-after-bind limits #6647, change other functions)NotImplemented_nid_no_patch); no stubs added.mdfiles or images added to the repositoryAI-assisted (Claude Code).
🤖 Generated with Claude Code