Repository navigation
fix(agc): journal the mesh output primitive size - #6784
Conversation
boykopovar#5884 added MeshConfiguration::outputPrimitiveSize, and boykopovar#5875's pipeline journal guards the layouts it encodes. The two merged separately, so on Linux with libstdc++ PipelineJournal.cpp no longer compiles: MeshConfiguration is 40 bytes, ShaderStages (which holds an optional one) 112. The guards now name those sizes, and the recipe codec writes and reads the new field, so a replayed line-strip geometry pipeline keeps its two-vertex primitives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
The journal codec changes, but FileFormat remains 2. A journal written by main after #5884 can already carry the new ShaderDiskCache::SourceVersion() (the recompiler source hash includes the new mesh field) while still using the old encodeMesh layout. PipelineJournal.cpp is not part of that source-version hash, so this new decoder accepts that old journal header and then reads the added outputPrimitiveSize from the following field(s). Please bump the journal format (or add an equivalent codec-version discriminator) and add a regression showing that a journal written with the old mesh record is discarded, rather than parsed with the new codec.
The layout guards only build on Linux with libstdc++, so a Windows or macOS build of main after boykopovar#5884 wrote journals whose mesh records lack outputPrimitiveSize under the new shader-cache source version. The journal format is now 3, so such a file is discarded instead of being read with the new codec. MarcZX100 pointed this out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks, you're right. The guards only build on Linux with libstdc++, so Windows and macOS builds of 2d5f761 bumps the journal format to 3. The journal test now rewrites a recorded journal's header to format 2 and requires the replay to load no module and no recipe. With the format left at 2 it fails 3 of 3 ( AI-assisted: yes |
|
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
There was a problem hiding this comment.
The format-3 bump closes the compatibility gap: old format-2 journals are discarded even when their shader source version matches, and the regression checks that replay loads neither recipes nor modules. The mesh round-trip test also checks outputPrimitiveSize alongside the neighboring field. Eligibility, conventions and report checks pass; the required relinker/platform jobs are skipped for this path. The focused runtime results are reported in the PR body. I found no remaining issue in the requested change.
|
Independent confirmation: unmodified main65's On that adapted pin, CPU journal tests pass3/3 on Windows and Linux, including the line-strip field and format-2 rejection assertions. GPU journal tests pass3/3 each on NVIDIA RTX5080 (610.62/32.0.16.1062), AMD Radeon Graphics (32.0.21036.18), Windows11 Enterprise26200 / WinLibs GCC15.2r7, and Linux Mesa25.2.8 llvmpipe. Physical/software identity is verified per GPU process. Affected libraries/targets build; this is not an exact-head full-suite claim. The separate CPU graphics error you noted reproduces3/3 on unmodified post-merge main |
|
Thanks for checking it across the four devices. The AI-assisted: yes |
What
main65d56a2 does not build on Linux with libstdc++ (GCC 13.3 here).PipelineJournal.cppstops on its own layout guards:#5884 added
MeshConfiguration::outputPrimitiveSize, and #5875 guards the sizes of the structs its recipe codec encodes. The two merged separately.MeshConfigurationis now 40 bytes, andShaderStages, which holds an optional one, is 112. The codec also didn't write the new field, so a replayed line-strip geometry pipeline would come back with the default of 3.How
encodeMeshanddecodeMeshwrite and readoutputPrimitiveSizeafteresgsItemSize.mainafter feat(agc): draw line-strip output of NGG geometry programs as mesh lines #5884 wrote journals whose mesh records lack the field, under the new shader-cache source version. Those are now discarded instead of being read with the new codec. MarcZX100 pointed this out.Tested
Linux (Ubuntu 24.04, GCC 13.3, Release), RTX 4070 Ti, NVIDIA 595.99.
agc_pipeline_journal(agc_driver_graphics_tests --pipeline-journal): new case. A recipe for a mesh stage withoutputPrimitiveSize2 must decode with 2. With the guards fixed but the codec as onmain, it fails 3 of 3 withthe recipe dropped the mesh output primitive size. With this change it passes 3 of 3, as doesagc_driver_pipeline_journal.a journal in format 2, whose mesh records lack the output primitive size, was loaded. With 3 it passes 3 of 3.main65d56a2 the library doesn't compile, so nothing above runs there.agc_driver_graphicsfails here 3 of 3 withShaderRecompiler::Recompile: VOP2 opcode is not implemented. It passed 3 of 3 on this change atmainaff725b, before fix(agc): stop pixel invocations on BDA faults when supported #5804 merged, so it is in a case fix(agc): stop pixel invocations on BDA faults when supported #5804 added. The request it dumps is a 12-word vertex program whose code words are all zero, the same length as fix(agc): stop pixel invocations on BDA faults when supported #5804'svertexBdaCode. I haven't found why; that should be a separate fix.Not checked on Windows, macOS, on AMD or on a console.
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)main🤖 Generated with Claude Code