Repository navigation
feat(libSceAgcDriver): take a one-layer 2D array view of a depth surface plane as the plane - #2076
Conversation
|
Thanks for the review. Both points are addressed; details are in the updated description.
|
This reverts commit f99c0e9 (upstream PR boykopovar#2076's commit) on the PPSA08427 branch. With stencil-plane storage images the title's compute writes CMASK codes 1-14 into the CMASK of a 2560x1440 fast-clear target; every later draw to a CMASK target is then skipped and the loading screen flickers between its art and black. With the old throw (the pass skipped) the codes stay 0/15 and the screen is stable (A/B on the same build: 7 vs 0 CMASK errors by the menu). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uzy15wqYTt6iWyxYXEzUWo
|
A case this PR should handle: a storage image at a stencil plane's address that is not the stencil plane. In Palworld (PPSA23126), with this PR's
The color target stays non-resident ( Seeding only an image shaped like the plane (an R8 format, one slice, one mip, the surface's extent) and treating anything else as reused memory fixes it locally: @@ void SeedStorageFromStencil(const Context& context, const std::shared_ptr<StorageTexture>& storage) {
if (found == list.rend()) return;
auto& surface = **found;
+ const auto& descriptor = storage->Descriptor();
+ const auto storageFormat = storage->StorageFormat();
+ const bool r8 = storageFormat == VK_FORMAT_R8_UINT || storageFormat == VK_FORMAT_R8_UNORM || storageFormat == VK_FORMAT_R8_SINT || storageFormat == VK_FORMAT_R8_SNORM;
+ const bool oneSlice = descriptor.dimension == TextureDimension::k2D || (descriptor.dimension == TextureDimension::k2DArray && descriptor.depthOrLastArray == 0);
+ if (!r8 || !oneSlice || descriptor.mipCount != 1 || descriptor.width != surface.target.extent.width || descriptor.height != surface.target.extent.height) return;
if (surface.stencilSeeded == storage.get() && surface.stencilWriter.lock() == storage) return;The condition is the one Limits of this report:
AI-assisted (Claude Code) |
|
@jdmagic for coordination, since the two overlap: #4500 adds storage images over both planes of a depth surface. The stencil plane is bound as an What this pull request covers beyond #4500:
The case @queeeef describes (a 2536x1428 If that works for you, these parts could build on #4500 once it merges. If you would rather structure it differently, I can adjust #4500. |
f08fc22 to
abe2f32
Compare
|
@ee6aba54 Thanks, building on #4500 works for me. I reworked this PR onto it; the description is updated. Of the parts you listed, only the one-layer 2D array view is still needed: PPSA01289 binds its stencil plane as a one-layer
@fisherjoey, thanks for 598f788. Its coverage is now in #4500's |
adriadam10
left a comment
There was a problem hiding this comment.
#4500 is merged, and it landed with two extra commits from @oneandonlydean (26dfc464a7, 848b97b611) that this PR's last commit doesn't contain. It conflicts with main in DepthSurface.cpp, tests/DepthSurfaceReuse.cpp and tests/execution/DepthStorage.cpp (add/add), and it isn't a mechanical rebase:
26dfc464a7makes a storage view that is not 2D throwstorage image access to depth/stencil surface ... is not implemented(DepthSurface.cpp:165). This PR's change is to take a one-layer 2D array view as the plane, so it has to be reworked as a narrow exception to that check (layer 0, one layer, plane extent and format), keeping the throw for every other non-2D view.- Main now has
agc_driver_depth_storage_dispatch(dispatch-based test with the refusals). Please put the one-layer array cases (depth, stencil, and the two-layer view that must still throw) there as well as inagc_driver_depth_storage, and check the new tests fail without the change. StoreBackthrough a view replaced by another view of the same plane now throws; say what happens when the dimension change recreates the storage image (the "recreated when the view's dimension changes" part) with respect to that.- Dean noted that #2491 will conflict in
DepthSurface.cppand thatStoreBackhas to bump the write serial there; keep that in mind when rebasing.
Please rebase on current main so the PR is only its own commit, rerun the depth/agc tests, and update the body's Tested section for that head (it now says it was tested on #4500's old head and the full ctest wasn't run).
…ace plane as the plane A storage view of a depth or stencil plane was taken for the depth surface only as a 2D image. A 2D array view of layer 0 with one layer and the plane's extent and format is the same plane, and now gets the plane's storage image too; a view with more layers still takes the ordinary path. The plane's storage image is recreated when the view's dimension changes. PPSA01289 (Sackboy: A Big Adventure) binds the stencil plane of its 2880x1620 D32S8 surface as a one-layer R8_UINT 2D array from a compute shader during presentation, and read guest memory instead of the stencil. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…w exception to the non-2D refusal Main refuses every storage view of a depth or stencil plane that is not 2D. A 2D array view is now taken as the plane only at layer 0 with one layer; a view with more layers or another base layer still throws. A one-layer 2D array view replaces a 2D view of the plane like any other view, so a store back through the replaced 2D view throws. The depth storage dispatch test runs one-layer views of both planes and checks that the depth and stencil tests see what they stored, and that a two-layer view still throws.
abe2f32 to
cc3b47d
Compare
|
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.
|
|
Rebased onto main 77b9f72 so only your commit remains, and added one on top for the review:
|
Met in cc3b47d: the one-layer array view is a narrow exception (layer 0, one layer) and every other non-2D view still throws; the cases are in agc_driver_depth_storage and agc_driver_depth_storage_dispatch and fail without the change; a store back through a replaced view throws. See the comment above.
127 files conflicted. Resolved per group (AgcDriver depth / graphics / execution, shader recompiler + libSceAgc, libc, libkernel, net/np, media, Usbd, misc, docs), each group checked against both sides by a second pass. Kept from local: the Astro Bot perf work (draw prep helper, per-unit pending image checks, depth plane copies only for changed slices, layer copy on the device, register clear without the key-fill wait, GDS DMA_DATA without drains, 32-bit fault words for validated V# atomics, request version 22 with frontFaceAllBits), local swscanf_s/vsscanf_s, cpuset_getaffinity, Mbed TLS, the libSceUsbd transfer tracking on top of main's libusb path, the libReactNative VSH modules main deleted (boykopovar#5010), the Ngs2 GeomApply and AudioIn Hq/Async open implementations. Took main: sceKernelFcntl (File::Fcntl), sceCoredumpDebugTextOut, asinl, nan/nanf, the depth/stencil plane storage path (boykopovar#4500/boykopovar#2076) in cachedStorageTexture, the epoll model in libSceHttp, Json2 setSpecialFloatFormatType storing any value (RE4, PRAGMATA call it), the per-library debt files under docs/dev/debt. Post-merge fixes: DrawKeyRegisters holds 50 ranges, draw prep passes the hidden user words, duplicate VideoOut stubs and libc definitions removed, APS5_LIST_ALL_COMPLETION_WRITERS also restores whole-write overlap tests in the readers, tests adjusted where the kept implementation changed on purpose (fcntl, pm4 message, pixel push slots, v19 request layout, font syllable state, Json2, depth plane drawn note, atomic fault words). Build clean, test_fast 1125/1125 (guest_raise_exception and guest_video_out_high_frame_rate pass alone; parallel-load timing). Progress 3248/3529 -> 3387/3716.
…ed path SILENT HILL: The Short Message (PPSA10112) stopped at "compute shader 0x2007a70000: AGC graphics: storage image access to depth/stencil surface 0x205da70000 is not implemented (storage=1024x1024 guest format=7 mip=0 dimension=2 tile=4)": a compute shader binds a depth surface as a 2D array storage image of more than one layer. Main's depth plane storage path (boykopovar#4500/boykopovar#2076), which replaced our boykopovar#425 port in the e965b46 merge, takes one layer only. DepthSurface::Storage now leaves a depth (not stencil) 2D array from layer 0 to the ordinary storage path, and cachedStorageTexture seeds that image from the depth slices again (SeedStorageFromDepth: every layer that is a depth surface of its own is copied in, the surface becomes its writer and takes its writes back when it is drawn to; the code the merge left without a caller). Stencil arrays and arrays from a later layer still refuse. Tests: agc_driver_depth_storage expects the two-layer depth view to be left to that path; agc_driver_depth_storage_dispatch now stores through a two-layer 2D array view and requires the depth test to see layer 0's stored row (was: expected the refusal). test_fast 1126/1127 (windows_import_modules = Defender WinError 225).
What
Depends on: #4500; its two commits are included. Only the last commit belongs to this PR.
#4500 takes a storage view of a depth or stencil plane for the depth surface only when the view is a 2D image. A 2D array view of layer 0 with one layer and the plane's extent and format is the same plane, but it took the ordinary storage path and read guest memory instead of the plane.
agc_driver_depth_storage: a one-layer 2D array view of the depth plane reads back depth stored into the surface (0.5 while guest memory holds 0.75), a one-layer 2D array view of the stencil plane reads back the cleared stencil over zeroed guest memory, and a two-layer array view of the stencil plane is not taken for the surface.Sackboy: A Big Adventure (PPSA01289) binds the stencil plane of its 2880x1620
D32_SFLOAT_S8_UINTsurface as a one-layerR8_UINT2D array from a compute shader during presentation.Tested
Windows 11, MinGW-w64 GCC 15.2.0 (WinLibs r7), NVIDIA GPU, on #4500's head cdfba51 (main 5efd09a + #4500).
agc_driver_depth_storage,agc_driver_depth_storage_reuse,agc_driver_depth_compare_copy,agc_driver_depth_fast_clear,agc_driver_depth_fast_clear_submit,agc_driver_depth_bias_per_face,agc_driver_graphics: 7/7 passed. The fullctestwas not run on this branch.agc_driver_depth_storagefails witha one-layer 2D array view of the depth plane was not taken for the depth surface.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code