From dc078360c201253e8a406c239861f2ca42336f0f Mon Sep 17 00:00:00 2001 From: drako <98249188+drakolordx7@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:14:35 -0500 Subject: [PATCH] fix(vif1): parse VIFcodes between a DIRECT image tag and its pixel data A VIF1 DIRECT can end with a PATH2 GIF IMAGE tag whose pixel data comes in a later DIRECT, typically "MARK; DIRECT n" in the TTE words of the next DMAtag. processVIF1Data() remembered the pending image (qwc) and then treated the bytes right after the first DIRECT as that pixel data. Those bytes are the next VIFcodes, so every such upload (fonts, CLUT and UI atlas uploads) was shifted by the two VIFcode words: 2 CT32 pixels or 16 T4 pixels. The last 8 bytes of the data were then parsed as VIFcodes, which showed up as garbled text and dithered sprite edges. VIFcodes are now always parsed. When an image is pending, only the payload of a DIRECT/DIRECTHL is re-wrapped in a synthesized IMAGE tag and forwarded to PATH2 (forwardVif1DirectData). Raw continuation without VIFcodes is kept only where hardware has it: a DIRECT cut off at the end of a processVIF1Data() buffer (m_vif1PendingDirectQwc / m_vif1PendingDirectHl, reset together with the pending image state). Two existing tests ("VIF1 DIRECT image tag can continue with raw image qwords" and "VIF1 DIRECT finds an image continuation after packed setup") placed the pixel qwords directly after a complete DIRECT with no VIFcode in front of them, which hardware would decode as a VIFcode. They now put the pixels behind a DIRECT (and the first one checks the buffer cut-off case, which is the one that really continues without VIFcodes). A new test covers "DIRECT; MARK; DIRECT" and fails without the fix. Found while bringing up a game's UI and font rendering, and verified byte for byte against the EE RAM of a real PCSX2 through a savestate. Made by drakolord and assisted with Claude Code. --- ps2xRuntime/include/runtime/ps2_memory.h | 5 ++ ps2xRuntime/src/lib/ps2_memory.cpp | 4 + ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp | 80 ++++++++++++++------ ps2xTest/src/ps2_memory_tests.cpp | 69 ++++++++++++++++- 4 files changed, 130 insertions(+), 28 deletions(-) diff --git a/ps2xRuntime/include/runtime/ps2_memory.h b/ps2xRuntime/include/runtime/ps2_memory.h index ea86a9cb2..4afdcbe85 100644 --- a/ps2xRuntime/include/runtime/ps2_memory.h +++ b/ps2xRuntime/include/runtime/ps2_memory.h @@ -335,6 +335,7 @@ class PS2Memory void flushMaskedPath3Packets(bool drainImmediately = true); void submitGifPacket(GifPathId pathId, const uint8_t *data, uint32_t sizeBytes, bool drainImmediately = true, bool path2DirectHl = false); + void forwardVif1DirectData(const uint8_t *data, uint32_t sizeBytes, bool directHl); void processGIFPacket(uint32_t srcPhysAddr, uint32_t qwCount); void processGIFPacket(const uint8_t *data, uint32_t sizeBytes); bool tryProcessNativeGifImageUploadChain(GS &gs, uint32_t tadr, uint32_t chcr); @@ -409,6 +410,10 @@ class PS2Memory bool m_path3Masked = false; uint32_t m_vif1PendingPath2ImageQwc = 0u; bool m_vif1PendingPath2DirectHl = false; + // Remaining data qwords of a VIF1 DIRECT/DIRECTHL cut off by the end of a processVIF1Data() buffer; the next + // buffer starts with that raw data (no VIFcodes). + uint32_t m_vif1PendingDirectQwc = 0u; + bool m_vif1PendingDirectHl = false; std::vector> m_path3MaskedFifo; struct PendingTransfer diff --git a/ps2xRuntime/src/lib/ps2_memory.cpp b/ps2xRuntime/src/lib/ps2_memory.cpp index 37b6a4dec..ab414231d 100644 --- a/ps2xRuntime/src/lib/ps2_memory.cpp +++ b/ps2xRuntime/src/lib/ps2_memory.cpp @@ -331,6 +331,8 @@ bool PS2Memory::initialize(size_t ramSize) m_path3MaskedFifo.clear(); m_vif1PendingPath2ImageQwc = 0u; m_vif1PendingPath2DirectHl = false; + m_vif1PendingDirectQwc = 0u; + m_vif1PendingDirectHl = false; resetEeTimers(); try @@ -1231,6 +1233,8 @@ bool PS2Memory::writeIORegister(uint32_t address, uint32_t value) std::memset(&vif1_regs, 0, sizeof(vif1_regs)); m_vif1PendingPath2ImageQwc = 0u; m_vif1PendingPath2DirectHl = false; + m_vif1PendingDirectQwc = 0u; + m_vif1PendingDirectHl = false; m_path3Masked = false; if (wasPath3Masked) flushMaskedPath3Packets(); diff --git a/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp b/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp index b02c7ca69..7043d35d6 100644 --- a/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp +++ b/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp @@ -295,6 +295,47 @@ void PS2Memory::processVIF1Data(uint32_t srcPhys, uint32_t sizeBytes) processVIF1Data(m_rdram + srcPhys, sizeBytes); } +// Forwards the data of a VIF1 DIRECT/DIRECTHL to GIF PATH2. The GS frontends consume each forwarded packet on its own, +// starting with a GIFtag, while on hardware PATH2 is one continuous GIF stream: a DIRECT may end with an IMAGE GIFtag +// whose pixel data arrives in a later DIRECT (typically "MARK; DIRECT n" in the next DMAtag's TTE words). Such +// continuation data is re-wrapped here in a synthesized IMAGE tag. Only data that really is DIRECT payload is +// wrapped; the VIFcodes between the DIRECTs are parsed as VIFcodes (they used to be taken as the first 8 bytes of +// the image, shifting every uploaded texture by two words). +void PS2Memory::forwardVif1DirectData(const uint8_t *data, uint32_t sizeBytes, bool directHl) +{ + while (sizeBytes >= 16u) + { + if (m_vif1PendingPath2ImageQwc != 0u) + { + const uint32_t chunkQw = std::min(m_vif1PendingPath2ImageQwc, sizeBytes / 16u); + std::vector imagePacket(16u + static_cast(chunkQw) * 16u, 0u); + const uint64_t imageTag = + static_cast(chunkQw & 0x7FFFu) | + ((m_vif1PendingPath2ImageQwc == chunkQw) ? (1ull << 15) : 0ull) | + (static_cast(kGifFmtImage) << 58); + std::memcpy(imagePacket.data(), &imageTag, sizeof(imageTag)); + std::memcpy(imagePacket.data() + 16u, data, static_cast(chunkQw) * 16u); + submitGifPacket(GifPathId::Path2, imagePacket.data(), static_cast(imagePacket.size()), true, + m_vif1PendingPath2DirectHl); + m_vif1PendingPath2ImageQwc -= chunkQw; + if (m_vif1PendingPath2ImageQwc == 0u) + m_vif1PendingPath2DirectHl = false; + data += chunkQw * 16u; + sizeBytes -= chunkQw * 16u; + continue; + } + + submitGifPacket(GifPathId::Path2, data, sizeBytes, true, directHl); + const uint32_t pendingImageQw = pendingGifImageQwc(data, sizeBytes); + if (pendingImageQw != 0u) + { + m_vif1PendingPath2ImageQwc = pendingImageQw; + m_vif1PendingPath2DirectHl = directHl; + } + break; + } +} + void PS2Memory::processVIF1Data(const uint8_t *data, uint32_t sizeBytes) { if (sizeBytes == 0u) @@ -304,30 +345,19 @@ void PS2Memory::processVIF1Data(const uint8_t *data, uint32_t sizeBytes) while (pos + 4 <= sizeBytes) { - if (m_vif1PendingPath2ImageQwc != 0u) + if (m_vif1PendingDirectQwc != 0u) { + // Continuation of a DIRECT that was cut off at the end of the previous buffer: raw GIF data, no VIFcodes. const uint32_t availableQw = (sizeBytes - pos) / 16u; if (availableQw == 0u) { break; } - const uint32_t chunkQw = std::min(m_vif1PendingPath2ImageQwc, availableQw); - std::vector imagePacket(16u + static_cast(chunkQw) * 16u, 0u); - const uint64_t imageTag = - static_cast(chunkQw & 0x7FFFu) | - ((m_vif1PendingPath2ImageQwc == chunkQw) ? (1ull << 15) : 0ull) | - (static_cast(kGifFmtImage) << 58); - std::memcpy(imagePacket.data(), &imageTag, sizeof(imageTag)); - std::memcpy(imagePacket.data() + 16u, data + pos, static_cast(chunkQw) * 16u); - submitGifPacket(GifPathId::Path2, imagePacket.data(), static_cast(imagePacket.size()), true, m_vif1PendingPath2DirectHl); - + const uint32_t chunkQw = std::min(m_vif1PendingDirectQwc, availableQw); + forwardVif1DirectData(data + pos, chunkQw * 16u, m_vif1PendingDirectHl); pos += chunkQw * 16u; - m_vif1PendingPath2ImageQwc -= chunkQw; - if (m_vif1PendingPath2ImageQwc == 0u) - { - m_vif1PendingPath2DirectHl = false; - } + m_vif1PendingDirectQwc -= chunkQw; continue; } @@ -490,22 +520,22 @@ void PS2Memory::processVIF1Data(const uint8_t *data, uint32_t sizeBytes) uint32_t qwCount = imm; if (qwCount == 0) qwCount = 65536; + const uint32_t requestedQw = qwCount; const uint32_t availableQw = (sizeBytes - pos) / 16u; const bool truncated = qwCount > availableQw; if (qwCount > availableQw) qwCount = availableQw; + const bool directHl = (opcode == VIF_DIRECTHL); + if (truncated) + { + // The rest of this DIRECT's data starts the next buffer (see m_vif1PendingDirectQwc). + m_vif1PendingDirectQwc = requestedQw - qwCount; + m_vif1PendingDirectHl = directHl; + } if (qwCount > 0) { - const bool directHl = (opcode == VIF_DIRECTHL); - submitGifPacket(GifPathId::Path2, data + pos, qwCount * 16, true, directHl); - - const uint32_t pendingImageQw = pendingGifImageQwc(data + pos, qwCount * 16u); - if (pendingImageQw != 0u) - { - m_vif1PendingPath2ImageQwc = pendingImageQw; - m_vif1PendingPath2DirectHl = directHl; - } + forwardVif1DirectData(data + pos, qwCount * 16u, directHl); } pos += qwCount * 16; diff --git a/ps2xTest/src/ps2_memory_tests.cpp b/ps2xTest/src/ps2_memory_tests.cpp index 0b22fe03c..2ad20fba2 100644 --- a/ps2xTest/src/ps2_memory_tests.cpp +++ b/ps2xTest/src/ps2_memory_tests.cpp @@ -2112,7 +2112,7 @@ void register_ps2_memory_tests() t.IsTrue(imageOk, "VIF1 DIRECT image should update GS VRAM through GIF path2"); }); - tc.Run("VIF1 DIRECT image tag can continue with raw image qwords", [](TestCase &t) + tc.Run("VIF1 DIRECT cut off at the end of a buffer continues with raw image qwords", [](TestCase &t) { PS2Memory mem; t.IsTrue(mem.initialize(), "PS2Memory initialize should succeed"); @@ -2137,10 +2137,72 @@ void register_ps2_memory_tests() gs.writeRegister(GS_REG_TRXREG, (4ull << 0) | (1ull << 32)); gs.writeRegister(GS_REG_TRXDIR, 0ull); + // DIRECT 2 QW: the IMAGE tag and one pixel qword. The buffer ends after the tag, so the pixel qword + // is the start of the next buffer and carries no VIFcode. + std::vector first; + appendU32(first, makeVifCmd(0x50u, 0u, 2u)); + appendU64(first, makeGifTag(1u, GIF_FMT_IMAGE, 0u, true)); + appendU64(first, 0ull); + + std::vector second; + for (uint32_t i = 0; i < 16u; ++i) + { + second.push_back(static_cast(0xA0u + i)); + } + + mem.processVIF1Data(first.data(), static_cast(first.size())); + mem.processVIF1Data(second.data(), static_cast(second.size())); + + const uint8_t *vramOut = mem.getGSVRAM(); + bool imageOk = true; + for (uint32_t x = 0; x < 4u && imageOk; ++x) + { + const uint32_t off = GSPSMCT32::addrPSMCT32(0u, 1u, x, 0u); + for (uint32_t c = 0; c < 4u; ++c) + { + if (vramOut[off + c] != static_cast(0xA0u + x * 4u + c)) + { + imageOk = false; + break; + } + } + } + t.IsTrue(imageOk, "raw qwords of a DIRECT cut off at a buffer end should continue the PATH2 image upload"); + }); + + tc.Run("VIF1 DIRECT image tag continues in a later DIRECT after intervening VIFcodes", [](TestCase &t) + { + PS2Memory mem; + t.IsTrue(mem.initialize(), "PS2Memory initialize should succeed"); + + GS gs; + gs.init(mem.getGSVRAM(), static_cast(PS2_GS_VRAM_SIZE), &mem.gs()); + GifArbiter arbiter([&](const uint8_t *data, uint32_t sizeBytes) + { + gs.processGIFPacket(data, sizeBytes); + }); + mem.setGifArbiter(&arbiter); + + const uint64_t bitblt = + (static_cast(0u) << 0) | + (static_cast(1u) << 16) | + (static_cast(0u) << 24) | + (static_cast(0u) << 32) | + (static_cast(1u) << 48) | + (static_cast(0u) << 56); + gs.writeRegister(GS_REG_BITBLTBUF, bitblt); + gs.writeRegister(GS_REG_TRXPOS, 0ull); + gs.writeRegister(GS_REG_TRXREG, (4ull << 0) | (1ull << 32)); + gs.writeRegister(GS_REG_TRXDIR, 0ull); + + // The first DIRECT ends with an IMAGE tag; its pixel data is the payload of a second DIRECT that follows + // "MARK; DIRECT 1". The VIFcodes in between must be parsed as VIFcodes, not taken as image data. std::vector packet; appendU32(packet, makeVifCmd(0x50u, 0u, 1u)); // DIRECT 1 QW payload: GIF IMAGE tag only. appendU64(packet, makeGifTag(1u, GIF_FMT_IMAGE, 0u, true)); appendU64(packet, 0ull); + appendU32(packet, makeVifCmd(0x07u, 0u, 0x1234u)); // MARK + appendU32(packet, makeVifCmd(0x50u, 0u, 1u)); // DIRECT 1 QW payload: the pixels. for (uint32_t i = 0; i < 16u; ++i) { packet.push_back(static_cast(0xA0u + i)); @@ -2162,7 +2224,7 @@ void register_ps2_memory_tests() } } } - t.IsTrue(imageOk, "raw qwords after a DIRECT image tag should continue the PATH2 image upload"); + t.IsTrue(imageOk, "image data in a later DIRECT should not be shifted by the VIFcodes before it"); }); tc.Run("VIF1 DIRECT finds an image continuation after packed setup", [](TestCase &t) @@ -2194,6 +2256,7 @@ void register_ps2_memory_tests() appendU64(packet, GS_REG_TEXA); appendU64(packet, makeGifTag(1u, GIF_FMT_IMAGE, 0u, true)); appendU64(packet, 0ull); + appendU32(packet, makeVifCmd(0x50u, 0u, 1u)); // DIRECT 1 QW payload: the pixels. for (uint32_t i = 0; i < 16u; ++i) packet.push_back(static_cast(0xC0u + i)); @@ -2213,7 +2276,7 @@ void register_ps2_memory_tests() } } } - t.IsTrue(imageOk, "raw image continuation after packed setup should not be decoded as VIF/GIF registers"); + t.IsTrue(imageOk, "image continuation after packed setup should not be decoded as VIF/GIF registers"); }); tc.Run("unaligned accesses throw", [](TestCase &t)