use our own Skia surface when using GPU screen drawing

Previously the code called window context's getBackbufferSurface()
once, and the repeatedly used it for drawing and then did
swapBuffers(). This worked until version chrome/m91, now Skia
requires that a screen drawing pass is calling getBackbufferSurface(),
drawing to it and calling swapBuffers(). Since we do not always
draw full window content and instead keep previous content, use
a separate offscreen surface for that and for actual screen drawing
just blit that to the screen surface.

Change-Id: I36a5b3bb23a085936f4473a0e00d8e04c6b40dab
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120966
Tested-by: Luboš Luňák <l.lunak@collabora.com>
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

4 files changed, 61 insertions, 76 deletions

diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 1a27d3719727..882816514362 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -289,7 +289,11 @@ SkiaSalGraphicsImpl::SkiaSalGraphicsImpl(SalGraphics& rParent, SalGeometryProvid
 {
 }
 
-SkiaSalGraphicsImpl::~SkiaSalGraphicsImpl() { assert(!mSurface); }
+SkiaSalGraphicsImpl::~SkiaSalGraphicsImpl()
+{
+    assert(!mSurface);
+    assert(!mWindowContext);
+}
 
 void SkiaSalGraphicsImpl::Init() {}
 
@@ -382,12 +386,39 @@ void SkiaSalGraphicsImpl::destroySurface()
     // but work around it here.
     if (mSurface)
         mSurface->flushAndSubmit();
-    if (!isOffscreen())
-        destroyWindowSurfaceInternal();
     mSurface.reset();
+    mWindowContext.reset();
     mIsGPU = false;
 }
 
+void SkiaSalGraphicsImpl::flushSurfaceToWindowContext(const SkIRect& rect)
+{
+    sk_sp<SkSurface> screenSurface = mWindowContext->getBackbufferSurface();
+    if (screenSurface != mSurface)
+    {
+        // GPU-based window contexts require calling getBackbufferSurface()
+        // for every swapBuffers(), for this reason mSurface is an offscreen surface
+        // where we keep the contents (LO does not do full redraws).
+        // So here blit the surface to the window context surface and then swap it.
+        assert(isGPU()); // Raster should always draw directly to backbuffer to save copying
+        SkPaint paint;
+        paint.setBlendMode(SkBlendMode::kSrc); // copy as is
+        screenSurface->getCanvas()->drawImage(makeCheckedImageSnapshot(mSurface), 0, 0,
+                                              SkSamplingOptions(), &paint);
+        screenSurface->flushAndSubmit(); // Otherwise the window is not drawn sometimes.
+        mWindowContext->swapBuffers(nullptr); // Must swap the entire surface.
+    }
+    else
+    {
+        // For raster mode use directly the backbuffer surface, it's just a bitmap
+        // surface anyway, and for those there's no real requirement to call
+        // getBackbufferSurface() repeatedly. Using our own surface would duplicate
+        // memory and cost time copying pixels around.
+        assert(!isGPU());
+        mWindowContext->swapBuffers(&rect);
+    }
+}
+
 void SkiaSalGraphicsImpl::DeInit() { destroySurface(); }
 
 void SkiaSalGraphicsImpl::preDraw()
diff --git a/vcl/skia/osx/gdiimpl.cxx b/vcl/skia/osx/gdiimpl.cxx
index 4b771ad372bb..34f343097c1c 100644
--- a/vcl/skia/osx/gdiimpl.cxx
+++ b/vcl/skia/osx/gdiimpl.cxx
@@ -43,20 +43,14 @@ AquaSkiaSalGraphicsImpl::AquaSkiaSalGraphicsImpl(AquaSalGraphics& rParent,
 AquaSkiaSalGraphicsImpl::~AquaSkiaSalGraphicsImpl()
 {
     DeInit(); // mac code doesn't call DeInit()
-    assert(!mWindowContext);
-}
-
-void AquaSkiaSalGraphicsImpl::DeInit()
-{
-    SkiaZone zone;
-    SkiaSalGraphicsImpl::DeInit();
-    mWindowContext.reset();
 }
 
 void AquaSkiaSalGraphicsImpl::freeResources() {}
 
 void AquaSkiaSalGraphicsImpl::createWindowSurfaceInternal(bool forceRaster)
 {
+    assert(!mWindowContext);
+    assert(!mSurface);
     SkiaZone zone;
     sk_app::DisplayParams displayParams;
     displayParams.fColorType = kN32_SkColorType;
@@ -71,15 +65,14 @@ void AquaSkiaSalGraphicsImpl::createWindowSurfaceInternal(bool forceRaster)
             mSurface = createSkSurface(GetWidth(), GetHeight());
             break;
         case RenderMetal:
-            // It appears that Metal surfaces cannot be read from, which may break things
-            // like copyArea(). Additionally sk_app::MetalWindowContext requires
-            // a new call to getBackbufferSurface() after every swapBuffers(), which
-            // normally would also require reading contents of the previous surface,
-            // because we do not redraw the complete area for every draw call.
-            // Handle that by using an offscreen surface and blit to the onscreen surface as necessary.
-            mSurface = createSkSurface(GetWidth(), GetHeight());
             mWindowContext
                 = sk_app::window_context_factory::MakeMetalForMac(macWindow, displayParams);
+            // Like with other GPU contexts, create a proxy offscreen surface (see
+            // flushSurfaceToWindowContext()). Here it's additionally needed because
+            // it appears that Metal surfaces cannot be read from, which would break things
+            // like copyArea().
+            if (mWindowContext)
+                mSurface = createSkSurface(GetWidth(), GetHeight());
             break;
         case RenderVulkan:
             abort();
@@ -87,12 +80,6 @@ void AquaSkiaSalGraphicsImpl::createWindowSurfaceInternal(bool forceRaster)
     }
 }
 
-void AquaSkiaSalGraphicsImpl::destroyWindowSurfaceInternal()
-{
-    mWindowContext.reset();
-    mSurface.reset();
-}
-
 void AquaSkiaSalGraphicsImpl::Flush() { performFlush(); }
 
 void AquaSkiaSalGraphicsImpl::Flush(const tools::Rectangle&) { performFlush(); }
@@ -105,10 +92,10 @@ void AquaSkiaSalGraphicsImpl::performFlush()
     {
         if (mDirtyRect.intersect(SkIRect::MakeWH(GetWidth(), GetHeight())))
         {
-            if (isGPU())
-                flushToScreenMetal(mDirtyRect);
+            if (!isGPU())
+                flushSurfaceToScreenCG(mDirtyRect);
             else
-                flushToScreenRaster(mDirtyRect);
+                flushSurfaceToWindowContext(mDirtyRect);
         }
         mDirtyRect.setEmpty();
     }
@@ -132,7 +119,7 @@ constexpr static uint32_t toCGBitmapType(SkColorType color, SkAlphaType alpha)
 }
 
 // For Raster we use our own screen blitting (see above).
-void AquaSkiaSalGraphicsImpl::flushToScreenRaster(const SkIRect& rect)
+void AquaSkiaSalGraphicsImpl::flushSurfaceToScreenCG(const SkIRect& rect)
 {
     // Based on AquaGraphicsBackend::drawBitmap().
     if (!mrShared.checkContext())
@@ -175,19 +162,6 @@ void AquaSkiaSalGraphicsImpl::flushToScreenRaster(const SkIRect& rect)
     mrShared.refreshRect(rect.left(), rect.top(), rect.width(), rect.height());
 }
 
-// For Metal we flush to the Metal surface and then swap buffers (see above).
-void AquaSkiaSalGraphicsImpl::flushToScreenMetal(const SkIRect&)
-{
-    // The rectangle argument is irrelevant, the whole surface must be used for Metal.
-    sk_sp<SkSurface> screenSurface = mWindowContext->getBackbufferSurface();
-    SkPaint paint;
-    paint.setBlendMode(SkBlendMode::kSrc); // copy as is
-    screenSurface->getCanvas()->drawImage(makeCheckedImageSnapshot(mSurface), 0, 0,
-                                          SkSamplingOptions(), &paint);
-    screenSurface->flushAndSubmit(); // Otherwise the window is not drawn sometimes.
-    mWindowContext->swapBuffers(nullptr);
-}
-
 bool AquaSkiaSalGraphicsImpl::drawNativeControl(ControlType nType, ControlPart nPart,
                                                 const tools::Rectangle& rControlRegion,
                                                 ControlState nState, const ImplControlValue& aValue)
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index c7f1fc1f51e6..6cc4acb42034 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -36,8 +36,6 @@ WinSkiaSalGraphicsImpl::WinSkiaSalGraphicsImpl(WinSalGraphics& rGraphics,
 {
 }
 
-WinSkiaSalGraphicsImpl::~WinSkiaSalGraphicsImpl() { assert(!mWindowContext); }
-
 void WinSkiaSalGraphicsImpl::createWindowSurfaceInternal(bool forceRaster)
 {
     assert(!mWindowContext);
@@ -51,30 +49,20 @@ void WinSkiaSalGraphicsImpl::createWindowSurfaceInternal(bool forceRaster)
         case RenderRaster:
             mWindowContext = sk_app::window_context_factory::MakeRasterForWin(mWinParent.gethWnd(),
                                                                               displayParams);
+            if (mWindowContext)
+                mSurface = mWindowContext->getBackbufferSurface();
             break;
         case RenderVulkan:
             mWindowContext = sk_app::window_context_factory::MakeVulkanForWin(mWinParent.gethWnd(),
                                                                               displayParams);
+            // See flushSurfaceToWindowContext().
+            if (mWindowContext)
+                mSurface = createSkSurface(GetWidth(), GetHeight());
             break;
         case RenderMetal:
             abort();
             break;
     }
-    if (mWindowContext)
-        mSurface = mWindowContext->getBackbufferSurface();
-}
-
-void WinSkiaSalGraphicsImpl::destroyWindowSurfaceInternal()
-{
-    mWindowContext.reset();
-    mSurface.reset();
-}
-
-void WinSkiaSalGraphicsImpl::DeInit()
-{
-    SkiaZone zone;
-    SkiaSalGraphicsImpl::DeInit();
-    mWindowContext.reset();
 }
 
 void WinSkiaSalGraphicsImpl::freeResources() {}
diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx
index ab0207fd94e6..b53e0bd5cea7 100644
--- a/vcl/skia/x11/gdiimpl.cxx
+++ b/vcl/skia/x11/gdiimpl.cxx
@@ -33,8 +33,6 @@ X11SkiaSalGraphicsImpl::X11SkiaSalGraphicsImpl(X11SalGraphics& rParent)
 {
 }
 
-X11SkiaSalGraphicsImpl::~X11SkiaSalGraphicsImpl() { assert(!mWindowContext); }
-
 void X11SkiaSalGraphicsImpl::Init()
 {
     // The m_pFrame and m_pVDev pointers are updated late in X11
@@ -47,17 +45,18 @@ void X11SkiaSalGraphicsImpl::createWindowSurfaceInternal(bool forceRaster)
     assert(!mWindowContext);
     assert(!mSurface);
     assert(mX11Parent.GetDrawable() != None);
+    RenderMethod renderMethod = forceRaster ? RenderRaster : renderMethodToUse();
     mWindowContext = createWindowContext(mX11Parent.GetXDisplay(), mX11Parent.GetDrawable(),
                                          &mX11Parent.GetVisual(), GetWidth(), GetHeight(),
-                                         forceRaster ? RenderRaster : renderMethodToUse(), false);
+                                         renderMethod, false);
     if (mWindowContext)
-        mSurface = mWindowContext->getBackbufferSurface();
-}
-
-void X11SkiaSalGraphicsImpl::destroyWindowSurfaceInternal()
-{
-    mWindowContext.reset();
-    mSurface.reset();
+    {
+        // See flushSurfaceToWindowContext().
+        if (renderMethod == RenderRaster)
+            mWindowContext->getBackbufferSurface();
+        else
+            mSurface = createSkSurface(GetWidth(), GetHeight());
+    }
 }
 
 std::unique_ptr<sk_app::WindowContext>
@@ -140,13 +139,6 @@ bool X11SkiaSalGraphicsImpl::avoidRecreateByResize() const
     return mSurface->width() == int(w) && mSurface->height() == int(h);
 }
 
-void X11SkiaSalGraphicsImpl::DeInit()
-{
-    SkiaZone zone;
-    SkiaSalGraphicsImpl::DeInit();
-    mWindowContext.reset();
-}
-
 void X11SkiaSalGraphicsImpl::freeResources() {}
 
 void X11SkiaSalGraphicsImpl::Flush() { performFlush(); }
@@ -159,7 +151,7 @@ void X11SkiaSalGraphicsImpl::performFlush()
     if (mWindowContext)
     {
         if (mDirtyRect.intersect(SkIRect::MakeWH(GetWidth(), GetHeight())))
-            mWindowContext->swapBuffers(&mDirtyRect);
+            flushSurfaceToWindowContext(mDirtyRect);
         mDirtyRect.setEmpty();
     }
 }