vcl/inc/skia/gdiimpl.hxx | 2 +- vcl/skia/gdiimpl.cxx | 28 ++++++---------------------- vcl/skia/osx/gdiimpl.cxx | 2 +- 3 files changed, 8 insertions(+), 24 deletions(-)
New commits: commit 9cb34aa3c98ac6254b7e70e26e92f3e2ba881fab Author: Luboš Luňák <[email protected]> AuthorDate: Thu Jan 6 18:33:04 2022 +0100 Commit: Luboš Luňák <[email protected]> CommitDate: Wed Jan 12 13:27:37 2022 +0100 reduce explicit Skia flushing The docs say that Skia handles this itself, and with Vulkan excessive flushing may show up in profiling in some cases, as it's apparently a non-trivial operation in GPU mode. Remove even the two workarounds, I cannot reproduce the problems anymore, so let's assume it's been fixed in Skia. But still keep the flushing after a certain number of operations, as too many pending operations still may overload Skia (or Vulkan?), besides tdf#136369 I can also reproduce it while loading bsc#1183308 which does a large number of tiny VirtualDevice's and does GetBitmap() on them. Also change the counter to be global, as we use just one Skia drawing context. Change-Id: I48b96c2a59f8e1eeef3da154dbe373a37857c4be Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128293 Tested-by: Jenkins Reviewed-by: Luboš Luňák <[email protected]> diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx index b5144a249207..b2814b2a6463 100644 --- a/vcl/inc/skia/gdiimpl.hxx +++ b/vcl/inc/skia/gdiimpl.hxx @@ -368,7 +368,7 @@ protected: double transparency; }; LastPolyPolygonInfo mLastPolyPolygonInfo; - int mPendingOperationsToFlush; + inline static int pendingOperationsToFlush = 0; int mScaling; // The scale factor for HiDPI screens. }; diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index 593e2ffb57e4..362508a5cdbe 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -265,7 +265,6 @@ SkiaSalGraphicsImpl::SkiaSalGraphicsImpl(SalGraphics& rParent, SalGeometryProvid , mFillColor(SALCOLOR_NONE) , mXorMode(XorMode::None) , mFlush(new SkiaFlushIdle(this)) - , mPendingOperationsToFlush(0) , mScaling(1) { } @@ -362,15 +361,6 @@ void SkiaSalGraphicsImpl::destroySurface() // if this fails, something forgot to use SkAutoCanvasRestore assert(mSurface->getCanvas()->getTotalMatrix() == SkMatrix::Scale(mScaling, mScaling)); } - // If we use e.g. Vulkan, we must destroy the surface before the context, - // otherwise destroying the surface will reference the context. This is - // handled by calling destroySurface() before destroying the context. - // However we also need to flush the surface before destroying it, - // otherwise when destroying the context later there still could be queued - // commands referring to the surface data. This is probably a Skia bug, - // but work around it here. - if (mSurface) - mSurface->flushAndSubmit(); mSurface.reset(); mWindowContext.reset(); mIsGPU = false; @@ -439,10 +429,12 @@ void SkiaSalGraphicsImpl::postDraw() // But tdf#136369 leads to creating and queueing many tiny bitmaps, which makes // Skia slow, and may make it even run out of memory. So force a flush if such // a problematic operation has been performed too many times without a flush. - if (mPendingOperationsToFlush > 1000) + // Note that the counter is a static variable, as all drawing shares the same Skia drawing + // context (and so the flush here will also flush all drawing). + if (pendingOperationsToFlush > 1000) { mSurface->flushAndSubmit(); - mPendingOperationsToFlush = 0; + pendingOperationsToFlush = 0; } SkiaZone::leave(); // matched in preDraw() // If there's a problem with the GPU context, abort. @@ -540,8 +532,7 @@ void SkiaSalGraphicsImpl::flushDrawing() if (!mSurface) return; checkPendingDrawing(); - mSurface->flushAndSubmit(); - mPendingOperationsToFlush = 0; + ++pendingOperationsToFlush; } void SkiaSalGraphicsImpl::setCanvasScalingAndClipping() @@ -901,13 +892,6 @@ void SkiaSalGraphicsImpl::performDrawPolyPolygon(const basegfx::B2DPolyPolygon& getDrawCanvas()->drawPath(polygonPath, aPaint); } postDraw(); -#if defined LINUX - // WORKAROUND: The logo in the about dialog has drawing errors. This seems to happen - // only on Linux (not Windows on the same machine), with both AMDGPU and Mesa, - // and only when antialiasing is enabled. Flushing seems to avoid the problem. - if (useAA && getVendor() == DriverBlocklist::VendorAMD) - mSurface->flushAndSubmit(); -#endif } namespace @@ -1786,7 +1770,7 @@ void SkiaSalGraphicsImpl::drawImage(const SalTwoRect& rPosAry, const sk_sp<SkIma getDrawCanvas()->drawImageRect(aImage, aSourceRect, aDestinationRect, makeSamplingOptions(rPosAry, mScaling, srcScaling), &aPaint, SkCanvas::kFast_SrcRectConstraint); - ++mPendingOperationsToFlush; // tdf#136369 + ++pendingOperationsToFlush; // tdf#136369 postDraw(); } diff --git a/vcl/skia/osx/gdiimpl.cxx b/vcl/skia/osx/gdiimpl.cxx index 643b38b52557..fa3f1c528322 100644 --- a/vcl/skia/osx/gdiimpl.cxx +++ b/vcl/skia/osx/gdiimpl.cxx @@ -255,7 +255,7 @@ bool AquaSkiaSalGraphicsImpl::drawNativeControl(ControlType nType, ControlPart n boundingRegion.GetWidth(), boundingRegion.GetHeight()); assert(drawRect.width() * mScaling == bitmap.width()); // no scaling should be needed getDrawCanvas()->drawImageRect(bitmap.asImage(), drawRect, SkSamplingOptions()); - ++mPendingOperationsToFlush; // tdf#136369 + ++pendingOperationsToFlush; // tdf#136369 postDraw(); } return bOK;
