more safe handling of destroying Skia surfaces/contexts

As the comment in SkiaSalGraphicsImpl::destroySurface() says,
they may both refer to each other's data when being destroyed,
so try to handle that.

Change-Id: I44353ed9d1888f8e8d15d93cd2c66414adfd372b

4 files changed, 22 insertions, 4 deletions

diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index a5036fbb3075..8190115a0714 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -204,7 +204,8 @@ protected:
     // Call to ensure that mSurface is valid. If mSurface is going to be modified,
     // use preDraw() instead of this.
     void checkSurface();
-    void resetSurface();
+    void recreateSurface();
+    void destroySurface();
 
     void privateDrawAlphaRect(long nX, long nY, long nWidth, long nHeight, double nTransparency);
 
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index d6a956e4e76f..bd9519c7243b 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -180,8 +180,9 @@ SkiaSalGraphicsImpl::~SkiaSalGraphicsImpl() {}
 
 void SkiaSalGraphicsImpl::Init() {}
 
-void SkiaSalGraphicsImpl::resetSurface()
+void SkiaSalGraphicsImpl::recreateSurface()
 {
+    destroySurface();
     createSurface();
     mSurface->getCanvas()->save(); // see SetClipRegion()
     mClipRegion = vcl::Region(tools::Rectangle(0, 0, GetWidth(), GetHeight()));
@@ -198,7 +199,21 @@ void SkiaSalGraphicsImpl::createSurface()
     mSurface = SkSurface::MakeRasterN32Premul(GetWidth(), GetHeight());
 }
 
-void SkiaSalGraphicsImpl::DeInit() { mSurface.reset(); }
+void SkiaSalGraphicsImpl::destroySurface()
+{
+    // 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 destroing 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->flush();
+    mSurface.reset();
+}
+
+void SkiaSalGraphicsImpl::DeInit() { destroySurface(); }
 
 void SkiaSalGraphicsImpl::preDraw() { checkSurface(); }
 
@@ -219,7 +234,7 @@ void SkiaSalGraphicsImpl::postDraw()
 void SkiaSalGraphicsImpl::checkSurface()
 {
     if (!mSurface || GetWidth() != mSurface->width() || GetHeight() != mSurface->height())
-        resetSurface();
+        recreateSurface();
 }
 
 static SkIRect toSkIRect(const tools::Rectangle& rectangle)
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index e583cea86f3c..c1a6d45353cb 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -50,6 +50,7 @@ void WinSkiaSalGraphicsImpl::createSurface()
     // valid here, but better check.
     assert(GetWidth() != 0 && GetHeight() != 0);
     sk_app::DisplayParams displayParams;
+    destroySurface();
     switch (renderMethodToUse())
     {
         case RenderRaster:
diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx
index 72f3e65b5859..29119f092ac9 100644
--- a/vcl/skia/x11/gdiimpl.cxx
+++ b/vcl/skia/x11/gdiimpl.cxx
@@ -55,6 +55,7 @@ void X11SkiaSalGraphicsImpl::createSurface()
     winInfo.fVisualInfo = const_cast<SalVisual*>(&mX11Parent.GetVisual());
     winInfo.fWidth = GetWidth();
     winInfo.fHeight = GetHeight();
+    destroySurface();
     switch (renderMethodToUse())
     {
         case RenderRaster: