From ab3c9fc21f98300be19c6e22ce617e486fdb2d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 3 Sep 2019 23:26:05 +0200 Subject: [PATCH 1/7] Vulkan: Move scissor/viewport rotation into the VulkanRenderManager. Fixes #12303. --- ext/native/base/display.cpp | 34 +++++++++++++++++++++++++ ext/native/base/display.h | 5 ++++ ext/native/math/math_util.h | 2 -- ext/native/thin3d/VulkanRenderManager.h | 24 +++++++++++++---- ext/native/thin3d/thin3d.cpp | 32 ----------------------- ext/native/thin3d/thin3d.h | 5 ---- ext/native/thin3d/thin3d_vulkan.cpp | 13 +--------- 7 files changed, 59 insertions(+), 56 deletions(-) diff --git a/ext/native/base/display.cpp b/ext/native/base/display.cpp index 2223753a7d..7c93865e69 100644 --- a/ext/native/base/display.cpp +++ b/ext/native/base/display.cpp @@ -1,3 +1,4 @@ +#include #include "base/display.h" int dp_xres; @@ -17,3 +18,36 @@ float display_hz = 60.0f; DisplayRotation g_display_rotation; Matrix4x4 g_display_rot_matrix; + +void RotateRectToDisplay(FRect &rect, float curRTWidth, float curRTHeight) { + switch (g_display_rotation) { + case DisplayRotation::ROTATE_180: + rect.x = curRTWidth - rect.w - rect.x; + rect.y = curRTHeight - rect.h - rect.y; + break; + case DisplayRotation::ROTATE_90: { + // Note that curRTWidth_ and curRTHeight_ are "swapped"! + float origX = rect.x; + float origY = rect.y; + float rtw = curRTHeight; + float rth = curRTWidth; + rect.x = rth - rect.h - origY; + rect.y = origX; + std::swap(rect.w, rect.h); + break; + } + case DisplayRotation::ROTATE_270: { + float origX = rect.x; + float origY = rect.y; + float rtw = curRTHeight; + float rth = curRTWidth; + rect.x = origY; + rect.y = rtw - rect.w - origX; + std::swap(rect.w, rect.h); + break; + } + case DisplayRotation::ROTATE_0: + default: + break; + } +} diff --git a/ext/native/base/display.h b/ext/native/base/display.h index ca01fe1997..f56db2ce89 100644 --- a/ext/native/base/display.h +++ b/ext/native/base/display.h @@ -30,3 +30,8 @@ enum class DisplayRotation { extern DisplayRotation g_display_rotation; extern Matrix4x4 g_display_rot_matrix; + +struct FRect { + float x, y, w, h; +}; +void RotateRectToDisplay(FRect &rect, float curRTWidth, float curRTHeight); diff --git a/ext/native/math/math_util.h b/ext/native/math/math_util.h index 23ee8969a3..4d30d080c6 100644 --- a/ext/native/math/math_util.h +++ b/ext/native/math/math_util.h @@ -220,5 +220,3 @@ void EnableFZ(); // very slow VFP units). // http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0274h/Babffifj.html void FPU_SetFastMode(); - - diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index a06332aa69..ee8a15ee56 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -10,6 +10,7 @@ #include #include +#include "base/display.h" #include "Common/Vulkan/VulkanContext.h" #include "math/dataconv.h" #include "math/math_util.h" @@ -119,13 +120,18 @@ public: void SetViewport(const VkViewport &vp) { _dbg_assert_(G3D, curRenderStep_ && curRenderStep_->stepType == VKRStepType::RENDER); _dbg_assert_(G3D, (int)vp.width >= 0); + FRect rc{ vp.x, vp.y, vp.width, vp.height }; + if (curRenderStep_->render.framebuffer == nullptr) { // Only the backbuffer is actually rotated wrong! + RotateRectToDisplay(rc, (float)vulkan_->GetBackbufferWidth(), (float)vulkan_->GetBackbufferHeight()); + } VkRenderData data{ VKRRenderCommand::VIEWPORT }; - data.viewport.vp.x = vp.x; - data.viewport.vp.y = vp.y; - data.viewport.vp.width = vp.width; - data.viewport.vp.height = vp.height; + data.viewport.vp.x = rc.x; + data.viewport.vp.y = rc.y; + data.viewport.vp.width = rc.w; + data.viewport.vp.height = rc.h; // We can't allow values outside this range unless we use VK_EXT_depth_range_unrestricted. // Sometimes state mapping produces 65536/65535 which is slightly outside. + // TODO: This should be fixed at the source. data.viewport.vp.maxDepth = clamp_value(vp.maxDepth, 0.0f, 1.0f); data.viewport.vp.minDepth = clamp_value(vp.minDepth, 0.0f, 1.0f); curRenderStep_->commands.push_back(data); @@ -136,7 +142,15 @@ public: _dbg_assert_(G3D, (int)rc.extent.width >= 0); _dbg_assert_(G3D, (int)rc.extent.height >= 0); VkRenderData data{ VKRRenderCommand::SCISSOR }; - data.scissor.scissor = rc; + if (curRenderStep_->render.framebuffer == nullptr) { + FRect frc{ (float)rc.offset.x, (float)rc.offset.y, (float)rc.extent.width, (float)rc.extent.height }; + if (curRenderStep_->render.framebuffer == nullptr) { // Only the backbuffer is actually rotated wrong! + RotateRectToDisplay(frc, (float)vulkan_->GetBackbufferWidth(), (float)vulkan_->GetBackbufferHeight()); + } + data.scissor.scissor = VkRect2D{ { (int32_t)frc.x, (int32_t)frc.y }, { (uint32_t)frc.w, (uint32_t)frc.h} }; + } else { + data.scissor.scissor = rc; + } curRenderStep_->commands.push_back(data); } diff --git a/ext/native/thin3d/thin3d.cpp b/ext/native/thin3d/thin3d.cpp index 82f372e42e..d142638e07 100644 --- a/ext/native/thin3d/thin3d.cpp +++ b/ext/native/thin3d/thin3d.cpp @@ -353,38 +353,6 @@ DrawContext::~DrawContext() { DestroyPresets(); } -void DrawContext::RotateRectToDisplay(FRect &rect, float curRTWidth, float curRTHeight) { - if (g_display_rotation == DisplayRotation::ROTATE_0) - return; - switch (g_display_rotation) { - case DisplayRotation::ROTATE_180: - rect.x = curRTWidth - rect.w - rect.x; - rect.y = curRTHeight - rect.h - rect.y; - break; - case DisplayRotation::ROTATE_90: { - // Note that curRTWidth_ and curRTHeight_ are "swapped"! - float origX = rect.x; - float origY = rect.y; - float rtw = curRTHeight; - float rth = curRTWidth; - rect.x = rth - rect.h - origY; - rect.y = origX; - std::swap(rect.w, rect.h); - break; - } - case DisplayRotation::ROTATE_270: { - float origX = rect.x; - float origY = rect.y; - float rtw = curRTHeight; - float rth = curRTWidth; - rect.x = origY; - rect.y = rtw - rect.w - origX; - std::swap(rect.w, rect.h); - break; - } - } -} - // TODO: SSE/NEON // Could also make C fake-simd for 64-bit, two 8888 pixels fit in a register :) void ConvertFromRGBA8888(uint8_t *dst, const uint8_t *src, uint32_t dstStride, uint32_t srcStride, uint32_t width, uint32_t height, DataFormat format) { diff --git a/ext/native/thin3d/thin3d.h b/ext/native/thin3d/thin3d.h index b7d717447f..c07b6130b5 100644 --- a/ext/native/thin3d/thin3d.h +++ b/ext/native/thin3d/thin3d.h @@ -661,11 +661,6 @@ public: virtual void FlushState() {} protected: - struct FRect { - float x, y, w, h; - }; - void RotateRectToDisplay(FRect &rect, float curRTWidth, float curRTHeight); - ShaderModule *vsPresets_[VS_MAX_PRESET]; ShaderModule *fsPresets_[FS_MAX_PRESET]; diff --git a/ext/native/thin3d/thin3d_vulkan.cpp b/ext/native/thin3d/thin3d_vulkan.cpp index 960f224180..896d6ef337 100644 --- a/ext/native/thin3d/thin3d_vulkan.cpp +++ b/ext/native/thin3d/thin3d_vulkan.cpp @@ -1059,13 +1059,7 @@ Pipeline *VKContext::CreateGraphicsPipeline(const PipelineDesc &desc) { } void VKContext::SetScissorRect(int left, int top, int width, int height) { - FRect rc{ (float)left, (float)top, (float)width, (float)height }; - if (curFramebuffer_ == nullptr) { // Only the backbuffer is actually rotated wrong! - int curRTWidth, curRTHeight; - GetFramebufferDimensions((Framebuffer *)curFramebuffer_, &curRTWidth, &curRTHeight); - RotateRectToDisplay(rc, (float)curRTWidth, (float)curRTHeight); - } - VkRect2D scissor{ {(int32_t)rc.x, (int32_t)rc.y}, {(uint32_t)rc.w, (uint32_t)rc.h} }; + VkRect2D scissor{ {(int32_t)left, (int32_t)top}, {(uint32_t)width, (uint32_t)height} }; renderManager_.SetScissor(scissor); } @@ -1074,11 +1068,6 @@ void VKContext::SetViewports(int count, Viewport *viewports) { // Ignore viewports more than the first. VkViewport viewport; FRect rc{ viewports[0].TopLeftX , viewports[0].TopLeftY, viewports[0].Width, viewports[0].Height }; - if (curFramebuffer_ == nullptr) { // Only the backbuffer is actually rotated wrong! - int curRTWidth, curRTHeight; - GetFramebufferDimensions((Framebuffer *)curFramebuffer_, &curRTWidth, &curRTHeight); - RotateRectToDisplay(rc, (float)curRTWidth, (float)curRTHeight); - } viewport.x = rc.x; viewport.y = rc.y; viewport.width = rc.w; From 025a9f4dae034aae003c99d7719ca50ec8ab8a12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 4 Sep 2019 21:42:47 +0200 Subject: [PATCH 2/7] Improve Mali driver version detection --- Common/Vulkan/VulkanContext.cpp | 40 +++++++++++++++++++++++++++++ Common/Vulkan/VulkanContext.h | 4 +++ GPU/Vulkan/GPU_Vulkan.cpp | 10 +++++--- ext/native/thin3d/thin3d_vulkan.cpp | 24 ----------------- 4 files changed, 51 insertions(+), 27 deletions(-) diff --git a/Common/Vulkan/VulkanContext.cpp b/Common/Vulkan/VulkanContext.cpp index 955039de51..be07dd66c6 100644 --- a/Common/Vulkan/VulkanContext.cpp +++ b/Common/Vulkan/VulkanContext.cpp @@ -1391,3 +1391,43 @@ void VulkanContext::GetImageMemoryRequirements(VkImage image, VkMemoryRequiremen *dedicatedAllocation = false; } } + +bool IsHashMaliDriverVersion(const VkPhysicalDeviceProperties &props) { + // ARM used to put a hash in place of the driver version. + // Now they only use major versions. We'll just make a bad heuristic. + uint32_t major = VK_VERSION_MAJOR(props.driverVersion); + uint32_t minor = VK_VERSION_MINOR(props.driverVersion); + uint32_t branch = VK_VERSION_PATCH(props.driverVersion); + if (branch > 0) + return true; + if (branch > 100 || major > 100) + return true; + return false; +} + +// From Sascha's code +std::string FormatDriverVersion(const VkPhysicalDeviceProperties &props) { + if (props.vendorID == VULKAN_VENDOR_NVIDIA) { + // For whatever reason, NVIDIA has their own scheme. + // 10 bits = major version (up to r1023) + // 8 bits = minor version (up to 255) + // 8 bits = secondary branch version/build version (up to 255) + // 6 bits = tertiary branch/build version (up to 63) + uint32_t major = (props.driverVersion >> 22) & 0x3ff; + uint32_t minor = (props.driverVersion >> 14) & 0x0ff; + uint32_t secondaryBranch = (props.driverVersion >> 6) & 0x0ff; + uint32_t tertiaryBranch = (props.driverVersion) & 0x003f; + return StringFromFormat("%d.%d.%d.%d", major, minor, secondaryBranch, tertiaryBranch); + } else if (props.vendorID == VULKAN_VENDOR_ARM) { + // ARM used to just put a hash here. No point in splitting it up. + if (IsHashMaliDriverVersion(props)) { + return StringFromFormat("(hash) %08x", props.driverVersion); + } + } + // Qualcomm has an inscrutable versioning scheme. Let's just display it as normal. + // Standard scheme, use the standard macros. + uint32_t major = VK_VERSION_MAJOR(props.driverVersion); + uint32_t minor = VK_VERSION_MINOR(props.driverVersion); + uint32_t branch = VK_VERSION_PATCH(props.driverVersion); + return StringFromFormat("%d.%d.%d (%08x)", major, minor, branch, props.driverVersion); +} diff --git a/Common/Vulkan/VulkanContext.h b/Common/Vulkan/VulkanContext.h index d3ecb96963..63fa5108e7 100644 --- a/Common/Vulkan/VulkanContext.h +++ b/Common/Vulkan/VulkanContext.h @@ -359,3 +359,7 @@ void finalize_glslang(); bool GLSLtoSPV(const VkShaderStageFlagBits shader_type, const char *pshader, std::vector &spirv, std::string *errorMessage = nullptr); const char *VulkanResultToString(VkResult res); +std::string FormatDriverVersion(const VkPhysicalDeviceProperties &props); + +// Simple heuristic. +bool IsHashMaliDriverVersion(const VkPhysicalDeviceProperties &props); diff --git a/GPU/Vulkan/GPU_Vulkan.cpp b/GPU/Vulkan/GPU_Vulkan.cpp index 0e55bbd84e..5acdc68a43 100644 --- a/GPU/Vulkan/GPU_Vulkan.cpp +++ b/GPU/Vulkan/GPU_Vulkan.cpp @@ -17,6 +17,7 @@ // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. #include + #include "base/logging.h" #include "base/timeutil.h" #include "profiler/profiler.h" @@ -197,12 +198,15 @@ void GPU_Vulkan::CheckGPUFeatures() { features |= GPU_SUPPORTS_ACCURATE_DEPTH; break; case VULKAN_VENDOR_ARM: - // Also required on older ARM Mali drivers, like the one on many Galaxy S7. - if (!PSP_CoreParameter().compat.flags().DisableAccurateDepth || - vulkan_->GetPhysicalDeviceProperties().properties.driverVersion <= VK_MAKE_VERSION(428, 811, 2674)) { + { + // This check is probably not exactly accurate. But old drivers had problems with reverse-Z, just like AMD and Qualcomm. + bool driverTooOld = IsHashMaliDriverVersion(vulkan_->GetPhysicalDeviceProperties().properties) + || VK_VERSION_MAJOR(vulkan_->GetPhysicalDeviceProperties().properties.driverVersion) < 14; + if (!PSP_CoreParameter().compat.flags().DisableAccurateDepth || driverTooOld) { features |= GPU_SUPPORTS_ACCURATE_DEPTH; } break; + } default: if (!PSP_CoreParameter().compat.flags().DisableAccurateDepth) features |= GPU_SUPPORTS_ACCURATE_DEPTH; diff --git a/ext/native/thin3d/thin3d_vulkan.cpp b/ext/native/thin3d/thin3d_vulkan.cpp index 960f224180..dafd15808d 100644 --- a/ext/native/thin3d/thin3d_vulkan.cpp +++ b/ext/native/thin3d/thin3d_vulkan.cpp @@ -429,30 +429,6 @@ public: void FlushState() override {} - // From Sascha's code - static std::string FormatDriverVersion(const VkPhysicalDeviceProperties &props) { - if (props.vendorID == VULKAN_VENDOR_NVIDIA) { - // 10 bits = major version (up to r1023) - // 8 bits = minor version (up to 255) - // 8 bits = secondary branch version/build version (up to 255) - // 6 bits = tertiary branch/build version (up to 63) - uint32_t major = (props.driverVersion >> 22) & 0x3ff; - uint32_t minor = (props.driverVersion >> 14) & 0x0ff; - uint32_t secondaryBranch = (props.driverVersion >> 6) & 0x0ff; - uint32_t tertiaryBranch = (props.driverVersion) & 0x003f; - return StringFromFormat("%d.%d.%d.%d (%08x)", major, minor, secondaryBranch, tertiaryBranch, props.driverVersion); - } else if (props.vendorID == VULKAN_VENDOR_ARM) { - // ARM just puts a hash here, let's just output it as is. - return StringFromFormat("%08x", props.driverVersion); - } else { - // Standard scheme, use the standard macros. - uint32_t major = VK_VERSION_MAJOR(props.driverVersion); - uint32_t minor = VK_VERSION_MINOR(props.driverVersion); - uint32_t branch = VK_VERSION_PATCH(props.driverVersion); - return StringFromFormat("%d.%d.%d (%08x)", major, minor, branch, props.driverVersion); - } - } - std::string GetInfoString(InfoField info) const override { // TODO: Make these actually query the right information switch (info) { From 3e43c4f008f900e6fc7e3f979f025fb1085a43b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 8 Sep 2019 21:49:10 +0200 Subject: [PATCH 3/7] Update lang submodule --- assets/lang | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/lang b/assets/lang index 7f098771e0..8ded8b378c 160000 --- a/assets/lang +++ b/assets/lang @@ -1 +1 @@ -Subproject commit 7f098771e00ce8fca6d116c973c711275deec68c +Subproject commit 8ded8b378ca86ce9509a9bdfb3ab19290a980048 From 0462c01228bc9a1f547ce6d6c97aabf62767e5eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 4 Sep 2019 23:20:57 +0200 Subject: [PATCH 4/7] Workaround ARM Mali depth hardware bug. Fixes #11937 When triangles coincide with the Z=1 plane in specific ways, triggered by Burnout Legends' sky for example, the depth buffer gets corrupted. This is worked around here by slightly rescaling Z. This type of workaround is recommended by ARM driver engineers. Ugly but what can you do when the hardware is bugged. I've done quick tests on a number of games with no issues. --- Common/Vulkan/VulkanContext.h | 3 +++ GPU/GPUState.h | 1 + GPU/Vulkan/DrawEngineVulkan.cpp | 6 +++++- GPU/Vulkan/GPU_Vulkan.cpp | 7 ++++++- GPU/Vulkan/VertexShaderGeneratorVulkan.cpp | 7 +++++-- ext/native/thin3d/VulkanRenderManager.h | 1 + 6 files changed, 21 insertions(+), 4 deletions(-) diff --git a/Common/Vulkan/VulkanContext.h b/Common/Vulkan/VulkanContext.h index 63fa5108e7..07388ce0f7 100644 --- a/Common/Vulkan/VulkanContext.h +++ b/Common/Vulkan/VulkanContext.h @@ -363,3 +363,6 @@ std::string FormatDriverVersion(const VkPhysicalDeviceProperties &props); // Simple heuristic. bool IsHashMaliDriverVersion(const VkPhysicalDeviceProperties &props); + +// For the ARM Mali depth scale hack. +#define DEPTH_SCALE_HACK_VALUE 0.9999f diff --git a/GPU/GPUState.h b/GPU/GPUState.h index c8481301ba..8afa4f4ef2 100644 --- a/GPU/GPUState.h +++ b/GPU/GPUState.h @@ -495,6 +495,7 @@ enum { GPU_SUPPORTS_ARB_FRAMEBUFFER_BLIT = FLAG_BIT(26), GPU_SUPPORTS_NV_FRAMEBUFFER_BLIT = FLAG_BIT(27), GPU_SUPPORTS_OES_TEXTURE_NPOT = FLAG_BIT(28), + GPU_NEEDS_DEPTH_SCALE_HACK = FLAG_BIT(29), GPU_PREFER_CPU_DOWNLOAD = FLAG_BIT(30), GPU_PREFER_REVERSE_COLOR_ORDER = FLAG_BIT(31), }; diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index 82c2656c84..3ce24f2696 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -962,6 +962,10 @@ void DrawEngineVulkan::DoFlush() { // We let the framebuffer manager handle the clear. It can use renderpasses to optimize on tilers. // If non-buffered though, it'll just do a plain clear. + float depth = result.depth; + if (gstate_c.Supports(GPU_NEEDS_DEPTH_SCALE_HACK)) { + depth *= DEPTH_SCALE_HACK_VALUE; + } framebufferManager_->NotifyClear(gstate.isClearModeColorMask(), gstate.isClearModeAlphaMask(), gstate.isClearModeDepthMask(), result.color, result.depth); int scissorX1 = gstate.getScissorX1(); @@ -970,7 +974,7 @@ void DrawEngineVulkan::DoFlush() { int scissorY2 = gstate.getScissorY2() + 1; framebufferManager_->SetSafeSize(scissorX2, scissorY2); - if ((gstate_c.featureFlags & GPU_USE_CLEAR_RAM_HACK) && gstate.isClearModeColorMask() && (gstate.isClearModeAlphaMask() || gstate.FrameBufFormat() == GE_FORMAT_565)) { + if (gstate_c.Supports(GPU_USE_CLEAR_RAM_HACK) && gstate.isClearModeColorMask() && (gstate.isClearModeAlphaMask() || gstate.FrameBufFormat() == GE_FORMAT_565)) { framebufferManager_->ApplyClearToMemory(scissorX1, scissorY1, scissorX2, scissorY2, result.color); } } diff --git a/GPU/Vulkan/GPU_Vulkan.cpp b/GPU/Vulkan/GPU_Vulkan.cpp index 5acdc68a43..cdb88d7894 100644 --- a/GPU/Vulkan/GPU_Vulkan.cpp +++ b/GPU/Vulkan/GPU_Vulkan.cpp @@ -205,11 +205,16 @@ void GPU_Vulkan::CheckGPUFeatures() { if (!PSP_CoreParameter().compat.flags().DisableAccurateDepth || driverTooOld) { features |= GPU_SUPPORTS_ACCURATE_DEPTH; } + // These GPUs (up to some certain hardware version?) has a bug where draws that touch the upper range of Z + // depth test incorrectly. This is easily worked around by simply scaling Z down a tiny bit, with a small risk + // for artifacts - haven't seen any though. + features |= GPU_NEEDS_DEPTH_SCALE_HACK; break; } default: - if (!PSP_CoreParameter().compat.flags().DisableAccurateDepth) + if (!PSP_CoreParameter().compat.flags().DisableAccurateDepth) { features |= GPU_SUPPORTS_ACCURATE_DEPTH; + } break; } diff --git a/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp b/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp index 88382f3b81..ad80589839 100644 --- a/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp +++ b/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp @@ -175,8 +175,7 @@ bool GenerateVulkanGLSLVertexShader(const VShaderID &id, char *buffer) { if (!useHWTransform && doTextureTransform && !isModeThrough) { WRITE(p, "layout (location = %d) in vec3 texcoord;\n", (int)PspAttributeLocation::TEXCOORD); texcoordInVec3 = true; - } - else + } else WRITE(p, "layout (location = %d) in vec2 texcoord;\n", (int)PspAttributeLocation::TEXCOORD); } if (hasColor) { @@ -637,6 +636,10 @@ bool GenerateVulkanGLSLVertexShader(const VShaderID &id, char *buffer) { } WRITE(p, " gl_Position = outPos;\n"); + if (gstate_c.Supports(GPU_NEEDS_DEPTH_SCALE_HACK)) { + WRITE(p, " gl_Position.z *= %f;\n", DEPTH_SCALE_HACK_VALUE); + } + WRITE(p, "}\n"); return true; } diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index ee8a15ee56..56a3fa1cf3 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -134,6 +134,7 @@ public: // TODO: This should be fixed at the source. data.viewport.vp.maxDepth = clamp_value(vp.maxDepth, 0.0f, 1.0f); data.viewport.vp.minDepth = clamp_value(vp.minDepth, 0.0f, 1.0f); + curRenderStep_->commands.push_back(data); } From f76adfd760403ce1a9390135c4e61c295c6b6e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 9 Sep 2019 00:09:57 +0200 Subject: [PATCH 5/7] Vulkan ARM mali Z hack: Modify the matrix instead of the shader. --- Common/Vulkan/VulkanContext.h | 3 --- GPU/Common/ShaderUniforms.cpp | 5 +++++ GPU/GPUState.h | 3 +++ GPU/Vulkan/VertexShaderGeneratorVulkan.cpp | 4 ---- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Common/Vulkan/VulkanContext.h b/Common/Vulkan/VulkanContext.h index 07388ce0f7..63fa5108e7 100644 --- a/Common/Vulkan/VulkanContext.h +++ b/Common/Vulkan/VulkanContext.h @@ -363,6 +363,3 @@ std::string FormatDriverVersion(const VkPhysicalDeviceProperties &props); // Simple heuristic. bool IsHashMaliDriverVersion(const VkPhysicalDeviceProperties &props); - -// For the ARM Mali depth scale hack. -#define DEPTH_SCALE_HACK_VALUE 0.9999f diff --git a/GPU/Common/ShaderUniforms.cpp b/GPU/Common/ShaderUniforms.cpp index 46ad4470e2..ca9320b749 100644 --- a/GPU/Common/ShaderUniforms.cpp +++ b/GPU/Common/ShaderUniforms.cpp @@ -151,6 +151,10 @@ void BaseUpdateUniforms(UB_VS_FS_Base *ub, uint64_t dirtyUniforms, bool flipView flippedMatrix = flippedMatrix * g_display_rot_matrix; } + if (gstate_c.Supports(GPU_NEEDS_DEPTH_SCALE_HACK)) { + flippedMatrix[10] *= DEPTH_SCALE_HACK_VALUE; + } + CopyMatrix4x4(ub->proj, flippedMatrix.getReadPtr()); } @@ -164,6 +168,7 @@ void BaseUpdateUniforms(UB_VS_FS_Base *ub, uint64_t dirtyUniforms, bool flipView if (g_Config.iRenderingMode == FB_NON_BUFFERED_MODE && g_display_rotation != DisplayRotation::ROTATE_0) { proj_through = proj_through * g_display_rot_matrix; } + proj_through[10] *= 0.999f; CopyMatrix4x4(ub->proj_through, proj_through.getReadPtr()); } diff --git a/GPU/GPUState.h b/GPU/GPUState.h index 8afa4f4ef2..19901ca437 100644 --- a/GPU/GPUState.h +++ b/GPU/GPUState.h @@ -500,6 +500,9 @@ enum { GPU_PREFER_REVERSE_COLOR_ORDER = FLAG_BIT(31), }; +// For the ARM Mali depth scale hack. +#define DEPTH_SCALE_HACK_VALUE 0.9999f + struct KnownVertexBounds { u16 minU; u16 minV; diff --git a/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp b/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp index ad80589839..c079285381 100644 --- a/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp +++ b/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp @@ -636,10 +636,6 @@ bool GenerateVulkanGLSLVertexShader(const VShaderID &id, char *buffer) { } WRITE(p, " gl_Position = outPos;\n"); - if (gstate_c.Supports(GPU_NEEDS_DEPTH_SCALE_HACK)) { - WRITE(p, " gl_Position.z *= %f;\n", DEPTH_SCALE_HACK_VALUE); - } - WRITE(p, "}\n"); return true; } From 70ec327b40fd61f6edfb57a44c351d067cb511a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 9 Sep 2019 00:33:04 +0200 Subject: [PATCH 6/7] Vulkan: Slim down and rename the Mali hack. --- GPU/Common/ShaderUniforms.cpp | 6 ------ GPU/GPUState.h | 5 +---- GPU/Vulkan/DrawEngineVulkan.cpp | 5 +---- GPU/Vulkan/GPU_Vulkan.cpp | 8 ++++---- GPU/Vulkan/VertexShaderGeneratorVulkan.cpp | 4 ++++ 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/GPU/Common/ShaderUniforms.cpp b/GPU/Common/ShaderUniforms.cpp index ca9320b749..ee90163c7d 100644 --- a/GPU/Common/ShaderUniforms.cpp +++ b/GPU/Common/ShaderUniforms.cpp @@ -150,11 +150,6 @@ void BaseUpdateUniforms(UB_VS_FS_Base *ub, uint64_t dirtyUniforms, bool flipView if (g_Config.iRenderingMode == FB_NON_BUFFERED_MODE && g_display_rotation != DisplayRotation::ROTATE_0) { flippedMatrix = flippedMatrix * g_display_rot_matrix; } - - if (gstate_c.Supports(GPU_NEEDS_DEPTH_SCALE_HACK)) { - flippedMatrix[10] *= DEPTH_SCALE_HACK_VALUE; - } - CopyMatrix4x4(ub->proj, flippedMatrix.getReadPtr()); } @@ -168,7 +163,6 @@ void BaseUpdateUniforms(UB_VS_FS_Base *ub, uint64_t dirtyUniforms, bool flipView if (g_Config.iRenderingMode == FB_NON_BUFFERED_MODE && g_display_rotation != DisplayRotation::ROTATE_0) { proj_through = proj_through * g_display_rot_matrix; } - proj_through[10] *= 0.999f; CopyMatrix4x4(ub->proj_through, proj_through.getReadPtr()); } diff --git a/GPU/GPUState.h b/GPU/GPUState.h index 19901ca437..2f33c3fb91 100644 --- a/GPU/GPUState.h +++ b/GPU/GPUState.h @@ -495,14 +495,11 @@ enum { GPU_SUPPORTS_ARB_FRAMEBUFFER_BLIT = FLAG_BIT(26), GPU_SUPPORTS_NV_FRAMEBUFFER_BLIT = FLAG_BIT(27), GPU_SUPPORTS_OES_TEXTURE_NPOT = FLAG_BIT(28), - GPU_NEEDS_DEPTH_SCALE_HACK = FLAG_BIT(29), + GPU_NEEDS_Z_EQUAL_W_HACK = FLAG_BIT(29), GPU_PREFER_CPU_DOWNLOAD = FLAG_BIT(30), GPU_PREFER_REVERSE_COLOR_ORDER = FLAG_BIT(31), }; -// For the ARM Mali depth scale hack. -#define DEPTH_SCALE_HACK_VALUE 0.9999f - struct KnownVertexBounds { u16 minU; u16 minV; diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index 3ce24f2696..ced84746e6 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -962,10 +962,7 @@ void DrawEngineVulkan::DoFlush() { // We let the framebuffer manager handle the clear. It can use renderpasses to optimize on tilers. // If non-buffered though, it'll just do a plain clear. - float depth = result.depth; - if (gstate_c.Supports(GPU_NEEDS_DEPTH_SCALE_HACK)) { - depth *= DEPTH_SCALE_HACK_VALUE; - } + // TODO: Don't we need to rescale result.depth if we use GPU_SCALE_DEPTH_FROM_24_TO_16_BIT and similar? framebufferManager_->NotifyClear(gstate.isClearModeColorMask(), gstate.isClearModeAlphaMask(), gstate.isClearModeDepthMask(), result.color, result.depth); int scissorX1 = gstate.getScissorX1(); diff --git a/GPU/Vulkan/GPU_Vulkan.cpp b/GPU/Vulkan/GPU_Vulkan.cpp index cdb88d7894..a1e6152503 100644 --- a/GPU/Vulkan/GPU_Vulkan.cpp +++ b/GPU/Vulkan/GPU_Vulkan.cpp @@ -205,10 +205,10 @@ void GPU_Vulkan::CheckGPUFeatures() { if (!PSP_CoreParameter().compat.flags().DisableAccurateDepth || driverTooOld) { features |= GPU_SUPPORTS_ACCURATE_DEPTH; } - // These GPUs (up to some certain hardware version?) has a bug where draws that touch the upper range of Z - // depth test incorrectly. This is easily worked around by simply scaling Z down a tiny bit, with a small risk - // for artifacts - haven't seen any though. - features |= GPU_NEEDS_DEPTH_SCALE_HACK; + // These GPUs (up to some certain hardware version?) has a bug where draws where gl_Position.w == .z + // corrupt the depth buffer. This is easily worked around by simply scaling Z down a tiny bit when this case + // is detected. See: https://github.com/hrydgard/ppsspp/issues/11937 + features |= GPU_NEEDS_Z_EQUAL_W_HACK; break; } default: diff --git a/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp b/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp index c079285381..265bcbdd0a 100644 --- a/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp +++ b/GPU/Vulkan/VertexShaderGeneratorVulkan.cpp @@ -636,6 +636,10 @@ bool GenerateVulkanGLSLVertexShader(const VShaderID &id, char *buffer) { } WRITE(p, " gl_Position = outPos;\n"); + if (gstate_c.Supports(GPU_NEEDS_Z_EQUAL_W_HACK)) { + // See comment in GPU_Vulkan.cpp. + WRITE(p, " if (gl_Position.z == gl_Position.w) gl_Position.z *= 0.999999;\n"); + } WRITE(p, "}\n"); return true; } From 02a96e29bbc6d9c6b2c8077c3ac353b138060410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 9 Sep 2019 00:52:23 +0200 Subject: [PATCH 7/7] Fix a comment --- GPU/Vulkan/DrawEngineVulkan.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index ced84746e6..af4ee397a5 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -962,7 +962,6 @@ void DrawEngineVulkan::DoFlush() { // We let the framebuffer manager handle the clear. It can use renderpasses to optimize on tilers. // If non-buffered though, it'll just do a plain clear. - // TODO: Don't we need to rescale result.depth if we use GPU_SCALE_DEPTH_FROM_24_TO_16_BIT and similar? framebufferManager_->NotifyClear(gstate.isClearModeColorMask(), gstate.isClearModeAlphaMask(), gstate.isClearModeDepthMask(), result.color, result.depth); int scissorX1 = gstate.getScissorX1();