From 131a1eedfb33f51a65409f214e76c454a3edd2cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 13 Jul 2020 23:11:52 +0200 Subject: [PATCH] Vulkan: Make sure textures/samplers are unbound at the end of PresentationCommon::CopyToOutput. Validation caught an issue where old stuff lingered in sampler 1 and texture 1. Bug probably introduced in #12921, but could also be others. --- GPU/Common/PresentationCommon.cpp | 1 + ext/native/thin3d/thin3d.h | 4 ++-- ext/native/thin3d/thin3d_d3d11.cpp | 6 ++++++ ext/native/thin3d/thin3d_d3d9.cpp | 6 ++++++ ext/native/thin3d/thin3d_gl.cpp | 9 +++++++++ ext/native/thin3d/thin3d_vulkan.cpp | 24 ++++++++++++++++++++++++ 6 files changed, 48 insertions(+), 2 deletions(-) diff --git a/GPU/Common/PresentationCommon.cpp b/GPU/Common/PresentationCommon.cpp index f0bda76a8bc4..09bf2c20090d 100644 --- a/GPU/Common/PresentationCommon.cpp +++ b/GPU/Common/PresentationCommon.cpp @@ -748,6 +748,7 @@ void PresentationCommon::CopyToOutput(OutputFlags flags, int uvRotation, float u DoRelease(srcFramebuffer_); DoRelease(srcTexture_); + // Unbinds all textures and samplers too, needed since sometimes a MakePixelTexture is deleted etc. draw_->BindPipeline(nullptr); } diff --git a/ext/native/thin3d/thin3d.h b/ext/native/thin3d/thin3d.h index 4e61672b7f7d..f78dbf8f8784 100644 --- a/ext/native/thin3d/thin3d.h +++ b/ext/native/thin3d/thin3d.h @@ -643,7 +643,7 @@ class DrawContext { BindTextures(stage, 1, textures); } // from sampler 0 and upwards - // Call this with 0 to signal that you have been drawing on your own, and need the state reset on the next pipeline bind. + // Call this with nullptr to signal that you're done with the stuff you've bound, like textures and samplers and stuff. virtual void BindPipeline(Pipeline *pipeline) = 0; virtual void Draw(int vertexCount, int offset) = 0; @@ -652,7 +652,7 @@ class DrawContext { // Frame management (for the purposes of sync and resource management, necessary with modern APIs). Default implementations here. virtual void BeginFrame() {} - virtual void EndFrame() {} + virtual void EndFrame() = 0; virtual void WipeQueue() {} // This should be avoided as much as possible, in favor of clearing when binding a render target, which is native diff --git a/ext/native/thin3d/thin3d_d3d11.cpp b/ext/native/thin3d/thin3d_d3d11.cpp index 96b9e1ad6346..8c53ffc7d80b 100644 --- a/ext/native/thin3d/thin3d_d3d11.cpp +++ b/ext/native/thin3d/thin3d_d3d11.cpp @@ -100,6 +100,8 @@ class D3D11DrawContext : public DrawContext { stencilRefDirty_ = true; } + void EndFrame() override; + void Draw(int vertexCount, int offset) override; void DrawIndexed(int vertexCount, int offset) override; void DrawUP(const void *vdata, int vertexCount) override; @@ -363,6 +365,10 @@ void D3D11DrawContext::HandleEvent(Event ev, int width, int height, void *param1 } } +void D3D11DrawContext::EndFrame() { + curPipeline_ = nullptr; +} + void D3D11DrawContext::SetViewports(int count, Viewport *viewports) { D3D11_VIEWPORT vp[4]; for (int i = 0; i < count; i++) { diff --git a/ext/native/thin3d/thin3d_d3d9.cpp b/ext/native/thin3d/thin3d_d3d9.cpp index d8556210ea5a..0b6740963f1e 100644 --- a/ext/native/thin3d/thin3d_d3d9.cpp +++ b/ext/native/thin3d/thin3d_d3d9.cpp @@ -559,6 +559,8 @@ class D3D9Context : public DrawContext { curPipeline_ = (D3D9Pipeline *)pipeline; } + void EndFrame() override; + void UpdateDynamicUniformBuffer(const void *ub, size_t size) override; // Raster state @@ -802,6 +804,10 @@ void D3D9Context::BindTextures(int start, int count, Texture **textures) { } } +void D3D9Context::EndFrame() { + curPipeline_ = nullptr; +} + static void SemanticToD3D9UsageAndIndex(int semantic, BYTE *usage, BYTE *index) { *index = 0; switch (semantic) { diff --git a/ext/native/thin3d/thin3d_gl.cpp b/ext/native/thin3d/thin3d_gl.cpp index b168cb02a58a..bc2eece3797f 100644 --- a/ext/native/thin3d/thin3d_gl.cpp +++ b/ext/native/thin3d/thin3d_gl.cpp @@ -618,6 +618,15 @@ void OpenGLContext::EndFrame() { FrameData &frameData = frameData_[renderManager_.GetCurFrame()]; renderManager_.EndPushBuffer(frameData.push); // upload the data! renderManager_.Finish(); + + // Unbind stuff. + for (auto &texture : boundTextures_) { + texture = nullptr; + } + for (auto &sampler : boundSamplers_) { + sampler = nullptr; + } + curPipeline_ = nullptr; } InputLayout *OpenGLContext::CreateInputLayout(const InputLayoutDesc &desc) { diff --git a/ext/native/thin3d/thin3d_vulkan.cpp b/ext/native/thin3d/thin3d_vulkan.cpp index 7bab5391971d..7e161fe51598 100644 --- a/ext/native/thin3d/thin3d_vulkan.cpp +++ b/ext/native/thin3d/thin3d_vulkan.cpp @@ -406,8 +406,13 @@ class VKContext : public DrawContext { void BindSamplerStates(int start, int count, SamplerState **state) override; void BindTextures(int start, int count, Texture **textures) override; + void BindPipeline(Pipeline *pipeline) override { curPipeline_ = (VKPipeline *)pipeline; + + if (!pipeline) { + UnbindBoundState(); + } } // TODO: Make VKBuffers proper buffers, and do a proper binding model. This is just silly. @@ -494,6 +499,8 @@ class VKContext : public DrawContext { } private: + void UnbindBoundState(); + VulkanTexture *GetNullTexture(); VulkanContext *vulkan_ = nullptr; @@ -913,6 +920,23 @@ void VKContext::EndFrame() { renderManager_.Finish(); push_ = nullptr; + + // Unbind stuff, to avoid accidentally relying on it across frames (and provide some protection against forgotten unbinds of deleted things). + UnbindBoundState(); +} + +void VKContext::UnbindBoundState() { + curPipeline_ = nullptr; + + for (auto &view : boundImageView_) { + view = nullptr; + } + for (auto &sampler : boundSamplers_) { + sampler = nullptr; + } + for (auto &texture : boundTextures_) { + texture = nullptr; + } } void VKContext::WipeQueue() {