From 37822b7c9383f7f84323c7ccf81ccb99316f7b33 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Fri, 1 Sep 2023 14:13:13 -0400 Subject: [PATCH] Fix unintentional cpu_sync=true and deprecate sk_sp APIs As per the linked bugs, this was causing a severe performance regression. Change-Id: Ibacf842408cee1eb434795437c8bbe0dc5244c8e Bug: b/291711510, chromium:1475906 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/750403 Reviewed-by: Greg Daniel Commit-Queue: Kevin Lubick (cherry picked from commit 02fa14799c6ca5581f6e21011438d6287b4c5e7a) Reviewed-on: https://skia-review.googlesource.com/c/skia/+/751516 --- include/gpu/GrDirectContext.h | 21 +++++++++++++++++---- relnotes/grdirectctx.md | 2 ++ src/gpu/ganesh/GrDirectContext.cpp | 21 ++++++++++++++++++++- src/gpu/ganesh/surface/SkSurface_Ganesh.cpp | 6 +++--- 4 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 relnotes/grdirectctx.md diff --git a/include/gpu/GrDirectContext.h b/include/gpu/GrDirectContext.h index 65287f93a05d..2dd699d5956f 100644 --- a/include/gpu/GrDirectContext.h +++ b/include/gpu/GrDirectContext.h @@ -462,12 +462,14 @@ class SK_API GrDirectContext : public GrRecordingContext { * @param access type of access the call will do on the backend object after flush * @param info flush options */ - GrSemaphoresSubmitted flush(sk_sp surface, + GrSemaphoresSubmitted flush(SkSurface* surface, SkSurfaces::BackendSurfaceAccess access, const GrFlushInfo& info); - GrSemaphoresSubmitted flush(SkSurface* surface, +#if !defined(SK_DISABLE_LEGACY_GRDIRECTCONTEXT_FLUSH) + GrSemaphoresSubmitted flush(sk_sp surface, SkSurfaces::BackendSurfaceAccess access, const GrFlushInfo& info); +#endif /** * Same as above except: @@ -488,12 +490,15 @@ class SK_API GrDirectContext : public GrRecordingContext { * @param info flush options * @param newState optional state change request after flush */ - GrSemaphoresSubmitted flush(sk_sp surface, + GrSemaphoresSubmitted flush(SkSurface* surface, const GrFlushInfo& info, const skgpu::MutableTextureState* newState = nullptr); - GrSemaphoresSubmitted flush(SkSurface* surface, +#if !defined(SK_DISABLE_LEGACY_GRDIRECTCONTEXT_FLUSH) + // TODO(kjlubick) Remove this variant to be consistent with flushAndSubmit + GrSemaphoresSubmitted flush(sk_sp surface, const GrFlushInfo& info, const skgpu::MutableTextureState* newState = nullptr); +#endif /** Call to ensure all reads/writes of the surface have been issued to the underlying 3D API. * Skia will correctly order its own draws and pixel operations. This must to be used to ensure @@ -503,14 +508,22 @@ class SK_API GrDirectContext : public GrRecordingContext { * * Has no effect on a CPU-backed surface. */ + void flushAndSubmit(SkSurface* surface, bool syncCpu = false); +#if !defined(SK_DISABLE_LEGACY_GRDIRECTCONTEXT_FLUSH) + // TODO(kjlubick) remove this as it is error prone https://crbug.com/1475906 void flushAndSubmit(sk_sp surface, bool syncCpu = false); +#endif /** * Flushes the given surface with the default GrFlushInfo. * * Has no effect on a CPU-backed surface. */ + void flush(SkSurface* surface); +#if !defined(SK_DISABLE_LEGACY_GRDIRECTCONTEXT_FLUSH) + // TODO(kjlubick) Remove this variant to be consistent with flushAndSubmit void flush(sk_sp surface); +#endif /** * Submit outstanding work to the gpu from all previously un-submitted flushes. The return diff --git a/relnotes/grdirectctx.md b/relnotes/grdirectctx.md new file mode 100644 index 000000000000..5458aeb37559 --- /dev/null +++ b/relnotes/grdirectctx.md @@ -0,0 +1,2 @@ +`GrDirectContext::flush` variants now expect a SkSurface pointer only, not +an sk_sp. \ No newline at end of file diff --git a/src/gpu/ganesh/GrDirectContext.cpp b/src/gpu/ganesh/GrDirectContext.cpp index c34d1e745f2e..e27fffb3d062 100644 --- a/src/gpu/ganesh/GrDirectContext.cpp +++ b/src/gpu/ganesh/GrDirectContext.cpp @@ -496,11 +496,13 @@ void GrDirectContext::flushAndSubmit(sk_sp image) { this->submit(); } +#if !defined(SK_DISABLE_LEGACY_GRDIRECTCONTEXT_FLUSH) GrSemaphoresSubmitted GrDirectContext::flush(sk_sp surface, SkSurfaces::BackendSurfaceAccess access, const GrFlushInfo& info) { return this->flush(surface.get(), access, info); } +#endif GrSemaphoresSubmitted GrDirectContext::flush(SkSurface* surface, SkSurfaces::BackendSurfaceAccess access, @@ -512,6 +514,7 @@ GrSemaphoresSubmitted GrDirectContext::flush(SkSurface* surface, if (!sb->isGaneshBacked()) { return GrSemaphoresSubmitted::kNo; } + auto gs = static_cast(surface); SkASSERT(this->priv().matches(gs->getDevice()->recordingContext()->asDirectContext())); GrRenderTargetProxy* rtp = gs->getDevice()->targetProxy(); @@ -519,11 +522,13 @@ GrSemaphoresSubmitted GrDirectContext::flush(SkSurface* surface, return this->priv().flushSurface(rtp, access, info, nullptr); } +#if !defined(SK_DISABLE_LEGACY_GRDIRECTCONTEXT_FLUSH) GrSemaphoresSubmitted GrDirectContext::flush(sk_sp surface, const GrFlushInfo& info, const skgpu::MutableTextureState* newState) { return this->flush(surface.get(), info, newState); } +#endif GrSemaphoresSubmitted GrDirectContext::flush(SkSurface* surface, const GrFlushInfo& info, @@ -535,6 +540,7 @@ GrSemaphoresSubmitted GrDirectContext::flush(SkSurface* surface, if (!sb->isGaneshBacked()) { return GrSemaphoresSubmitted::kNo; } + auto gs = static_cast(surface); SkASSERT(this->priv().matches(gs->getDevice()->recordingContext()->asDirectContext())); GrRenderTargetProxy* rtp = gs->getDevice()->targetProxy(); @@ -543,14 +549,27 @@ GrSemaphoresSubmitted GrDirectContext::flush(SkSurface* surface, rtp, SkSurfaces::BackendSurfaceAccess::kNoAccess, info, newState); } +void GrDirectContext::flushAndSubmit(SkSurface* surface, bool syncCpu) { + this->flush(surface, SkSurfaces::BackendSurfaceAccess::kNoAccess, GrFlushInfo()); + this->submit(syncCpu); +} + +#if !defined(SK_DISABLE_LEGACY_GRDIRECTCONTEXT_FLUSH) void GrDirectContext::flushAndSubmit(sk_sp surface, bool syncCpu) { this->flush(surface.get(), SkSurfaces::BackendSurfaceAccess::kNoAccess, GrFlushInfo()); this->submit(syncCpu); } +#endif +void GrDirectContext::flush(SkSurface* surface) { + this->flush(surface, GrFlushInfo(), nullptr); +} + +#if !defined(SK_DISABLE_LEGACY_GRDIRECTCONTEXT_FLUSH) void GrDirectContext::flush(sk_sp surface) { - this->flush(surface.get(), GrFlushInfo(), nullptr); + this->flush(surface, GrFlushInfo(), nullptr); } +#endif //////////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/ganesh/surface/SkSurface_Ganesh.cpp b/src/gpu/ganesh/surface/SkSurface_Ganesh.cpp index 64492cfb05b3..5d7667b58af5 100644 --- a/src/gpu/ganesh/surface/SkSurface_Ganesh.cpp +++ b/src/gpu/ganesh/surface/SkSurface_Ganesh.cpp @@ -775,7 +775,7 @@ GrSemaphoresSubmitted Flush(sk_sp surface) { return GrSemaphoresSubmitted::kNo; } if (auto rContext = surface->recordingContext(); rContext != nullptr) { - return rContext->asDirectContext()->flush(surface, {}); + return rContext->asDirectContext()->flush(surface.get(), {}); } return GrSemaphoresSubmitted::kNo; } @@ -785,7 +785,7 @@ void FlushAndSubmit(SkSurface* surface) { return; } if (auto rContext = surface->recordingContext(); rContext != nullptr) { - rContext->asDirectContext()->flushAndSubmit(surface); + rContext->asDirectContext()->flushAndSubmit(surface, false); } } @@ -794,7 +794,7 @@ void FlushAndSubmit(sk_sp surface) { return; } if (auto rContext = surface->recordingContext(); rContext != nullptr) { - rContext->asDirectContext()->flushAndSubmit(surface); + rContext->asDirectContext()->flushAndSubmit(surface.get(), false); } }