[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - vcl/inc vcl/skia

[Jan-Marek Glogowski (via logerrit)]

 vcl/inc/skia/gdiimpl.hxx |    1 -
 vcl/skia/gdiimpl.cxx     |   25 ++++++++++++++++---------
 2 files changed, 16 insertions(+), 10 deletions(-)

New commits:
commit 838ff2e26e484f2601b0c18756e9230cd7a5d9e9
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Mon Nov 16 22:43:51 2020 +0100
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Tue Nov 17 20:29:51 2020 +0100

    tdf#138022 Skia don't recreate empty surfaces
    
    Skia can't create empty surfaces, so the recreation will hit the
    std::abort() in SkiaSalGraphicsImpl::createWindowSurface. Origin
    of the backtrace is some queued Resize event, which will hit
    this a few times via SkiaSalGraphicsImpl::checkSurface.
    
    This feels a bit like tdf#130831, where VCL tried to track damange
    for an empty Qt image...
    
    This also includes commit 6a3a17c8dcd860506781b28b14c3df7036eaaaba
    ("createSurface() should be enough if the surface doesn't exist
    yet"), reviewed on https://gerrit.libreoffice.org/c/core/+/103184
    
    Change-Id: I75e22c987ba633e7a403541db8d580df33c68964
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105963
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
    (cherry picked from commit 42e30c24615402c49351f80cc8a47d61d47267c6)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105992

diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index e84c251cdde1..ca227e4e809d 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -227,7 +227,6 @@ protected:
     // Call to ensure that mSurface is valid. If mSurface is going to be modified,
     // use preDraw() instead of this.
     void checkSurface();
-    void recreateSurface();
     void destroySurface();
     // Reimplemented for X11.
     virtual bool avoidRecreateByResize() const { return false; }
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index fa2f386d6c35..40bd2181c9b1 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -259,12 +259,6 @@ SkiaSalGraphicsImpl::~SkiaSalGraphicsImpl()
 
 void SkiaSalGraphicsImpl::Init() {}
 
-void SkiaSalGraphicsImpl::recreateSurface()
-{
-    destroySurface();
-    createSurface();
-}
-
 void SkiaSalGraphicsImpl::createSurface()
 {
     SkiaZone zone;
@@ -419,13 +413,23 @@ void SkiaSalGraphicsImpl::checkSurface()
 {
     if (!mSurface)
     {
-        recreateSurface();
+        createSurface();
         SAL_INFO("vcl.skia.trace",
                  "create(" << this << "): " << Size(mSurface->width(), mSurface->height()));
     }
     else if (GetWidth() != mSurface->width() || GetHeight() != mSurface->height())
     {
-        if (!avoidRecreateByResize())
+        if (avoidRecreateByResize())
+            return;
+
+        if (!GetWidth() || !GetHeight())
+        {
+            SAL_WARN("vcl.skia", "recreate(" << this << "): can't create empty surface "
+                                             << Size(GetWidth(), GetHeight())
+                                             << " => keeping old one!");
+            return;
+        }
+
         {
             Size oldSize(mSurface->width(), mSurface->height());
             // Recreating a surface means that the old SkSurface contents will be lost.
@@ -440,7 +444,10 @@ void SkiaSalGraphicsImpl::checkSurface()
                 flushDrawing();
                 snapshot = SkiaHelper::makeCheckedImageSnapshot(mSurface);
             }
-            recreateSurface();
+
+            destroySurface();
+            createSurface();
+
             if (snapshot)
             {
                 SkPaint paint;