Repository navigation
fix(agc): widen a vertex format the device lacks to the buffer element's - #9436
Conversation
A 16_16_16_16, 8_8_8_8 or 32_32_32_32 attribute that the shader reads with fewer components is bound with the narrower Vulkan format, such as R16G16B16_SFLOAT for three components of 16_16_16_16_FLOAT. A device without vertex buffer support for that format (MADiSON on an RX 5600 XT stops with "device does not support vertex format 90") now gets the next wider form of the same buffer format it supports, up to the element's own width: the shader input is unchanged and Vulkan discards the extra components. The vertex copy and the zero buffer grow by the widened bytes so the last vertex's read stays inside them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The implementation is narrowly scoped and the reported wider-format path exercises the intended Vulkan fallback, but three merge blockers remain under the repository checklist:
-
The PR changes AGC vertex input handling, and
CONTRIBUTING.mdrequires the same focused tests onmainand this change withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1, andANYPS5_REQUIRE_VULKAN=1, withctest -Vlogs confirming the layer loaded and a comparison showing no newVUID-errors. The description says the layer is not installed and no comparison was made. Please rerun that paired validation on a machine with the layer, or leave this as an explicit unresolved validation blocker for a maintainer with the required environment. The hosted Linux and Windows jobs are skipped, so they do not supply this evidence. -
The overlap report names open PR #8083 in
Draw.cpp. The description says whichever lands second will need a one-line rebase, but the checklist saysDepends on: noneand does not record an agreed landing order. Please coordinate the order with #8083's author and list the prerequisite on the checklist, or document that the overlap has been resolved and refresh the overlap check. -
The
Unblocksline identifies a MADiSON stop but explicitly lacks both the title ID and a Game bring-up discussion. The repository checklist requires both for a title-blocking PR. Please add the verified title ID and discussion reference, or usenoneif the report cannot be tied to a qualifying bring-up entry.
The change-specific tests in the description are author-reported (15 focused tests on main and the branch, repeated twice); I did not run them locally because no matching PR checkout/build is available in this workspace. Current automated checks show conventions, eligibility, labels, and report passing; Linux, Windows, relinker, ASAN, register-context, and write-watch jobs are skipped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@MarcZX10 thanks. On the three points:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dismissing a stale review: the requested changes were addressed after it (new head and/or updated description with the asked-for runs) and the merge queue blocks on it. Please request changes again if anything is still open.
|
Re-checked against current |
What
On a GPU without vertex buffer support for the Vulkan format
DecodeVertexFormatpicks, a draw stops inBuildVertexInputLayoutwithAGC graphics: device does not support vertex format N. MADiSON stops there on Windows with an AMD Radeon RX 5600 XT, as reported by Hiro_02 on the AnyPS5 Discord (#showcase thread "MADiSON"; Ryzen 5 5600G, RX 5600 XT, 16 GB). The menu runs, and entering the level ends with:Format 90 is
VK_FORMAT_R16G16B16_SFLOAT. The attribute's buffer format is 71,16_16_16_16_FLOAT, whose elements are 8 bytes, and the shader reads three components, soDecodeVertexFormattakes the three-component format.BuildVertexInputLayoutkeeps that format when the device supports it, as before. Otherwise it takes the next wider form of the same buffer format that the device supports, up to the element's own component count: four components for8_8_8_8_*,16_16_16_16_*and32_32_32_32_*, two for8_8_*,16_16_*and32_32_*. The read stays inside the guest element, the numeric type and component width do not change, and the shader input does not change (VertexAttributeSignaturestill uses the attribute's component count), since Vulkan discards the components the input does not consume ("If the vertex shader has fewer components, the extra components are discarded", Vertex Input Extraction). A format with no wider form, such as32_32_32_*, still throws.The wider format reads more bytes per vertex, so
prepareDrawInputsadds the difference to the vertex copy and to the zero buffer of a null or out-of-range attribute; otherwise the last vertex's read would end past the copy. The difference comes fromVertexReadBytes, the size of the format the layout holds, so a layout taken from the cache or a recipe gets it as well.Tested
Windows 11 Home 10.0.26200, MinGW-w64 GCC 15.2.0 (ucrt-posix-seh r7), Release,
BUILD_TESTING=ON, NVIDIA GeForce RTX 4070 SUPER (616.92), onmain0c6a5bd.agc_driver_graphics, extended: with a faked format query that lacksR16G16B16_SFLOATandR8G8B8_UNORMbut has the four-component formats, a three-component16_16_16_16_FLOATattribute getsR16G16B16A16_SFLOAT(8 bytes read, stride 8) with the shader input stillf32x3, a three-component8_8_8_8_UNORMone getsR8G8B8A8_UNORM, and a three-component32_32_32_FLOATone still throwsdevice does not support vertex format 106; the existing four-component case still throws.main0c6a5bd and on this branch, withANYPS5_REQUIRE_VULKAN=1andANYPS5_NO_SHADER_CACHE=1,--repeat until-fail:2: all pass on both (agc_driver_graphics,vertex_fetch,zero_stride_vertex_fetch,zero_position_vertex,draw_cache_reuseand its_inline,_offand_verifyruns,draw_offset_indices,pipelined_indirect_draw,pipelined_prefetch,draw_write_ranges,vertex_user_data_tail,queued_index_read_order,inactive_vertex_point_size).mainda54c65 into this branch (head7b391f8889): the same 15 tests (their ctest names start withagc_driver_) on thatmainand on this head, built one after the other in the same build directory, withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(Vulkan SDK 1.4.363.0),ANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1andctest -V, on the RTX 4070 SUPER (616.92) and on the Radeon iGPU (25.10.30.02), each picked withANYPS5_GPU. All four runs pass 15/15. No log reportswas not found but was requested by env var VK_INSTANCE_LAYERS, and none has aVUID-error. Each log has the same 10 layer warnings on both trees: 9Undefined-Value-ShaderOutputNotConsumed-DynamicRenderingand 1Undefined-Value-ShaderOutputNotConsumed. The runs are from a non-elevated process: an elevated one ignoresVK_LAYER_PATH, and runs made that way had neither those warnings nor thewas not foundline.main'sImageArrayWindowPastLayerLimit.cppdoes not compile on MinGW (the alignment error test(agc): align the array window test's texels at run time #8310 fixes), so both trees were built withninja -k 0; no other target failed.check_conventions.py --base origin/mainpasses on7b391f8889.pr_overlap.pyreports feat(agc): draw triangle strips through the barycentric tessellation stages #8083, which addsStripPatchesright abovezeroVertexBufferinDraw.cpp; merged together, that is the only conflict. feat(agc): draw triangle strips through the barycentric tessellation stages #8083's author agreed that feat(agc): draw triangle strips through the barycentric tessellation stages #8083 lands first and this PR then rebases onto it, keeping both.Not tested: a GPU that lacks one of these formats. The RTX 4070 SUPER, a GTX 1650 SUPER and the Radeon iGPU of a Ryzen 9 7900 (AMD 25.10.30.02) in this machine all report
VERTEX_BUFFER_BITfor format 90 (vulkaninfo --show-formats), so the wider path ran only with the faked query. MADiSON was not run here; the RX 5600 XT run is the check.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)Draw.cpp)🤖 Generated with Claude Code