From c846c2dfa8551976e177436ef7bfa21e87cb374f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 3 Sep 2022 14:47:47 +0200 Subject: [PATCH] Remove confusing resetFramebufferRead flag from secondary framebuffer binding --- GPU/Common/DrawEngineCommon.h | 1 - GPU/Common/GPUStateUtils.cpp | 4 ---- GPU/Common/GPUStateUtils.h | 1 - GPU/D3D11/DrawEngineD3D11.h | 1 - GPU/D3D11/StateMappingD3D11.cpp | 22 +++++++++------------- GPU/Directx9/DrawEngineDX9.h | 3 ++- GPU/Directx9/StateMappingDX9.cpp | 18 +++++++++--------- GPU/GLES/DrawEngineGLES.h | 1 - GPU/GLES/StateMappingGLES.cpp | 26 +++++++++++--------------- GPU/Vulkan/DrawEngineVulkan.h | 2 +- GPU/Vulkan/StateMappingVulkan.cpp | 16 +++++++--------- 11 files changed, 39 insertions(+), 56 deletions(-) diff --git a/GPU/Common/DrawEngineCommon.h b/GPU/Common/DrawEngineCommon.h index 3186766bb7..a8997454d9 100644 --- a/GPU/Common/DrawEngineCommon.h +++ b/GPU/Common/DrawEngineCommon.h @@ -187,7 +187,6 @@ protected: GEPrimitiveType prevPrim_ = GE_PRIM_INVALID; // Shader blending state - bool fboTexNeedsBind_ = false; bool fboTexBound_ = false; // Sometimes, unusual situations mean we need to reset dirty flags after state calc finishes. diff --git a/GPU/Common/GPUStateUtils.cpp b/GPU/Common/GPUStateUtils.cpp index 93db4f15b0..a78bad276b 100644 --- a/GPU/Common/GPUStateUtils.cpp +++ b/GPU/Common/GPUStateUtils.cpp @@ -1067,7 +1067,6 @@ void ConvertBlendState(GenericBlendState &blendState, bool forceReplaceBlend) { switch (replaceBlend) { case REPLACE_BLEND_NO: - blendState.resetFramebufferRead = true; // We may still want to do something about stencil -> alpha. ApplyStencilReplaceAndLogicOpIgnoreBlend(replaceAlphaWithStencil, blendState); return; @@ -1081,7 +1080,6 @@ void ConvertBlendState(GenericBlendState &blendState, bool forceReplaceBlend) { case REPLACE_BLEND_READ_FRAMEBUFFER: blendState.blendEnabled = true; blendState.applyFramebufferRead = true; - blendState.resetFramebufferRead = false; break; case REPLACE_BLEND_PRE_SRC: @@ -1097,8 +1095,6 @@ void ConvertBlendState(GenericBlendState &blendState, bool forceReplaceBlend) { break; } - blendState.resetFramebufferRead = true; - const GEBlendMode blendFuncEq = gstate.getBlendEq(); GEBlendSrcFactor blendFuncA = gstate.getBlendFuncA(); GEBlendDstFactor blendFuncB = gstate.getBlendFuncB(); diff --git a/GPU/Common/GPUStateUtils.h b/GPU/Common/GPUStateUtils.h index 60dce4da80..dcbb134226 100644 --- a/GPU/Common/GPUStateUtils.h +++ b/GPU/Common/GPUStateUtils.h @@ -139,7 +139,6 @@ enum class BlendEq : uint8_t { // Computed blend setup, including shader stuff. struct GenericBlendState { - bool resetFramebufferRead; bool applyFramebufferRead; bool dirtyShaderBlendFixValues; diff --git a/GPU/D3D11/DrawEngineD3D11.h b/GPU/D3D11/DrawEngineD3D11.h index acfbc01be3..ca7fb04dbb 100644 --- a/GPU/D3D11/DrawEngineD3D11.h +++ b/GPU/D3D11/DrawEngineD3D11.h @@ -159,7 +159,6 @@ private: void ApplyDrawState(int prim); void ApplyDrawStateLate(bool applyStencilRef, uint8_t stencilRef); - void ResetFramebufferRead(); ID3D11InputLayout *SetupDecFmtForDraw(D3D11VertexShader *vshader, const DecVtxFormat &decFmt, u32 pspFmt); diff --git a/GPU/D3D11/StateMappingD3D11.cpp b/GPU/D3D11/StateMappingD3D11.cpp index b905cb35ab..d764374e05 100644 --- a/GPU/D3D11/StateMappingD3D11.cpp +++ b/GPU/D3D11/StateMappingD3D11.cpp @@ -122,12 +122,6 @@ static const D3D11_LOGIC_OP logicOps[] = { D3D11_LOGIC_OP_SET, }; -void DrawEngineD3D11::ResetFramebufferRead() { - if (fboTexBound_) { - fboTexBound_ = false; - } -} - class FramebufferManagerD3D11; class ShaderManagerD3D11; @@ -159,15 +153,15 @@ void DrawEngineD3D11::ApplyDrawState(int prim) { ConvertBlendState(blendState, maskState.applyFramebufferRead); if (blendState.applyFramebufferRead || maskState.applyFramebufferRead) { - ApplyFramebufferRead(&fboTexNeedsBind_); + bool fboTexNeedsBind = false; + ApplyFramebufferRead(&fboTexNeedsBind); // The shader takes over the responsibility for blending, so recompute. ApplyStencilReplaceAndLogicOpIgnoreBlend(blendState.replaceAlphaWithStencil, blendState); - if (fboTexNeedsBind_) { + if (fboTexNeedsBind) { framebufferManager_->BindFramebufferAsColorTexture(1, framebufferManager_->GetCurrentRenderVFB(), BINDFBCOLOR_MAY_COPY); // No sampler required, we do a plain Load in the pixel shader. fboTexBound_ = true; - fboTexNeedsBind_ = false; framebufferManager_->RebindFramebuffer("RebindFramebuffer - ApplyDrawState"); // Must dirty blend state here so we re-copy next time. Example: Lunar's spell effects. @@ -177,10 +171,12 @@ void DrawEngineD3D11::ApplyDrawState(int prim) { dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); - } else if (blendState.resetFramebufferRead) { - ResetFramebufferRead(); - dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; - gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); + } else { + if (fboTexBound_) { + fboTexBound_ = false; + dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; + gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); + } } if (blendState.blendEnabled) { diff --git a/GPU/Directx9/DrawEngineDX9.h b/GPU/Directx9/DrawEngineDX9.h index 616620a412..a0ee23e60d 100644 --- a/GPU/Directx9/DrawEngineDX9.h +++ b/GPU/Directx9/DrawEngineDX9.h @@ -148,7 +148,6 @@ private: void ApplyDrawState(int prim); void ApplyDrawStateLate(); - void ResetFramebufferRead(); IDirect3DVertexDeclaration9 *SetupDecFmtForDraw(VSShader *vshader, const DecVtxFormat &decFmt, u32 pspFmt); @@ -172,4 +171,6 @@ private: TessellationDataTransferDX9 *tessDataTransferDX9; int lastRenderStepId_ = -1; + + bool fboTexNeedsBind_ = false; }; diff --git a/GPU/Directx9/StateMappingDX9.cpp b/GPU/Directx9/StateMappingDX9.cpp index a7dafacea6..dd0e7690dc 100644 --- a/GPU/Directx9/StateMappingDX9.cpp +++ b/GPU/Directx9/StateMappingDX9.cpp @@ -88,10 +88,6 @@ static const D3DSTENCILOP stencilOps[] = { D3DSTENCILOP_KEEP, // reserved }; -inline void DrawEngineDX9::ResetFramebufferRead() { - fboTexBound_ = false; -} - void DrawEngineDX9::ApplyDrawState(int prim) { if (!gstate_c.IsDirty(DIRTY_BLEND_STATE | DIRTY_TEXTURE_IMAGE | DIRTY_TEXTURE_PARAMS | DIRTY_VIEWPORTSCISSOR_STATE | DIRTY_RASTER_STATE | DIRTY_DEPTHSTENCIL_STATE)) { // nothing to do @@ -137,26 +133,30 @@ void DrawEngineDX9::ApplyDrawState(int prim) { ConvertBlendState(blendState, maskState.applyFramebufferRead); if (blendState.applyFramebufferRead || maskState.applyFramebufferRead) { - ApplyFramebufferRead(&fboTexNeedsBind_); + bool fboTexNeedsBind = false; + ApplyFramebufferRead(&fboTexNeedsBind); // The shader takes over the responsibility for blending, so recompute. ApplyStencilReplaceAndLogicOpIgnoreBlend(blendState.replaceAlphaWithStencil, blendState); - if (fboTexNeedsBind_) { + if (fboTexNeedsBind) { // Note that this is positions, not UVs, that we need the copy from. framebufferManager_->BindFramebufferAsColorTexture(1, framebufferManager_->GetCurrentRenderVFB(), BINDFBCOLOR_MAY_COPY); // If we are rendering at a higher resolution, linear is probably best for the dest color. device_->SetSamplerState(1, D3DSAMP_MAGFILTER, D3DTEXF_LINEAR); device_->SetSamplerState(1, D3DSAMP_MINFILTER, D3DTEXF_LINEAR); fboTexBound_ = true; - fboTexNeedsBind_ = false; dirtyRequiresRecheck_ |= DIRTY_BLEND_STATE; gstate_c.Dirty(DIRTY_BLEND_STATE); } dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); - } else if (blendState.resetFramebufferRead) { - ResetFramebufferRead(); + } else { + if (fboTexBound_) { + dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; + gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); + fboTexBound_ = false; + } } if (blendState.blendEnabled) { diff --git a/GPU/GLES/DrawEngineGLES.h b/GPU/GLES/DrawEngineGLES.h index 8d45ff0d84..d843c83ff4 100644 --- a/GPU/GLES/DrawEngineGLES.h +++ b/GPU/GLES/DrawEngineGLES.h @@ -123,7 +123,6 @@ private: void DoFlush(); void ApplyDrawState(int prim); void ApplyDrawStateLate(bool setStencil, int stencilValue); - void ResetFramebufferRead(); GLRInputLayout *SetupDecFmtForDraw(LinkedShader *program, const DecVtxFormat &decFmt); diff --git a/GPU/GLES/StateMappingGLES.cpp b/GPU/GLES/StateMappingGLES.cpp index 273c75e630..386f6d7f17 100644 --- a/GPU/GLES/StateMappingGLES.cpp +++ b/GPU/GLES/StateMappingGLES.cpp @@ -122,14 +122,6 @@ static const GLushort logicOps[] = { }; #endif -inline void DrawEngineGLES::ResetFramebufferRead() { - if (fboTexBound_) { - GLRenderManager *renderManager = (GLRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); - renderManager->BindTexture(TEX_SLOT_SHADERBLEND_SRC, nullptr); - fboTexBound_ = false; - } -} - void DrawEngineGLES::ApplyDrawState(int prim) { GLRenderManager *renderManager = (GLRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); @@ -157,18 +149,18 @@ void DrawEngineGLES::ApplyDrawState(int prim) { ConvertBlendState(blendState, maskState.applyFramebufferRead); if (blendState.applyFramebufferRead || maskState.applyFramebufferRead) { - ApplyFramebufferRead(&fboTexNeedsBind_); + bool fboTexNeedsBind = false; + ApplyFramebufferRead(&fboTexNeedsBind); // The shader takes over the responsibility for blending, so recompute. ApplyStencilReplaceAndLogicOpIgnoreBlend(blendState.replaceAlphaWithStencil, blendState); // We copy the framebuffer here, as doing so will wipe any blend state if we do it later. - if (fboTexNeedsBind_) { + if (fboTexNeedsBind) { // Note that this is positions, not UVs, that we need the copy from. framebufferManager_->BindFramebufferAsColorTexture(1, framebufferManager_->GetCurrentRenderVFB(), BINDFBCOLOR_MAY_COPY); // If we are rendering at a higher resolution, linear is probably best for the dest color. renderManager->SetTextureSampler(1, GL_CLAMP_TO_EDGE, GL_CLAMP_TO_EDGE, GL_LINEAR, GL_LINEAR, 0.0f); fboTexBound_ = true; - fboTexNeedsBind_ = false; framebufferManager_->RebindFramebuffer("RebindFramebuffer - ApplyDrawState"); // Must dirty blend state here so we re-copy next time. Example: Lunar's spell effects. @@ -177,10 +169,14 @@ void DrawEngineGLES::ApplyDrawState(int prim) { } dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); - } else if (blendState.resetFramebufferRead) { - ResetFramebufferRead(); - dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; - gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); + } else { + if (fboTexBound_) { + GLRenderManager *renderManager = (GLRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); + renderManager->BindTexture(TEX_SLOT_SHADERBLEND_SRC, nullptr); + fboTexBound_ = false; + dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; + gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); + } } if (blendState.blendEnabled) { diff --git a/GPU/Vulkan/DrawEngineVulkan.h b/GPU/Vulkan/DrawEngineVulkan.h index f89a28b7f9..531e05c4ed 100644 --- a/GPU/Vulkan/DrawEngineVulkan.h +++ b/GPU/Vulkan/DrawEngineVulkan.h @@ -195,7 +195,6 @@ private: void ApplyDrawStateLate(VulkanRenderManager *renderManager, bool applyStencilRef, uint8_t stencilRef, bool useBlendConstant); void ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManager, ShaderManagerVulkan *shaderManager, int prim, VulkanPipelineRasterStateKey &key, VulkanDynamicState &dynState); void BindShaderBlendTex(); - void ResetFramebufferRead(); void InitDeviceObjects(); void DestroyDeviceObjects(); @@ -282,6 +281,7 @@ private: VulkanDynamicState dynState_{}; int tessOffset_ = 0; + bool fboTexNeedsBind_ = false; // Hardware tessellation TessellationDataTransferVulkan *tessDataTransferVulkan; diff --git a/GPU/Vulkan/StateMappingVulkan.cpp b/GPU/Vulkan/StateMappingVulkan.cpp index c249aefbb8..1955f1b772 100644 --- a/GPU/Vulkan/StateMappingVulkan.cpp +++ b/GPU/Vulkan/StateMappingVulkan.cpp @@ -122,11 +122,6 @@ static const VkLogicOp logicOps[] = { VK_LOGIC_OP_SET, }; -void DrawEngineVulkan::ResetFramebufferRead() { - boundSecondary_ = VK_NULL_HANDLE; - fboTexBound_ = false; -} - // In Vulkan, we simply collect all the state together into a "pipeline key" - we don't actually set any state here // (the caller is responsible for setting the little dynamic state that is supported, dynState). void DrawEngineVulkan::ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManager, ShaderManagerVulkan *shaderManager, int prim, VulkanPipelineRasterStateKey &key, VulkanDynamicState &dynState) { @@ -172,10 +167,13 @@ void DrawEngineVulkan::ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManag ApplyStencilReplaceAndLogicOpIgnoreBlend(blendState.replaceAlphaWithStencil, blendState); dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); - } else if (blendState.resetFramebufferRead) { - ResetFramebufferRead(); - dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; - gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); + } else { + if (fboTexBound_) { + boundSecondary_ = VK_NULL_HANDLE; + fboTexBound_ = false; + dirtyRequiresRecheck_ |= DIRTY_FRAGMENTSHADER_STATE; + gstate_c.Dirty(DIRTY_FRAGMENTSHADER_STATE); + } } if (blendState.blendEnabled) {