From 77f0499f7f4f1dc4105fe05d29005261294dce8e Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Aug 2018 17:11:51 -0700 Subject: [PATCH 1/6] GPU: Rename clipping flag to depth clamp. It seems to just to depth clamp. When depth clamp happens, it affects clipping a little, but only for vertices that needed clamping. --- GPU/Common/GPUStateUtils.cpp | 2 +- GPU/GPUState.h | 5 +++-- GPU/Software/Clipper.cpp | 9 +++++++-- GPU/Software/TransformUnit.cpp | 2 +- GPU/Vulkan/StateMappingVulkan.cpp | 2 +- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/GPU/Common/GPUStateUtils.cpp b/GPU/Common/GPUStateUtils.cpp index 0353e3ac963e..824d3827b1ee 100644 --- a/GPU/Common/GPUStateUtils.cpp +++ b/GPU/Common/GPUStateUtils.cpp @@ -676,7 +676,7 @@ void ConvertViewportAndScissor(bool useBufferedRendering, float renderWidth, flo float minz = gstate.getDepthRangeMin(); float maxz = gstate.getDepthRangeMax(); - if (gstate.isClippingEnabled() && (minz == 0 || maxz == 65535)) { + if (gstate.isDepthClampEnabled() && (minz == 0 || maxz == 65535)) { // Here, we should "clamp." But clamping per fragment would be slow. // So, instead, we just increase the available range and hope. // If depthSliceFactor is 4, it means (75% / 2) of the depth lies in each direction. diff --git a/GPU/GPUState.h b/GPU/GPUState.h index 06b14867c465..a73023927fc2 100644 --- a/GPU/GPUState.h +++ b/GPU/GPUState.h @@ -58,7 +58,7 @@ struct GPUgstate { region2, lightingEnable, lightEnable[4], - clipEnable, + depthClampEnable, cullfaceEnable, textureMapEnable, // 0x1E GE_CMD_TEXTUREMAPENABLE fogEnable, @@ -392,8 +392,9 @@ struct GPUgstate { int getRegionX2() const { return (region2 & 0x3FF); } int getRegionY2() const { return (region2 >> 10) & 0x3FF; } + bool isDepthClampEnabled() const { return depthClampEnable & 1; } + // Note that the X1/Y1/Z1 here does not mean the upper-left corner, but half the dimensions. X2/Y2/Z2 are the center. - bool isClippingEnabled() const { return clipEnable & 1; } float getViewportXScale() const { return getFloat24(viewportxscale); } float getViewportYScale() const { return getFloat24(viewportyscale); } float getViewportZScale() const { return getFloat24(viewportzscale); } diff --git a/GPU/Software/Clipper.cpp b/GPU/Software/Clipper.cpp index 2dcb1dd91951..85c9fc6efd06 100644 --- a/GPU/Software/Clipper.cpp +++ b/GPU/Software/Clipper.cpp @@ -39,6 +39,7 @@ enum { static inline int CalcClipMask(const ClipCoords& v) { int mask = 0; + // This checks `x / w` compared to 1 or -1, skipping the division. if (v.x > v.w) mask |= CLIP_POS_X_BIT; if (v.x < -v.w) mask |= CLIP_NEG_X_BIT; if (v.y > v.w) mask |= CLIP_POS_Y_BIT; @@ -255,7 +256,9 @@ void ProcessLine(VertexData& v0, VertexData& v1) return; } - if (mask && gstate.isClippingEnabled()) { + if (mask && gstate.isDepthClampEnabled()) { + // TODO: Validate if this logic is correct / should be in depthClampEnabled. + // discard if any vertex is outside the near clipping plane if (mask & CLIP_NEG_Z_BIT) return; @@ -303,7 +306,9 @@ void ProcessTriangle(VertexData& v0, VertexData& v1, VertexData& v2) mask |= CalcClipMask(v1.clippos); mask |= CalcClipMask(v2.clippos); - if (mask && gstate.isClippingEnabled()) { + if (mask && gstate.isDepthClampEnabled()) { + // TODO: Validate if this logic is correct / should be in depthClampEnabled. + // discard if any vertex is outside the near clipping plane if (mask & CLIP_NEG_Z_BIT) return; diff --git a/GPU/Software/TransformUnit.cpp b/GPU/Software/TransformUnit.cpp index f1e7e9581bfd..911bdbf94f78 100644 --- a/GPU/Software/TransformUnit.cpp +++ b/GPU/Software/TransformUnit.cpp @@ -105,7 +105,7 @@ static inline ScreenCoords ClipToScreenInternal(const ClipCoords& coords, bool * float z = coords.z * zScale / coords.w + zCenter; // This matches hardware tests - depth is clamped when this flag is on. - if (gstate.isClippingEnabled()) { + if (gstate.isDepthClampEnabled()) { if (z < 0.f) z = 0.f; if (z > 65535.f) diff --git a/GPU/Vulkan/StateMappingVulkan.cpp b/GPU/Vulkan/StateMappingVulkan.cpp index 1e78e352ef4b..68bc711a1da5 100644 --- a/GPU/Vulkan/StateMappingVulkan.cpp +++ b/GPU/Vulkan/StateMappingVulkan.cpp @@ -247,7 +247,7 @@ void DrawEngineVulkan::ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManag // Set cull bool wantCull = !gstate.isModeThrough() && prim != GE_PRIM_RECTANGLES && gstate.isCullEnabled(); key.cullMode = wantCull ? (gstate.getCullMode() ? VK_CULL_MODE_FRONT_BIT : VK_CULL_MODE_BACK_BIT) : VK_CULL_MODE_NONE; - key.depthClampEnable = gstate.isClippingEnabled() && gstate_c.Supports(GPU_SUPPORTS_DEPTH_CLAMP); + key.depthClampEnable = gstate.isDepthClampEnabled() && gstate_c.Supports(GPU_SUPPORTS_DEPTH_CLAMP); } } From 921727f1632bd56c5ab8d94aff34685f6488939b Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Aug 2018 17:12:46 -0700 Subject: [PATCH 2/6] GE Debugger: Fix preview crash. In some cases on first draw (e.g. from a test), this might be null and crash. It was supposed to be the other program. --- Windows/GEDebugger/VertexPreview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Windows/GEDebugger/VertexPreview.cpp b/Windows/GEDebugger/VertexPreview.cpp index 42e006efd0e9..3dd9c8486b05 100644 --- a/Windows/GEDebugger/VertexPreview.cpp +++ b/Windows/GEDebugger/VertexPreview.cpp @@ -494,7 +494,7 @@ void CGEDebugger::UpdatePrimPreview(u32 op, int which) { } if (texPreviewVao == 0) { - glDisableVertexAttribArray(previewProgram->a_position); + glDisableVertexAttribArray(texPreviewProgram->a_position); } secondWindow->End(); From cd6b1f73c1a87c2e9b6e0f8c9ec810e061dba43c Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Aug 2018 17:15:44 -0700 Subject: [PATCH 3/6] SoftGPU: Drop verts only when depth not clamped. Depth clamping bypasses the 4096x4096 box check. --- GPU/Software/TransformUnit.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/GPU/Software/TransformUnit.cpp b/GPU/Software/TransformUnit.cpp index 911bdbf94f78..7bd578a25f26 100644 --- a/GPU/Software/TransformUnit.cpp +++ b/GPU/Software/TransformUnit.cpp @@ -106,14 +106,16 @@ static inline ScreenCoords ClipToScreenInternal(const ClipCoords& coords, bool * // This matches hardware tests - depth is clamped when this flag is on. if (gstate.isDepthClampEnabled()) { + // Note: if the depth is clamped, the outside_range_flag should NOT be set, even for x and y. if (z < 0.f) z = 0.f; - if (z > 65535.f) + else if (z > 65535.f) z = 65535.f; - } - - if (outside_range_flag && (x > 4095.9375f || y > 4095.9375f || x < 0 || y < 0 || z < 0 || z > 65535.f)) + else if (outside_range_flag && (x > 4095.9375f || y > 4095.9375f || x < 0 || y < 0)) + *outside_range_flag = true; + } else if (outside_range_flag && (x > 4095.9375f || y > 4095.9375f || x < 0 || y < 0 || z < 0 || z > 65535.f)) { *outside_range_flag = true; + } // 16 = 0xFFFF / 4095.9375 // Round up at 0.625 to the nearest subpixel. From e22cc7ef6d057b4a7907f373ac3d2254609df57d Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Aug 2018 17:47:05 -0700 Subject: [PATCH 4/6] SoftGPU: Always clip, without special neg z case. Depth clamping doesn't change whether it clips. Also, avoid culling when a vertex is behind the near plane. --- GPU/Software/Clipper.cpp | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/GPU/Software/Clipper.cpp b/GPU/Software/Clipper.cpp index 85c9fc6efd06..851747807d90 100644 --- a/GPU/Software/Clipper.cpp +++ b/GPU/Software/Clipper.cpp @@ -256,13 +256,7 @@ void ProcessLine(VertexData& v0, VertexData& v1) return; } - if (mask && gstate.isDepthClampEnabled()) { - // TODO: Validate if this logic is correct / should be in depthClampEnabled. - - // discard if any vertex is outside the near clipping plane - if (mask & CLIP_NEG_Z_BIT) - return; - + if (mask) { CLIP_LINE(CLIP_POS_X_BIT, -1, 0, 0, 1); CLIP_LINE(CLIP_NEG_X_BIT, 1, 0, 0, 1); CLIP_LINE(CLIP_POS_Y_BIT, 0, -1, 0, 1); @@ -306,13 +300,7 @@ void ProcessTriangle(VertexData& v0, VertexData& v1, VertexData& v2) mask |= CalcClipMask(v1.clippos); mask |= CalcClipMask(v2.clippos); - if (mask && gstate.isDepthClampEnabled()) { - // TODO: Validate if this logic is correct / should be in depthClampEnabled. - - // discard if any vertex is outside the near clipping plane - if (mask & CLIP_NEG_Z_BIT) - return; - + if (mask) { for (int i = 0; i < 3; i += 3) { int vlist[2][2*6+1]; int *inlist = vlist[0], *outlist = vlist[1]; From 44be615cf5612b883fb5cee9b6adade00def858f Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Aug 2018 19:52:20 -0700 Subject: [PATCH 5/6] GE Debugger: Arrange matrices properly. They were all off before for 4x3. --- Windows/GEDebugger/TabVertices.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Windows/GEDebugger/TabVertices.cpp b/Windows/GEDebugger/TabVertices.cpp index e878105fdd9b..f1f7e2f16426 100644 --- a/Windows/GEDebugger/TabVertices.cpp +++ b/Windows/GEDebugger/TabVertices.cpp @@ -369,7 +369,7 @@ void CtrlMatrixList::GetColumnText(wchar_t *dest, int row, int col) { if (row >= MATRIXLIST_ROW_BONE_0_0) { int b = (row - MATRIXLIST_ROW_BONE_0_0) / 3; int r = (row - MATRIXLIST_ROW_BONE_0_0) % 3; - int offset = (row - MATRIXLIST_ROW_BONE_0_0) * 4 + col - 1; + int offset = b * 12 + r + (col - 1) * 3; switch (col) { case MATRIXLIST_COL_NAME: @@ -382,7 +382,7 @@ void CtrlMatrixList::GetColumnText(wchar_t *dest, int row, int col) { } } else if (row >= MATRIXLIST_ROW_TGEN_0) { int r = row - MATRIXLIST_ROW_TGEN_0; - int offset = r * 4 + col - 1; + int offset = r + (col - 1) * 4; switch (col) { case MATRIXLIST_COL_NAME: @@ -395,7 +395,7 @@ void CtrlMatrixList::GetColumnText(wchar_t *dest, int row, int col) { } } else if (row >= MATRIXLIST_ROW_PROJ_0) { int r = row - MATRIXLIST_ROW_PROJ_0; - int offset = r * 4 + col - 1; + int offset = r + (col - 1) * 4; switch (col) { case MATRIXLIST_COL_NAME: @@ -408,7 +408,7 @@ void CtrlMatrixList::GetColumnText(wchar_t *dest, int row, int col) { } } else if (row >= MATRIXLIST_ROW_VIEW_0) { int r = row - MATRIXLIST_ROW_VIEW_0; - int offset = r * 4 + col - 1; + int offset = r + (col - 1) * 3; switch (col) { case MATRIXLIST_COL_NAME: @@ -421,7 +421,7 @@ void CtrlMatrixList::GetColumnText(wchar_t *dest, int row, int col) { } } else { int r = row - MATRIXLIST_ROW_WORLD_0; - int offset = r * 4 + col - 1; + int offset = r + (col - 1) * 3; switch (col) { case MATRIXLIST_COL_NAME: From 31d5c39858060d7d04861fa3af2d4fe877def37d Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Aug 2018 20:07:45 -0700 Subject: [PATCH 6/6] SoftGPU: Fix some minor rounding on viewport cull. Had some tests failing when on the edge due to this. --- GPU/Software/TransformUnit.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/GPU/Software/TransformUnit.cpp b/GPU/Software/TransformUnit.cpp index 7bd578a25f26..5810a36c2d6f 100644 --- a/GPU/Software/TransformUnit.cpp +++ b/GPU/Software/TransformUnit.cpp @@ -104,16 +104,21 @@ static inline ScreenCoords ClipToScreenInternal(const ClipCoords& coords, bool * float y = coords.y * yScale / coords.w + yCenter; float z = coords.z * zScale / coords.w + zCenter; + // Account for rounding for X and Y. + const float SCREEN_BOUND = 4095.0f + (15.0f / 16.0f) + 0.375f; + // TODO: Validate actual rounding range. + const float DEPTH_BOUND = 65535.5f; + // This matches hardware tests - depth is clamped when this flag is on. if (gstate.isDepthClampEnabled()) { // Note: if the depth is clamped, the outside_range_flag should NOT be set, even for x and y. if (z < 0.f) z = 0.f; - else if (z > 65535.f) - z = 65535.f; - else if (outside_range_flag && (x > 4095.9375f || y > 4095.9375f || x < 0 || y < 0)) + else if (z > 65535.0f) + z = 65535.0f; + else if (outside_range_flag && (x >= SCREEN_BOUND || y >= SCREEN_BOUND || x < 0 || y < 0)) *outside_range_flag = true; - } else if (outside_range_flag && (x > 4095.9375f || y > 4095.9375f || x < 0 || y < 0 || z < 0 || z > 65535.f)) { + } else if (outside_range_flag && (x > 4095.9675f || y >= SCREEN_BOUND || x < 0 || y < 0 || z < 0 || z >= DEPTH_BOUND)) { *outside_range_flag = true; }