Repository navigation
Upstream PR batch 1 U7: tessellation and draw-state keys - #483
Conversation
Ported from upstream PR boykopovar#5234, adapted to our triangle/fractional-odd only tessellation path: VGT_TF_PARAM.PARTITIONING 0 decodes, emits equal spacing and bounds factors at 64. Metal still refuses guest tessellation draws in MetalRenderState. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
Ported from upstream PR boykopovar#5402 (its integer-partitioning base commit is the previous commit), adapted to our triangle-only tessellation path: VGT_TF_PARAM.PARTITIONING 3 decodes, emits fractional-even spacing and bounds factors at 64. Metal still refuses guest tessellation draws. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
…ol-point slot Ported from upstream PR boykopovar#4723 (shader hunk; its Vulkan execution test is not ported). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
…partial) Ported from upstream PR boykopovar#5457, commits 1, 4 and 5 of 5 (hull exec masks restored through phis, input stride from the patch term, one output control point per patch). Vulkan execution-test hunks are not ported. Skipped from boykopovar#5457, with reasons: - relative patch index rebuilt as data: needs GE_CNTL.PRIM_GRP_SIZE and BREAK_WAVE_AT_EOI in TessellationConfiguration, VGT_LS_HS_CONFIG patchesPerGroup and the request key/serializer additions. None of these exist on main, which only lowers triangle fractional-odd hulls. - recording BDA faults in hull programs: drops a refusal whose safety rests on upstream hull work we lack; the refusal stays. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
…face input Ported from upstream PR boykopovar#5314. Tracks SPI_BARYC_CNTL in the draw key and shader request key, and bumps the request serialization to version 24 (shared with the following slice-offset port, which adds no second bump). Not ported: the ShaderMemory serializer-version assertions of the PR, which build on upstream test scaffolding absent from main. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
Ported from upstream PR boykopovar#5500, adapted to main: the draw decode sets the color view slice as layerBase (the layer-export decode this PR also touches does not exist on main, so its diagnostic and layers hunks are skipped). layerBase is added to the pixel info, request key, shader info, disk cache and pipeline specialization constants. The request serialization stays at version 24, bumped once by the front-face commit. The shader disk cache format is bumped to 36. Not ported: unrelated hunks the upstream commit carried in Recompiler.cpp (dot-product capability option) and its ShaderMemory serializer tests. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aacc6d831a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| state.module.AddFunction(spv::OpFOrdGreaterThan, TypeBool(state), inside, floating, lower); | ||
| const auto clamped = state.module.AllocateId(); | ||
| state.module.AddFunction(spv::OpExtInst, TypeF32(state), clamped, GlslStd450(state), GLSLstd450FMin, floating, ConstantF32Value(state, 63.0f)); | ||
| state.module.AddFunction(spv::OpExtInst, TypeF32(state), clamped, GlslStd450(state), GLSLstd450FMin, floating, ConstantF32Value(state, state.inputInfo.vertex->tess.partitioning == 2u ? 63.0f : 64.0f)); |
There was a problem hiding this comment.
For integer or fractional-even tessellation, this emits factors up to 64, but DecodeShaderStages still accepts VGT_HOS_MAX_TESS_LEVEL values as low as 63. A draw with one of the newly accepted partition modes, a maximum clamp below 64, and a factor above that clamp therefore reaches the host tessellator with a larger factor than the guest hardware permits, changing segment counts or fractional spacing. Require a maximum of at least 64 for partitioning 0/3, or specialize this clamp from the decoded register.
Useful? React with 👍 / 👎.
…rsion 37) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
…ayerBase and serializer v24 Fix the stale serialization version expectation in the shader memory test, add pixel ancillary rows with a non-zero layerBase, extract PixelLayerBase for a derivation test, and point the FrontFace note at TechnicalDebt.md. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj
2 participants
Upstream PR batch 1, unit U7: tessellation and draw-state keys (non-stacked part). One commit per upstream PR, in the order 5234, 5402, 4723, 5457 (partial), 5314, 5500. Upstream boykopovar/AnyPS5 was only read.
Ported
VGT_TF_PARAM.PARTITIONING0, equal spacing, factors bounded at 64. Adapted to main's triangle-only, fractional-odd-only path.SPI_BARYC_CNTL.FRONT_FACE_ALL_BITSfor the front-face input: SPI_BARYC_CNTL joins the draw key range and request key, request serialization goes to version 24.LayerBase): added to pixel info, draw compare, request key, shader info, shader disk cache and the specialization constants. It reuses version 24, so the serializer is bumped once (23 to 24). Shader disk cacheFormatVersion36 to 37 (main's Upstream PR batch 1 U8a: Metal image-mode views #485 took 36; history line added) and theShaderInfofield-count assertion 23 to 24.tests/execution/FrontFaceAllBits.cpp(from 5314). Equivalent SPIR-V-level tests that need no device are added totests/ShaderMemory.cpp:ShaderInputInfoBuilder;constant % stride % 16u;frontFaceAllBitsandlayerBasethrough the v24 serializer, and the request prefix now expects version 24.PixelAncillary.cpp: two rows with non-zerolayerBase(exported and not exported layer) and two whole-word rows, so theexportedLayer + LayerBaseadd is checked.PixelLayerBase(const State&)is extracted from DrawDecode.cpp and tested intests/Graphics.cpp. It differs from upstream on purpose: upstream takes the color view slice as is, here it is 0 for layered draws (the vertex stage exports the layer, adding the view slice would double count it) and for draws with no color target.ShaderMemory.cppadditions beyond serializer assertions (domain and half-wave reduction assertions, from 5457's hull exec-mask work) are in the diff; the stale version-19 expectation is fixed to 24 and the unsupported-version probe moved from 20 to 25.Skipped, with reasons
GE_CNTL.PRIM_GRP_SIZE,BREAK_WAVE_AT_EOIandVGT_LS_HS_CONFIGpatches-per-group in the tessellation configuration, plus their request key/serializer fields. None exist on main.color.layershunks: the layer-export decode they build on is absent on main. The Recompiler.cpp dot-product option hunk in that commit is unrelated and left out. Draw decode setslayerBasefrom the color view slice (0 for layered draws or no color target).Metal
Metal still refuses guest tessellation draws (
MetalRenderState.mmrequires no tessellation stage). Everything about partitioning (5234, 5402), the hull AccessChain slot placement (4723), the 64 vs 63 factor bound and the builder's partitioning check therefore matters only off Metal; their tests are SPIR-V-level. Front-face andlayerBasealso reach Metal through the shared request key. The newLayerBaseconstant flows through the existing specialization path to Metal.Testing
pixel_ancillary_tests(x86_64 default configuration): passed.agc_shader_memory_tests: does not build on macOS as the tree stands (GuestMemory.cppusespthread_getattr_np,atomic_ref<const T>and a Linux page query). I ran it with a local, uncommitted macOS shim ofGuestMemory.cpp(x86_64 under Rosetta): passed. The new tests were checked to fail when the 64 bound, the AccessChain fix or the builder's partitioning check is removed.agc_driver_graphics_tests(needslinux/udmabuf.h;tests/Graphics.cppalso has compile errors on macOS already on main), so thePixelLayerBaseand partitioning-decode assertions there are unverified, and the Vulkan execution tests.tools/smoke.sh, 130 s, Rosetta build) was run before the merge and the test-only changes; it reached the first movie, main menu and leader select with no fault. Not rerun: the later changes are tests, a pure extraction (PixelLayerBase) and the merge.🤖 Generated with Claude Code
https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj