From b9443e54e3e8268b11f040b6b279631d0914c191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Fri, 3 Nov 2017 00:03:35 +0100 Subject: [PATCH 01/38] Vulkan: Minor fixes. --- GPU/Common/FramebufferCommon.cpp | 3 +++ ext/native/thin3d/VulkanQueueRunner.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index a684bd68f9..7e1f9e9ca0 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -1914,6 +1914,9 @@ bool FramebufferManagerCommon::GetFramebuffer(u32 fb_address, int fb_stride, GEB bool flipY = (g_Config.iGPUBackend == GPU_BACKEND_OPENGL && !useBufferedRendering_) ? true : false; buffer.Allocate(w, h, GE_FORMAT_8888, flipY, true); bool retval = draw_->CopyFramebufferToMemorySync(bound, Draw::FB_COLOR_BIT, 0, 0, w, h, Draw::DataFormat::R8G8B8A8_UNORM, buffer.GetData(), w); + // Vulkan requires us to re-apply all dynamic state for each command buffer, and the above will cause us to start a new cmdbuf. + // So let's dirty the things that are involved in Vulkan dynamic state. Readbacks are not frequent so this won't hurt other backends. + gstate_c.Dirty(DIRTY_VIEWPORTSCISSOR_STATE | DIRTY_BLEND_STATE | DIRTY_DEPTHSTENCIL_STATE); // We may have blitted to a temp FBO. RebindFramebuffer(); return retval; diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index 83511ecca9..f40daf80a6 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -814,8 +814,10 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd VkImageMemoryBarrier barrier{ VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER }; VkPipelineStageFlags stage = 0; - SetupTransitionToTransferSrc(*srcImage, barrier, stage, step.readback.aspectMask); - vkCmdPipelineBarrier(cmd, stage, VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier); + if (srcImage->layout != VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL) { + SetupTransitionToTransferSrc(*srcImage, barrier, stage, step.readback.aspectMask); + vkCmdPipelineBarrier(cmd, stage, VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier); + } VkBufferImageCopy region{}; region.imageOffset = { step.readback.srcRect.offset.x, step.readback.srcRect.offset.y, 0 }; From d5f8d484dd3f9e0d7da5220553f1ae26f2c2110c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 4 Nov 2017 10:08:53 +0100 Subject: [PATCH 02/38] Tiny cleanups. --- GPU/Vulkan/GPU_Vulkan.cpp | 1 - GPU/Vulkan/ShaderManagerVulkan.cpp | 2 -- GPU/Vulkan/ShaderManagerVulkan.h | 2 -- ext/native/thin3d/VulkanQueueRunner.cpp | 1 + 4 files changed, 1 insertion(+), 5 deletions(-) diff --git a/GPU/Vulkan/GPU_Vulkan.cpp b/GPU/Vulkan/GPU_Vulkan.cpp index 5fdf6d7add..cfaae884f3 100644 --- a/GPU/Vulkan/GPU_Vulkan.cpp +++ b/GPU/Vulkan/GPU_Vulkan.cpp @@ -173,7 +173,6 @@ GPU_Vulkan::~GPU_Vulkan() { void GPU_Vulkan::CheckGPUFeatures() { uint32_t features = 0; - features |= GPU_SUPPORTS_16BIT_FORMATS; if (vulkan_->GetFeaturesEnabled().wideLines) { features |= GPU_SUPPORTS_WIDE_LINES; } diff --git a/GPU/Vulkan/ShaderManagerVulkan.cpp b/GPU/Vulkan/ShaderManagerVulkan.cpp index c5a70db8f4..9c657d9099 100644 --- a/GPU/Vulkan/ShaderManagerVulkan.cpp +++ b/GPU/Vulkan/ShaderManagerVulkan.cpp @@ -19,8 +19,6 @@ //#define SHADERLOG #endif -#include - #include "base/logging.h" #include "math/lin/matrix4x4.h" #include "math/math_util.h" diff --git a/GPU/Vulkan/ShaderManagerVulkan.h b/GPU/Vulkan/ShaderManagerVulkan.h index 0135b76141..c1157784a0 100644 --- a/GPU/Vulkan/ShaderManagerVulkan.h +++ b/GPU/Vulkan/ShaderManagerVulkan.h @@ -17,8 +17,6 @@ #pragma once -#include - #include "base/basictypes.h" #include "Common/Hashmaps.h" #include "Common/Vulkan/VulkanMemory.h" diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index f40daf80a6..e1969f69fa 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -187,6 +187,7 @@ void VulkanQueueRunner::InitRenderpasses() { } } } + void VulkanQueueRunner::RunSteps(VkCommandBuffer cmd, const std::vector &steps) { // Optimizes renderpasses, then sequences them. for (int i = 0; i < steps.size(); i++) { From 9e65ae1cef8a76e394ac3aa595b3919ca891379f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 4 Nov 2017 11:41:44 +0100 Subject: [PATCH 03/38] Fix issue in framebuffer size calculation when viewport is bogus in through mode. Fixes #10043. --- GPU/Common/FramebufferCommon.cpp | 26 +++++++++++++++++------ GPU/Common/FramebufferCommon.h | 2 +- ext/native/thin3d/VulkanRenderManager.cpp | 2 ++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index 7e1f9e9ca0..f97b2117a3 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -17,6 +17,7 @@ #include #include +#include #include "ext/native/thin3d/thin3d.h" #include "base/timeutil.h" @@ -213,7 +214,7 @@ void FramebufferManagerCommon::EstimateDrawingSize(u32 fb_address, GEBufferForma static const int MAX_FRAMEBUF_HEIGHT = 512; // Games don't always set any of these. Take the greatest parameter that looks valid based on stride. - if (viewport_width > 4 && viewport_width <= fb_stride) { + if (viewport_width > 4 && viewport_width <= fb_stride && viewport_height > 0) { drawing_width = viewport_width; drawing_height = viewport_height; // Some games specify a viewport with 0.5, but don't have VRAM for 273. 480x272 is the buffer size. @@ -300,8 +301,19 @@ void GetFramebufferHeuristicInputs(FramebufferHeuristicParams *params, const GPU params->isModeThrough = gstate.isModeThrough(); // Viewport-X1 and Y1 are not the upper left corner, but half the width/height. A bit confusing. - params->viewportWidth = (int)(fabsf(gstate.getViewportXScale()*2.0f)); - params->viewportHeight = (int)(fabsf(gstate.getViewportYScale()*2.0f)); + float vpx = gstate.getViewportXScale(); + float vpy = gstate.getViewportYScale(); + + // Work around problem in F1 Grand Prix, where it draws in through mode with a bogus viewport. + // We set bad values to 0 which causes the framebuffer size heuristic to rely on the other parameters instead. + if (isnan(vpx) || vpx > 10000000.0f) { + vpx = 0.f; + } + if (isnan(vpy) || vpy > 10000000.0f) { + vpy = 0.f; + } + params->viewportWidth = (int)(fabsf(vpx) * 2.0f); + params->viewportHeight = (int)(fabsf(vpy) * 2.0f); params->regionWidth = gstate.getRegionX2() + 1; params->regionHeight = gstate.getRegionY2() + 1; params->scissorWidth = gstate.getScissorX2() + 1; @@ -1108,7 +1120,9 @@ void FramebufferManagerCommon::DecimateFBOs() { } } -void FramebufferManagerCommon::ResizeFramebufFBO(VirtualFramebuffer *vfb, u16 w, u16 h, bool force, bool skipCopy) { +void FramebufferManagerCommon::ResizeFramebufFBO(VirtualFramebuffer *vfb, int w, int h, bool force, bool skipCopy) { + assert(w > 0); + assert(h > 0); VirtualFramebuffer old = *vfb; int oldWidth = vfb->bufferWidth; @@ -1123,8 +1137,8 @@ void FramebufferManagerCommon::ResizeFramebufFBO(VirtualFramebuffer *vfb, u16 w, } // In case it gets thin and wide, don't resize down either side. - vfb->bufferWidth = std::max(vfb->bufferWidth, w); - vfb->bufferHeight = std::max(vfb->bufferHeight, h); + vfb->bufferWidth = std::max((int)vfb->bufferWidth, w); + vfb->bufferHeight = std::max((int)vfb->bufferHeight, h); } SetRenderSize(vfb); diff --git a/GPU/Common/FramebufferCommon.h b/GPU/Common/FramebufferCommon.h index 62befe6913..75224a0cc5 100644 --- a/GPU/Common/FramebufferCommon.h +++ b/GPU/Common/FramebufferCommon.h @@ -338,7 +338,7 @@ protected: virtual void ReformatFramebufferFrom(VirtualFramebuffer *vfb, GEBufferFormat old) = 0; virtual void BlitFramebufferDepth(VirtualFramebuffer *src, VirtualFramebuffer *dst) = 0; - void ResizeFramebufFBO(VirtualFramebuffer *vfb, u16 w, u16 h, bool force = false, bool skipCopy = false); + void ResizeFramebufFBO(VirtualFramebuffer *vfb, int w, int h, bool force = false, bool skipCopy = false); void ShowScreenResolution(); bool ShouldDownloadFramebuffer(const VirtualFramebuffer *vfb) const; diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 1d67e3fe49..bd9692c55e 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -318,7 +318,9 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR curRenderStep_ = nullptr; } if (curRenderStep_ && curRenderStep_->commands.size() == 0) { +#ifdef _DEBUG ILOG("Empty render step. Usually happens after uploading pixels.."); +#endif } VKRStep *step = new VKRStep{ VKRStepType::RENDER }; From 8edc6eaf5efa62f365445a0ac51aac58686859b5 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 20:44:11 -0700 Subject: [PATCH 04/38] Vulkan: Fix segfault on swapchain fail. This at least allows us to detect that the backend failed to init. Happens when switching backends with debugger attached (probably driver bug?) --- Common/Vulkan/VulkanContext.cpp | 1 + Common/Vulkan/VulkanContext.h | 2 +- Windows/GPU/WindowsVulkanContext.cpp | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Common/Vulkan/VulkanContext.cpp b/Common/Vulkan/VulkanContext.cpp index 3fcfcc49cb..de0000c382 100644 --- a/Common/Vulkan/VulkanContext.cpp +++ b/Common/Vulkan/VulkanContext.cpp @@ -137,6 +137,7 @@ VkResult VulkanContext::CreateInstance(const char *app_name, int app_ver, uint32 if (res != VK_SUCCESS) { init_error_ = "Failed to enumerate physical devices"; vkDestroyInstance(instance_, nullptr); + instance_ = nullptr; return res; } diff --git a/Common/Vulkan/VulkanContext.h b/Common/Vulkan/VulkanContext.h index f647f4d0f2..64b17f229e 100644 --- a/Common/Vulkan/VulkanContext.h +++ b/Common/Vulkan/VulkanContext.h @@ -378,7 +378,7 @@ private: std::vector msg_callbacks; - VkSwapchainKHR swapchain_; + VkSwapchainKHR swapchain_ = VK_NULL_HANDLE; VkFormat swapchainFormat_; uint32_t queue_count = 0; diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp index 21dcb026ea..f8263b565e 100644 --- a/Windows/GPU/WindowsVulkanContext.cpp +++ b/Windows/GPU/WindowsVulkanContext.cpp @@ -208,7 +208,8 @@ bool WindowsVulkanContext::Init(HINSTANCE hInst, HWND hWnd, std::string *error_m } void WindowsVulkanContext::Shutdown() { - draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight()); + if (draw_) + draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight()); delete draw_; draw_ = nullptr; From ec0f640f569a2cc6367403f8bfde7f5e5782253b Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 21:04:07 -0700 Subject: [PATCH 05/38] Vulkan: Trigger condvar only at frame end. --- ext/native/thin3d/VulkanRenderManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index bd9692c55e..4e54b21de4 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -640,7 +640,8 @@ void VulkanRenderManager::Submit(int frame, bool triggerFence) { assert(res == VK_SUCCESS); } - if (useThread) { + // TODO: If !triggerFence, we're actually not using the thread right now. + if (useThread && triggerFence) { VLOG("PULL: Frame %d.readyForFence = true", frame); std::unique_lock lock(frameData.push_mutex); frameData.readyForFence = true; From 8b55940a3de0f5a0dcdc32bbd1a0596f59353d79 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 22:21:47 -0700 Subject: [PATCH 06/38] Vulkan: Fix out-of-sync frames on threading. We end up with a second thread start at frame 1, so the thread needs to start at frame 1 too. --- ext/native/thin3d/VulkanRenderManager.cpp | 23 +++++++++++++++++++---- ext/native/thin3d/VulkanRenderManager.h | 1 + 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 4e54b21de4..c2fbf46f93 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -179,7 +179,15 @@ void VulkanRenderManager::CreateBackbuffers() { // Start the thread. if (useThread) { + for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { + // Reset all the frameData. Might be dirty from previous thread stop. + frameData_[i].readyForRun = false; + frameData_[i].readyForFence = true; + } + run_ = true; + // Won't necessarily be 0. + threadInitFrame_ = vulkan_->GetCurFrame(); thread_ = std::thread(&VulkanRenderManager::ThreadFunc, this); } } @@ -236,12 +244,15 @@ VulkanRenderManager::~VulkanRenderManager() { // TODO: Activate this code. void VulkanRenderManager::ThreadFunc() { setCurrentThreadName("RenderMan"); - int threadFrame = -1; // Increment first, start at 0. + int threadFrame = threadInitFrame_; + bool nextFrame = false; while (run_) { { - threadFrame++; - if (threadFrame >= vulkan_->GetInflightFrames()) - threadFrame = 0; + if (nextFrame) { + threadFrame++; + if (threadFrame >= vulkan_->GetInflightFrames()) + threadFrame = 0; + } FrameData &frameData = frameData_[threadFrame]; std::unique_lock lock(frameData.pull_mutex); while (!frameData.readyForRun && run_) { @@ -252,10 +263,14 @@ void VulkanRenderManager::ThreadFunc() { frameData.readyForRun = false; if (!run_) // quick exit if bailing. return; + + nextFrame = true; } VLOG("PULL: Running frame %d", threadFrame); Run(threadFrame); + VLOG("PULL: Finished frame %d", threadFrame); } + VLOG("PULL: Quitting"); } void VulkanRenderManager::BeginFrame() { diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 44d34c6cc1..53a90a1bcc 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -249,6 +249,7 @@ private: VulkanContext *vulkan_; std::thread thread_; std::mutex mutex_; + int threadInitFrame_ = 0; VulkanQueueRunner queueRunner_; // Swap chain management From 97fa0a7461eff5cea48748bf3b056db5e7245435 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 22:23:01 -0700 Subject: [PATCH 07/38] Vulkan: Allow sync when using threading. --- ext/native/thin3d/VulkanRenderManager.cpp | 68 ++++++++++++++++++----- ext/native/thin3d/VulkanRenderManager.h | 9 ++- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index c2fbf46f93..e701d12f12 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -264,7 +264,9 @@ void VulkanRenderManager::ThreadFunc() { if (!run_) // quick exit if bailing. return; - nextFrame = true; + // Only increment next time if we're done. + nextFrame = frameData.type == VKRRunType::END; + assert(frameData.type == VKRRunType::END || frameData.type == VKRRunType::SYNC); } VLOG("PULL: Running frame %d", threadFrame); Run(threadFrame); @@ -560,12 +562,14 @@ void VulkanRenderManager::Finish() { FrameData &frameData = frameData_[curFrame]; if (!useThread) { frameData.steps = std::move(steps_); + frameData.type = VKRRunType::END; Run(curFrame); } else { std::unique_lock lock(frameData.pull_mutex); VLOG("PUSH: Frame[%d].readyForRun = true", curFrame); frameData.steps = std::move(steps_); frameData.readyForRun = true; + frameData.type = VKRRunType::END; frameData.pull_condVar.notify_all(); } vulkan_->EndFrame(); @@ -690,8 +694,6 @@ void VulkanRenderManager::EndSubmitFrame(int frame) { } void VulkanRenderManager::Run(int frame) { - VkDevice device = vulkan_->GetDevice(); - BeginSubmitFrame(frame); FrameData &frameData = frameData_[frame]; @@ -701,23 +703,24 @@ void VulkanRenderManager::Run(int frame) { queueRunner_.RunSteps(cmd, stepsOnThread); stepsOnThread.clear(); - EndSubmitFrame(frame); + switch (frameData.type) { + case VKRRunType::END: + EndSubmitFrame(frame); + break; + + case VKRRunType::SYNC: + EndSyncFrame(frame); + break; + + default: + assert(false); + } VLOG("PULL: Finished running frame %d", frame); } -void VulkanRenderManager::FlushSync() { - int frame = vulkan_->GetCurFrame(); - BeginSubmitFrame(frame); - +void VulkanRenderManager::EndSyncFrame(int frame) { FrameData &frameData = frameData_[frame]; - frameData.steps = std::move(steps_); - - auto &stepsOnThread = frameData_[frame].steps; - VkCommandBuffer cmd = frameData.mainCmd; - queueRunner_.RunSteps(cmd, stepsOnThread); - stepsOnThread.clear(); - Submit(frame, false); // This is brutal! Should probably wait for a fence instead, not that it'll matter much since we'll @@ -734,4 +737,39 @@ void VulkanRenderManager::FlushSync() { }; VkResult res = vkBeginCommandBuffer(frameData.mainCmd, &begin); assert(res == VK_SUCCESS); + + if (useThread) { + std::unique_lock lock(frameData.push_mutex); + frameData.readyForFence = true; + frameData.push_condVar.notify_all(); + } +} + +void VulkanRenderManager::FlushSync() { + // TODO: Reset curRenderStep_? + int curFrame = vulkan_->GetCurFrame(); + FrameData &frameData = frameData_[curFrame]; + if (!useThread) { + frameData.steps = std::move(steps_); + frameData.type = VKRRunType::SYNC; + Run(curFrame); + } else { + std::unique_lock lock(frameData.pull_mutex); + VLOG("PUSH: Frame[%d].readyForRun = true (sync)", curFrame); + frameData.steps = std::move(steps_); + frameData.readyForRun = true; + assert(frameData.readyForFence == false); + frameData.type = VKRRunType::SYNC; + frameData.pull_condVar.notify_all(); + } + + if (useThread) { + std::unique_lock lock(frameData.push_mutex); + // Wait for the flush to be hit, since we're syncing. + while (!frameData.readyForFence) { + VLOG("PUSH: Waiting for frame[%d].readyForFence = 1 (sync)", curFrame); + frameData.push_condVar.wait(lock); + } + frameData.readyForFence = false; + } } diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 53a90a1bcc..50fb2f56d8 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -70,6 +70,11 @@ private: VulkanContext *vulkan_; }; +enum class VKRRunType { + END, + SYNC, +}; + class VulkanRenderManager { public: VulkanRenderManager(VulkanContext *vulkan); @@ -206,6 +211,7 @@ private: // Bad for performance but sometimes necessary for synchronous CPU readbacks (screenshots and whatnot). void FlushSync(); + void EndSyncFrame(int frame); // Permanent objects VkSemaphore acquireSemaphore_; @@ -220,8 +226,9 @@ private: std::condition_variable pull_condVar; bool readyForFence = true; - bool readyForRun = false; + VKRRunType type = VKRRunType::END; + VkFence fence; // These are on different threads so need separate pools. VkCommandPool cmdPoolInit; From dc200a4fbcb276f2f4033339abeeb5a8f7cc2438 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 4 Nov 2017 22:29:12 -0700 Subject: [PATCH 08/38] Vulkan: Fix a comment. --- ext/native/thin3d/VulkanRenderManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index e701d12f12..038e85939d 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -659,7 +659,7 @@ void VulkanRenderManager::Submit(int frame, bool triggerFence) { assert(res == VK_SUCCESS); } - // TODO: If !triggerFence, we're actually not using the thread right now. + // When !triggerFence, we notify after syncing with Vulkan. if (useThread && triggerFence) { VLOG("PULL: Frame %d.readyForFence = true", frame); std::unique_lock lock(frameData.push_mutex); From b991dd5bd397643bc2fccc02e147c4373d943da6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 5 Nov 2017 10:01:03 +0100 Subject: [PATCH 09/38] Should be bufferWidth here, not width. Thanks unknown. Get rid of Source.cpp, oops. --- GPU/Common/FramebufferCommon.cpp | 6 +++--- GPU/GPU.vcxproj | 3 +-- GPU/GPU.vcxproj.filters | 5 +---- GPU/Source.cpp | 0 4 files changed, 5 insertions(+), 9 deletions(-) delete mode 100644 GPU/Source.cpp diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index f97b2117a3..677340c170 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -2033,9 +2033,9 @@ void FramebufferManagerCommon::PackFramebufferSync_(VirtualFramebuffer *vfb, int } void FramebufferManagerCommon::ReadFramebufferToMemory(VirtualFramebuffer *vfb, bool sync, int x, int y, int w, int h) { - // Clamp to width. Sometimes block transfers can cause this to hit. - if (x + w >= vfb->width) { - w = vfb->width - x; + // Clamp to bufferWidth. Sometimes block transfers can cause this to hit. + if (x + w >= vfb->bufferWidth) { + w = vfb->bufferWidth - x; } if (vfb) { // We'll pseudo-blit framebuffers here to get a resized version of vfb. diff --git a/GPU/GPU.vcxproj b/GPU/GPU.vcxproj index 9c68a6fee3..dc3d6f11d9 100644 --- a/GPU/GPU.vcxproj +++ b/GPU/GPU.vcxproj @@ -360,7 +360,6 @@ - @@ -386,4 +385,4 @@ - \ No newline at end of file + diff --git a/GPU/GPU.vcxproj.filters b/GPU/GPU.vcxproj.filters index 6e85d8e48f..8aa5382057 100644 --- a/GPU/GPU.vcxproj.filters +++ b/GPU/GPU.vcxproj.filters @@ -531,11 +531,8 @@ Vulkan - - Common - Common - \ No newline at end of file + diff --git a/GPU/Source.cpp b/GPU/Source.cpp deleted file mode 100644 index e69de29bb2..0000000000 From d670948ad2ba0f1b759db2292dd11e9305ec2822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 5 Nov 2017 10:08:41 +0100 Subject: [PATCH 10/38] Cleanups and fixes around Vulkan stencil upload. thx unknown --- GPU/D3D11/StencilBufferD3D11.cpp | 3 --- GPU/Vulkan/StencilBufferVulkan.cpp | 19 ++++++------------- GPU/Vulkan/VulkanUtil.cpp | 2 +- 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/GPU/D3D11/StencilBufferD3D11.cpp b/GPU/D3D11/StencilBufferD3D11.cpp index c8825d121b..9a8a44a888 100644 --- a/GPU/D3D11/StencilBufferD3D11.cpp +++ b/GPU/D3D11/StencilBufferD3D11.cpp @@ -28,9 +28,6 @@ #include "GPU/D3D11/TextureCacheD3D11.h" #include "GPU/D3D11/D3D11Util.h" -#define STR_HELPER(x) #x -#define STR(x) STR_HELPER(x) - struct StencilValueUB { uint32_t u_stencilValue[4]; }; diff --git a/GPU/Vulkan/StencilBufferVulkan.cpp b/GPU/Vulkan/StencilBufferVulkan.cpp index 2958ba6518..effbb8a779 100644 --- a/GPU/Vulkan/StencilBufferVulkan.cpp +++ b/GPU/Vulkan/StencilBufferVulkan.cpp @@ -27,29 +27,22 @@ #include "GPU/Vulkan/TextureCacheVulkan.h" #include "GPU/Vulkan/VulkanUtil.h" -#define STR_HELPER(x) #x -#define STR(x) STR_HELPER(x) - -struct StencilValueUB { - uint32_t u_stencilValue[4]; -}; - static const char *stencil_fs = R"(#version 400 #extension GL_ARB_separate_shader_objects : enable #extension GL_ARB_shading_language_420pack : enable layout (binding = 0) uniform sampler2D tex; layout(push_constant) uniform params { - int u_stencilValue; + int u_stencilValue; }; layout (location = 0) in vec2 v_texcoord0; layout (location = 0) out vec4 fragColor0; void main() { - vec4 index = texture(tex, v_texcoord0); + vec4 index = texture(tex, v_texcoord0); int indexBits = int(floor(index.a * 255.99)) & 0xFF; if ((indexBits & u_stencilValue) == 0) discard; - fragColor0 = index.aaaa; + fragColor0 = index.aaaa; } )"; @@ -60,9 +53,9 @@ layout (location = 0) out vec2 v_texcoord0; out gl_PerVertex { vec4 gl_Position; }; void main() { int id = gl_VertexIndex; - v_texcoord0.x = (id == 2) ? 2.0 : 0.0; - v_texcoord0.y = (id == 1) ? 2.0 : 0.0; - gl_Position = vec4(v_texcoord0 * vec2(2.0, 2.0) + vec2(-1.0, -1.0), 0.0, 1.0); + v_texcoord0.x = (id == 2) ? 2.0 : 0.0; + v_texcoord0.y = (id == 1) ? 2.0 : 0.0; + gl_Position = vec4(v_texcoord0 * vec2(2.0, 2.0) + vec2(-1.0, -1.0), 0.0, 1.0); } )"; diff --git a/GPU/Vulkan/VulkanUtil.cpp b/GPU/Vulkan/VulkanUtil.cpp index 14ba80dccb..01d001a0e3 100644 --- a/GPU/Vulkan/VulkanUtil.cpp +++ b/GPU/Vulkan/VulkanUtil.cpp @@ -210,7 +210,7 @@ VkPipeline Vulkan2D::GetPipeline(VkRenderPass rp, VkShaderModule vs, VkShaderMod VkPipelineColorBlendAttachmentState blend0 = {}; blend0.blendEnable = false; - blend0.colorWriteMask = depthStencilMode == VK2DDepthStencilMode::STENCIL_REPLACE_ALWAYS ? 0 : 0xF; + blend0.colorWriteMask = depthStencilMode == VK2DDepthStencilMode::STENCIL_REPLACE_ALWAYS ? VK_COLOR_COMPONENT_A_BIT : 0xF; VkPipelineColorBlendStateCreateInfo cbs = { VK_STRUCTURE_TYPE_PIPELINE_COLOR_BLEND_STATE_CREATE_INFO }; cbs.pAttachments = &blend0; From eb300d715c7651424be8776f301df59d12af5e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 5 Nov 2017 10:11:00 +0100 Subject: [PATCH 11/38] Add include. Will hopefully help #10046 --- GPU/Common/FramebufferCommon.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index 677340c170..e8667c00c6 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include "ext/native/thin3d/thin3d.h" #include "base/timeutil.h" From 066ad469153808d75bc59cfcde73d3c7acd3711c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 5 Nov 2017 10:20:18 +0100 Subject: [PATCH 12/38] Minor depal state dirtying fix --- GPU/Vulkan/DepalettizeShaderVulkan.h | 2 +- GPU/Vulkan/TextureCacheVulkan.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/GPU/Vulkan/DepalettizeShaderVulkan.h b/GPU/Vulkan/DepalettizeShaderVulkan.h index 4e6e119b84..068d2c5014 100644 --- a/GPU/Vulkan/DepalettizeShaderVulkan.h +++ b/GPU/Vulkan/DepalettizeShaderVulkan.h @@ -33,7 +33,7 @@ public: } // A Vulkan2D pipeline. Set texture to slot 0 and palette texture to slot 1. VkPipeline pipeline = VK_NULL_HANDLE; - const char *code = nullptr;; + const char *code = nullptr; }; class DepalTextureVulkan { diff --git a/GPU/Vulkan/TextureCacheVulkan.cpp b/GPU/Vulkan/TextureCacheVulkan.cpp index 9869c40a9e..b6e3119073 100644 --- a/GPU/Vulkan/TextureCacheVulkan.cpp +++ b/GPU/Vulkan/TextureCacheVulkan.cpp @@ -450,6 +450,7 @@ void TextureCacheVulkan::ApplyTextureFramebuffer(TexCacheEntry *entry, VirtualFr // Need to rebind the pipeline since we switched it. drawEngine_->DirtyPipeline(); + gstate_c.Dirty(DIRTY_VIEWPORTSCISSOR_STATE); } else { entry->status &= ~TexCacheEntry::STATUS_DEPALETTIZE; From 2b7d1c1ded9dde964d0e0cfdc9261b6025d2003b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 5 Nov 2017 10:37:01 +0100 Subject: [PATCH 13/38] Vulkan: Fix bug in depal causing Sonic Rivals to be grayscale. --- GPU/Vulkan/TextureCacheVulkan.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/GPU/Vulkan/TextureCacheVulkan.cpp b/GPU/Vulkan/TextureCacheVulkan.cpp index b6e3119073..8826278327 100644 --- a/GPU/Vulkan/TextureCacheVulkan.cpp +++ b/GPU/Vulkan/TextureCacheVulkan.cpp @@ -362,12 +362,13 @@ void TextureCacheVulkan::Unbind() { void TextureCacheVulkan::ApplyTextureFramebuffer(TexCacheEntry *entry, VirtualFramebuffer *framebuffer) { DepalShaderVulkan *depalShader = nullptr; - const GEPaletteFormat clutFormat = gstate.getClutPaletteFormat(); + uint32_t clutMode = gstate.clutformat & 0xFFFFFF; if ((entry->status & TexCacheEntry::STATUS_DEPALETTIZE) && !g_Config.bDisableSlowFramebufEffects) { - depalShader = depalShaderCache_->GetDepalettizeShader(clutFormat, framebuffer->drawnFormat); + depalShader = depalShaderCache_->GetDepalettizeShader(clutMode, framebuffer->drawnFormat); } if (depalShader) { depalShaderCache_->SetPushBuffer(drawEngine_->GetPushBufferForTextureData()); + const GEPaletteFormat clutFormat = gstate.getClutPaletteFormat(); VulkanTexture *clutTexture = depalShaderCache_->GetClutTexture(clutFormat, clutHash_, clutBuf_); Draw::Framebuffer *depalFBO = framebufferManager_->GetTempFBO( From 5d0bd85a709b24aafd492333d01f566fa4919e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 5 Nov 2017 10:40:21 +0100 Subject: [PATCH 14/38] Unify a little bit of depal code. --- GPU/Common/DepalettizeShaderCommon.cpp | 9 +++++++++ GPU/Common/DepalettizeShaderCommon.h | 10 ++++++++++ GPU/D3D11/DepalettizeShaderD3D11.cpp | 12 ++++-------- GPU/D3D11/DepalettizeShaderD3D11.h | 5 ++--- GPU/Directx9/DepalettizeShaderDX9.cpp | 12 ++++-------- GPU/Directx9/DepalettizeShaderDX9.h | 7 +++---- GPU/GLES/DepalettizeShaderGLES.cpp | 12 ++++-------- GPU/GLES/DepalettizeShaderGLES.h | 4 ++-- GPU/Vulkan/DepalettizeShaderVulkan.cpp | 15 +++++---------- GPU/Vulkan/DepalettizeShaderVulkan.h | 5 ++--- 10 files changed, 45 insertions(+), 46 deletions(-) diff --git a/GPU/Common/DepalettizeShaderCommon.cpp b/GPU/Common/DepalettizeShaderCommon.cpp index 340d26ded3..a230ce163c 100644 --- a/GPU/Common/DepalettizeShaderCommon.cpp +++ b/GPU/Common/DepalettizeShaderCommon.cpp @@ -288,4 +288,13 @@ void GenerateDepalShader(char *buffer, GEBufferFormat pixelFormat, ShaderLanguag } } +uint32_t DepalShaderCacheCommon::GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat) const { + return (clutMode & 0xFFFFFF) | (pixelFormat << 24); +} + +uint32_t DepalShaderCacheCommon::GetClutID(GEPaletteFormat clutFormat, uint32_t clutHash) const { + // Simplistic. + return clutHash ^ (uint32_t)clutFormat; +} + #undef WRITE diff --git a/GPU/Common/DepalettizeShaderCommon.h b/GPU/Common/DepalettizeShaderCommon.h index 933d4c02d9..3520fded93 100644 --- a/GPU/Common/DepalettizeShaderCommon.h +++ b/GPU/Common/DepalettizeShaderCommon.h @@ -17,9 +17,19 @@ #pragma once +#include #include "GPU/ge_constants.h" #include "GPU/Common/ShaderCommon.h" static const int DEPAL_TEXTURE_OLD_AGE = 120; void GenerateDepalShader(char *buffer, GEBufferFormat pixelFormat, ShaderLanguage language); + +class DepalShaderCacheCommon { +public: + virtual ~DepalShaderCacheCommon() {} + +protected: + uint32_t GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat) const; + uint32_t GetClutID(GEPaletteFormat clutFormat, uint32_t clutHash) const; +}; \ No newline at end of file diff --git a/GPU/D3D11/DepalettizeShaderD3D11.cpp b/GPU/D3D11/DepalettizeShaderD3D11.cpp index 1ab84dbd38..88520f404b 100644 --- a/GPU/D3D11/DepalettizeShaderD3D11.cpp +++ b/GPU/D3D11/DepalettizeShaderD3D11.cpp @@ -70,14 +70,10 @@ DepalShaderCacheD3D11::~DepalShaderCacheD3D11() { inputLayout_->Release(); } -u32 DepalShaderCacheD3D11::GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat) { - return (clutMode & 0xFFFFFF) | (pixelFormat << 24); -} +ID3D11ShaderResourceView *DepalShaderCacheD3D11::GetClutTexture(GEPaletteFormat clutFormat, const u32 clutHash, u32 *rawClut, bool expandTo32bit) { + const u32 clutId = GetClutID(clutFormat, clutHash); -ID3D11ShaderResourceView *DepalShaderCacheD3D11::GetClutTexture(GEPaletteFormat clutFormat, const u32 clutID, u32 *rawClut, bool expandTo32bit) { - const u32 realClutID = clutID ^ clutFormat; - - auto oldtex = texCache_.find(realClutID); + auto oldtex = texCache_.find(clutId); if (oldtex != texCache_.end()) { oldtex->second->lastFrame = gpuStats.numFlips; return oldtex->second->view; @@ -128,7 +124,7 @@ ID3D11ShaderResourceView *DepalShaderCacheD3D11::GetClutTexture(GEPaletteFormat ASSERT_SUCCESS(device_->CreateTexture2D(&desc, &data, &tex->texture)); ASSERT_SUCCESS(device_->CreateShaderResourceView(tex->texture, nullptr, &tex->view)); tex->lastFrame = gpuStats.numFlips; - texCache_[realClutID] = tex; + texCache_[clutId] = tex; if (expandTo32bit) { delete[] expanded; diff --git a/GPU/D3D11/DepalettizeShaderD3D11.h b/GPU/D3D11/DepalettizeShaderD3D11.h index 8c0239a699..df9c4f5133 100644 --- a/GPU/D3D11/DepalettizeShaderD3D11.h +++ b/GPU/D3D11/DepalettizeShaderD3D11.h @@ -23,6 +23,7 @@ #include "Common/CommonTypes.h" #include "GPU/ge_constants.h" #include "thin3d/thin3d.h" +#include "GPU/Common/DepalettizeShaderCommon.h" class DepalShaderD3D11 { public: @@ -44,7 +45,7 @@ public: }; // Caches both shaders and palette textures. -class DepalShaderCacheD3D11 { +class DepalShaderCacheD3D11 : public DepalShaderCacheCommon { public: DepalShaderCacheD3D11(Draw::DrawContext *draw); ~DepalShaderCacheD3D11(); @@ -60,8 +61,6 @@ public: std::string DebugGetShaderString(std::string id, DebugShaderType type, DebugShaderStringType stringType); private: - u32 GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat); - ID3D11Device *device_; ID3D11DeviceContext *context_; D3D_FEATURE_LEVEL featureLevel_; diff --git a/GPU/Directx9/DepalettizeShaderDX9.cpp b/GPU/Directx9/DepalettizeShaderDX9.cpp index fec1e61c68..08824b7a98 100644 --- a/GPU/Directx9/DepalettizeShaderDX9.cpp +++ b/GPU/Directx9/DepalettizeShaderDX9.cpp @@ -67,14 +67,10 @@ DepalShaderCacheDX9::~DepalShaderCacheDX9() { } } -u32 DepalShaderCacheDX9::GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat) { - return (clutMode & 0xFFFFFF) | (pixelFormat << 24); -} +LPDIRECT3DTEXTURE9 DepalShaderCacheDX9::GetClutTexture(GEPaletteFormat clutFormat, u32 clutHash, u32 *rawClut) { + u32 clutId = GetClutID(clutFormat, clutHash); -LPDIRECT3DTEXTURE9 DepalShaderCacheDX9::GetClutTexture(GEPaletteFormat clutFormat, const u32 clutID, u32 *rawClut) { - const u32 realClutID = clutID ^ clutFormat; - - auto oldtex = texCache_.find(realClutID); + auto oldtex = texCache_.find(clutId); if (oldtex != texCache_.end()) { oldtex->second->lastFrame = gpuStats.numFlips; return oldtex->second->texture; @@ -117,7 +113,7 @@ LPDIRECT3DTEXTURE9 DepalShaderCacheDX9::GetClutTexture(GEPaletteFormat clutForma device_->SetSamplerState(1, D3DSAMP_MAGFILTER, D3DTEXF_POINT); tex->lastFrame = gpuStats.numFlips; - texCache_[realClutID] = tex; + texCache_[clutId] = tex; return tex->texture; } diff --git a/GPU/Directx9/DepalettizeShaderDX9.h b/GPU/Directx9/DepalettizeShaderDX9.h index a98ac9ff98..b6c77aa6f2 100644 --- a/GPU/Directx9/DepalettizeShaderDX9.h +++ b/GPU/Directx9/DepalettizeShaderDX9.h @@ -21,6 +21,7 @@ #include "Common/CommonTypes.h" #include "GPU/ge_constants.h" #include "GPU/Common/ShaderCommon.h" +#include "GPU/Common/DepalettizeShaderCommon.h" namespace DX9 { @@ -37,7 +38,7 @@ public: }; // Caches both shaders and palette textures. -class DepalShaderCacheDX9 { +class DepalShaderCacheDX9 : public DepalShaderCacheCommon { public: DepalShaderCacheDX9(Draw::DrawContext *draw); ~DepalShaderCacheDX9(); @@ -45,15 +46,13 @@ public: // This also uploads the palette and binds the correct texture. LPDIRECT3DPIXELSHADER9 GetDepalettizePixelShader(uint32_t clutMode, GEBufferFormat pixelFormat); LPDIRECT3DVERTEXSHADER9 GetDepalettizeVertexShader() { return vertexShader_; } - LPDIRECT3DTEXTURE9 GetClutTexture(GEPaletteFormat clutFormat, const u32 clutHash, u32 *rawClut); + LPDIRECT3DTEXTURE9 GetClutTexture(GEPaletteFormat clutFormat, u32 clutHash, u32 *rawClut); void Clear(); void Decimate(); std::vector DebugGetShaderIDs(DebugShaderType type); std::string DebugGetShaderString(std::string id, DebugShaderType type, DebugShaderStringType stringType); private: - u32 GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat); - LPDIRECT3DDEVICE9 device_; LPDIRECT3DVERTEXSHADER9 vertexShader_; std::map cache_; diff --git a/GPU/GLES/DepalettizeShaderGLES.cpp b/GPU/GLES/DepalettizeShaderGLES.cpp index b6f5c2e7d6..71ef33d4c2 100644 --- a/GPU/GLES/DepalettizeShaderGLES.cpp +++ b/GPU/GLES/DepalettizeShaderGLES.cpp @@ -120,14 +120,10 @@ bool DepalShaderCacheGLES::CreateVertexShader() { return !vertexShaderFailed_; } -u32 DepalShaderCacheGLES::GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat) { - return (clutMode & 0xFFFFFF) | (pixelFormat << 24); -} +GLuint DepalShaderCacheGLES::GetClutTexture(GEPaletteFormat clutFormat, const u32 clutHash, u32 *rawClut) { + u32 clutId = GetClutID(clutFormat, clutHash); -GLuint DepalShaderCacheGLES::GetClutTexture(GEPaletteFormat clutFormat, const u32 clutID, u32 *rawClut) { - const u32 realClutID = clutID ^ clutFormat; - - auto oldtex = texCache_.find(realClutID); + auto oldtex = texCache_.find(clutId); if (oldtex != texCache_.end()) { oldtex->second->lastFrame = gpuStats.numFlips; return oldtex->second->texture; @@ -151,7 +147,7 @@ GLuint DepalShaderCacheGLES::GetClutTexture(GEPaletteFormat clutFormat, const u3 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); tex->lastFrame = gpuStats.numFlips; - texCache_[realClutID] = tex; + texCache_[clutId] = tex; return tex->texture; } diff --git a/GPU/GLES/DepalettizeShaderGLES.h b/GPU/GLES/DepalettizeShaderGLES.h index effc0b4673..a32acb2cb3 100644 --- a/GPU/GLES/DepalettizeShaderGLES.h +++ b/GPU/GLES/DepalettizeShaderGLES.h @@ -21,6 +21,7 @@ #include "gfx/gl_common.h" #include "GPU/ge_constants.h" #include "GPU/Common/ShaderCommon.h" +#include "GPU/Common/DepalettizeShaderCommon.h" class DepalShader { public: @@ -38,7 +39,7 @@ public: }; // Caches both shaders and palette textures. -class DepalShaderCacheGLES { +class DepalShaderCacheGLES : public DepalShaderCacheCommon { public: DepalShaderCacheGLES(); ~DepalShaderCacheGLES(); @@ -52,7 +53,6 @@ public: std::string DebugGetShaderString(std::string id, DebugShaderType type, DebugShaderStringType stringType); private: - u32 GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat); bool CreateVertexShader(); bool useGL3_; diff --git a/GPU/Vulkan/DepalettizeShaderVulkan.cpp b/GPU/Vulkan/DepalettizeShaderVulkan.cpp index 406afe614c..ce71a9babd 100644 --- a/GPU/Vulkan/DepalettizeShaderVulkan.cpp +++ b/GPU/Vulkan/DepalettizeShaderVulkan.cpp @@ -103,11 +103,9 @@ DepalShaderVulkan *DepalShaderCacheVulkan::GetDepalettizeShader(uint32_t clutMod return depal; } -VulkanTexture *DepalShaderCacheVulkan::GetClutTexture(GEPaletteFormat clutFormat, u32 clutID, u32 *rawClut) { - VkCommandBuffer cmd = (VkCommandBuffer)draw_->GetNativeObject(Draw::NativeObject::INIT_COMMANDBUFFER); - const u32 realClutID = clutID ^ clutFormat; - - auto oldtex = texCache_.find(realClutID); +VulkanTexture *DepalShaderCacheVulkan::GetClutTexture(GEPaletteFormat clutFormat, u32 clutHash, u32 *rawClut) { + u32 clutId = GetClutID(clutFormat, clutHash); + auto oldtex = texCache_.find(clutId); if (oldtex != texCache_.end()) { oldtex->second->lastFrame = gpuStats.numFlips; return oldtex->second->texture; @@ -121,6 +119,7 @@ VulkanTexture *DepalShaderCacheVulkan::GetClutTexture(GEPaletteFormat clutFormat uint32_t pushOffset = push_->PushAligned(rawClut, 1024, 4, &pushBuffer); VulkanTexture *vktex = new VulkanTexture(vulkan_, alloc_); + VkCommandBuffer cmd = (VkCommandBuffer)draw_->GetNativeObject(Draw::NativeObject::INIT_COMMANDBUFFER); if (!vktex->CreateDirect(cmd, texturePixels, 1, 1, destFormat, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_USAGE_SAMPLED_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT, &componentMapping)) { Crash(); @@ -131,7 +130,7 @@ VulkanTexture *DepalShaderCacheVulkan::GetClutTexture(GEPaletteFormat clutFormat DepalTextureVulkan *tex = new DepalTextureVulkan(); tex->texture = vktex; tex->lastFrame = gpuStats.numFlips; - texCache_[realClutID] = tex; + texCache_[clutId] = tex; return tex->texture; } @@ -161,7 +160,3 @@ void DepalShaderCacheVulkan::Decimate() { } } } - -u32 DepalShaderCacheVulkan::GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat) { - return (clutMode & 0xFFFFFF) | (pixelFormat << 24); -} diff --git a/GPU/Vulkan/DepalettizeShaderVulkan.h b/GPU/Vulkan/DepalettizeShaderVulkan.h index 068d2c5014..bba40a2c09 100644 --- a/GPU/Vulkan/DepalettizeShaderVulkan.h +++ b/GPU/Vulkan/DepalettizeShaderVulkan.h @@ -25,6 +25,7 @@ #include "Common/Vulkan/VulkanMemory.h" #include "GPU/ge_constants.h" #include "thin3d/thin3d.h" +#include "GPU/Common/DepalettizeShaderCommon.h" class DepalShaderVulkan { public: @@ -47,7 +48,7 @@ class Vulkan2D; // Caches both shaders and palette textures. // Could even avoid bothering with palette texture and just use uniform data... -class DepalShaderCacheVulkan { +class DepalShaderCacheVulkan : public DepalShaderCacheCommon { public: DepalShaderCacheVulkan(Draw::DrawContext *draw, VulkanContext *vulkan); ~DepalShaderCacheVulkan(); @@ -64,8 +65,6 @@ public: void SetVShader(VkShaderModule vshader) { vshader_ = vshader; } private: - u32 GenerateShaderID(uint32_t clutMode, GEBufferFormat pixelFormat); - Draw::DrawContext *draw_ = nullptr; VulkanContext *vulkan_ = nullptr; VulkanPushBuffer *push_ = nullptr; From fc60a2692c8082e2589e0e9b29ca200da05ce5cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 5 Nov 2017 13:44:23 +0100 Subject: [PATCH 15/38] Buildfix again. Should fix #10046 this time. --- GPU/Common/FramebufferCommon.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index e8667c00c6..30bafd0287 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -307,10 +307,10 @@ void GetFramebufferHeuristicInputs(FramebufferHeuristicParams *params, const GPU // Work around problem in F1 Grand Prix, where it draws in through mode with a bogus viewport. // We set bad values to 0 which causes the framebuffer size heuristic to rely on the other parameters instead. - if (isnan(vpx) || vpx > 10000000.0f) { + if (std::isnan(vpx) || vpx > 10000000.0f) { vpx = 0.f; } - if (isnan(vpy) || vpy > 10000000.0f) { + if (std::isnan(vpy) || vpy > 10000000.0f) { vpy = 0.f; } params->viewportWidth = (int)(fabsf(vpx) * 2.0f); From db80e3fb234bee5abbb46f1f7b4338275ab10046 Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Sun, 5 Nov 2017 15:24:47 +0100 Subject: [PATCH 16/38] Add missing Google Maven repository --- build.gradle | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 62b62b545c..6013117aa1 100644 --- a/build.gradle +++ b/build.gradle @@ -1,7 +1,8 @@ // Top-level build file where you can add configuration options common to all sub-projects/modules. buildscript { repositories { - jcenter() + google() + jcenter() } dependencies { classpath 'com.android.tools.build:gradle:3.0.0' @@ -10,6 +11,7 @@ buildscript { allprojects { repositories { + google() jcenter() } } From ab9aee9a07a853006eb35c6a3071e5f53e867e3c Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 07:07:14 -0800 Subject: [PATCH 17/38] Vulkan: Account inside frame from push side. We don't want a dependency on the thread state, of course. --- ext/native/thin3d/VulkanRenderManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 038e85939d..ea38d21eaf 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -573,6 +573,8 @@ void VulkanRenderManager::Finish() { frameData.pull_condVar.notify_all(); } vulkan_->EndFrame(); + + insideFrame_ = false; } void VulkanRenderManager::Wipe() { @@ -671,7 +673,6 @@ void VulkanRenderManager::Submit(int frame, bool triggerFence) { void VulkanRenderManager::EndSubmitFrame(int frame) { FrameData &frameData = frameData_[frame]; frameData.hasBegun = false; - insideFrame_ = false; Submit(frame, true); From 2ad9eb047edfe73ffc5e640373b47cc4b440c169 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:13:18 -0800 Subject: [PATCH 18/38] Vulkan: Refcount framebuffer deletes. Fixes crash in GoW when using a thread. --- GPU/Common/FramebufferCommon.cpp | 2 +- ext/native/thin3d/VulkanQueueRunner.cpp | 13 +++++++++++++ ext/native/thin3d/VulkanRenderManager.cpp | 19 +++++++++++++++++++ ext/native/thin3d/VulkanRenderManager.h | 14 +++++++++++--- ext/native/thin3d/thin3d_vulkan.cpp | 3 ++- 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index f97b2117a3..371a43e371 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -1072,7 +1072,7 @@ void FramebufferManagerCommon::DecimateFBOs() { currentRenderVfb_ = 0; for (auto iter : fbosToDelete_) { - delete iter; + iter->Release(); } fbosToDelete_.clear(); diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index e1969f69fa..10948f82b2 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -326,6 +326,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c vkCmdPipelineBarrier(cmd, srcStage, dstStage, 0, 0, nullptr, 0, nullptr, 1, &barrier); iter.fb->color.layout = barrier.newLayout; + iter.fb->Release(); } } @@ -573,6 +574,10 @@ void VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKRStep &step rp_begin.clearValueCount = numClearVals; rp_begin.pClearValues = numClearVals ? clearVal : nullptr; vkCmdBeginRenderPass(cmd, &rp_begin, VK_SUBPASS_CONTENTS_INLINE); + + if (step.render.framebuffer) { + step.render.framebuffer->Release(); + } } void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { @@ -646,6 +651,9 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { } vkCmdCopyImage(cmd, src->depth.image, src->depth.layout, dst->depth.image, dst->depth.layout, 1, ©); } + + src->Release(); + dst->Release(); } void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) { @@ -729,6 +737,9 @@ void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) { } vkCmdBlitImage(cmd, src->depth.image, src->depth.layout, dst->depth.image, dst->depth.layout, 1, &blit, step.blit.filter); } + + src->Release(); + dst->Release(); } void VulkanQueueRunner::SetupTransitionToTransferSrc(VKRImage &img, VkImageMemoryBarrier &barrier, VkPipelineStageFlags &stage, VkImageAspectFlags aspect) { @@ -831,6 +842,8 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd vkCmdCopyImageToBuffer(cmd, srcImage->image, srcImage->layout, readbackBuffer_, 1, ®ion); // NOTE: Can't read the buffer using the CPU here - need to sync first. + + step.readback.src->Release(); } void VulkanQueueRunner::CopyReadbackBuffer(int width, int height, Draw::DataFormat destFormat, int pixelStride, uint8_t *pixels) { diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index ea38d21eaf..c2c89a2100 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -96,6 +96,16 @@ void CreateImage(VulkanContext *vulkan, VkCommandBuffer cmd, VKRImage &img, int img.format = format; } +bool VKRFramebuffer::Release() { + if (--refcount_ == 0) { + delete this; + return true; + } else if (refcount_ >= 10000 || refcount_ < 0) { + ELOG("Refcount (%d) invalid for object %p - corrupt?", refcount_.load(), this); + } + return false; +} + VulkanRenderManager::VulkanRenderManager(VulkanContext *vulkan) : vulkan_(vulkan), queueRunner_(vulkan) { VkSemaphoreCreateInfo semaphoreCreateInfo = { VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO }; semaphoreCreateInfo.flags = 0; @@ -351,6 +361,9 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR step->render.numDraws = 0; step->render.finalColorLayout = !fb ? VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL : VK_IMAGE_LAYOUT_UNDEFINED; steps_.push_back(step); + if (fb) { + fb->AddRef(); + } curRenderStep_ = step; curWidth_ = fb ? fb->width : vulkan_->GetBackbufferWidth(); @@ -364,6 +377,7 @@ void VulkanRenderManager::CopyFramebufferToMemorySync(VKRFramebuffer *src, int a step->readback.srcRect.offset = { x, y }; step->readback.srcRect.extent = { (uint32_t)w, (uint32_t)h }; steps_.push_back(step); + src->AddRef(); curRenderStep_ = nullptr; @@ -505,6 +519,8 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, step->copy.srcRect = srcRect; step->copy.dst = dst; step->copy.dstPos = dstPos; + src->AddRef(); + dst->AddRef(); std::unique_lock lock(mutex_); steps_.push_back(step); @@ -530,6 +546,8 @@ void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, step->blit.dst = dst; step->blit.dstRect = dstRect; step->blit.filter = filter; + src->AddRef(); + dst->AddRef(); std::unique_lock lock(mutex_); steps_.push_back(step); @@ -553,6 +571,7 @@ VkImageView VulkanRenderManager::BindFramebufferAsTexture(VKRFramebuffer *fb, in } curRenderStep_->preTransitions.push_back({ fb, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL }); + fb->AddRef(); return fb->color.imageView; } diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 50fb2f56d8..9b9ae5f679 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -4,10 +4,11 @@ // Only draws and binds are handled here, resource creation and allocations are handled as normal - // that's the nice thing with Vulkan. -#include -#include -#include +#include #include +#include +#include +#include #include "Common/Vulkan/VulkanContext.h" #include "math/dataconv.h" @@ -28,6 +29,7 @@ void CreateImage(VulkanContext *vulkan, VkCommandBuffer cmd, VKRImage &img, int class VKRFramebuffer { public: VKRFramebuffer(VulkanContext *vk, VkCommandBuffer initCmd, VkRenderPass renderPass, int _width, int _height) : vulkan_(vk) { + refcount_ = 1; width = _width; height = _height; @@ -58,6 +60,11 @@ public: vulkan_->Delete().QueueDeleteFramebuffer(framebuf); } + void AddRef() { + refcount_++; + } + bool Release(); + int numShadows = 1; // TODO: Support this. VkFramebuffer framebuf = VK_NULL_HANDLE; @@ -68,6 +75,7 @@ public: private: VulkanContext *vulkan_; + std::atomic refcount_; }; enum class VKRRunType { diff --git a/ext/native/thin3d/thin3d_vulkan.cpp b/ext/native/thin3d/thin3d_vulkan.cpp index 4e6eff5ddd..70a7cdd65b 100644 --- a/ext/native/thin3d/thin3d_vulkan.cpp +++ b/ext/native/thin3d/thin3d_vulkan.cpp @@ -1248,9 +1248,10 @@ uint32_t VKContext::GetDataFormatSupport(DataFormat fmt) const { // use this frame's init command buffer. class VKFramebuffer : public Framebuffer { public: + // Inherits ownership so no AddRef. VKFramebuffer(VKRFramebuffer *fb) : buf_(fb) {} ~VKFramebuffer() { - delete buf_; + buf_->Release(); } VKRFramebuffer *GetFB() const { return buf_; } private: From efbba31608fb41715e74b3d37f880ced77fb7d0b Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:13:43 -0800 Subject: [PATCH 19/38] GPU: Restore initial readback on output. Needed when the last rendered FB needs to be downloaded. --- GPU/Common/FramebufferCommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index 371a43e371..3173705c59 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -837,7 +837,7 @@ void FramebufferManagerCommon::SetViewport2D(int x, int y, int w, int h) { } void FramebufferManagerCommon::CopyDisplayToOutput() { - // DownloadFramebufferOnSwitch(currentRenderVfb_); + DownloadFramebufferOnSwitch(currentRenderVfb_); currentRenderVfb_ = 0; From ebe9dcafde21a81aea29ea6895bde623eb768880 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:39:02 -0800 Subject: [PATCH 20/38] Vulkan: Wait for queue idle in DestroyBackbuffers. Otherwise it's only done after destroying Draw, so no need to mutex. --- Windows/GPU/WindowsVulkanContext.cpp | 1 - android/jni/app-android.cpp | 1 - ext/native/thin3d/VulkanRenderManager.cpp | 2 ++ 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp index f8263b565e..4d8e8f152f 100644 --- a/Windows/GPU/WindowsVulkanContext.cpp +++ b/Windows/GPU/WindowsVulkanContext.cpp @@ -228,7 +228,6 @@ void WindowsVulkanContext::SwapBuffers() { } void WindowsVulkanContext::Resize() { - g_Vulkan->WaitUntilQueueIdle(); draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight()); g_Vulkan->DestroyObjects(); diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index e0e772d1c1..19a133502f 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -341,7 +341,6 @@ void AndroidVulkanContext::SwapBuffers() { } void AndroidVulkanContext::Resize() { - g_Vulkan->WaitUntilQueueIdle(); draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight()); g_Vulkan->DestroyObjects(); diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index c2c89a2100..0b6c7e015b 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -218,6 +218,8 @@ void VulkanRenderManager::DestroyBackbuffers() { } thread_.join(); } + vulkan_->WaitUntilQueueIdle(); + VkDevice device = vulkan_->GetDevice(); for (uint32_t i = 0; i < swapchainImageCount_; i++) { vulkan_->Delete().QueueDeleteImageView(swapchainImages_[i].view); From 56d34402ffe3f00af6ceb5786dd5addd4bd8bba4 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:40:11 -0800 Subject: [PATCH 21/38] Vulkan: Resignal unexecuted fences on thread stop. When resizing or similar, we may end up with frames we never ran. This also happens on startup. We need them signaled at start so we can wait on them, or we may deadlock. --- ext/native/thin3d/VulkanRenderManager.cpp | 38 ++++++++++++++--------- ext/native/thin3d/VulkanRenderManager.h | 2 ++ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 0b6c7e015b..53db35ffc6 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -189,12 +189,6 @@ void VulkanRenderManager::CreateBackbuffers() { // Start the thread. if (useThread) { - for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { - // Reset all the frameData. Might be dirty from previous thread stop. - frameData_[i].readyForRun = false; - frameData_[i].readyForFence = true; - } - run_ = true; // Won't necessarily be 0. threadInitFrame_ = vulkan_->GetCurFrame(); @@ -202,22 +196,37 @@ void VulkanRenderManager::CreateBackbuffers() { } } -void VulkanRenderManager::DestroyBackbuffers() { - if (useThread) { +void VulkanRenderManager::StopThread(bool shutdown) { + if (useThread && run_) { run_ = false; // Stop the thread. for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { + auto &frameData = frameData_[i]; { - std::unique_lock lock(frameData_[i].push_mutex); - frameData_[i].push_condVar.notify_all(); + std::unique_lock lock(frameData.push_mutex); + frameData.push_condVar.notify_all(); } { - std::unique_lock lock(frameData_[i].pull_mutex); - frameData_[i].pull_condVar.notify_all(); + std::unique_lock lock(frameData.pull_mutex); + frameData.pull_condVar.notify_all(); } } thread_.join(); + + // Resignal fences for next time around - must be done after join. + for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { + auto &frameData = frameData_[i]; + frameData.readyForRun = false; + if (!shutdown && !frameData.readyForFence) { + vkDestroyFence(vulkan_->GetDevice(), frameData.fence, nullptr); + frameData.fence = vulkan_->CreateFence(true); + } + } } +} + +void VulkanRenderManager::DestroyBackbuffers() { + StopThread(false); vulkan_->WaitUntilQueueIdle(); VkDevice device = vulkan_->GetDevice(); @@ -237,9 +246,10 @@ void VulkanRenderManager::DestroyBackbuffers() { } VulkanRenderManager::~VulkanRenderManager() { - run_ = false; - VkDevice device = vulkan_->GetDevice(); + StopThread(true); vulkan_->WaitUntilQueueIdle(); + + VkDevice device = vulkan_->GetDevice(); vkDestroySemaphore(device, acquireSemaphore_, nullptr); vkDestroySemaphore(device, renderingCompleteSemaphore_, nullptr); for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 9b9ae5f679..f567163fa5 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -221,6 +221,8 @@ private: void FlushSync(); void EndSyncFrame(int frame); + void StopThread(bool shutdown); + // Permanent objects VkSemaphore acquireSemaphore_; VkSemaphore renderingCompleteSemaphore_; From ead4c5f9a8036939877f8897b95ab33a96c6b716 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 08:48:18 -0800 Subject: [PATCH 22/38] Vulkan: Make sure backbuffer is bound for UI. Fixes #10048. --- UI/EmuScreen.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/UI/EmuScreen.cpp b/UI/EmuScreen.cpp index 39a1c3fb87..8ce39c35a5 100644 --- a/UI/EmuScreen.cpp +++ b/UI/EmuScreen.cpp @@ -1038,6 +1038,9 @@ void EmuScreen::render() { if (!osm.IsEmpty() || g_Config.bShowDebugStats || g_Config.iShowFPSCounter || g_Config.bShowTouchControls || g_Config.bShowDeveloperMenu || g_Config.bShowAudioDebug || saveStatePreview_->GetVisibility() != UI::V_GONE || g_Config.bShowFrameProfiler) { DrawContext *thin3d = screenManager()->getDrawContext(); + // It's possible we never ended up outputted anything - make sure we have the backbuffer. + thin3d->BindFramebufferAsRenderTarget(nullptr, { RPAction::KEEP, RPAction::KEEP }); + // This sets up some important states but not the viewport. screenManager()->getUIContext()->Begin(); From 66ddf592b4d5d997ea00cb28ee602b3011af7074 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 10:04:22 -0800 Subject: [PATCH 23/38] SaveState: Use malloc to avoid Android OOM crash. We have exceptions disabled on Android, which is exactly where we need the OOM check most. --- Common/ChunkFile.cpp | 39 +++++++++++++++++++++------------------ Common/ChunkFile.h | 13 +++---------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/Common/ChunkFile.cpp b/Common/ChunkFile.cpp index ecf422b0f9..5f380347fd 100644 --- a/Common/ChunkFile.cpp +++ b/Common/ChunkFile.cpp @@ -15,6 +15,7 @@ // Official SVN repository and contact information can be found at // http://code.google.com/p/dolphin-emu/ +#include #include #include @@ -271,18 +272,23 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, INFO_LOG(SAVESTATE, "ChunkReader: Writing %s", filename.c_str()); File::IOFile pFile(filename, "wb"); - if (!pFile) - { + if (!pFile) { ERROR_LOG(SAVESTATE, "ChunkReader: Error opening file for write"); - delete[] buffer; + free(buffer); return ERROR_BAD_FILE; } - bool compress = true; + // Make sure we can allocate a buffer to compress before compressing. + size_t comp_len = snappy_max_compressed_length(sz); + u8 *compressed_buffer = (u8 *)malloc(comp_len); + if (!compressed_buffer) { + ERROR_LOG(SAVESTATE, "ChunkReader: Unable to allocate compressed buffer"); + // We'll save uncompressed. Better than not saving... + } // Create header SChunkHeader header; - header.Compress = compress ? 1 : 0; + header.Compress = compressed_buffer ? 1 : 0; header.Revision = REVISION_CURRENT; header.ExpectedSize = (u32)sz; header.UncompressedSize = (u32)sz; @@ -293,12 +299,11 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, truncate_cpy(titleFixed, title.c_str()); // Write to file - if (compress) { - size_t comp_len = snappy_max_compressed_length(sz); - u8 *compressed_buffer = new u8[comp_len]; + if (compressed_buffer) { snappy_compress((const char *)buffer, sz, (char *)compressed_buffer, &comp_len); - delete [] buffer; + free(buffer); header.ExpectedSize = (u32)comp_len; + if (!pFile.WriteArray(&header, 1)) { ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); return ERROR_BAD_FILE; @@ -307,27 +312,25 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing title"); return ERROR_BAD_FILE; } - if (!pFile.WriteBytes(&compressed_buffer[0], comp_len)) { + if (!pFile.WriteBytes(compressed_buffer, comp_len)) { ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing compressed data"); return ERROR_BAD_FILE; } else { INFO_LOG(SAVESTATE, "Savestate: Compressed %i bytes into %i", (int)sz, (int)comp_len); } - delete [] compressed_buffer; + free(compressed_buffer); } else { - if (!pFile.WriteArray(&header, 1)) - { + if (!pFile.WriteArray(&header, 1)) { ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); - delete[] buffer; + free(buffer); return ERROR_BAD_FILE; } - if (!pFile.WriteBytes(&buffer[0], sz)) - { + if (!pFile.WriteBytes(&buffer[0], sz)) { ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing data"); - delete[] buffer; + free(buffer); return ERROR_BAD_FILE; } - delete [] buffer; + free(buffer); } INFO_LOG(SAVESTATE, "ChunkReader: Done writing %s", filename.c_str()); diff --git a/Common/ChunkFile.h b/Common/ChunkFile.h index 1820dc2192..6fc76ddb29 100644 --- a/Common/ChunkFile.h +++ b/Common/ChunkFile.h @@ -29,6 +29,7 @@ // + Sections can be versioned for backwards/forwards compatibility // - Serialization code for anything complex has to be manually written. +#include #include #include #include @@ -646,17 +647,9 @@ public: { // Get data size_t const sz = MeasurePtr(_class); - u8 *buffer = nullptr; -#if PPSSPP_PLATFORM(ANDROID) - buffer = new u8[sz]; -#else - try { - buffer = new u8[sz]; - } - catch (std::bad_alloc e) { + u8 *buffer = (u8 *)malloc(sz); + if (!buffer) return ERROR_BAD_ALLOC; - } -#endif Error error = SavePtr(buffer, _class); // SaveFile takes ownership of buffer From 349b36f000538ff29b15a97cd837004f436e5a9d Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 10:17:41 -0800 Subject: [PATCH 24/38] SaveState: Correct free on error and uncomp format. The uncompressed path had diverged, so refactor to avoid this in the future. Also, some errors weren't freeing all buffers. --- Common/ChunkFile.cpp | 65 ++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/Common/ChunkFile.cpp b/Common/ChunkFile.cpp index 5f380347fd..3560c6eeb4 100644 --- a/Common/ChunkFile.cpp +++ b/Common/ChunkFile.cpp @@ -279,18 +279,25 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, } // Make sure we can allocate a buffer to compress before compressing. - size_t comp_len = snappy_max_compressed_length(sz); - u8 *compressed_buffer = (u8 *)malloc(comp_len); + size_t write_len = snappy_max_compressed_length(sz); + u8 *compressed_buffer = (u8 *)malloc(write_len); + u8 *write_buffer = buffer; if (!compressed_buffer) { ERROR_LOG(SAVESTATE, "ChunkReader: Unable to allocate compressed buffer"); // We'll save uncompressed. Better than not saving... + write_len = sz; + } else { + snappy_compress((const char *)buffer, sz, (char *)compressed_buffer, &write_len); + free(buffer); + + write_buffer = compressed_buffer; } // Create header SChunkHeader header; header.Compress = compressed_buffer ? 1 : 0; header.Revision = REVISION_CURRENT; - header.ExpectedSize = (u32)sz; + header.ExpectedSize = (u32)write_len; header.UncompressedSize = (u32)sz; truncate_cpy(header.GitVersion, gitVersion); @@ -298,40 +305,26 @@ CChunkFileReader::Error CChunkFileReader::SaveFile(const std::string &filename, char titleFixed[128]; truncate_cpy(titleFixed, title.c_str()); - // Write to file - if (compressed_buffer) { - snappy_compress((const char *)buffer, sz, (char *)compressed_buffer, &comp_len); - free(buffer); - header.ExpectedSize = (u32)comp_len; - - if (!pFile.WriteArray(&header, 1)) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); - return ERROR_BAD_FILE; - } - if (!pFile.WriteArray(titleFixed, sizeof(titleFixed))) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing title"); - return ERROR_BAD_FILE; - } - if (!pFile.WriteBytes(compressed_buffer, comp_len)) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing compressed data"); - return ERROR_BAD_FILE; - } else { - INFO_LOG(SAVESTATE, "Savestate: Compressed %i bytes into %i", (int)sz, (int)comp_len); - } - free(compressed_buffer); - } else { - if (!pFile.WriteArray(&header, 1)) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); - free(buffer); - return ERROR_BAD_FILE; - } - if (!pFile.WriteBytes(&buffer[0], sz)) { - ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing data"); - free(buffer); - return ERROR_BAD_FILE; - } - free(buffer); + // Now let's start writing out the file... + if (!pFile.WriteArray(&header, 1)) { + ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing header"); + free(write_buffer); + return ERROR_BAD_FILE; } + if (!pFile.WriteArray(titleFixed, sizeof(titleFixed))) { + ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing title"); + free(write_buffer); + return ERROR_BAD_FILE; + } + + if (!pFile.WriteBytes(write_buffer, write_len)) { + ERROR_LOG(SAVESTATE, "ChunkReader: Failed writing compressed data"); + free(write_buffer); + return ERROR_BAD_FILE; + } else if (sz != write_len) { + INFO_LOG(SAVESTATE, "Savestate: Compressed %i bytes into %i", (int)sz, (int)write_len); + } + free(write_buffer); INFO_LOG(SAVESTATE, "ChunkReader: Done writing %s", filename.c_str()); return ERROR_NONE; From 78b5565035763e05acdb28733788e9a986e816a0 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 12:45:02 -0800 Subject: [PATCH 25/38] GPU: Properly Release() FBOs. In case we end up wanting to use those reference counts, let's use the right API. --- GPU/Common/FramebufferCommon.cpp | 6 +++--- GPU/Common/TextureCacheCommon.cpp | 2 +- GPU/D3D11/FramebufferManagerD3D11.cpp | 2 +- GPU/Directx9/FramebufferDX9.cpp | 6 +++--- GPU/GLES/FramebufferManagerGLES.cpp | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/GPU/Common/FramebufferCommon.cpp b/GPU/Common/FramebufferCommon.cpp index 8a47967d6a..9bce79605e 100644 --- a/GPU/Common/FramebufferCommon.cpp +++ b/GPU/Common/FramebufferCommon.cpp @@ -198,7 +198,7 @@ bool FramebufferManagerCommon::ShouldDownloadFramebuffer(const VirtualFramebuffe void FramebufferManagerCommon::SetNumExtraFBOs(int num) { for (size_t i = 0; i < extraFBOs_.size(); i++) { - delete extraFBOs_[i]; + extraFBOs_[i]->Release(); } extraFBOs_.clear(); for (int i = 0; i < num; i++) { @@ -620,7 +620,7 @@ void FramebufferManagerCommon::NotifyRenderFramebufferSwitched(VirtualFramebuffe if (vfb->fbo) { // This should only happen very briefly when toggling useBufferedRendering_. textureCache_->NotifyFramebuffer(vfb->fb_address, vfb, NOTIFY_FB_DESTROYED); - delete vfb->fbo; + vfb->fbo->Release(); vfb->fbo = nullptr; } @@ -1173,7 +1173,7 @@ void FramebufferManagerCommon::ResizeFramebufFBO(VirtualFramebuffer *vfb, int w, if (!useBufferedRendering_) { if (vfb->fbo) { - delete vfb->fbo; + vfb->fbo->Release(); vfb->fbo = nullptr; } return; diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index c40475f944..774a5d1e2c 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -781,7 +781,7 @@ void TextureCacheCommon::SetTextureFramebuffer(TexCacheEntry *entry, VirtualFram nextTexture_ = entry; } else { if (framebuffer->fbo) { - delete framebuffer->fbo; + framebuffer->fbo->Release(); framebuffer->fbo = nullptr; } Unbind(); diff --git a/GPU/D3D11/FramebufferManagerD3D11.cpp b/GPU/D3D11/FramebufferManagerD3D11.cpp index ee1e13e64d..095d659856 100644 --- a/GPU/D3D11/FramebufferManagerD3D11.cpp +++ b/GPU/D3D11/FramebufferManagerD3D11.cpp @@ -738,7 +738,7 @@ void FramebufferManagerD3D11::DestroyAllFBOs() { bvfbs_.clear(); for (auto it = tempFBOs_.begin(), end = tempFBOs_.end(); it != end; ++it) { - delete it->second.fbo; + it->second.fbo->Release(); } tempFBOs_.clear(); diff --git a/GPU/Directx9/FramebufferDX9.cpp b/GPU/Directx9/FramebufferDX9.cpp index c5b319d950..af12aa999a 100644 --- a/GPU/Directx9/FramebufferDX9.cpp +++ b/GPU/Directx9/FramebufferDX9.cpp @@ -131,7 +131,7 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = { drawPixelsTex_->Release(); } for (auto it = tempFBOs_.begin(), end = tempFBOs_.end(); it != end; ++it) { - delete it->second.fbo; + it->second.fbo->Release(); } for (auto it = offscreenSurfaces_.begin(), end = offscreenSurfaces_.end(); it != end; ++it) { it->second.surface->Release(); @@ -726,7 +726,7 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = { bvfbs_.clear(); for (auto it = tempFBOs_.begin(), end = tempFBOs_.end(); it != end; ++it) { - delete it->second.fbo; + it->second.fbo->Release(); } tempFBOs_.clear(); @@ -777,7 +777,7 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = { success = GetRenderTargetFramebuffer(renderTarget, offscreen, w, h, buffer); } if (tempFBO) { - delete tempFBO; + tempFBO->Release(); } } diff --git a/GPU/GLES/FramebufferManagerGLES.cpp b/GPU/GLES/FramebufferManagerGLES.cpp index 3eb62e0bcd..6dede8ff5b 100644 --- a/GPU/GLES/FramebufferManagerGLES.cpp +++ b/GPU/GLES/FramebufferManagerGLES.cpp @@ -254,7 +254,7 @@ FramebufferManagerGLES::~FramebufferManagerGLES() { SetNumExtraFBOs(0); for (auto it = tempFBOs_.begin(), end = tempFBOs_.end(); it != end; ++it) { - delete it->second.fbo; + it->second.fbo->Release(); } delete [] pixelBufObj_; @@ -1048,7 +1048,7 @@ void FramebufferManagerGLES::DestroyAllFBOs() { bvfbs_.clear(); for (auto it = tempFBOs_.begin(), end = tempFBOs_.end(); it != end; ++it) { - delete it->second.fbo; + it->second.fbo->Release(); } tempFBOs_.clear(); From 7ae451db1fb6dcce317239696ff072e668f48e65 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 13:01:50 -0800 Subject: [PATCH 26/38] TexCache: Detect textures in FBO gutters. If it's not even within the framebuffer's bufferWidth, we can just go to the RAM as normal and skip the FBO. Fixes #10045. --- GPU/Common/TextureCacheCommon.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index c40475f944..69621b4dde 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -654,6 +654,7 @@ bool TextureCacheCommon::AttachFramebuffer(TexCacheEntry *entry, u32 address, Vi const u32 texaddr = ((entry->addr + texaddrOffset) & ~mirrorMask); const bool noOffset = texaddr == addr; const bool exactMatch = noOffset && entry->format < 4; + const u32 w = 1 << ((entry->dim >> 0) & 0xf); const u32 h = 1 << ((entry->dim >> 8) & 0xf); // 512 on a 272 framebuffer is sane, so let's be lenient. const u32 minSubareaHeight = h / 4; @@ -703,6 +704,13 @@ bool TextureCacheCommon::AttachFramebuffer(TexCacheEntry *entry, u32 address, Vi } } + // Check if it's in bufferWidth (which might be higher than width and may indicate the framebuffer includes the data.) + if (fbInfo.xOffset >= framebuffer->bufferWidth && fbInfo.xOffset + w <= framebuffer->fb_stride) { + // This happens in Brave Story, see #10045 - the texture is in the space between strides, with matching stride. + DetachFramebuffer(entry, address, framebuffer); + return false; + } + if (fbInfo.yOffset + minSubareaHeight >= framebuffer->height) { // Can't be inside the framebuffer then, ram. Detach to be safe. DetachFramebuffer(entry, address, framebuffer); From 702e354a6aec88fdbdcd60351baf80989ec09c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 5 Nov 2017 22:18:28 +0100 Subject: [PATCH 27/38] Vulkan multithread: Fix race condition. --- ext/native/thin3d/VulkanRenderManager.cpp | 10 ++++++++-- ext/native/thin3d/VulkanRenderManager.h | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 53db35ffc6..b87f9a6d63 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -187,11 +187,14 @@ void VulkanRenderManager::CreateBackbuffers() { curWidth_ = -1; curHeight_ = -1; + VLOG("Backbuffers Created"); + // Start the thread. if (useThread) { run_ = true; // Won't necessarily be 0. threadInitFrame_ = vulkan_->GetCurFrame(); + VLOG("starting thread"); thread_ = std::thread(&VulkanRenderManager::ThreadFunc, this); } } @@ -212,6 +215,7 @@ void VulkanRenderManager::StopThread(bool shutdown) { } } thread_.join(); + VLOG("thread joined."); // Resignal fences for next time around - must be done after join. for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { @@ -243,6 +247,7 @@ void VulkanRenderManager::DestroyBackbuffers() { framebuffers_.clear(); swapchainImages_.clear(); + VLOG("Backbuffers Destroyed"); } VulkanRenderManager::~VulkanRenderManager() { @@ -283,8 +288,8 @@ void VulkanRenderManager::ThreadFunc() { } VLOG("PULL: frame[%d].readyForRun = false", threadFrame); frameData.readyForRun = false; - if (!run_) // quick exit if bailing. - return; + // Previously we had a quick exit here that avoided calling Run() if run_ was suddenly false, + // but that created a race condition where frames could end up not finished properly on resize etc. // Only increment next time if we're done. nextFrame = frameData.type == VKRRunType::END; @@ -298,6 +303,7 @@ void VulkanRenderManager::ThreadFunc() { } void VulkanRenderManager::BeginFrame() { + VLOG("BeginFrame"); VkDevice device = vulkan_->GetDevice(); int curFrame = vulkan_->GetCurFrame(); diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index f567163fa5..22f2649da0 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -250,7 +250,7 @@ private: // Swapchain. bool hasBegun = false; - uint32_t curSwapchainImage; + uint32_t curSwapchainImage = -1; }; FrameData frameData_[VulkanContext::MAX_INFLIGHT_FRAMES]; From ef55cf1e745447c095be994306378c9cdf639125 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 13:26:18 -0800 Subject: [PATCH 28/38] Vulkan: Correct logicop dirtying. Needs to be under blend state. --- GPU/Vulkan/StateMappingVulkan.cpp | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/GPU/Vulkan/StateMappingVulkan.cpp b/GPU/Vulkan/StateMappingVulkan.cpp index c1d955fc99..4b2f1f3e3c 100644 --- a/GPU/Vulkan/StateMappingVulkan.cpp +++ b/GPU/Vulkan/StateMappingVulkan.cpp @@ -152,8 +152,13 @@ void DrawEngineVulkan::ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManag bool alphaMask = gstate.isClearModeAlphaMask(); key.colorWriteMask = (colorMask ? (VK_COLOR_COMPONENT_R_BIT | VK_COLOR_COMPONENT_G_BIT | VK_COLOR_COMPONENT_B_BIT) : 0) | (alphaMask ? VK_COLOR_COMPONENT_A_BIT : 0); } else { - key.logicOpEnable = false; - key.logicOp = VK_LOGIC_OP_CLEAR; + if (gstate_c.Supports(GPU_SUPPORTS_LOGIC_OP) && gstate.isLogicOpEnabled() && gstate.getLogicOp() != GE_LOGIC_COPY) { + key.logicOpEnable = true; + key.logicOp = logicOps[gstate.getLogicOp()]; + } else { + key.logicOpEnable = false; + key.logicOp = VK_LOGIC_OP_CLEAR; + } // Set blend - unless we need to do it in the shader. GenericBlendState blendState; @@ -275,16 +280,6 @@ void DrawEngineVulkan::ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManag dynState.useStencil = false; } } else { - if (gstate_c.Supports(GPU_SUPPORTS_LOGIC_OP)) { - // Logic Ops - if (gstate.isLogicOpEnabled() && gstate.getLogicOp() != GE_LOGIC_COPY) { - key.logicOpEnable = true; - key.logicOp = logicOps[gstate.getLogicOp()]; - } else { - key.logicOpEnable = false; - } - } - // Depth Test if (gstate.isDepthTestEnabled()) { key.depthTestEnable = true; From 63e78ef3e8003c01a26c76dff4029a68bdb8eb97 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 14:21:41 -0800 Subject: [PATCH 29/38] Vulkan: Fix framebuffer leak. Evident in God of War. --- ext/native/thin3d/VulkanQueueRunner.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index 10948f82b2..50fbf123bd 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -326,8 +326,8 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c vkCmdPipelineBarrier(cmd, srcStage, dstStage, 0, 0, nullptr, 0, nullptr, 1, &barrier); iter.fb->color.layout = barrier.newLayout; - iter.fb->Release(); } + iter.fb->Release(); } // This is supposed to bind a vulkan render pass to the command buffer. From b2039bd14fdd0aef755beb2689d3aae6e271ddf6 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 14:22:09 -0800 Subject: [PATCH 30/38] Vulkan: Fix leak in WipeQueue(). --- ext/native/thin3d/VulkanRenderManager.cpp | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index b87f9a6d63..88009d666a 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -616,8 +616,27 @@ void VulkanRenderManager::Finish() { void VulkanRenderManager::Wipe() { int curFrame = vulkan_->GetCurFrame(); - for (auto iter : steps_) { - delete iter; + for (auto step : steps_) { + // Need to release held framebuffers. + switch (step->stepType) { + case VKRStepType::RENDER: + for (const auto &iter : step->preTransitions) { + iter.fb->Release(); + } + break; + case VKRStepType::COPY: + step->copy.src->Release(); + step->copy.dst->Release(); + break; + case VKRStepType::BLIT: + step->blit.src->Release(); + step->blit.dst->Release(); + break; + case VKRStepType::READBACK: + step->readback.src->Release(); + break; + } + delete step; } steps_.clear(); } From 9fc355eee445aa5e553eae17cd105a304265288c Mon Sep 17 00:00:00 2001 From: "Anthony J. Bentley" Date: Sun, 5 Nov 2017 17:06:09 -0700 Subject: [PATCH 31/38] Use W^X on OpenBSD. --- Common/MemoryUtil.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Common/MemoryUtil.cpp b/Common/MemoryUtil.cpp index 7ea29b9715..36c7c74088 100644 --- a/Common/MemoryUtil.cpp +++ b/Common/MemoryUtil.cpp @@ -284,9 +284,9 @@ void FreeAlignedMemory(void* ptr) { } bool PlatformIsWXExclusive() { - // Only iOS really needs this mode currently. Even without block linking, still should be much faster than IR JIT. + // Needed on platforms that disable W^X pages for security. Even without block linking, still should be much faster than IR JIT. // This might also come in useful for UWP (Universal Windows Platform) if I'm understanding things correctly. -#if defined(IOS) || PPSSPP_PLATFORM(UWP) +#if defined(IOS) || PPSSPP_PLATFORM(UWP) || defined(__OpenBSD__) return true; #else // Returning true here lets you test the W^X path on Windows and other non-W^X platforms. From 1f33907f20439d62f757f9e0557aa5b533b76630 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 17:59:34 -0800 Subject: [PATCH 32/38] TexCache: Warning fix. --- GPU/Common/TextureCacheCommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index ad65820ed7..d00dfd8b90 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -705,7 +705,7 @@ bool TextureCacheCommon::AttachFramebuffer(TexCacheEntry *entry, u32 address, Vi } // Check if it's in bufferWidth (which might be higher than width and may indicate the framebuffer includes the data.) - if (fbInfo.xOffset >= framebuffer->bufferWidth && fbInfo.xOffset + w <= framebuffer->fb_stride) { + if (fbInfo.xOffset >= framebuffer->bufferWidth && fbInfo.xOffset + w <= (u32)framebuffer->fb_stride) { // This happens in Brave Story, see #10045 - the texture is in the space between strides, with matching stride. DetachFramebuffer(entry, address, framebuffer); return false; From cd6249d0803f25f7e9db869355f9360a828a2745 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 18:00:14 -0800 Subject: [PATCH 33/38] D3D11: Show FBO-as-texture in GE debugger. --- GPU/D3D11/TextureCacheD3D11.cpp | 14 ++++++++++++++ GPU/GPU.vcxproj.filters | 4 +++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/GPU/D3D11/TextureCacheD3D11.cpp b/GPU/D3D11/TextureCacheD3D11.cpp index 26744c22b8..5f190a6eb1 100644 --- a/GPU/D3D11/TextureCacheD3D11.cpp +++ b/GPU/D3D11/TextureCacheD3D11.cpp @@ -819,6 +819,20 @@ bool TextureCacheD3D11::GetCurrentTextureDebug(GPUDebugBuffer &buffer, int level if (!nextTexture_) return false; + // TODO: Centralize. + if (nextTexture_->framebuffer) { + VirtualFramebuffer *vfb = nextTexture_->framebuffer; + bool flipY = g_Config.iGPUBackend == GPU_BACKEND_OPENGL && g_Config.iRenderingMode != FB_NON_BUFFERED_MODE; + buffer.Allocate(vfb->bufferWidth, vfb->bufferHeight, GPU_DBG_FORMAT_8888, flipY); + bool retval = draw_->CopyFramebufferToMemorySync(vfb->fbo, Draw::FB_COLOR_BIT, 0, 0, vfb->bufferWidth, vfb->bufferHeight, Draw::DataFormat::R8G8B8A8_UNORM, buffer.GetData(), vfb->bufferWidth); + // Vulkan requires us to re-apply all dynamic state for each command buffer, and the above will cause us to start a new cmdbuf. + // So let's dirty the things that are involved in Vulkan dynamic state. Readbacks are not frequent so this won't hurt other backends. + gstate_c.Dirty(DIRTY_VIEWPORTSCISSOR_STATE | DIRTY_BLEND_STATE | DIRTY_DEPTHSTENCIL_STATE); + // We may have blitted to a temp FBO. + framebufferManager_->RebindFramebuffer(); + return retval; + } + ID3D11Texture2D *texture = (ID3D11Texture2D *)nextTexture_->texturePtr; if (!texture) return false; diff --git a/GPU/GPU.vcxproj.filters b/GPU/GPU.vcxproj.filters index 8aa5382057..ad9211a28c 100644 --- a/GPU/GPU.vcxproj.filters +++ b/GPU/GPU.vcxproj.filters @@ -255,7 +255,6 @@ D3D11 - Common @@ -268,6 +267,9 @@ Common + + D3D11 + From 9dfc9ff6aea0a7c41f494ada16a9c9e6a4cf4140 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 22:14:53 -0800 Subject: [PATCH 34/38] Vulkan: Allow clearing the backbuffer to any color. Helps with debugging sometimes. --- UI/EmuScreen.cpp | 1 - ext/native/thin3d/VulkanQueueRunner.cpp | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/UI/EmuScreen.cpp b/UI/EmuScreen.cpp index 8ce39c35a5..8090ad5733 100644 --- a/UI/EmuScreen.cpp +++ b/UI/EmuScreen.cpp @@ -965,7 +965,6 @@ void EmuScreen::preRender() { bool useBufferedRendering = g_Config.iRenderingMode != FB_NON_BUFFERED_MODE; if ((!useBufferedRendering && !g_Config.bSoftwareRendering) || Core_IsStepping()) { // We need to clear here already so that drawing during the frame is done on a clean slate. - DrawContext *draw = screenManager()->getDrawContext(); draw->BindFramebufferAsRenderTarget(nullptr, { RPAction::CLEAR, RPAction::CLEAR, 0xFF000000 }); Viewport viewport; diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index 50fbf123bd..4cc6052964 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -559,6 +559,9 @@ void VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKRStep &step } } else { renderPass = GetBackbufferRenderPass(); + assert(step.render.color == VKRRenderPassAction::CLEAR || step.render.color == VKRRenderPassAction::DONT_CARE); + assert(step.render.depthStencil == VKRRenderPassAction::CLEAR || step.render.depthStencil == VKRRenderPassAction::DONT_CARE); + Uint8x4ToFloat4(clearVal[0].color.float32, step.render.clearColor); numClearVals = 2; // We don't bother with a depth buffer here. clearVal[1].depthStencil.depth = 0.0f; clearVal[1].depthStencil.stencil = 0; From 4a08410d7d5d24b0d66d095f2b1770b065f581cd Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 22:15:27 -0800 Subject: [PATCH 35/38] Vulkan: Fix non-16 bit indexed drawing. Only really affects softgpu, since games are rewritten to 16 bit. --- ext/native/thin3d/VulkanQueueRunner.cpp | 2 +- ext/native/thin3d/thin3d_vulkan.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index 4cc6052964..2b50d77324 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -380,7 +380,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c case VKRRenderCommand::DRAW_INDEXED: vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, c.drawIndexed.pipelineLayout, 0, 1, &c.drawIndexed.ds, c.drawIndexed.numUboOffsets, c.drawIndexed.uboOffsets); - vkCmdBindIndexBuffer(cmd, c.drawIndexed.ibuffer, c.drawIndexed.ioffset, VK_INDEX_TYPE_UINT16); + vkCmdBindIndexBuffer(cmd, c.drawIndexed.ibuffer, c.drawIndexed.ioffset, c.drawIndexed.indexType); vkCmdBindVertexBuffers(cmd, 0, 1, &c.drawIndexed.vbuffer, &c.drawIndexed.voffset); vkCmdDrawIndexed(cmd, c.drawIndexed.count, c.drawIndexed.instances, 0, 0, 0); break; diff --git a/ext/native/thin3d/thin3d_vulkan.cpp b/ext/native/thin3d/thin3d_vulkan.cpp index 70a7cdd65b..55945e57ee 100644 --- a/ext/native/thin3d/thin3d_vulkan.cpp +++ b/ext/native/thin3d/thin3d_vulkan.cpp @@ -1118,8 +1118,8 @@ void VKContext::Draw(int vertexCount, int offset) { } void VKContext::DrawIndexed(int vertexCount, int offset) { - VKBuffer *ibuf = static_cast(curIBuffer_); - VKBuffer *vbuf = static_cast(curVBuffers_[0]); + VKBuffer *ibuf = curIBuffer_; + VKBuffer *vbuf = curVBuffers_[0]; VkBuffer vulkanVbuf, vulkanIbuf, vulkanUBObuf; uint32_t ubo_offset = (uint32_t)curPipeline_->PushUBO(push_, vulkan_, &vulkanUBObuf); From 138837ac5405b4df60f302190fc455c78104a8bb Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 18:03:53 -0800 Subject: [PATCH 36/38] Vulkan: Add texture image readback. This way we can see how it was decoded, scaled, etc. This also helps seeing texture cache invalidation issues. --- ext/native/thin3d/DataFormat.h | 6 +++ ext/native/thin3d/VulkanQueueRunner.cpp | 65 +++++++++++++++++++++-- ext/native/thin3d/VulkanQueueRunner.h | 10 +++- ext/native/thin3d/VulkanRenderManager.cpp | 41 +++++++++++++- ext/native/thin3d/VulkanRenderManager.h | 1 + ext/native/thin3d/thin3d.h | 6 --- 6 files changed, 115 insertions(+), 14 deletions(-) diff --git a/ext/native/thin3d/DataFormat.h b/ext/native/thin3d/DataFormat.h index fa9a8325be..bc89107c56 100644 --- a/ext/native/thin3d/DataFormat.h +++ b/ext/native/thin3d/DataFormat.h @@ -66,6 +66,12 @@ enum class DataFormat : uint8_t { D32F_S8, }; +size_t DataFormatSizeInBytes(DataFormat fmt); +bool DataFormatIsDepthStencil(DataFormat fmt); +inline bool DataFormatIsColor(DataFormat fmt) { + return !DataFormatIsDepthStencil(fmt); +} + void ConvertFromRGBA8888(uint8_t *dst, const uint8_t *src, uint32_t dstStride, uint32_t srcStride, uint32_t width, uint32_t height, DataFormat format); } // namespace diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index 50fbf123bd..d06596488c 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -2,6 +2,8 @@ #include "VulkanQueueRunner.h" #include "VulkanRenderManager.h" +// TODO: This is only enough for 4x render resolution / 4x texture upscale for debugger. +// Maybe we should use a dynamically allocated one for larger? const uint32_t readbackBufferSize = 2048 * 2048 * 4; void VulkanQueueRunner::CreateDeviceObjects() { @@ -205,6 +207,9 @@ void VulkanQueueRunner::RunSteps(VkCommandBuffer cmd, const std::vector &steps) { case VKRStepType::READBACK: LogReadback(step); break; + case VKRStepType::READBACK_IMAGE: + LogReadbackImage(step); + break; } } } @@ -280,6 +288,10 @@ void VulkanQueueRunner::LogReadback(const VKRStep &pass) { ILOG("Readback"); } +void VulkanQueueRunner::LogReadbackImage(const VKRStep &pass) { + ILOG("ReadbackImage"); +} + void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer cmd) { // TODO: If there are multiple, we can transition them together. for (const auto &iter : step.preTransitions) { @@ -822,8 +834,6 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd assert(false); } - assert(srcImage->format == VK_FORMAT_R8G8B8A8_UNORM); - VkImageMemoryBarrier barrier{ VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER }; VkPipelineStageFlags stage = 0; if (srcImage->layout != VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL) { @@ -846,13 +856,58 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd step.readback.src->Release(); } -void VulkanQueueRunner::CopyReadbackBuffer(int width, int height, Draw::DataFormat destFormat, int pixelStride, uint8_t *pixels) { +void VulkanQueueRunner::PerformReadbackImage(const VKRStep &step, VkCommandBuffer cmd) { + // TODO: Clean this up - just reusing `SetupTransitionToTransferSrc`. + VKRImage srcImage; + srcImage.image = step.readback_image.image; + srcImage.layout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; + + VkImageMemoryBarrier barrier{ VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER }; + VkPipelineStageFlags stage = 0; + SetupTransitionToTransferSrc(srcImage, barrier, stage, VK_IMAGE_ASPECT_COLOR_BIT); + vkCmdPipelineBarrier(cmd, stage, VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier); + + VkBufferImageCopy region{}; + region.imageOffset = { step.readback_image.srcRect.offset.x, step.readback_image.srcRect.offset.y, 0 }; + region.imageExtent = { step.readback_image.srcRect.extent.width, step.readback_image.srcRect.extent.height, 1 }; + region.imageSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; + region.imageSubresource.layerCount = 1; + region.imageSubresource.mipLevel = step.readback_image.mipLevel; + region.bufferOffset = 0; + region.bufferRowLength = step.readback_image.srcRect.extent.width; + region.bufferImageHeight = step.readback_image.srcRect.extent.height; + vkCmdCopyImageToBuffer(cmd, step.readback_image.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, readbackBuffer_, 1, ®ion); + + // Now transfer it back to a texture. + TransitionImageLayout2(cmd, step.readback_image.image, + VK_IMAGE_ASPECT_COLOR_BIT, + VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, + VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT, + VK_ACCESS_TRANSFER_READ_BIT, VK_ACCESS_SHADER_READ_BIT); + + // NOTE: Can't read the buffer using the CPU here - need to sync first. +} + +void VulkanQueueRunner::CopyReadbackBuffer(int width, int height, Draw::DataFormat srcFormat, Draw::DataFormat destFormat, int pixelStride, uint8_t *pixels) { // Read back to the requested address in ram from buffer. void *mappedData; - const int srcPixelSize = 4; // TODO: Fix. + const size_t srcPixelSize = DataFormatSizeInBytes(destFormat); VkResult res = vkMapMemory(vulkan_->GetDevice(), readbackMemory_, 0, width * height * srcPixelSize, 0, &mappedData); assert(res == VK_SUCCESS); - ConvertFromRGBA8888(pixels, (const uint8_t *)mappedData, pixelStride, width, width, height, destFormat); + if (srcFormat == Draw::DataFormat::R8G8B8A8_UNORM) { + ConvertFromRGBA8888(pixels, (const uint8_t *)mappedData, pixelStride, width, width, height, destFormat); + } else if (srcFormat == destFormat) { + uint8_t *dst = pixels; + const uint8_t *src = (const uint8_t *)mappedData; + for (int y = 0; y < height; ++y) { + memcpy(dst, src, width * srcPixelSize); + src += width * srcPixelSize; + dst += pixelStride * srcPixelSize; + } + } else { + // TODO: Maybe depth conversion or something? + assert(false); + } vkUnmapMemory(vulkan_->GetDevice(), readbackMemory_); } diff --git a/ext/native/thin3d/VulkanQueueRunner.h b/ext/native/thin3d/VulkanQueueRunner.h index 06a85a0721..7b70a14f23 100644 --- a/ext/native/thin3d/VulkanQueueRunner.h +++ b/ext/native/thin3d/VulkanQueueRunner.h @@ -84,6 +84,7 @@ enum class VKRStepType : uint8_t { COPY, BLIT, READBACK, + READBACK_IMAGE, }; enum class VKRRenderPassAction { @@ -133,6 +134,11 @@ struct VKRStep { VKRFramebuffer *src; VkRect2D srcRect; } readback; + struct { + VkImage image; + VkRect2D srcRect; + int mipLevel; + } readback_image; }; }; @@ -159,7 +165,7 @@ public: return (int)depth * 3 + (int)color; } - void CopyReadbackBuffer(int width, int height, Draw::DataFormat destFormat, int pixelStride, uint8_t *pixels); + void CopyReadbackBuffer(int width, int height, Draw::DataFormat srcFormat, Draw::DataFormat destFormat, int pixelStride, uint8_t *pixels); private: void InitBackbufferRenderPass(); @@ -170,11 +176,13 @@ private: void PerformCopy(const VKRStep &pass, VkCommandBuffer cmd); void PerformBlit(const VKRStep &pass, VkCommandBuffer cmd); void PerformReadback(const VKRStep &pass, VkCommandBuffer cmd); + void PerformReadbackImage(const VKRStep &pass, VkCommandBuffer cmd); void LogRenderPass(const VKRStep &pass); void LogCopy(const VKRStep &pass); void LogBlit(const VKRStep &pass); void LogReadback(const VKRStep &pass); + void LogReadbackImage(const VKRStep &pass); static void SetupTransitionToTransferSrc(VKRImage &img, VkImageMemoryBarrier &barrier, VkPipelineStageFlags &stage, VkImageAspectFlags aspect); static void SetupTransitionToTransferDst(VKRImage &img, VkImageMemoryBarrier &barrier, VkPipelineStageFlags &stage, VkImageAspectFlags aspect); diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index 88009d666a..c239bdb5ff 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -401,8 +401,41 @@ void VulkanRenderManager::CopyFramebufferToMemorySync(VKRFramebuffer *src, int a FlushSync(); - // Need to call this after FlushSyfnc so the pixels are guaranteed to be ready in CPU-accessible VRAM. - queueRunner_.CopyReadbackBuffer(w, h, destFormat, pixelStride, pixels); + Draw::DataFormat srcFormat; + if (aspectBits & VK_IMAGE_ASPECT_COLOR_BIT) { + switch (src->color.format) { + case VK_FORMAT_R8G8B8A8_UNORM: srcFormat = Draw::DataFormat::R8G8B8A8_UNORM; break; + default: assert(false); + } + } else if (aspectBits & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) { + switch (src->depth.format) { + case VK_FORMAT_D24_UNORM_S8_UINT: srcFormat = Draw::DataFormat::D24_S8; break; + case VK_FORMAT_D32_SFLOAT_S8_UINT: srcFormat = Draw::DataFormat::D32F_S8; break; + // TODO: Wrong. + case VK_FORMAT_D16_UNORM_S8_UINT: srcFormat = Draw::DataFormat::D16; break; + default: assert(false); + } + } else { + assert(false); + } + // Need to call this after FlushSync so the pixels are guaranteed to be ready in CPU-accessible VRAM. + queueRunner_.CopyReadbackBuffer(w, h, srcFormat, destFormat, pixelStride, pixels); +} + +void VulkanRenderManager::CopyImageToMemorySync(VkImage image, int mipLevel, int x, int y, int w, int h, Draw::DataFormat destFormat, uint8_t *pixels, int pixelStride) { + VKRStep *step = new VKRStep{ VKRStepType::READBACK_IMAGE }; + step->readback_image.image = image; + step->readback_image.srcRect.offset = { x, y }; + step->readback_image.srcRect.extent = { (uint32_t)w, (uint32_t)h }; + step->readback_image.mipLevel = mipLevel; + steps_.push_back(step); + + curRenderStep_ = nullptr; + + FlushSync(); + + // Need to call this after FlushSync so the pixels are guaranteed to be ready in CPU-accessible VRAM. + queueRunner_.CopyReadbackBuffer(w, h, destFormat, destFormat, pixelStride, pixels); } void VulkanRenderManager::InitBackbufferFramebuffers(int width, int height) { @@ -635,6 +668,10 @@ void VulkanRenderManager::Wipe() { case VKRStepType::READBACK: step->readback.src->Release(); break; + case VKRStepType::READBACK_IMAGE: + break; + default: + assert(false); } delete step; } diff --git a/ext/native/thin3d/VulkanRenderManager.h b/ext/native/thin3d/VulkanRenderManager.h index 22f2649da0..607beaf9ab 100644 --- a/ext/native/thin3d/VulkanRenderManager.h +++ b/ext/native/thin3d/VulkanRenderManager.h @@ -102,6 +102,7 @@ public: void BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRRenderPassAction color, VKRRenderPassAction depth, uint32_t clearColor, float clearDepth, uint8_t clearStencil); VkImageView BindFramebufferAsTexture(VKRFramebuffer *fb, int binding, int aspectBit, int attachment); void CopyFramebufferToMemorySync(VKRFramebuffer *src, int aspectBits, int x, int y, int w, int h, Draw::DataFormat destFormat, uint8_t *pixels, int pixelStride); + void CopyImageToMemorySync(VkImage image, int mipLevel, int x, int y, int w, int h, Draw::DataFormat destFormat, uint8_t *pixels, int pixelStride); void CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, VKRFramebuffer *dst, VkOffset2D dstPos, int aspectMask); void BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect, VKRFramebuffer *dst, VkRect2D dstRect, int aspectMask, VkFilter filter); diff --git a/ext/native/thin3d/thin3d.h b/ext/native/thin3d/thin3d.h index bd1a25d99a..9373489833 100644 --- a/ext/native/thin3d/thin3d.h +++ b/ext/native/thin3d/thin3d.h @@ -644,12 +644,6 @@ protected: int targetHeight_; }; -size_t DataFormatSizeInBytes(DataFormat fmt); -bool DataFormatIsDepthStencil(DataFormat fmt); -inline bool DataFormatIsColor(DataFormat fmt) { - return !DataFormatIsDepthStencil(fmt); -} - DrawContext *T3DCreateGLContext(); extern const UniformBufferDesc UBPresetDesc; From fc32a7b24efb7a80651d261d643201d9fae8e622 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 18:04:56 -0800 Subject: [PATCH 37/38] Vulkan: Show textures in GE debugger. --- Common/Vulkan/VulkanImage.h | 3 +- GPU/Vulkan/TextureCacheVulkan.cpp | 60 +++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/Common/Vulkan/VulkanImage.h b/Common/Vulkan/VulkanImage.h index cc303a7460..b84913c0d0 100644 --- a/Common/Vulkan/VulkanImage.h +++ b/Common/Vulkan/VulkanImage.h @@ -35,7 +35,6 @@ public: void UploadMip(VkCommandBuffer cmd, int mip, int mipWidth, int mipHeight, VkBuffer buffer, uint32_t offset, size_t rowLength); // rowLength is in pixels void EndCreate(VkCommandBuffer cmd); - int GetNumMips() const { return numMips_; } void Destroy(); // Used in image copies, etc. @@ -46,6 +45,8 @@ public: int32_t GetWidth() const { return tex_width; } int32_t GetHeight() const { return tex_height; } + int32_t GetNumMips() const { return numMips_; } + VkFormat GetFormat() const { return format_; } private: void CreateMappableImage(); diff --git a/GPU/Vulkan/TextureCacheVulkan.cpp b/GPU/Vulkan/TextureCacheVulkan.cpp index 8826278327..20664c7831 100644 --- a/GPU/Vulkan/TextureCacheVulkan.cpp +++ b/GPU/Vulkan/TextureCacheVulkan.cpp @@ -809,6 +809,62 @@ void TextureCacheVulkan::LoadTextureLevel(TexCacheEntry &entry, uint8_t *writePt } bool TextureCacheVulkan::GetCurrentTextureDebug(GPUDebugBuffer &buffer, int level) { - // TODO - return false; + ApplyTexture(); + SetTexture(false); + if (!nextTexture_) + return false; + + // TODO: Centralize? + if (nextTexture_->framebuffer) { + VirtualFramebuffer *vfb = nextTexture_->framebuffer; + bool flipY = g_Config.iGPUBackend == GPU_BACKEND_OPENGL && g_Config.iRenderingMode != FB_NON_BUFFERED_MODE; + buffer.Allocate(vfb->bufferWidth, vfb->bufferHeight, GPU_DBG_FORMAT_8888, flipY); + bool retval = draw_->CopyFramebufferToMemorySync(vfb->fbo, Draw::FB_COLOR_BIT, 0, 0, vfb->bufferWidth, vfb->bufferHeight, Draw::DataFormat::R8G8B8A8_UNORM, buffer.GetData(), vfb->bufferWidth); + // Vulkan requires us to re-apply all dynamic state for each command buffer, and the above will cause us to start a new cmdbuf. + // So let's dirty the things that are involved in Vulkan dynamic state. Readbacks are not frequent so this won't hurt other backends. + gstate_c.Dirty(DIRTY_VIEWPORTSCISSOR_STATE | DIRTY_BLEND_STATE | DIRTY_DEPTHSTENCIL_STATE); + // We may have blitted to a temp FBO. + framebufferManager_->RebindFramebuffer(); + return retval; + } + + if (!nextTexture_->vkTex || !nextTexture_->vkTex->texture_) + return false; + VulkanTexture *texture = nextTexture_->vkTex->texture_; + VulkanRenderManager *renderManager = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); + + GPUDebugBufferFormat bufferFormat; + Draw::DataFormat drawFormat; + switch (texture->GetFormat()) { + case VULKAN_565_FORMAT: + bufferFormat = GPU_DBG_FORMAT_565; + drawFormat = Draw::DataFormat::B5G6R5_UNORM_PACK16; + break; + case VULKAN_1555_FORMAT: + bufferFormat = GPU_DBG_FORMAT_5551; + drawFormat = Draw::DataFormat::B5G5R5A1_UNORM_PACK16; + break; + case VULKAN_4444_FORMAT: + bufferFormat = GPU_DBG_FORMAT_4444; + drawFormat = Draw::DataFormat::B4G4R4A4_UNORM_PACK16; + break; + case VULKAN_8888_FORMAT: + default: + bufferFormat = GPU_DBG_FORMAT_8888; + drawFormat = Draw::DataFormat::R8G8B8A8_UNORM; + break; + } + + int w = texture->GetWidth(); + int h = texture->GetHeight(); + buffer.Allocate(w, h, bufferFormat); + + renderManager->CopyImageToMemorySync(texture->GetImage(), level, 0, 0, w, h, drawFormat, (uint8_t *)buffer.GetData(), w); + + // Vulkan requires us to re-apply all dynamic state for each command buffer, and the above will cause us to start a new cmdbuf. + // So let's dirty the things that are involved in Vulkan dynamic state. Readbacks are not frequent so this won't hurt other backends. + gstate_c.Dirty(DIRTY_VIEWPORTSCISSOR_STATE | DIRTY_BLEND_STATE | DIRTY_DEPTHSTENCIL_STATE); + framebufferManager_->RebindFramebuffer(); + + return true; } From 3a84b637777e8307cf4ecbd20537f7fe3d10ef6d Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 5 Nov 2017 18:40:27 -0800 Subject: [PATCH 38/38] Vulkan: Convert depth format on readback. Although, the GPU debugger stuff already handles more formats, it's hard to get the current format to avoid the conversion... --- ext/native/thin3d/DataFormat.h | 1 + ext/native/thin3d/VulkanQueueRunner.cpp | 6 ++- ext/native/thin3d/VulkanRenderManager.cpp | 8 ++-- ext/native/thin3d/thin3d.cpp | 47 +++++++++++++++++++++++ 4 files changed, 57 insertions(+), 5 deletions(-) diff --git a/ext/native/thin3d/DataFormat.h b/ext/native/thin3d/DataFormat.h index bc89107c56..831b34d626 100644 --- a/ext/native/thin3d/DataFormat.h +++ b/ext/native/thin3d/DataFormat.h @@ -73,5 +73,6 @@ inline bool DataFormatIsColor(DataFormat fmt) { } void ConvertFromRGBA8888(uint8_t *dst, const uint8_t *src, uint32_t dstStride, uint32_t srcStride, uint32_t width, uint32_t height, DataFormat format); +void ConvertToD32F(uint8_t *dst, const uint8_t *src, uint32_t dstStride, uint32_t srcStride, uint32_t width, uint32_t height, DataFormat format); } // namespace diff --git a/ext/native/thin3d/VulkanQueueRunner.cpp b/ext/native/thin3d/VulkanQueueRunner.cpp index d06596488c..fbd6dd40fa 100644 --- a/ext/native/thin3d/VulkanQueueRunner.cpp +++ b/ext/native/thin3d/VulkanQueueRunner.cpp @@ -891,7 +891,7 @@ void VulkanQueueRunner::PerformReadbackImage(const VKRStep &step, VkCommandBuffe void VulkanQueueRunner::CopyReadbackBuffer(int width, int height, Draw::DataFormat srcFormat, Draw::DataFormat destFormat, int pixelStride, uint8_t *pixels) { // Read back to the requested address in ram from buffer. void *mappedData; - const size_t srcPixelSize = DataFormatSizeInBytes(destFormat); + const size_t srcPixelSize = DataFormatSizeInBytes(srcFormat); VkResult res = vkMapMemory(vulkan_->GetDevice(), readbackMemory_, 0, width * height * srcPixelSize, 0, &mappedData); assert(res == VK_SUCCESS); @@ -905,8 +905,10 @@ void VulkanQueueRunner::CopyReadbackBuffer(int width, int height, Draw::DataForm src += width * srcPixelSize; dst += pixelStride * srcPixelSize; } + } else if (destFormat == Draw::DataFormat::D32F) { + ConvertToD32F(pixels, (const uint8_t *)mappedData, pixelStride, width, width, height, srcFormat); } else { - // TODO: Maybe depth conversion or something? + // TODO: Maybe a depth conversion or something? assert(false); } vkUnmapMemory(vulkan_->GetDevice(), readbackMemory_); diff --git a/ext/native/thin3d/VulkanRenderManager.cpp b/ext/native/thin3d/VulkanRenderManager.cpp index c239bdb5ff..773a7fa59b 100644 --- a/ext/native/thin3d/VulkanRenderManager.cpp +++ b/ext/native/thin3d/VulkanRenderManager.cpp @@ -407,11 +407,13 @@ void VulkanRenderManager::CopyFramebufferToMemorySync(VKRFramebuffer *src, int a case VK_FORMAT_R8G8B8A8_UNORM: srcFormat = Draw::DataFormat::R8G8B8A8_UNORM; break; default: assert(false); } - } else if (aspectBits & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) { + } else if (aspectBits & VK_IMAGE_ASPECT_STENCIL_BIT) { + // Copies from stencil are always S8. + srcFormat = Draw::DataFormat::S8; + } else if (aspectBits & VK_IMAGE_ASPECT_DEPTH_BIT) { switch (src->depth.format) { case VK_FORMAT_D24_UNORM_S8_UINT: srcFormat = Draw::DataFormat::D24_S8; break; - case VK_FORMAT_D32_SFLOAT_S8_UINT: srcFormat = Draw::DataFormat::D32F_S8; break; - // TODO: Wrong. + case VK_FORMAT_D32_SFLOAT_S8_UINT: srcFormat = Draw::DataFormat::D32F; break; case VK_FORMAT_D16_UNORM_S8_UINT: srcFormat = Draw::DataFormat::D16; break; default: assert(false); } diff --git a/ext/native/thin3d/thin3d.cpp b/ext/native/thin3d/thin3d.cpp index 39bdc447bc..70a7fb0feb 100644 --- a/ext/native/thin3d/thin3d.cpp +++ b/ext/native/thin3d/thin3d.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -13,6 +14,7 @@ size_t DataFormatSizeInBytes(DataFormat fmt) { case DataFormat::R8G8_UNORM: return 2; case DataFormat::R8G8B8_UNORM: return 3; + case DataFormat::R4G4_UNORM_PACK8: return 1; case DataFormat::R4G4B4A4_UNORM_PACK16: return 2; case DataFormat::B4G4R4A4_UNORM_PACK16: return 2; case DataFormat::A4R4G4B4_UNORM_PACK16: return 2; @@ -38,6 +40,13 @@ size_t DataFormatSizeInBytes(DataFormat fmt) { case DataFormat::R32G32B32_FLOAT: return 12; case DataFormat::R32G32B32A32_FLOAT: return 16; + case DataFormat::S8: return 1; + case DataFormat::D16: return 2; + case DataFormat::D24_S8: return 4; + case DataFormat::D32F: return 4; + // Or maybe 8... + case DataFormat::D32F_S8: return 5; + default: return 0; } @@ -372,5 +381,43 @@ void ConvertFromRGBA8888(uint8_t *dst, const uint8_t *src, uint32_t dstStride, u } } +void ConvertToD32F(uint8_t *dst, const uint8_t *src, uint32_t dstStride, uint32_t srcStride, uint32_t width, uint32_t height, DataFormat format) { + if (format == Draw::DataFormat::D32F) { + const float *src32 = (const float *)src; + float *dst32 = (float *)dst; + if (src == dst) { + return; + } else { + for (uint32_t y = 0; y < height; ++y) { + memcpy(dst32, src32, width * 4); + src32 += srcStride; + dst32 += dstStride; + } + } + } else if (format == Draw::DataFormat::D16) { + const uint16_t *src16 = (const uint16_t *)src; + float *dst32 = (float *)dst; + for (uint32_t y = 0; y < height; ++y) { + for (uint32_t x = 0; x < width; ++x) { + dst32[x] = (float)(int)src16[x] / 65535.0f; + } + src16 += srcStride; + dst32 += dstStride; + } + } else if (format == Draw::DataFormat::D24_S8) { + const uint32_t *src32 = (const uint32_t *)src; + float *dst32 = (float *)dst; + for (uint32_t y = 0; y < height; ++y) { + for (uint32_t x = 0; x < width; ++x) { + dst32[x] = (src32[x] & 0x00FFFFFF) / 16777215.0f; + } + src32 += srcStride; + dst32 += dstStride; + } + } else { + assert(false); + } +} + } // namespace Draw