From a763f66ed0d77a484e18f93566e16f65c84bafff Mon Sep 17 00:00:00 2001 From: RINNECODER <93304526+RINNECODER@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:34:43 -0500 Subject: [PATCH 1/5] fix(relinker): decode scalar compares behind address-size and segment prefixes A scalar compare carrying an address-size override (67 C5 FA C2 ..., 67 F3 0F C2 ...) or with F2/F3 on either side of another legacy prefix was not decoded, so it stayed in its Rosetta block and could still lose the later load of its destination. Legacy prefixes are now read in any order, the last F2/F3 being the mandatory one; F2/F3 before a VEX escape is still not a compare. Co-Authored-By: Claude Opus 5.5 --- .../codegen/src/x86/ScalarCompareOperands.cpp | 20 +++++++++++++------ .../codegen/tests/Amd64OnlyConverterTests.cpp | 17 ++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/core/relinker/codegen/src/x86/ScalarCompareOperands.cpp b/core/relinker/codegen/src/x86/ScalarCompareOperands.cpp index b45fad8d8c5..f5521156381 100644 --- a/core/relinker/codegen/src/x86/ScalarCompareOperands.cpp +++ b/core/relinker/codegen/src/x86/ScalarCompareOperands.cpp @@ -17,22 +17,31 @@ constexpr std::uint8_t kEqOq = 0x00; constexpr std::uint8_t kLtOs = 0x01; constexpr std::uint8_t kLeOs = 0x02; -bool _segmentPrefix(const std::uint8_t byte) { +// Legacy prefixes that leave a scalar compare a scalar compare: segment and address-size overrides. +bool _neutralPrefix(const std::uint8_t byte) { using namespace X64OpcodeConstants; - return byte == PrefixSegCs || byte == PrefixSegSs || byte == PrefixSegDs || byte == PrefixSegEs || byte == PrefixSegFs || byte == PrefixSegGs; + return byte == PrefixSegCs || byte == PrefixSegSs || byte == PrefixSegDs || byte == PrefixSegEs || byte == PrefixSegFs || byte == PrefixSegGs || + byte == PrefixAddressSize; } } std::optional DecodeScalarCompare(const std::uint8_t* data, const std::size_t length) { using namespace X64OpcodeConstants; + // Legacy prefixes come in any order; the last F2/F3 among them is the mandatory one. std::size_t pos = 0; - while (pos < length && _segmentPrefix(data[pos])) + std::uint8_t repeat = 0; + while (pos < length && (_neutralPrefix(data[pos]) || data[pos] == PrefixRep || data[pos] == PrefixRepne)) { + if (data[pos] == PrefixRep || data[pos] == PrefixRepne) + repeat = data[pos]; ++pos; + } if (pos >= length) return std::nullopt; // The immediate is the instruction's last byte; the ModRM byte (and any SIB/displacement) sits between it and the opcode. if (data[pos] == OneByteVex2 || data[pos] == OneByteVex3) { + if (repeat != 0) + return std::nullopt; // F2/F3 before a VEX escape is #UD const bool vex3 = data[pos] == OneByteVex3; const std::size_t opcodeOffset = pos + (vex3 ? 3 : 2); if (opcodeOffset + 2 >= length || data[opcodeOffset] != kOpcodeCmp) @@ -44,10 +53,9 @@ std::optional DecodeScalarCompare(const std::uint8_t* data, const return std::nullopt; return ScalarCompare{true, pp == kVexPpF2, static_cast(data[length - 1] & kVexPredicateMask)}; } - if (data[pos] != PrefixRep && data[pos] != PrefixRepne) + if (repeat == 0) return std::nullopt; - const bool isDouble = data[pos] == PrefixRepne; - ++pos; + const bool isDouble = repeat == PrefixRepne; if (pos < length && data[pos] >= RexMin && data[pos] <= RexMax) ++pos; if (pos + 3 >= length || data[pos] != TwoByteOpcodeEscape || data[pos + 1] != kOpcodeCmp) diff --git a/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp b/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp index 7d82263a9c9..8b6d9130080 100644 --- a/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp +++ b/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp @@ -1708,6 +1708,18 @@ void converterScalarCompare() { require(Codegen::RosettaSplitsScalarCompare(*unord) && Codegen::RosettaSplitsScalarCompare(*neq) && Codegen::RosettaSplitsScalarCompare(*nle) && Codegen::RosettaSplitsScalarCompare(*trueUs) && Codegen::RosettaSplitsScalarCompare(*decoded(vexFalse)), "A compare Rosetta mistranslates is not split"); require(!Codegen::RosettaSplitsScalarCompare(*lt) && !Codegen::RosettaSplitsScalarCompare(*decoded(vexLe)), "LT or LE was split"); + // An address-size override (or a segment override) may precede the VEX escape or sit on either side of F2/F3. + const Bytes addr32Vex = {0x67, 0xC5, 0xFA, 0xC2, 0x05, 0x10, 0x00, 0x00, 0x00, 0x04}; // vcmpneqss xmm0, xmm0, [eip + 0x10] + const Bytes addr32Legacy = {0x67, 0xF3, 0x0F, 0xC2, 0x05, 0x10, 0x00, 0x00, 0x00, 0x04}; // cmpneqss xmm0, [eip + 0x10] + const Bytes repThenAddr32 = {0xF2, 0x67, 0x41, 0x0F, 0xC2, 0x00, 0x06}; // cmpnlesd xmm0, [r8d] + const Bytes repBeforeVex = {0xF3, 0xC5, 0xFA, 0xC2, 0xC1, 0x04}; // #UD, not a compare + const auto addr32VexCompare = decoded(addr32Vex); + require(addr32VexCompare && addr32VexCompare->Vex && !addr32VexCompare->Double && addr32VexCompare->Predicate == 4, "VCMPNEQSS with an address-size override was not decoded"); + const auto addr32LegacyCompare = decoded(addr32Legacy); + require(addr32LegacyCompare && !addr32LegacyCompare->Vex && !addr32LegacyCompare->Double && addr32LegacyCompare->Predicate == 4, "CMPNEQSS with an address-size override was not decoded"); + const auto repThenAddr32Compare = decoded(repThenAddr32); + require(repThenAddr32Compare && !repThenAddr32Compare->Vex && repThenAddr32Compare->Double && repThenAddr32Compare->Predicate == 6, "CMPNLESD with F2 before an address-size override was not decoded"); + require(!decoded(repBeforeVex), "A VEX escape after F3 was decoded as a compare"); const auto rosetta = Codegen::MakeAmd64OnlyInstructionMatcher(Codegen::Amd64OnlyTarget::Rosetta); const auto lowered = rosetta->Match(vexUnord.data(), vexUnord.size()); @@ -1715,6 +1727,11 @@ void converterScalarCompare() { require(lowered && Bytes(lowered->StubBody.begin(), lowered->StubBody.begin() + static_cast(lowered->ReturnBranchOffset)) == vexUnord, "The compare stub does not run the unchanged instruction"); const auto rip = rosetta->Match(ripNeq.data(), ripNeq.size()); require(rip && rip->Relocations.size() == 1 && rip->Relocations[0].SiteTarget == 9 + 0x10 && rip->Relocations[0].DisplacementOffset == 4, "A RIP-relative compare does not keep its target in the stub"); + for (const auto* bytes : {&addr32Vex, &addr32Legacy}) { + const auto moved = rosetta->Match(bytes->data(), bytes->size()); + require(moved && moved->Lowering == Codegen::Amd64OnlyLowering::Trampoline && moved->Relocations.size() == 1 && moved->Relocations[0].SiteTarget == 10 + 0x10 + && moved->Relocations[0].DisplacementOffset == 5, "An address-size-prefixed compare was not moved out of line with its target"); + } require(!rosetta->Match(vexLe.data(), vexLe.size()) && !rosetta->Match(packed.data(), packed.size()), "VCMPLESS or a packed compare was moved out of line"); require(!Codegen::MakeAmd64OnlyInstructionMatcher()->Match(vexUnord.data(), vexUnord.size()), "A scalar compare was moved out of line for an Intel host"); From 10bfba7628ac663a352efb73ed9ec93651448ec8 Mon Sep 17 00:00:00 2001 From: RINNECODER <93304526+RINNECODER@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:34:44 -0500 Subject: [PATCH 2/5] fix(agc): retire depth surfaces over every MSAA fragment and every layered slice on Metal A multisampled color target now retires the depth surfaces over all of its fragments (bytes * samples), as MetalColorSurface::OverlapsMappings already measures it. Each extra slice of a layered depth draw retires the surfaces over its depth and stencil planes, not only one at exactly its depth address. Co-Authored-By: Claude Opus 5.5 --- .../Graphics/Metal/MetalDepthSurface.hpp | 5 ++++ .../Graphics/Metal/MetalDepthSurface.mm | 11 ++++++++ .../Graphics/Metal/MetalDraw.mm | 13 +++------- .../tests/metal/DepthResource.mm | 26 +++++++++++++++++++ 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.hpp b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.hpp index aced19360f6..106fc49b73e 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.hpp @@ -102,6 +102,11 @@ void DropReplacedDepthSurfaces(std::vector>& // brings it back with the texels it held. void RetireDepthSurfaces(std::vector>& surfaces, std::uint64_t address, std::uint64_t bytes, const MetalDepthSurface* keep = nullptr); +// The same over the depth and stencil planes of a depth target, or over every fragment of a color target. +void RetireDepthSurfaces(std::vector>& surfaces, const Graphics::DepthTarget& target, + const MetalDepthSurface* keep = nullptr); +void RetireDepthSurfaces(std::vector>& surfaces, const Graphics::ColorTarget& target, + const MetalDepthSurface* keep = nullptr); // A color target with several fragments (CB_COLOR_ATTRIB NUM_FRAGMENTS) renders into a resident 2D // multisample texture, like multisampled depth: the guest's interleaved fragment and FMASK layout is diff --git a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.mm b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.mm index dfccb829e03..f80ff211a54 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.mm +++ b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.mm @@ -267,6 +267,17 @@ void RetireDepthSurfaces(std::vector>& surfac std::erase_if(surfaces, [&](const auto& surface) { return surface.get() != keep && surface->Overlaps(address, bytes); }); } +void RetireDepthSurfaces(std::vector>& surfaces, const Graphics::DepthTarget& target, + const MetalDepthSurface* keep) { + RetireDepthSurfaces(surfaces, target.address, Graphics::DepthSliceBytes(target.extent, hasD16(target) ? 2u : 4u) * target.samples, keep); + if (target.stencilAddress != 0) RetireDepthSurfaces(surfaces, target.stencilAddress, Graphics::DepthSliceBytes(target.extent, 1u) * target.samples, keep); +} + +void RetireDepthSurfaces(std::vector>& surfaces, const Graphics::ColorTarget& target, + const MetalDepthSurface* keep) { + RetireDepthSurfaces(surfaces, target.address, target.bytes * std::max(target.samples, 1u), keep); +} + bool MetalDepthSurface::OverlapsMappings(std::span changed) const { const bool d16 = target.format == VK_FORMAT_D16_UNORM || target.format == VK_FORMAT_D16_UNORM_S8_UINT; const auto overlaps = [&](std::uint64_t address, std::uint64_t bytes) { diff --git a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDraw.mm b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDraw.mm index 47f2fbadb04..797737d091c 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDraw.mm +++ b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDraw.mm @@ -678,16 +678,11 @@ MTLTextureSwizzle channelSwizzle(std::uint8_t value) { // depth surfaces its planes overlap (another stencil pairing of its depth plane, another extent // at its address) are retired until they are bound again. std::erase(depthSurfaces, depth); - const auto& target = depth->Target(); - const bool d16 = target.format == VK_FORMAT_D16_UNORM || target.format == VK_FORMAT_D16_UNORM_S8_UINT; - RetireDepthSurfaces(depthSurfaces, target.address, Graphics::DepthSliceBytes(target.extent, d16 ? 2u : 4u) * target.samples); - if (target.stencilAddress != 0) { - RetireDepthSurfaces(depthSurfaces, target.stencilAddress, Graphics::DepthSliceBytes(target.extent, 1u) * target.samples); - } + RetireDepthSurfaces(depthSurfaces, depth->Target()); depthSurfaces.push_back(depth); } - // A color target over a depth surface's memory writes where its planes were. - for (const auto& color : state.colors) RetireDepthSurfaces(depthSurfaces, color.address, color.bytes, depth.get()); + // A color target over a depth surface's memory writes where its planes were (all its fragments). + for (const auto& color : state.colors) RetireDepthSurfaces(depthSurfaces, color, depth.get()); // A depth-only draw that exports a render target index renders into slices slice..lastSlice of its // depth array at once; each slice also gets its surface, so later single-slice and array readers find it. const std::uint32_t depthLayers = depth && state.layered && state.colors.empty() && state.depth->samples == 1 ? @@ -701,7 +696,7 @@ MTLTextureSwizzle channelSwizzle(std::uint8_t value) { if (slice.stencilAddress != 0) slice.stencilAddress += std::uint64_t{layer} * Graphics::DepthSliceBytes(target.extent, 1u); slice.slice += layer; auto surface = depthCache->Acquire(slice); - std::erase_if(depthSurfaces, [&](const auto& other) { return other != surface && other->Target().address == slice.address; }); + RetireDepthSurfaces(depthSurfaces, slice, surface.get()); if (std::find(depthSurfaces.begin(), depthSurfaces.end(), surface) == depthSurfaces.end()) depthSurfaces.push_back(surface); } } diff --git a/core/libs/prx/libSceAgcDriver/tests/metal/DepthResource.mm b/core/libs/prx/libSceAgcDriver/tests/metal/DepthResource.mm index 4be4e5b5d14..957bdca7135 100644 --- a/core/libs/prx/libSceAgcDriver/tests/metal/DepthResource.mm +++ b/core/libs/prx/libSceAgcDriver/tests/metal/DepthResource.mm @@ -402,6 +402,32 @@ void retiredDepth(const AgcDriver::Metal::MetalDevice& backend) { surfaces = {surface, wide}; AgcDriver::Metal::RetireDepthSurfaces(surfaces, wide->Target().address, depthBytes * 2, wide.get()); MetalTests::Require(surfaces.size() == 1 && surfaces[0] == wide, "Binding a depth target of another extent at the address kept the other surface"); + + // A multisampled color target covers bytes * samples: one whose later fragments reach a depth plane retires it. + AgcDriver::Graphics::ColorTarget color{}; + color.address = target.address - 0x1000; + color.bytes = 0x1000; + color.samples = 4; + surfaces = {surface}; + AgcDriver::Metal::RetireDepthSurfaces(surfaces, color); + MetalTests::Require(surfaces.empty(), "A multisampled color target over a depth plane in its later fragments did not retire it"); + color.samples = 1; + surfaces = {surface}; + AgcDriver::Metal::RetireDepthSurfaces(surfaces, color); + MetalTests::Require(surfaces.size() == 1, "A single-sample color target next to a depth plane retired it"); + + // A depth target retires surfaces over its stencil plane too, and not only those at its depth address: + // a layered draw retires per slice what each slice's planes overlap. + auto slice = target; + slice.address += depthBytes; + slice.stencilAddress += stencilBytes; + auto sliceSurface = cache.Acquire(slice); + auto insideDepth = cache.Acquire(AgcDriver::Graphics::DepthTarget{slice.address + 0x100, 0, {width, height}, VK_FORMAT_D32_SFLOAT, 1, 0}); + auto onStencil = cache.Acquire(AgcDriver::Graphics::DepthTarget{0x2000000, slice.stencilAddress + 0x40, {width, height}, target.format, 1, 0}); + surfaces = {surface, insideDepth, onStencil, sliceSurface}; + AgcDriver::Metal::RetireDepthSurfaces(surfaces, slice, sliceSurface.get()); + MetalTests::Require(surfaces.size() == 2 && surfaces[0] == surface && surfaces[1] == sliceSurface, + "A depth slice did not retire the surfaces inside its depth or stencil plane, or retired its neighbour or itself"); } } From b905cdd61d34f034dc32371af184e54e012a1125 Mon Sep 17 00:00:00 2001 From: RINNECODER <93304526+RINNECODER@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:34:44 -0500 Subject: [PATCH 3/5] fix(recompiler): skip null images before FORCE_DEGAMMA validation A null T# (TYPE=0 or zero base) reads zero whatever its other words say, so a null image left with an RGBA8_UINT format under a FORCE_DEGAMMA sampler no longer fails the one-slice-volume check. Co-Authored-By: Claude Opus 5.5 --- .../tests/execution/ImagePlaneBiasOnSingleLevelVolume.cpp | 8 ++++++++ .../recompiler/Optimization/src/ResourceMaterializer.cpp | 2 ++ 2 files changed, 10 insertions(+) diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/ImagePlaneBiasOnSingleLevelVolume.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/ImagePlaneBiasOnSingleLevelVolume.cpp index 29875f7792f..7fae1537bd5 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/ImagePlaneBiasOnSingleLevelVolume.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/ImagePlaneBiasOnSingleLevelVolume.cpp @@ -215,6 +215,14 @@ int main() { Draw(*device, integerTexture, forcedSampler, bias); Check(true); } + // A null T# (TYPE=0) reads zero whatever its other words say, also under FORCE_DEGAMMA. + auto nullTexture = integerTexture; + nullTexture[3] &= ~(0xfu << 28u); + nullTexture[4] = 1u; + Draw(*device, nullTexture, forcedSampler, 0.0f); + for (std::size_t index = 0; index < Pixels.size(); ++index) { + Require(std::to_integer(Pixels[index]) == 0u, "a null FORCE_DEGAMMA image read " + std::to_string(std::to_integer(Pixels[index])) + " at byte " + std::to_string(index)); + } std::string mixedRefusal; try { Draw(*device, integerTexture, forcedSampler, 0.0f, true); diff --git a/core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp b/core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp index 57b1451a811..25951054f85 100644 --- a/core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp +++ b/core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp @@ -509,6 +509,8 @@ void materializeSnapshot(const IrResourcePlan& plan, const SrtRuntime& runtime, void materializeForcedDegamma(const IrResourcePlan& plan, ResourceSnapshot& snapshot) { for (std::uint32_t index = 0; index < plan.info.images.size(); ++index) { auto& descriptor = snapshot.images.at(index); + // A null image reads zero whatever its other words say; its format and extent are not checked. + if (nullImageDescriptor(descriptor)) continue; if (((descriptor.dwords[1] >> 20u) & 0x1ffu) != static_cast(IrBufferFormat::Format8_8_8_8UInt)) continue; bool forced = false; bool plain = false; From fd6737bd1cbf76dac39324e044e8d593369ed098 Mon Sep 17 00:00:00 2001 From: RINNECODER <93304526+RINNECODER@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:34:44 -0500 Subject: [PATCH 4/5] test(agc): pin a swizzled T# sampled through the FORCE_DEGAMMA sRGB view An R/B-swapped UNORM and sRGB texture sampled under FORCE_DEGAMMA keeps its mapping once. Measured: a Metal view of a view takes its swizzle relative to the root texture, so passing the parent's swizzle is right and an identity swizzle drops the mapping. Co-Authored-By: Claude Opus 5.5 --- .../Graphics/Metal/MetalShaderResources.mm | 4 +++- .../execution/ImageSampleForceDegamma.cpp | 20 +++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalShaderResources.mm b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalShaderResources.mm index 0c480dc1c25..2f895322ff6 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalShaderResources.mm +++ b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalShaderResources.mm @@ -677,7 +677,9 @@ MTLPixelFormat SrgbTwin(MTLPixelFormat format) { } } -// The same levels, slices and swizzle of `view` read through `format`. +// The same levels, slices and swizzle of `view` read through `format`. A view of a view takes its swizzle +// relative to the root texture, not composed with the parent's: passing `view.swizzle` keeps a non-identity +// T# channel mapping applied once (ImageSampleForceDegamma samples an R/B swap through this view). id SrgbView(id view, MTLPixelFormat format) { const auto type = view.textureType; const NSUInteger slices = type == MTLTextureType3D ? 1u : type == MTLTextureTypeCube || type == MTLTextureTypeCubeArray ? view.arrayLength * 6u : view.arrayLength; diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/ImageSampleForceDegamma.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/ImageSampleForceDegamma.cpp index 8aced8d8808..917ba7f7b81 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/ImageSampleForceDegamma.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/ImageSampleForceDegamma.cpp @@ -44,13 +44,17 @@ std::array BufferDescriptor(const void* data, std::uint32_t by return {static_cast(address), static_cast((address >> 32u) & 0xffffu), bytes, 0x31016facu}; } -std::array TextureDescriptor(std::uint32_t format) { +// DST_SEL_X..W in T# word 3: identity (R, G, B, A) and an R/B swap (B, G, R, A). +constexpr std::uint32_t IdentitySelect = 4u | (5u << 3u) | (6u << 6u) | (7u << 9u); +constexpr std::uint32_t SwapRbSelect = 6u | (5u << 3u) | (4u << 6u) | (7u << 9u); + +std::array TextureDescriptor(std::uint32_t format, std::uint32_t select = IdentitySelect) { const auto address = static_cast(reinterpret_cast(Texels.data())); return { static_cast(address >> 8u), static_cast((address >> 40u) & 0xffu) | (format << 20u) | (((Size - 1u) & 3u) << 30u), ((Size - 1u) >> 2u) | ((Size - 1u) << 14u), - 0xfacu | (Type2D << 28u), + select | (Type2D << 28u), 0u, 0u, 0u, @@ -93,11 +97,11 @@ float SrgbToLinear(std::uint8_t code) { return c <= 0.04045f ? c / 12.92f : std::pow((c + 0.055f) / 1.055f, 2.4f); } -std::array Sample(AgcDriver::VulkanDevice& device, std::uint32_t format, std::uint32_t filter, std::uint32_t forced) { +std::array Sample(AgcDriver::VulkanDevice& device, std::uint32_t format, std::uint32_t filter, std::uint32_t forced, std::uint32_t select = IdentitySelect) { std::vector userData(20, 0u); const auto input = BufferDescriptor(Input.data(), static_cast(sizeof(Input))); const auto output = BufferDescriptor(Output.data(), static_cast(sizeof(Output))); - const auto texture = TextureDescriptor(format); + const auto texture = TextureDescriptor(format, select); const auto sampler = SamplerDescriptor(filter, forced); std::copy(input.begin(), input.end(), userData.begin()); std::copy(output.begin(), output.end(), userData.begin() + 4); @@ -125,12 +129,12 @@ void RequireSame(const std::array& forced, const std::array< } } -void RequireDecoded(const std::array& sampled, const char* name) { +void RequireDecoded(const std::array& sampled, const char* name, bool swapRb = false) { for (std::uint32_t tid = 0; tid < Threads; ++tid) { const auto x = tid % Size; const auto y = (tid / Size) % Size; for (std::uint32_t channel = 0; channel < 4u; ++channel) { - const auto code = Code8(x, y, channel); + const auto code = Code8(x, y, swapRb && channel != 1u && channel != 3u ? 2u - channel : channel); const float expected = channel == 3u ? static_cast(code) / 255.0f : SrgbToLinear(code); const float value = sampled[tid * 4u + channel]; Require(std::fabs(value - expected) <= 0.005f, std::string(name) + " through FORCE_DEGAMMA: thread " + std::to_string(tid) + " component " + std::to_string(channel) + " sampled " + std::to_string(value) + ", expected " + std::to_string(expected)); @@ -170,6 +174,10 @@ int main() { Require(unormPlain[0] != unormPoint[0] || unormPlain[4] != unormPoint[4], "8_8_8_8_UNORM without FORCE_DEGAMMA sampled the decoded values"); FillInput(false); RequireSame(Sample(*device, Format8888UNorm, FilterBilinear, ForceDegamma), forcedBilinear, "8_8_8_8_UNORM, bilinear"); + // A swizzled T# (R/B swap) keeps its mapping through the sRGB view: applied once, not composed twice. + FillInput(true); + RequireDecoded(Sample(*device, Format8888UNorm, FilterPoint, ForceDegamma, SwapRbSelect), "8_8_8_8_UNORM, R/B swapped", true); + RequireDecoded(Sample(*device, Format8888Srgb, FilterPoint, ForceDegamma, SwapRbSelect), "8_8_8_8_SRGB, R/B swapped", true); Reject(*device, Format32Float, "guest format 22, which is not sRGB and has no sRGB view, is sampled through a sampler that forces sRGB decoding"); std::puts("image sample force degamma tests passed"); return 0; From 37aa5ed5f74f46d3239767bb7d10a6c7afc270fc Mon Sep 17 00:00:00 2001 From: RINNECODER <93304526+RINNECODER@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:04:48 -0500 Subject: [PATCH 5/5] fix(agc): keep a stencil-only target's absent depth plane absent in its layered slices The extra slices of a layered draw offset the depth address even when it is zero (a stencil-only target), and the range retirement then treated that fabricated address as a depth plane. DepthArraySlice leaves an absent plane at zero. Co-Authored-By: Claude Opus 5.5 --- .../Graphics/Metal/MetalDepthSurface.hpp | 3 +++ .../Graphics/Metal/MetalDepthSurface.mm | 8 ++++++++ .../libSceAgcDriver/Graphics/Metal/MetalDraw.mm | 7 +------ .../libSceAgcDriver/tests/metal/DepthResource.mm | 16 +++++++++++++--- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.hpp b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.hpp index 106fc49b73e..14b9d7bfb74 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.hpp @@ -105,6 +105,9 @@ void RetireDepthSurfaces(std::vector>& surfac // The same over the depth and stencil planes of a depth target, or over every fragment of a color target. void RetireDepthSurfaces(std::vector>& surfaces, const Graphics::DepthTarget& target, const MetalDepthSurface* keep = nullptr); +// Slice `slice + layer` of a target's depth array; an absent plane (address 0, as the depth plane of a +// stencil-only target) stays absent. +Graphics::DepthTarget DepthArraySlice(const Graphics::DepthTarget& target, std::uint32_t layer); void RetireDepthSurfaces(std::vector>& surfaces, const Graphics::ColorTarget& target, const MetalDepthSurface* keep = nullptr); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.mm b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.mm index f80ff211a54..efee6ed4d3b 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.mm +++ b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDepthSurface.mm @@ -273,6 +273,14 @@ void RetireDepthSurfaces(std::vector>& surfac if (target.stencilAddress != 0) RetireDepthSurfaces(surfaces, target.stencilAddress, Graphics::DepthSliceBytes(target.extent, 1u) * target.samples, keep); } +Graphics::DepthTarget DepthArraySlice(const Graphics::DepthTarget& target, std::uint32_t layer) { + auto slice = target; + if (slice.address != 0) slice.address += std::uint64_t{layer} * Graphics::DepthSliceBytes(target.extent, hasD16(target) ? 2u : 4u); + if (slice.stencilAddress != 0) slice.stencilAddress += std::uint64_t{layer} * Graphics::DepthSliceBytes(target.extent, 1u); + slice.slice += layer; + return slice; +} + void RetireDepthSurfaces(std::vector>& surfaces, const Graphics::ColorTarget& target, const MetalDepthSurface* keep) { RetireDepthSurfaces(surfaces, target.address, target.bytes * std::max(target.samples, 1u), keep); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDraw.mm b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDraw.mm index 797737d091c..8dbfe68549f 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDraw.mm +++ b/core/libs/prx/libSceAgcDriver/Graphics/Metal/MetalDraw.mm @@ -688,13 +688,8 @@ MTLTextureSwizzle channelSwizzle(std::uint8_t value) { const std::uint32_t depthLayers = depth && state.layered && state.colors.empty() && state.depth->samples == 1 ? state.depth->lastSlice - state.depth->slice + 1u : 0u; if (depthLayers > 1) { - const auto& target = *state.depth; - const bool d16 = target.format == VK_FORMAT_D16_UNORM || target.format == VK_FORMAT_D16_UNORM_S8_UINT; for (std::uint32_t layer = 1; layer < depthLayers; ++layer) { - auto slice = target; - slice.address += std::uint64_t{layer} * Graphics::DepthSliceBytes(target.extent, d16 ? 2u : 4u); - if (slice.stencilAddress != 0) slice.stencilAddress += std::uint64_t{layer} * Graphics::DepthSliceBytes(target.extent, 1u); - slice.slice += layer; + const auto slice = DepthArraySlice(*state.depth, layer); auto surface = depthCache->Acquire(slice); RetireDepthSurfaces(depthSurfaces, slice, surface.get()); if (std::find(depthSurfaces.begin(), depthSurfaces.end(), surface) == depthSurfaces.end()) depthSurfaces.push_back(surface); diff --git a/core/libs/prx/libSceAgcDriver/tests/metal/DepthResource.mm b/core/libs/prx/libSceAgcDriver/tests/metal/DepthResource.mm index 957bdca7135..2c5323821fb 100644 --- a/core/libs/prx/libSceAgcDriver/tests/metal/DepthResource.mm +++ b/core/libs/prx/libSceAgcDriver/tests/metal/DepthResource.mm @@ -418,9 +418,9 @@ void retiredDepth(const AgcDriver::Metal::MetalDevice& backend) { // A depth target retires surfaces over its stencil plane too, and not only those at its depth address: // a layered draw retires per slice what each slice's planes overlap. - auto slice = target; - slice.address += depthBytes; - slice.stencilAddress += stencilBytes; + const auto slice = AgcDriver::Metal::DepthArraySlice(target, 1); + MetalTests::Require(slice.address == target.address + depthBytes && slice.stencilAddress == target.stencilAddress + stencilBytes && slice.slice == target.slice + 1, + "A depth array slice does not start one slice past the target"); auto sliceSurface = cache.Acquire(slice); auto insideDepth = cache.Acquire(AgcDriver::Graphics::DepthTarget{slice.address + 0x100, 0, {width, height}, VK_FORMAT_D32_SFLOAT, 1, 0}); auto onStencil = cache.Acquire(AgcDriver::Graphics::DepthTarget{0x2000000, slice.stencilAddress + 0x40, {width, height}, target.format, 1, 0}); @@ -428,6 +428,16 @@ void retiredDepth(const AgcDriver::Metal::MetalDevice& backend) { AgcDriver::Metal::RetireDepthSurfaces(surfaces, slice, sliceSurface.get()); MetalTests::Require(surfaces.size() == 2 && surfaces[0] == surface && surfaces[1] == sliceSurface, "A depth slice did not retire the surfaces inside its depth or stencil plane, or retired its neighbour or itself"); + + // A stencil-only target has no depth plane: its slices have none either, and retire nothing at low addresses. + const AgcDriver::Graphics::DepthTarget stencilOnly{0, 0x800000, {width, height}, VK_FORMAT_S8_UINT, 1, 0}; + const auto stencilSlice = AgcDriver::Metal::DepthArraySlice(stencilOnly, 2); + MetalTests::Require(stencilSlice.address == 0 && stencilSlice.stencilAddress == stencilOnly.stencilAddress + 2 * stencilBytes, + "A stencil-only slice was given a depth plane"); + auto low = cache.Acquire(AgcDriver::Graphics::DepthTarget{2 * depthBytes, 0, {width, height}, VK_FORMAT_D32_SFLOAT, 1, 0}); + surfaces = {low}; + AgcDriver::Metal::RetireDepthSurfaces(surfaces, stencilSlice); + MetalTests::Require(surfaces.size() == 1, "A stencil-only slice retired a depth surface at a fabricated depth address"); } }