vcl/inc/skia/gdiimpl.hxx     |   12 +++-------
 vcl/inc/skia/osx/gdiimpl.hxx |    8 ------
 vcl/inc/skia/win/gdiimpl.hxx |    7 ------
 vcl/inc/skia/x11/gdiimpl.hxx |    5 ----
 vcl/skia/gdiimpl.cxx         |   37 +++++++++++++++++++++++++++++--
 vcl/skia/osx/gdiimpl.cxx     |   50 ++++++++++---------------------------------
 vcl/skia/win/gdiimpl.cxx     |   22 ++++--------------
 vcl/skia/x11/gdiimpl.cxx     |   28 ++++++++----------------
 8 files changed, 67 insertions(+), 102 deletions(-)

New commits:
commit 148ac7de372d0af3c3d20a5c94c12ca0b9cfd4ab
Author:     Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue Aug 24 21:32:20 2021 +0200
Commit:     Luboš Luňák <l.lu...@collabora.com>
CommitDate: Wed Aug 25 11:36:59 2021 +0200

    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.lu...@collabora.com>
    Reviewed-by: Luboš Luňák <l.lu...@collabora.com>

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();
     }
 }

Reply via email to