Repository navigation
fix(agc): preserve fresh HTILE depth and stencil clears - #6438
Conversation
|
I ran this PR's depth tests on Linux with lavapipe (Mesa 26.2.4, LLVM 23.1.1), GCC 16.2.1. The checks this PR adds to Setup: head 9870b37 merged into main at 92e5041 (no conflicts; the PR is based on that main), clean environment ( Default build (CMAKE_BUILD_TYPE empty, BUILD_TESTING=ON):
Release build (
So in this setup, the check at that message fails in the unoptimized build whether or not the fix is there, and the whole test passes in Release whether or not the fix is there. All build and test jobs (Linux and Windows) were skipped for this head, so CI has not run the PR's new checks yet. I have not found the cause of the build-type difference. I did not check Windows or macOS, or a GPU other than lavapipe. AI-assisted: yes |
9870b37 to
8d44a5e
Compare
|
Reproduced the unoptimized failure and corrected the fixture in The small depth/stencil arrays did not cover their 64 KiB tiled footprints. Declaration-order placement put HTILE writes inside those ranges. With Release runtime libraries held fixed, Corrected fixture at Final normal Release build and six focused CTests pass on each driver, no skips. The updated description separates this isolated |
|
Re-ran the same check on head 8d44a5e, same setup as before except that the head is now based directly on main 5d9dcc4 instead of merged into it: lavapipe (Mesa 26.2.4, LLVM 23.1.1), GCC 16.2.1, a clean environment, The failure I reported in the default build ("a fresh depth-only HTILE clear lost its initial depth or inherited stencil data") is gone with the full-footprint fixtures. That fits your explanation that the undersized arrays let HTILE writes land in neighbouring globals depending on layout, though I only tested the new head and did not rerun the old fixture on the new base. This is a full unoptimized build ( AI-assisted: yes |
8d44a5e to
14f92f5
Compare
|
Windows NVIDIA run, since the description lists RADV and lavapipe only. Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 (driver 617.42), AMD Ryzen 7 5800X, WinLibs GCC 15.2.0 r7, Ninja, Release, bundled dependencies,
No title run. AI-assisted: yes (runs done with Claude Code). |
14f92f5 to
cb113f2
Compare
cb113f2 to
6326d05
Compare
What
A fresh depth/stencil attachment receives
DB_DEPTH_CLEARandDB_STENCIL_CLEARin its constructor. Repeating a partial HTILE clear on its first bind loses the initialized stencil value in the tested RADV sequence; DOOM + DOOM II shows duplicated/outlined collection text.Use the constructor's clear for the cleared planes of a new attachment, inherit only the other planes, and retain the pending-clear path for reused attachments. HTILE decoding, expanded metadata write-back, caching, pipelined draws and native-owner guards remain in place. No general Vulkan-driver defect or speedup is claimed.
Review follow-up
The reported unoptimized fixture failure was real. Its small depth/stencil arrays held only logical pixel data, but
DepthSliceBytestracks a 64 KiB tiled footprint for each plane. The declaration-order layout put subsequently written HTILE bytes inside those undersized ranges. Changing only test translation-unit ordering reproduced the distinction:-O0 -ftoplevel-reorderpassed, while-O3 -fno-toplevel-reorderfailed at the inherited-stencil assertion, with the runtime libraries unchanged in Release mode.Allocate the complete plane footprints and check their sizes before drawing. This keeps all inherited-plane assertions and makes the
-O0fixture pass on both drivers. The corrected fixture still distinguishes the production fix on RADV; lavapipe passing with and without the production fix was already the expected driver-specific result.Dextroid17 independently retested corrected head
8d44a5e6con lavapipe Mesa 26.2.4 / LLVM 23.1.1 with GCC 16.2.1: the three depth checks pass in three runs of both a full unoptimized build and Release, with or without the production change. This confirms the fixture repair on that driver; their setup has no RADV and does not test the RADV negative control. Independent result.Tested
Latest refresh: four linear topic commits on main
55f82fee838c9895dd19ae8a997cbc935ef566e1, head6326d05f02868252bf64cc3e1699bb60e84fdf9f. Resolves the sharedDepthFastClearHarness.hppconflict after #7044 merged, preserving both configurable target dimensions and main’s configurable viewport depth range. The other three topic commits are unchanged by range-diff. Release driver and seven focused test executables build; 7/7 checks pass on RADV and 7/7 on lavapipe, zero skips and zero core-validation errors/VUIDs. Conventions and whitespace pass. No fresh title run, full suite, Windows run or performance claim; prior negative controls remain historical. Pinned refresh report.Earlier refresh: four linear commits on main
c0a7468f6f666a46b1c3e2fb14bb651fed05afc5, headcb113f2a645973a1bb2e5834ca5377c7d1148dc4. Main's newer plane-fill handling is retained. Fresh attachments inherit an explicit pending depth/stencil plane fill while constructor-initialized HTILE clears still avoid a redundant partial clear. New assertions cover both pending-fill aliases. The unadapted rebase fails the new stencil assertion on RADV. With unchanged main'sDepthSurface.cpp, the existing HTILE regression fails and the new plane-fill assertions pass; restoring the fix passes both.Release driver and seven focused executables build. The six earlier focused checks plus
agc_driver_depth_plane_fillpass 7/7 on RADV and 7/7 on lavapipe, zero skips, zero core-validation errors/VUIDs (0.81 s / 1.09 s). Conventions and whitespace pass. No fresh title run, full suite, Windows run or performance claim in this refresh. Pinned report, RADV checks, lavapipe checks, main control.Earlier validation head
14f92f555ead00380215da5bd9da193feabb25d4rebases the same three topic commits onto main66e290f1c04a6aa08271f106d0dc83ba06561bfc. Full Releaselibsand the six focused executables build with the published, clean SPIRV-Tools revision7f2d9ee926f98fc77a3ed1e1e0f113b8c9c49458, without a private dependency change. The six checks listed below pass 6/6 on RADV and 6/6 on lavapipe, zero skips (0.45 s / 0.56 s); conventions and whitespace pass.A separate isolated Override 2 PPSA01907 v01.007.000 startup run reaches the menu and closes normally with exit 0 after 93.41 s, restoring temporary settings. Its 80 s screenshot still has very dark 3D output; this topic does not resolve that observation. No new Override compatibility or FPS gain is claimed. Current validation and diagnostic screenshot:
htile-main66-validation-public.json,htile-main66-override2-menu.png, and the twohtile-main66-*-tests.logfiles.Earlier public head
8d44a5e6cd06da28a47b13fd3afcb479ae0f6298: three linear commits on main5d9dcc42d. Linux 7.2.9-arch1-1, GCC 16.2.1, Ryzen 9 7950X, RX 6650 XT / RADV Mesa 26.2.4-arch1.1; lavapipe Mesa 26.2.4 / LLVM 23.1.1.libsand six focused test executables build.agc_driver_depth_htile_write,agc_driver_depth_fast_clear,agc_driver_depth_fast_clear_submit,agc_driver_native_multisample_depth,agc_driver_depth_storageandagc_driver_depth_surface_last_writer: 6/6 pass on RADV and 6/6 on lavapipe, zero skips. Tests require Vulkan and select each driver explicitly.-O0, leaving runtime libraries in Release: both drivers pass. Restore main'sDepthSurface.cppwithout rebuilding that executable: RADV failsa depth-only HTILE clear discarded the initial stencil clear; lavapipe passes. Restore the fix: both pass. Fixture SHA-256 remains335a11180bf09d9651e41e67b6bd7622279dea644f7974c0e7e07bba287a51fbthroughout. This isolates the fixture failure and checks the production regression; it is not a full unoptimized-runtime build.ff23657f3plus shared boot prerequisites: control53f2e521ce260772f8fc47f8b1043e36e704cec9has corrupted collection-description text at 30 s; adding this production fix in4cb5da916d5706ca1e37ed6456546bfd32e52c36gives readable text at the matching capture and reaches E1M1: Hangar by 170 s. Both end at the scheduled watchdog (124); host configuration is restored. No fresh DOOM run on the new base, sustained gameplay, audible-audio result or FPS gain is claimed.Earlier validation validation/htile-review-5d, head
0a1fd1b43, adds only the common #6476 source-level validator repair to this PR on main5d9dcc42d. The three changed source/test files are byte-identical to this public branch. It uses the published third-party revision, not the earlier private submodule repair; #6476 is excluded from this PR. Earlier native integrations also share #5916 and #6348 to pass audio initialization. I have not run a full unoptimized runtime, fresh Windows build, complete CTest suite, or fresh DOOM comparison; the independent lavapipe result is credited above. No game assets, raw title logs or binaries are attached.Merge ordering: #6423 first. It changes the same creation/clear block; rebase retaining its
Refusediagnostic after it merges. #6300's native-MSAA handling and the manually reviewed stale overlaps #1395, #1368, #5499 and #6292 do not implement this fresh single-sample plane correction.Neithennn independently reports a full build on Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 driver 617.42, Ryzen 7 5800X, WinLibs GCC 15.2.0 r7 / Ninja / Release, bundled dependencies,
BUILD_TESTING=ON,ANYPS5_NO_SHADER_CACHE=1. This exact PR head14f92f555was merged onto mainf51724078: 8/8 focused depth tests pass withANYPS5_REQUIRE_VULKAN=1. Full CTest is 1397/1398:guest_memoryexits0xc0000409once, then passes 5/5 serial repeats; unchanged main passes 1398/1398. This remains an unexplained intermittent full-suite failure, not an all-green result. No title run. Contributor-reported Windows/NVIDIA validation; not independently reproduced locally.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)