From 11dbae3457c82f3ff2fabd640629bb3dd183ad2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 15 Dec 2024 13:42:05 +0100 Subject: [PATCH] Remove the "DispatchFlush" mechanism, not convinced it's a win --- GPU/Common/DrawEngineCommon.cpp | 8 ++++---- GPU/Common/DrawEngineCommon.h | 5 ++--- GPU/Common/FramebufferManagerCommon.cpp | 2 +- GPU/Common/GPUDebugInterface.h | 2 +- GPU/Common/SplineCommon.cpp | 2 +- GPU/D3D11/DrawEngineD3D11.cpp | 6 ++++-- GPU/D3D11/DrawEngineD3D11.h | 17 +---------------- GPU/Debugger/Stepping.cpp | 2 +- GPU/Directx9/DrawEngineDX9.cpp | 6 ++++-- GPU/Directx9/DrawEngineDX9.h | 15 +-------------- GPU/GLES/DrawEngineGLES.cpp | 5 ++++- GPU/GLES/DrawEngineGLES.h | 15 +-------------- GPU/GPUCommon.cpp | 10 +++------- GPU/GPUCommon.h | 2 -- GPU/GPUCommonHW.cpp | 12 ++++++------ GPU/Software/TransformUnit.cpp | 2 +- GPU/Software/TransformUnit.h | 2 +- GPU/Vulkan/DrawEngineVulkan.cpp | 6 +++++- GPU/Vulkan/DrawEngineVulkan.h | 18 ++---------------- 19 files changed, 43 insertions(+), 94 deletions(-) diff --git a/GPU/Common/DrawEngineCommon.cpp b/GPU/Common/DrawEngineCommon.cpp index 7ef2a88041..8724325f17 100644 --- a/GPU/Common/DrawEngineCommon.cpp +++ b/GPU/Common/DrawEngineCommon.cpp @@ -189,7 +189,7 @@ void DrawEngineCommon::DispatchSubmitImm(GEPrimitiveType prim, TransformedVertex bool clockwise = !gstate.isCullEnabled() || gstate.getCullMode() == cullMode; SubmitPrim(&temp[0], nullptr, prim, vertexCount, vertTypeID, clockwise, &bytesRead); - DispatchFlush(); + Flush(); if (!prevThrough) { gstate.vertType &= ~GE_VTYPE_THROUGH; @@ -924,7 +924,7 @@ int DrawEngineCommon::ExtendNonIndexedPrim(const uint32_t *cmd, const uint32_t * void DrawEngineCommon::SkipPrim(GEPrimitiveType prim, int vertexCount, u32 vertTypeID, int *bytesRead) { if (!indexGen.PrimCompatible(prevPrim_, prim)) { - DispatchFlush(); + Flush(); } // This isn't exactly right, if we flushed, since prims can straddle previous calls. @@ -950,7 +950,7 @@ void DrawEngineCommon::SkipPrim(GEPrimitiveType prim, int vertexCount, u32 vertT // vertTypeID is the vertex type but with the UVGen mode smashed into the top bits. bool DrawEngineCommon::SubmitPrim(const void *verts, const void *inds, GEPrimitiveType prim, int vertexCount, u32 vertTypeID, bool clockwise, int *bytesRead) { if (!indexGen.PrimCompatible(prevPrim_, prim) || numDrawVerts_ >= MAX_DEFERRED_DRAW_VERTS || numDrawInds_ >= MAX_DEFERRED_DRAW_INDS || vertexCountInDrawCalls_ + vertexCount > VERTEX_BUFFER_MAX) { - DispatchFlush(); + Flush(); } _dbg_assert_(numDrawVerts_ < MAX_DEFERRED_DRAW_VERTS); _dbg_assert_(numDrawInds_ < MAX_DEFERRED_DRAW_INDS); @@ -1033,7 +1033,7 @@ bool DrawEngineCommon::SubmitPrim(const void *verts, const void *inds, GEPrimiti if (prim == GE_PRIM_RECTANGLES && (gstate.getTextureAddress(0) & 0x3FFFFFFF) == (gstate.getFrameBufAddress() & 0x3FFFFFFF)) { // This prevents issues with consecutive self-renders in Ridge Racer. gstate_c.Dirty(DIRTY_TEXTURE_PARAMS); - DispatchFlush(); + Flush(); } return true; } diff --git a/GPU/Common/DrawEngineCommon.h b/GPU/Common/DrawEngineCommon.h index 7ec1a92a78..ea8b74d8e3 100644 --- a/GPU/Common/DrawEngineCommon.h +++ b/GPU/Common/DrawEngineCommon.h @@ -89,9 +89,8 @@ public: static u32 NormalizeVertices(u8 *outPtr, u8 *bufPtr, const u8 *inPtr, VertexDecoder *dec, int lowerBound, int upperBound, u32 vertType); - // Flush is normally non-virtual but here's a virtual way to call it, used by the shared spline code, which is expensive anyway. - // Not really sure if these wrappers are worth it... - virtual void DispatchFlush() = 0; + // Dispatches the queued-up draws. + virtual void Flush() = 0; // This would seem to be unnecessary now, but is still required for splines/beziers to work in the software backend since SubmitPrim // is different. Should probably refactor that. diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 446225b136..1e8784bee0 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -3256,7 +3256,7 @@ void FramebufferManagerCommon::FlushBeforeCopy() { // all the irrelevant state checking it'll use to decide what to do. Should // do something more focused here. SetRenderFrameBuffer(gstate_c.IsDirty(DIRTY_FRAMEBUF), gstate_c.skipDrawReason); - drawEngine_->DispatchFlush(); + drawEngine_->Flush(); } } diff --git a/GPU/Common/GPUDebugInterface.h b/GPU/Common/GPUDebugInterface.h index 079798d5ca..5d1c47279c 100644 --- a/GPU/Common/GPUDebugInterface.h +++ b/GPU/Common/GPUDebugInterface.h @@ -225,7 +225,7 @@ public: // Needs to be called from the GPU thread. // Calling from a separate thread (e.g. UI) may fail. virtual void SetCmdValue(u32 op) = 0; - virtual void DispatchFlush() = 0; + virtual void Flush() = 0; virtual void GetStats(char *buffer, size_t bufsize) = 0; diff --git a/GPU/Common/SplineCommon.cpp b/GPU/Common/SplineCommon.cpp index de22c14fcf..9100e0b21e 100644 --- a/GPU/Common/SplineCommon.cpp +++ b/GPU/Common/SplineCommon.cpp @@ -580,7 +580,7 @@ void DrawEngineCommon::SubmitCurve(const void *control_points, const void *indic DispatchSubmitPrim(output.vertices, output.indices, PatchPrimToPrim(surface.primType), output.count, vertTypeID, true, &generatedBytesRead); if (flushOnParams_) - DispatchFlush(); + Flush(); if (origVertType & GE_VTYPE_TC_MASK) { gstate_c.uv = prevUVScale; diff --git a/GPU/D3D11/DrawEngineD3D11.cpp b/GPU/D3D11/DrawEngineD3D11.cpp index e1cfacdc90..60a3e72456 100644 --- a/GPU/D3D11/DrawEngineD3D11.cpp +++ b/GPU/D3D11/DrawEngineD3D11.cpp @@ -252,8 +252,10 @@ void DrawEngineD3D11::Invalidate(InvalidationCallbackFlags flags) { } } -// The inline wrapper in the header checks for numDrawCalls_ == 0 -void DrawEngineD3D11::DoFlush() { +void DrawEngineD3D11::Flush() { + if (!numDrawVerts_) { + return; + } bool textureNeedsApply = false; if (gstate_c.IsDirty(DIRTY_TEXTURE_IMAGE | DIRTY_TEXTURE_PARAMS) && !gstate.isModeClear() && gstate.isTextureMapEnabled()) { textureCache_->SetTexture(); diff --git a/GPU/D3D11/DrawEngineD3D11.h b/GPU/D3D11/DrawEngineD3D11.h index 9f365ab247..d870ae1850 100644 --- a/GPU/D3D11/DrawEngineD3D11.h +++ b/GPU/D3D11/DrawEngineD3D11.h @@ -77,25 +77,12 @@ public: void BeginFrame(); - // So that this can be inlined - void Flush() { - if (!numDrawVerts_) - return; - DoFlush(); - } + void Flush() override; void FinishDeferred() { - if (!numDrawVerts_) - return; DecodeVerts(decoded_); } - void DispatchFlush() override { - if (!numDrawVerts_) - return; - Flush(); - } - void NotifyConfigChanged() override; void ClearInputLayoutMap(); @@ -103,8 +90,6 @@ public: private: void Invalidate(InvalidationCallbackFlags flags); - void DoFlush(); - void ApplyDrawState(int prim); void ApplyDrawStateLate(bool applyStencilRef, uint8_t stencilRef); diff --git a/GPU/Debugger/Stepping.cpp b/GPU/Debugger/Stepping.cpp index ff15a566a6..0860c1a292 100644 --- a/GPU/Debugger/Stepping.cpp +++ b/GPU/Debugger/Stepping.cpp @@ -143,7 +143,7 @@ static void RunPauseAction() { break; case PAUSE_FLUSHDRAW: - gpuDebug->DispatchFlush(); + gpuDebug->Flush(); break; default: diff --git a/GPU/Directx9/DrawEngineDX9.cpp b/GPU/Directx9/DrawEngineDX9.cpp index 50f830b385..e973d0370c 100644 --- a/GPU/Directx9/DrawEngineDX9.cpp +++ b/GPU/Directx9/DrawEngineDX9.cpp @@ -229,8 +229,10 @@ void DrawEngineDX9::Invalidate(InvalidationCallbackFlags flags) { } } -// The inline wrapper in the header checks for numDrawCalls_ == 0 -void DrawEngineDX9::DoFlush() { +void DrawEngineDX9::Flush() { + if (!numDrawVerts_) { + return; + } bool textureNeedsApply = false; if (gstate_c.IsDirty(DIRTY_TEXTURE_IMAGE | DIRTY_TEXTURE_PARAMS) && !gstate.isModeClear() && gstate.isTextureMapEnabled()) { textureCache_->SetTexture(); diff --git a/GPU/Directx9/DrawEngineDX9.h b/GPU/Directx9/DrawEngineDX9.h index 2a21c85eec..6bbf6ffcab 100644 --- a/GPU/Directx9/DrawEngineDX9.h +++ b/GPU/Directx9/DrawEngineDX9.h @@ -68,31 +68,18 @@ public: void BeginFrame(); // So that this can be inlined - void Flush() { - if (!numDrawVerts_) - return; - DoFlush(); - } + void Flush() override; void FinishDeferred() { - if (!numDrawVerts_) - return; DecodeVerts(decoded_); } - void DispatchFlush() override { - if (!numDrawVerts_) - return; - Flush(); - } - protected: // Not currently supported. bool UpdateUseHWTessellation(bool enable) const override { return false; } private: void Invalidate(InvalidationCallbackFlags flags); - void DoFlush(); void ApplyDrawState(int prim); void ApplyDrawStateLate(); diff --git a/GPU/GLES/DrawEngineGLES.cpp b/GPU/GLES/DrawEngineGLES.cpp index 08df5d630b..13adfc2ea9 100644 --- a/GPU/GLES/DrawEngineGLES.cpp +++ b/GPU/GLES/DrawEngineGLES.cpp @@ -230,7 +230,10 @@ void DrawEngineGLES::Invalidate(InvalidationCallbackFlags flags) { } } -void DrawEngineGLES::DoFlush() { +void DrawEngineGLES::Flush() { + if (!numDrawVerts_) { + return; + } PROFILE_THIS_SCOPE("flush"); FrameData &frameData = frameData_[render_->GetCurFrame()]; VShaderID vsid; diff --git a/GPU/GLES/DrawEngineGLES.h b/GPU/GLES/DrawEngineGLES.h index 06264637f7..167635af5a 100644 --- a/GPU/GLES/DrawEngineGLES.h +++ b/GPU/GLES/DrawEngineGLES.h @@ -85,21 +85,8 @@ public: void EndFrame(); // So that this can be inlined - void Flush() { - if (!numDrawVerts_) - return; - DoFlush(); - } - + void Flush() override; void FinishDeferred() { - if (!numDrawVerts_) - return; - DoFlush(); - } - - void DispatchFlush() override { - if (!numDrawVerts_) - return; Flush(); } diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 304173f064..0ba269e5a2 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -47,11 +47,7 @@ #include "GPU/Debugger/Stepping.h" void GPUCommon::Flush() { - drawEngineCommon_->DispatchFlush(); -} - -void GPUCommon::DispatchFlush() { - drawEngineCommon_->DispatchFlush(); + drawEngineCommon_->Flush(); } GPUCommon::GPUCommon(GraphicsContext *gfxCtx, Draw::DrawContext *draw) : @@ -1348,7 +1344,7 @@ void GPUCommon::FlushImm() { bool changed = texturing != prevTexturing || cullEnable != prevCullEnable || dither != prevDither; changed = changed || prevShading != shading || prevFog != fog; if (changed) { - DispatchFlush(); + Flush(); gstate.antiAliasEnable = (GE_CMD_ANTIALIASENABLE << 24) | (int)antialias; gstate.shademodel = (GE_CMD_SHADEMODE << 24) | (int)shading; gstate.cullfaceEnable = (GE_CMD_CULLFACEENABLE << 24) | (int)cullEnable; @@ -1363,7 +1359,7 @@ void GPUCommon::FlushImm() { immFirstSent_ = true; if (changed) { - DispatchFlush(); + Flush(); gstate.antiAliasEnable = (GE_CMD_ANTIALIASENABLE << 24) | (int)prevAntialias; gstate.shademodel = (GE_CMD_SHADEMODE << 24) | (int)prevShading; gstate.cullfaceEnable = (GE_CMD_CULLFACEENABLE << 24) | (int)prevCullEnable; diff --git a/GPU/GPUCommon.h b/GPU/GPUCommon.h index f929db8feb..3e91671b70 100644 --- a/GPU/GPUCommon.h +++ b/GPU/GPUCommon.h @@ -305,9 +305,7 @@ public: static int EstimatePerVertexCost(); - // Note: Not virtual! void Flush(); - void DispatchFlush() override; #ifdef USE_CRT_DBG #undef new diff --git a/GPU/GPUCommonHW.cpp b/GPU/GPUCommonHW.cpp index 34b77b1fc3..08937ee9ba 100644 --- a/GPU/GPUCommonHW.cpp +++ b/GPU/GPUCommonHW.cpp @@ -524,7 +524,7 @@ void GPUCommonHW::CheckFlushOp(int cmd, u32 diff) { if (dumpThisFrame_) { NOTICE_LOG(Log::G3D, "================ FLUSH ================"); } - drawEngineCommon_->DispatchFlush(); + drawEngineCommon_->Flush(); } } @@ -534,7 +534,7 @@ void GPUCommonHW::PreExecuteOp(u32 op, u32 diff) { void GPUCommonHW::CopyDisplayToOutput(bool reallyDirty) { // Flush anything left over. - drawEngineCommon_->DispatchFlush(); + drawEngineCommon_->Flush(); shaderManager_->DirtyLastShader(); @@ -850,7 +850,7 @@ void GPUCommonHW::FastRunLoop(DisplayList &list) { } else { uint64_t flags = info.flags; if (flags & FLAG_FLUSHBEFOREONCHANGE) { - drawEngineCommon_->DispatchFlush(); + drawEngineCommon_->Flush(); } gstate.cmdmem[cmd] = op; if (flags & (FLAG_EXECUTE | FLAG_EXECUTEONCHANGE)) { @@ -1256,7 +1256,7 @@ bail: // flush back cull mode if (cullMode != gstate.getCullMode()) { // We rewrote everything to the old cull mode, so flush first. - drawEngineCommon_->DispatchFlush(); + drawEngineCommon_->Flush(); // Now update things for next time. gstate.cmdmem[GE_CMD_CULL] ^= 1; @@ -1303,7 +1303,7 @@ void GPUCommonHW::Execute_Bezier(u32 op, u32 diff) { // Can't flush after setting gstate_c.submitType below since it'll be a mess - it must be done already. if (flushOnParams_) - drawEngineCommon_->DispatchFlush(); + drawEngineCommon_->Flush(); Spline::BezierSurface surface; surface.tess_u = gstate.getPatchDivisionU(); @@ -1375,7 +1375,7 @@ void GPUCommonHW::Execute_Spline(u32 op, u32 diff) { // Can't flush after setting gstate_c.submitType below since it'll be a mess - it must be done already. if (flushOnParams_) - drawEngineCommon_->DispatchFlush(); + drawEngineCommon_->Flush(); Spline::SplineSurface surface; surface.tess_u = gstate.getPatchDivisionU(); diff --git a/GPU/Software/TransformUnit.cpp b/GPU/Software/TransformUnit.cpp index 2a8e4c72ed..be2e4ea71a 100644 --- a/GPU/Software/TransformUnit.cpp +++ b/GPU/Software/TransformUnit.cpp @@ -66,7 +66,7 @@ void SoftwareDrawEngine::NotifyConfigChanged() { decOptions_.applySkinInDecode = true; } -void SoftwareDrawEngine::DispatchFlush() { +void SoftwareDrawEngine::Flush() { transformUnit.Flush("debug"); } diff --git a/GPU/Software/TransformUnit.h b/GPU/Software/TransformUnit.h index 488a31980f..6945448ec2 100644 --- a/GPU/Software/TransformUnit.h +++ b/GPU/Software/TransformUnit.h @@ -177,7 +177,7 @@ public: void DeviceRestore(Draw::DrawContext *draw) override {} void NotifyConfigChanged() override; - void DispatchFlush() override; + void Flush() override; void DispatchSubmitPrim(const void *verts, const void *inds, GEPrimitiveType prim, int vertexCount, u32 vertType, bool clockwise, int *bytesRead) override; void DispatchSubmitImm(GEPrimitiveType prim, TransformedVertex *buffer, int vertexCount, int cullMode, bool continuation) override; diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index 47b466fc11..5e7ba14a10 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -216,7 +216,11 @@ void DrawEngineVulkan::Invalidate(InvalidationCallbackFlags flags) { } // The inline wrapper in the header checks for numDrawCalls_ == 0 -void DrawEngineVulkan::DoFlush() { +void DrawEngineVulkan::Flush() { + if (!numDrawVerts_) { + return; + } + VulkanRenderManager *renderManager = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); renderManager->AssertInRenderPass(); diff --git a/GPU/Vulkan/DrawEngineVulkan.h b/GPU/Vulkan/DrawEngineVulkan.h index d191fe0f8a..185f66f9a2 100644 --- a/GPU/Vulkan/DrawEngineVulkan.h +++ b/GPU/Vulkan/DrawEngineVulkan.h @@ -123,26 +123,13 @@ public: void DeviceLost() override; void DeviceRestore(Draw::DrawContext *draw) override; - // So that this can be inlined - void Flush() { - if (!numDrawInds_) - return; - DoFlush(); - } + void Flush() override; void FinishDeferred() { - if (!numDrawInds_) - return; // Decode any pending vertices. And also flush while we're at it, for simplicity. // It might be possible to only decode like in the other backends, but meh, it can't matter. // Issue #10095 has a nice example of where this is required. - DoFlush(); - } - - void DispatchFlush() override { - if (!numDrawInds_) - return; - DoFlush(); + Flush(); } VKRPipelineLayout *GetPipelineLayout() const { @@ -183,7 +170,6 @@ private: void DestroyDeviceObjects(); - void DoFlush(); void UpdateUBOs(); NO_INLINE void ResetAfterDraw();