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>

8 files changed, 67 insertions, 102 deletions

diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index a7ba10c3273b..c914c26752cc 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -25,13 +25,7 @@
 #include <salgdiimpl.hxx>
 #include <salgeom.hxx>
 
-#include <premac.h>
-#include <SkSurface.h>
-#include <SkRegion.h>
-#include <postmac.h>
-
-#include <prewin.h>
-#include <postwin.h>
+#include <skia/utils.hxx>
 
 class SkiaFlushIdle;
 class GenericSalLayout;
@@ -240,8 +234,8 @@ protected:
     virtual bool avoidRecreateByResize() const;
     void createWindowSurface(bool forceRaster = false);
     virtual void createWindowSurfaceInternal(bool forceRaster = false) = 0;
-    virtual void destroyWindowSurfaceInternal() = 0;
     void createOffscreenSurface();
+    void flushSurfaceToWindowContext(const SkIRect& rect);
 
     void privateDrawAlphaRect(tools::Long nX, tools::Long nY, tools::Long nWidth,
                               tools::Long nHeight, double nTransparency, bool blockAA = false);
@@ -321,6 +315,8 @@ protected:
     SalGeometryProvider* mProvider;
     // The Skia surface that is target of all the rendering.
     sk_sp<SkSurface> mSurface;
+    // Note that mSurface may be a proxy surface and not the one from the window context.
+    std::unique_ptr<sk_app::WindowContext> mWindowContext;
     bool mIsGPU; // whether the surface is GPU-backed
     SkIRect mDirtyRect; // the area that has been changed since the last performFlush()
     vcl::Region mClipRegion;
diff --git a/vcl/inc/skia/osx/gdiimpl.hxx b/vcl/inc/skia/osx/gdiimpl.hxx
index b9adccb370ea..4ffac5985edb 100644
--- a/vcl/inc/skia/osx/gdiimpl.hxx
+++ b/vcl/inc/skia/osx/gdiimpl.hxx
@@ -26,9 +26,7 @@ public:
     AquaSkiaSalGraphicsImpl(AquaSalGraphics& rParent, AquaSharedAttributes& rShared);
     virtual ~AquaSkiaSalGraphicsImpl() override;
 
-    virtual void DeInit() override;
     virtual void freeResources() override;
-    //    virtual void Flush() override;
 
     virtual void UpdateGeometryProvider(SalGeometryProvider* provider) override
     {
@@ -47,13 +45,9 @@ public:
 
 private:
     virtual void createWindowSurfaceInternal(bool forceRaster = false) override;
-    virtual void destroyWindowSurfaceInternal() override;
     virtual void performFlush() override;
-    void flushToScreenRaster(const SkIRect& rect);
-    void flushToScreenMetal(const SkIRect& rect);
+    void flushSurfaceToScreenCG(const SkIRect& rect);
     static inline sk_sp<SkFontMgr> fontManager;
-    // This one is used only for Metal, and only indirectly.
-    std::unique_ptr<sk_app::WindowContext> mWindowContext;
 };
 
 #endif // INCLUDED_VCL_INC_SKIA_OSX_GDIIMPL_HXX
diff --git a/vcl/inc/skia/win/gdiimpl.hxx b/vcl/inc/skia/win/gdiimpl.hxx
index 2f85a9e3cf66..7e3f37ce435c 100644
--- a/vcl/inc/skia/win/gdiimpl.hxx
+++ b/vcl/inc/skia/win/gdiimpl.hxx
@@ -43,10 +43,6 @@ private:
 
 public:
     WinSkiaSalGraphicsImpl(WinSalGraphics& rGraphics, SalGeometryProvider* mpProvider);
-    virtual ~WinSkiaSalGraphicsImpl() override;
-
-    virtual void DeInit() override;
-    virtual void freeResources() override;
 
     virtual bool UseRenderNativeControl() const override { return true; }
     virtual bool TryRenderCachedNativeControl(ControlCacheKey const& rControlCacheKey, int nX,
@@ -57,13 +53,13 @@ public:
     virtual bool DrawTextLayout(const GenericSalLayout& layout) override;
     virtual void ClearDevFontCache() override;
 
+    virtual void freeResources() override;
     virtual void Flush() override;
 
     static void prepareSkia();
 
 protected:
     virtual void createWindowSurfaceInternal(bool forceRaster = false) override;
-    virtual void destroyWindowSurfaceInternal() override;
     virtual void performFlush() override;
     static sk_sp<SkTypeface> createDirectWriteTypeface(HDC hdc, HFONT hfont);
     static void initFontInfo();
@@ -72,7 +68,6 @@ protected:
     inline static sk_sp<SkFontMgr> dwriteFontMgr;
     inline static bool dwriteDone = false;
     static SkFont::Edging fontEdging;
-    std::unique_ptr<sk_app::WindowContext> mWindowContext;
 };
 
 typedef std::pair<ControlCacheKey, sk_sp<SkImage>> SkiaControlCachePair;
diff --git a/vcl/inc/skia/x11/gdiimpl.hxx b/vcl/inc/skia/x11/gdiimpl.hxx
index df9421f54720..d85c7dc0e5c7 100644
--- a/vcl/inc/skia/x11/gdiimpl.hxx
+++ b/vcl/inc/skia/x11/gdiimpl.hxx
@@ -15,7 +15,6 @@
 #include <unx/salgdi.h>
 #include <unx/x11/x11gdiimpl.h>
 #include <skia/gdiimpl.hxx>
-#include <skia/utils.hxx>
 
 class VCL_PLUGIN_PUBLIC X11SkiaSalGraphicsImpl final : public SkiaSalGraphicsImpl,
                                                        public X11GraphicsImpl
@@ -25,10 +24,8 @@ private:
 
 public:
     X11SkiaSalGraphicsImpl(X11SalGraphics& rParent);
-    virtual ~X11SkiaSalGraphicsImpl() override;
 
     virtual void Init() override;
-    virtual void DeInit() override;
     virtual void freeResources() override;
     virtual void Flush() override;
 
@@ -36,14 +33,12 @@ public:
 
 private:
     virtual void createWindowSurfaceInternal(bool forceRaster = false) override;
-    virtual void destroyWindowSurfaceInternal() override;
     virtual void performFlush() override;
     virtual bool avoidRecreateByResize() const override;
     static std::unique_ptr<sk_app::WindowContext>
     createWindowContext(Display* display, Drawable drawable, const XVisualInfo* visual, int width,
                         int height, SkiaHelper::RenderMethod renderMethod, bool temporary);
     friend std::unique_ptr<sk_app::WindowContext> createVulkanWindowContext(bool);
-    std::unique_ptr<sk_app::WindowContext> mWindowContext;
 };
 
 #endif // INCLUDED_VCL_INC_SKIA_X11_GDIIMPL_HXX
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();
     }
 }