From 096985f51f64af22053788ed83fbae9fc165d8dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 17 Jul 2024 13:26:14 +0200 Subject: [PATCH] Provoking vertex (software transform): Simpler solution Simply rotate each primitive in the index buffer to simulate a different provoking vertex. Since at this point we have already generated a plain primitive index buffer, it's easy to manipulate like this. An even better solution would be to generate rotated index buffers directly during decode, although that code is super critical and does not need more complexity.. We could now also enable this for hardware transform but I'm leaving that for later. --- GPU/Common/SoftwareTransformCommon.cpp | 99 +++++++++----------------- GPU/Common/SoftwareTransformCommon.h | 6 +- GPU/D3D11/D3D11Util.cpp | 4 +- GPU/D3D11/DrawEngineD3D11.cpp | 7 +- GPU/Directx9/DrawEngineDX9.cpp | 8 ++- GPU/GLES/DrawEngineGLES.cpp | 1 - GPU/Vulkan/DrawEngineVulkan.cpp | 12 ++-- 7 files changed, 63 insertions(+), 74 deletions(-) diff --git a/GPU/Common/SoftwareTransformCommon.cpp b/GPU/Common/SoftwareTransformCommon.cpp index 6e41c1476b..8d1624000c 100644 --- a/GPU/Common/SoftwareTransformCommon.cpp +++ b/GPU/Common/SoftwareTransformCommon.cpp @@ -125,33 +125,6 @@ static bool IsReallyAClear(const TransformedVertex *transformed, int numVerts, f return true; } -static int ColorIndexOffset(int prim, GEShadeMode shadeMode, bool clearMode) { - if (shadeMode != GE_SHADE_FLAT || clearMode) { - return 0; - } - - switch (prim) { - case GE_PRIM_LINES: - case GE_PRIM_LINE_STRIP: - return 1; - - case GE_PRIM_TRIANGLES: - case GE_PRIM_TRIANGLE_STRIP: - return 2; - - case GE_PRIM_TRIANGLE_FAN: - return 1; - - case GE_PRIM_RECTANGLES: - // We already use BR color when expanding, so no need to offset. - return 0; - - default: - break; - } - return 0; -} - void SoftwareTransform::SetProjMatrix(const float mtx[14], bool invertedX, bool invertedY, const Lin::Vec3 &trans, const Lin::Vec3 &scale) { memcpy(&projMatrix_.m, mtx, 16 * sizeof(float)); @@ -202,11 +175,6 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de fog_slope = std::signbit(fog_slope) ? -65535.0f : 65535.0f; } - int provokeIndOffset = 0; - if (!params_.provokingVertexLast) { - provokeIndOffset = ColorIndexOffset(prim, gstate.getShadeMode(), gstate.isModeClear()); - } - VertexReader reader(decoded, decVtxFormat, vertType); if (throughmode) { const u32 materialAmbientRGBA = gstate.getMaterialAmbientRGBA(); @@ -221,13 +189,7 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de vert.pos_w = 1.0f; if (hasColor) { - if (provokeIndOffset != 0 && index + provokeIndOffset < numDecodedVerts) { - reader.Goto(index + provokeIndOffset); - vert.color0_32 = reader.ReadColor0_8888(); - reader.Goto(index); - } else { - vert.color0_32 = reader.ReadColor0_8888(); - } + vert.color0_32 = reader.ReadColor0_8888(); } else { vert.color0_32 = materialAmbientRGBA; } @@ -268,10 +230,7 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de if (reader.hasUV()) reader.ReadUV(ruv); - // Read all the provoking vertex values here. Vec4f unlitColor; - if (provokeIndOffset != 0 && index + provokeIndOffset < numDecodedVerts) - reader.Goto(index + provokeIndOffset); if (reader.hasColor0()) reader.ReadColor0(unlitColor.AsArray()); else @@ -342,34 +301,14 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de break; case GE_PROJMAP_NORMALIZED_NORMAL: // Use normalized normal as source - // Flat uses the vertex normal, not provoking. - if (provokeIndOffset == 0) { - source = normal.Normalized(cpu_info.bSSE4_1); - } else { - reader.Goto(index); - if (reader.hasNormal()) - reader.ReadNrm(source.AsArray()); - if (gstate.areNormalsReversed()) - source = -source; - source.Normalize(); - } + source = normal.Normalized(cpu_info.bSSE4_1); if (!reader.hasNormal()) { ERROR_LOG_REPORT(Log::G3D, "Normal projection mapping without normal?"); } break; case GE_PROJMAP_NORMAL: // Use non-normalized normal as source! - // Flat uses the vertex normal, not provoking. - if (provokeIndOffset == 0) { - source = normal; - } else { - // Need to read the normal for this vertex and weight it again.. - reader.Goto(index); - if (reader.hasNormal()) - reader.ReadNrm(source.AsArray()); - if (gstate.areNormalsReversed()) - source = -source; - } + source = normal; if (!reader.hasNormal()) { ERROR_LOG_REPORT(Log::G3D, "Normal projection mapping without normal?"); } @@ -751,6 +690,38 @@ bool SoftwareTransform::ExpandRectangles(int vertexCount, int &numDecodedVerts, return true; } +// In-place. So, better not be doing this on GPU memory! +void IndexBufferProvokingLastToFirst(int prim, u16 *inds, int indsSize) { + switch (prim) { + case GE_PRIM_LINES: + // Swap every two indices. + for (int i = 0; i < indsSize - 1; i += 2) { + u16 temp = inds[i]; + inds[i] = inds[i + 1]; + inds[i + 1] = temp; + } + break; + case GE_PRIM_TRIANGLES: + // Rotate the triangle so the last becomes the first, without changing the winding order. + // This could be done with a series of pshufb. + for (int i = 0; i < indsSize - 2; i += 3) { + u16 temp = inds[i + 2]; + inds[i + 2] = inds[i + 1]; + inds[i + 1] = inds[i]; + inds[i] = temp; + } + break; + case GE_PRIM_POINTS: + // Nothing to do, + break; + case GE_PRIM_RECTANGLES: + // Nothing to do, already using the 2nd vertex. + break; + default: + _dbg_assert_msg_(false, "IndexBufferProvokingFirstToLast: Only works with plain indexed primitives, no strips or fans") + } +} + bool SoftwareTransform::ExpandLines(int vertexCount, int &numDecodedVerts, int vertsSize, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode) { // Before we start, do a sanity check - does the output fit? if ((vertexCount / 2) * 6 > indsSize) { diff --git a/GPU/Common/SoftwareTransformCommon.h b/GPU/Common/SoftwareTransformCommon.h index 9896b04bda..457fd6a253 100644 --- a/GPU/Common/SoftwareTransformCommon.h +++ b/GPU/Common/SoftwareTransformCommon.h @@ -57,9 +57,13 @@ struct SoftwareTransformParams { bool allowSeparateAlphaClear; bool flippedY; bool usesHalfZ; - bool provokingVertexLast; }; +// Converts an index buffer to make the provoking vertex the last. +// In-place. So, better not be doing this on GPU memory! +// TODO: We could do this already during index decode. +void IndexBufferProvokingLastToFirst(int prim, u16 *inds, int indsSize); + class SoftwareTransform { public: SoftwareTransform(SoftwareTransformParams ¶ms) : params_(params) {} diff --git a/GPU/D3D11/D3D11Util.cpp b/GPU/D3D11/D3D11Util.cpp index b136ca57bc..f84b188d56 100644 --- a/GPU/D3D11/D3D11Util.cpp +++ b/GPU/D3D11/D3D11Util.cpp @@ -36,7 +36,9 @@ std::vector CompileShaderToBytecodeD3D11(const char *code, size_t codeS if (trimmed.find("pow(f, e) will not work for negative f") != std::string::npos) { continue; } - WARN_LOG(Log::G3D, "%.*s", (int)trimmed.length(), trimmed.data()); + if (trimmed.size() > 1) { // ignore single nulls, not sure how they appear. + WARN_LOG(Log::G3D, "%.*s", (int)trimmed.length(), trimmed.data()); + } } } else { ERROR_LOG(Log::G3D, "%s: %s\n\n%s", "errors", errors.c_str(), numberedCode.c_str()); diff --git a/GPU/D3D11/DrawEngineD3D11.cpp b/GPU/D3D11/DrawEngineD3D11.cpp index 37582772c9..db0440c6c4 100644 --- a/GPU/D3D11/DrawEngineD3D11.cpp +++ b/GPU/D3D11/DrawEngineD3D11.cpp @@ -382,10 +382,15 @@ void DrawEngineD3D11::DoFlush() { params.texCache = textureCache_; params.allowClear = true; params.allowSeparateAlphaClear = false; // D3D11 doesn't support separate alpha clears - params.provokingVertexLast = false; params.flippedY = false; params.usesHalfZ = true; + if (gstate.getShadeMode() == GE_SHADE_FLAT) { + // We need to rotate the index buffer to simulate a different provoking vertex. + // We do this before line expansion etc. + IndexBufferProvokingLastToFirst(prim, inds, vertexCount); + } + // We need correct viewport values in gstate_c already. if (gstate_c.IsDirty(DIRTY_VIEWPORTSCISSOR_STATE)) { ViewportAndScissor vpAndScissor; diff --git a/GPU/Directx9/DrawEngineDX9.cpp b/GPU/Directx9/DrawEngineDX9.cpp index 46b314dbc8..ce20f88859 100644 --- a/GPU/Directx9/DrawEngineDX9.cpp +++ b/GPU/Directx9/DrawEngineDX9.cpp @@ -334,10 +334,16 @@ void DrawEngineDX9::DoFlush() { params.texCache = textureCache_; params.allowClear = true; params.allowSeparateAlphaClear = false; - params.provokingVertexLast = false; params.flippedY = false; params.usesHalfZ = true; + if (gstate.getShadeMode() == GE_SHADE_FLAT) { + // We need to rotate the index buffer to simulate a different provoking vertex. + // We do this before line expansion etc. + int indexCount = RemainingIndices(inds); + IndexBufferProvokingLastToFirst(prim, inds, vertexCount); + } + // We need correct viewport values in gstate_c already. if (gstate_c.IsDirty(DIRTY_VIEWPORTSCISSOR_STATE)) { ViewportAndScissor vpAndScissor; diff --git a/GPU/GLES/DrawEngineGLES.cpp b/GPU/GLES/DrawEngineGLES.cpp index e4f342af79..e2161572d3 100644 --- a/GPU/GLES/DrawEngineGLES.cpp +++ b/GPU/GLES/DrawEngineGLES.cpp @@ -356,7 +356,6 @@ void DrawEngineGLES::DoFlush() { params.texCache = textureCache_; params.allowClear = true; // Clear in OpenGL respects scissor rects, so we'll use it. params.allowSeparateAlphaClear = true; - params.provokingVertexLast = true; params.flippedY = framebufferManager_->UseBufferedRendering(); params.usesHalfZ = false; diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index bdae445a75..b03a8556a0 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -405,11 +405,13 @@ void DrawEngineVulkan::DoFlush() { // do not respect scissor rects. params.allowClear = framebufferManager_->UseBufferedRendering(); params.allowSeparateAlphaClear = false; - if (renderManager->GetVulkanContext()->GetDeviceFeatures().enabled.provokingVertex.provokingVertexLast) { - // We can get the OpenGL behavior, no need for workarounds. - params.provokingVertexLast = true; - } else { - params.provokingVertexLast = false; + + if (gstate.getShadeMode() == GE_SHADE_FLAT) { + if (!renderManager->GetVulkanContext()->GetDeviceFeatures().enabled.provokingVertex.provokingVertexLast) { + // If we can't have the hardware do it, we need to rotate the index buffer to simulate a different provoking vertex. + // We do this before line expansion etc. + IndexBufferProvokingLastToFirst(prim, inds, vertexCount); + } } params.flippedY = true; params.usesHalfZ = true;