From b361ecd54c95fdbfb486daeb6ab7caa2816cf731 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 13:29:17 +0000 Subject: [PATCH] Stop draws from merging or reading past their own vertices Incomplete primitives no longer emit indices: quads drop a one- or two-vertex tail and draw a three-vertex tail as a triangle, triangle lists ignore leftover vertices, and fans or strips under three vertices emit nothing. Draws with no complete primitive are skipped. Previously an incomplete quad indexed vertices that belonged to the next merged draw, or past the end of the buffer. Merging now stops before the 16-bit index offset would wrap, never folds triangles into a single-instance line or point draw, and breaks after GXInvalidateVtxCache or a vertex-format switch, so the next draw uploads fresh arrays and uses its own format's shader. From upstream patchzyy/Wiicompiled 6f14bde (#244, KartPad batch). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Wg7mB8ogCWmp9GH19Uc82B --- aurora-main/lib/gx/command_processor.cpp | 52 ++++++++--- aurora-main/lib/gx/pipeline.hpp | 1 + aurora-main/tests/CMakeLists.txt | 1 + aurora-main/tests/gx_fifo_test.cpp | 3 +- .../tests/renderer_regression_test.cpp | 86 +++++++++++++++++++ 5 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 aurora-main/tests/renderer_regression_test.cpp diff --git a/aurora-main/lib/gx/command_processor.cpp b/aurora-main/lib/gx/command_processor.cpp index 5b52139..328cb5c 100644 --- a/aurora-main/lib/gx/command_processor.cpp +++ b/aurora-main/lib/gx/command_processor.cpp @@ -33,10 +33,11 @@ using IndexBuffer = std::vector; static u32 prepare_idx_template(IndexBuffer& buf, GXPrimitive prim, u16 vtxCount) { size_t writePos = 0; if (prim == GX_QUADS) { - // Retain the existing incomplete-quad behavior: every started group emits a complete six-index quad. - buf.resize(((static_cast(vtxCount) + 3u) / 4u) * 6u); + // GX renders a three-vertex remainder as a triangle. One/two are ignored. + const u32 completeVertices = static_cast(vtxCount) & ~3u; + buf.resize((completeVertices / 4u) * 6u + (vtxCount % 4u == 3u ? 3u : 0u)); - for (u16 v = 0; v < vtxCount; v += 4) { + for (u32 v = 0; v < completeVertices; v += 4) { const u16 idx0 = v; const u16 idx1 = static_cast(v + 1); const u16 idx2 = static_cast(v + 2); @@ -48,15 +49,21 @@ static u32 prepare_idx_template(IndexBuffer& buf, GXPrimitive prim, u16 vtxCount buf[writePos++] = idx3; buf[writePos++] = idx0; } + if (vtxCount % 4u == 3u) { + buf[writePos++] = static_cast(completeVertices); + buf[writePos++] = static_cast(completeVertices + 1u); + buf[writePos++] = static_cast(completeVertices + 2u); + } } else if (prim == GX_TRIANGLES) { - buf.resize(vtxCount); - for (u16 v = 0; v < vtxCount; ++v) { + const u32 completeVertices = (static_cast(vtxCount) / 3u) * 3u; + buf.resize(completeVertices); + for (u32 v = 0; v < completeVertices; ++v) { buf[writePos++] = v; } } else if (prim == GX_TRIANGLEFAN) { - const u32 indexCount = vtxCount <= 3 ? vtxCount : 3u + (static_cast(vtxCount) - 3u) * 3u; + const u32 indexCount = vtxCount < 3 ? 0u : (static_cast(vtxCount) - 2u) * 3u; buf.resize(indexCount); - for (u16 v = 0; v < vtxCount; ++v) { + for (u32 v = 0; indexCount != 0 && v < vtxCount; ++v) { if (v < 3) { buf[writePos++] = v; continue; @@ -66,9 +73,9 @@ static u32 prepare_idx_template(IndexBuffer& buf, GXPrimitive prim, u16 vtxCount buf[writePos++] = v; } } else if (prim == GX_TRIANGLESTRIP) { - const u32 indexCount = vtxCount <= 3 ? vtxCount : 3u + (static_cast(vtxCount) - 3u) * 3u; + const u32 indexCount = vtxCount < 3 ? 0u : (static_cast(vtxCount) - 2u) * 3u; buf.resize(indexCount); - for (u16 v = 0; v < vtxCount; ++v) { + for (u32 v = 0; indexCount != 0 && v < vtxCount; ++v) { if (v < 3) { buf[writePos++] = v; continue; @@ -91,6 +98,13 @@ static u32 prepare_idx_template(IndexBuffer& buf, GXPrimitive prim, u16 vtxCount return static_cast(writePos); } +// Empty/incomplete draws consume FIFO bytes but cannot produce a primitive. +static bool has_complete_primitive(GXPrimitive prim, u16 count) { + if (prim == GX_POINTS) return count >= 1; + if (prim == GX_LINES || prim == GX_LINESTRIP) return count >= 2; + return count >= 3; +} + // GX FIFO opcodes - use CP_ prefix to avoid clashing with GXCommandList.h macros static constexpr u8 CP_CMD_NOP = GX_NOP; static constexpr u8 CP_CMD_LOAD_CP_REG = GX_LOAD_CP_REG; @@ -552,6 +566,10 @@ void process(const u8* data, u32 size, bool bigEndian) { for (int i = GX_VA_POS; i <= GX_VA_TEX7; ++i) { g_gxState.arrays[i].cachedRange = {}; } + // A merged draw retains its previous array uploads. Force a new draw so + // handle_draw_unmerged observes the invalidation and uploads fresh data. + // Pipeline configuration itself did not change. + g_gxState.stateDirty = true; break; } @@ -1926,6 +1944,10 @@ static u32 calculate_last_vtx_size(GXVtxFmt fmt) { g_gxState.lastVtxFmt = fmt; g_gxState.lastVtxSize = vtxSize; + // The format is selected by the draw opcode, without a register write. + // Even equal-stride formats may decode bytes differently, so do not merge + // into a draw using the previous format's shader and uniform layout. + g_gxState.stateDirty = true; return vtxSize; } @@ -2366,6 +2388,8 @@ bool submit_raw_draw(GXPrimitive prim, GXVtxFmt fmt, const uint8_t* vertices, ui return false; } + if (!has_complete_primitive(prim, vtxCount)) return true; + // This entry point bypasses process(), so it owns the renderer lock itself. std::lock_guard gpuLock(aurora::renderer_gpu_mutex()); if (model_array_hidden()) return true; @@ -2414,7 +2438,7 @@ static bool handle_draw(u8 cmd, const u8* data, u32& pos, u32 size, bool bigEndi return false; } - if (model_array_hidden()) { + if (!has_complete_primitive(prim, vtxCount) || model_array_hidden()) { pos += totalVtxBytes; return true; } @@ -2436,9 +2460,12 @@ static bool handle_draw(u8 cmd, const u8* data, u32& pos, u32 size, bool bigEndi // Only if the previous draw call was a single instance draw (no lines/points handling), and only into a draw // that resolved the same animated array: the merged whole renders through that draw's binding. Anything the // decision cache cannot vouch for (a command it was not recorded against) stays unmerged. + // Expanded lines/points have different vertex interpretation even with one instance. + // Triangle-list output has no restart index; index 65535 is usable. + // Overflow would address earlier vertices instead of the appended geometry. if (lastDraw != nullptr && prim != GX_LINES && prim != GX_LINESTRIP && prim != GX_POINTS && - !lastDraw->uniformReplayLayout.vertexMotion.enabled && - lastDraw->instanceCount == 1 && + !lastDraw->uniformReplayLayout.vertexMotion.enabled && !lastDraw->expandedPrimitive && + lastDraw->instanceCount == 1 && uint64_t(lastDraw->vtxCount) + vtxCount <= 65536u && (nativeWheelArrays.empty() || (nativeWheelLastDrawCommand == lastDraw && nativeWheelLastDecision == nativeWheel))) LIKELY { @@ -2615,6 +2642,7 @@ static void handle_draw_unmerged(GXPrimitive prim, GXVtxFmt fmt, u16 vtxCount, g .vtxCount = vtxCount, .indexCount = numIndices, .instanceCount = instanceCount, + .expandedPrimitive = prim == GX_LINES || prim == GX_LINESTRIP || prim == GX_POINTS, .bindGroups = bindGroups, .dstAlpha = pipelineState.dstAlpha, .screenRect = screen_rect(prim, fmt, vertices, vtxCount, vtxStride), diff --git a/aurora-main/lib/gx/pipeline.hpp b/aurora-main/lib/gx/pipeline.hpp index d52fe92..4ae19e6 100644 --- a/aurora-main/lib/gx/pipeline.hpp +++ b/aurora-main/lib/gx/pipeline.hpp @@ -29,6 +29,7 @@ struct DrawData { uint32_t vtxCount; uint32_t indexCount; uint32_t instanceCount; + bool expandedPrimitive; GXBindGroups bindGroups; uint32_t dstAlpha; // Valid only for simple orthographic rectangles/lines (textured or not). diff --git a/aurora-main/tests/CMakeLists.txt b/aurora-main/tests/CMakeLists.txt index ab34673..13d1428 100644 --- a/aurora-main/tests/CMakeLists.txt +++ b/aurora-main/tests/CMakeLists.txt @@ -64,6 +64,7 @@ if (AURORA_ENABLE_GX) add_executable(gx_fifo_tests gx_fifo_test.cpp gx_test_stubs.cpp + renderer_regression_test.cpp stereo_replay_test.cpp stereo_interpolation_test.cpp stereo_mirror_test.cpp diff --git a/aurora-main/tests/gx_fifo_test.cpp b/aurora-main/tests/gx_fifo_test.cpp index 8eb5a08..6433551 100644 --- a/aurora-main/tests/gx_fifo_test.cpp +++ b/aurora-main/tests/gx_fifo_test.cpp @@ -2861,6 +2861,7 @@ TEST_F(GXFifoTest, DrawTopologyTemplatesPreserveExactGxIndexOrder) { const auto decodeAndReadIndices = [&](GXPrimitive primitive, u16 count) { std::vector fifo; append_test_draw(fifo, primitive, count); + aurora::gfx::testing::reset_vertex_push_record(); decode_fifo(fifo); return aurora::gfx::testing::last_pushed_indices(); }; @@ -2871,7 +2872,7 @@ TEST_F(GXFifoTest, DrawTopologyTemplatesPreserveExactGxIndexOrder) { g_gxState.stateDirty = true; EXPECT_EQ(decodeAndReadIndices(GX_TRIANGLEFAN, 5), (std::vector{0, 1, 2, 0, 2, 3, 0, 3, 4})); g_gxState.stateDirty = true; - EXPECT_EQ(decodeAndReadIndices(GX_TRIANGLEFAN, 2), (std::vector{0, 1})); + EXPECT_EQ(decodeAndReadIndices(GX_TRIANGLEFAN, 2), (std::vector{})); g_gxState.stateDirty = true; EXPECT_EQ(decodeAndReadIndices(GX_TRIANGLESTRIP, 6), (std::vector{0, 1, 2, 2, 1, 3, 2, 3, 4, 4, 3, 5})); g_gxState.stateDirty = true; diff --git a/aurora-main/tests/renderer_regression_test.cpp b/aurora-main/tests/renderer_regression_test.cpp new file mode 100644 index 0000000..0a845b5 --- /dev/null +++ b/aurora-main/tests/renderer_regression_test.cpp @@ -0,0 +1,86 @@ +#include "gx_test_common.hpp" +#include "gx/pipeline.hpp" + +using aurora::gx::g_gxState; + +namespace { +std::vector draw(GXPrimitive primitive, u16 count, GXVtxFmt format = GX_VTXFMT0) { + std::vector bytes{static_cast(primitive | format), static_cast(count >> 8), static_cast(count)}; + bytes.resize(3 + count); + return bytes; +} +} // namespace + +TEST_F(GXFifoTest, MaximumQuadCountTerminatesWithoutOutOfRangeIndices) { + g_gxState.lastVtxFmt = GX_VTXFMT0; + g_gxState.lastVtxSize = 1; + for (const u16 count : {65532, 65533, 65534, 65535}) { + g_gxState.stateDirty = true; + decode_fifo(draw(GX_QUADS, count)); + const auto& indices = aurora::gfx::testing::last_pushed_indices(); + ASSERT_EQ(indices.size(), (count / 4) * 6 + (count % 4 == 3 ? 3 : 0)); + for (const auto index : indices) + ASSERT_LT(index, count); + } +} + +TEST_F(GXFifoTest, IncompletePrimitivesNeverJoinAcrossDraws) { + g_gxState.lastVtxFmt = GX_VTXFMT0; + g_gxState.lastVtxSize = 1; + aurora::gfx::testing::use_draw_command_tracking(true); + decode_fifo(draw(GX_TRIANGLES, 4)); + EXPECT_EQ(aurora::gfx::testing::last_pushed_indices(), (std::vector{0, 1, 2})); + decode_fifo(draw(GX_TRIANGLES, 5)); + EXPECT_EQ(aurora::gfx::testing::last_pushed_indices(), (std::vector{4, 5, 6})); + const auto before = aurora::gfx::testing::last_pushed_indices(); + decode_fifo(draw(GX_TRIANGLEFAN, 2)); + EXPECT_EQ(aurora::gfx::testing::last_pushed_indices(), before); +} + +TEST_F(GXFifoTest, MergeStopsBeforeSixteenBitIndexOverflow) { + g_gxState.lastVtxFmt = GX_VTXFMT0; + g_gxState.lastVtxSize = 1; + aurora::gfx::testing::use_draw_command_tracking(true); + decode_fifo(draw(GX_TRIANGLES, 65535)); + decode_fifo(draw(GX_TRIANGLES, 3)); + EXPECT_EQ(aurora::gfx::g_mergedDrawCallCount, 0u); + EXPECT_EQ(aurora::gfx::testing::last_pushed_indices(), (std::vector{0, 1, 2})); +} + +TEST_F(GXFifoTest, VertexCacheInvalidationBreaksDrawMerging) { + g_gxState.lastVtxFmt = GX_VTXFMT0; + g_gxState.lastVtxSize = 1; + aurora::gfx::testing::use_draw_command_tracking(true); + decode_fifo(draw(GX_TRIANGLES, 3)); + decode_fifo({GX_CMD_INVL_VC}); + EXPECT_TRUE(g_gxState.stateDirty); + decode_fifo(draw(GX_TRIANGLES, 3)); + EXPECT_EQ(aurora::gfx::g_mergedDrawCallCount, 0u); +} + +TEST_F(GXFifoTest, EqualStrideVertexFormatChangeBreaksDrawMerging) { + aurora::gfx::testing::use_real_vertex_format_helpers(true); + g_gxState.vtxDesc[GX_VA_POS] = GX_DIRECT; + for (const auto format : {GX_VTXFMT0, GX_VTXFMT1}) { + g_gxState.vtxFmts[format].attrs[GX_VA_POS].cnt = GX_POS_XY; + g_gxState.vtxFmts[format].attrs[GX_VA_POS].type = GX_U8; + } + g_gxState.vtxFmts[GX_VTXFMT1].attrs[GX_VA_POS].frac = 1; + aurora::gfx::testing::use_draw_command_tracking(true); + for (const auto format : {GX_VTXFMT0, GX_VTXFMT1}) { + auto bytes = draw(GX_TRIANGLES, 3, format); + bytes.resize(9); + decode_fifo(bytes); + } + EXPECT_EQ(aurora::gfx::g_mergedDrawCallCount, 0u); +} + +TEST_F(GXFifoTest, SingleExpandedPrimitiveCannotMergeWithTriangles) { + g_gxState.lastVtxFmt = GX_VTXFMT0; + g_gxState.lastVtxSize = 1; + aurora::gfx::testing::use_draw_command_tracking(true); + decode_fifo(draw(GX_POINTS, 1)); + decode_fifo(draw(GX_TRIANGLES, 3)); + EXPECT_EQ(aurora::gfx::g_mergedDrawCallCount, 0u); + EXPECT_EQ(aurora::gfx::testing::last_pushed_indices(), (std::vector{0, 1, 2})); +}