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 <l.lunak@collabora.com>

3 files changed, 8 insertions, 24 deletions

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;