[Libreoffice-commits] core.git: 3 commits - include/vcl vcl/backendtest vcl/inc vcl/skia vcl/unx vcl/win

Luboš Luňák (via logerrit) logerrit at kemper.freedesktop.org
Fri Dec 6 13:28:26 UTC 2019


 include/vcl/skia/SkiaHelper.hxx       |   34 +++---
 vcl/backendtest/VisualBackendTest.cxx |    9 +
 vcl/inc/skia/gdiimpl.hxx              |   12 --
 vcl/inc/skia/salbmp.hxx               |   32 +++--
 vcl/inc/skia/utils.hxx                |   56 ++++++++++
 vcl/skia/SkiaHelper.cxx               |  131 ++++++++++++++++++++++-
 vcl/skia/gdiimpl.cxx                  |  188 +++++++++++++---------------------
 vcl/skia/salbmp.cxx                   |  164 +++++++++++++++++------------
 vcl/skia/win/gdiimpl.cxx              |   34 +++---
 vcl/unx/generic/app/salinst.cxx       |    4 
 vcl/win/app/salinst.cxx               |    3 
 11 files changed, 423 insertions(+), 244 deletions(-)

New commits:
commit 8fede4e7870579251c58823566c88476df2038b0
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Wed Dec 4 18:01:47 2019 +0100
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Dec 6 14:26:45 2019 +0100

    make all Skia drawing GPU-backed, if possible
    
    This primarily means using SkiaHelper::createSkSurface(), which will
    create a GPU-backed SkSurface if Vulkan is used, and it is used in place
    of temporary SkBitmap instances, which are always raster-based.
    
    Change-Id: I3fe35866f962030f464d5c1d1c4bf518c20ee9af
    Reviewed-on: https://gerrit.libreoffice.org/84562
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>

diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index 7365d58f9173..d54c09d67eb8 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -193,10 +193,11 @@ public:
 
 #ifdef DBG_UTIL
     void dump(const char* file) const;
-    static void dump(const SkBitmap& bitmap, const char* file);
 #endif
 
     // Default blend mode for SkPaint is SkBlendMode::kSrcOver
+    void drawImage(const SalTwoRect& rPosAry, const sk_sp<SkImage>& aImage,
+                   SkBlendMode eBlendMode = SkBlendMode::kSrcOver);
     void drawBitmap(const SalTwoRect& rPosAry, const SkBitmap& aBitmap,
                     SkBlendMode eBlendMode = SkBlendMode::kSrcOver);
 
@@ -237,7 +238,7 @@ protected:
     // get the height of the device
     int GetHeight() const { return mProvider ? mProvider->GetHeight() : 1; }
 
-    void drawMask(const SalTwoRect& rPosAry, const SkImage& rImage, Color nMaskColor);
+    void drawMask(const SalTwoRect& rPosAry, const sk_sp<SkImage>& rImage, Color nMaskColor);
 
     // When drawing using GPU, rounding errors may result in off-by-one errors,
     // see https://bugs.chromium.org/p/skia/issues/detail?id=9611 . Compensate for
@@ -246,10 +247,6 @@ protected:
     SkScalar toSkX(long x) const { return mIsGPU ? x + 0.5 : x; }
     SkScalar toSkY(long y) const { return mIsGPU ? y + 0.5 : y; }
 
-#ifdef DBG_UTIL
-    void prefillSurface();
-#endif
-
     template <typename charT, typename traits>
     friend inline std::basic_ostream<charT, traits>&
     operator<<(std::basic_ostream<charT, traits>& stream, const SkiaSalGraphicsImpl* graphics)
diff --git a/vcl/inc/skia/salbmp.hxx b/vcl/inc/skia/salbmp.hxx
index 0b0e1aa5439c..ea25d477c985 100644
--- a/vcl/inc/skia/salbmp.hxx
+++ b/vcl/inc/skia/salbmp.hxx
@@ -22,15 +22,13 @@
 
 #include <salbmp.hxx>
 
-#include <SkBitmap.h>
-
-class SkImage;
+#include <SkImage.h>
 
 class VCL_PLUGIN_PUBLIC SkiaSalBitmap : public SalBitmap
 {
 public:
     SkiaSalBitmap();
-    SkiaSalBitmap(const SkImage& image);
+    SkiaSalBitmap(const sk_sp<SkImage>& image);
     virtual ~SkiaSalBitmap() override;
 
     // SalBitmap methods
@@ -59,12 +57,11 @@ public:
                          sal_uInt8 nTol) override;
     virtual bool ConvertToGreyscale() override;
 
-    // Accesses the internal SkBitmap. If the bit count is one that Skia does
-    // not support natively, data from the internal buffer is converted
-    // to a 32bpp SkBitmap.
-    const SkBitmap& GetSkBitmap() const;
+    // Returns the contents as SkImage (possibly GPU-backed).
+    const sk_sp<SkImage>& GetSkImage() const;
 
-    const SkBitmap& GetAlphaSkBitmap() const;
+    // Returns the contents as alpha SkImage (possibly GPU-backed)
+    const sk_sp<SkImage>& GetAlphaSkImage() const;
 
 #ifdef DBG_UTIL
     void dump(const char* file) const;
@@ -72,6 +69,7 @@ public:
 
 private:
     void ResetCachedBitmap();
+    SkBitmap GetAsSkBitmap() const;
 #ifdef DBG_UTIL
     void verify() const;
 #else
@@ -82,16 +80,18 @@ private:
     friend inline std::basic_ostream<charT, traits>&
     operator<<(std::basic_ostream<charT, traits>& stream, const SkiaSalBitmap* bitmap)
     { // TODO GPU-based, once it's done
-        // B - has SkBitmap, A - has alpha SkBitmap, D - has data buffer
+        // B - has SkBitmap, D - has data buffer, I/i - has SkImage (on GPU/CPU),
+        // A/a - has alpha SkImage (on GPU/CPU)
         return stream << static_cast<const void*>(bitmap) << " " << bitmap->GetSize() << "/"
                       << bitmap->mBitCount << (!bitmap->mBitmap.drawsNothing() ? "B" : "")
-                      << (!bitmap->mAlphaBitmap.drawsNothing() ? "A" : "")
-                      << (bitmap->mBuffer.get() ? "D" : "");
+                      << (bitmap->mBuffer.get() ? "D" : "")
+                      << (bitmap->mImage ? (bitmap->mImage->isTextureBacked() ? "I" : "i") : "")
+                      << (bitmap->mAlphaImage ? (bitmap->mAlphaImage->isTextureBacked() ? "A" : "a")
+                                              : "");
     }
 
-    // TODO use something GPU-backed, or at least cache it for when drawing it to something GPU-backed?
+    // Bitmap pixels, if format is supported by Skia. If not, mBuffer is used.
     SkBitmap mBitmap;
-    SkBitmap mAlphaBitmap; // TODO for use as an alpha channel or mask
     BitmapPalette mPalette;
     int mBitCount; // bpp
     Size mSize;
@@ -99,6 +99,8 @@ private:
     // in a buffer (and converted to 32bpp SkBitmap on-demand using GetSkBitmap()).
     std::unique_ptr<sal_uInt8[]> mBuffer;
     int mScanlineSize; // size of one row in mBuffer
+    sk_sp<SkImage> mImage; // cached contents, possibly GPU-backed
+    sk_sp<SkImage> mAlphaImage; // cached contents as alpha image, possibly GPU-backed
 };
 
 #endif // INCLUDED_VCL_INC_SKIA_SALBMP_H
diff --git a/vcl/inc/skia/utils.hxx b/vcl/inc/skia/utils.hxx
index 5d824400030a..27ca2b1f0b72 100644
--- a/vcl/inc/skia/utils.hxx
+++ b/vcl/inc/skia/utils.hxx
@@ -22,6 +22,8 @@
 
 #include <vcl/skia/SkiaHelper.hxx>
 
+#include <tools/gen.hxx>
+
 #include <tools/sk_app/VulkanWindowContext.h>
 
 namespace SkiaHelper
@@ -31,6 +33,22 @@ GrContext* getSharedGrContext();
 
 void disableRenderMethod(RenderMethod method);
 
+// Create SkSurface, GPU-backed if possible.
+VCL_DLLPUBLIC sk_sp<SkSurface> createSkSurface(int width, int height,
+                                               SkColorType type = kN32_SkColorType);
+
+inline sk_sp<SkSurface> createSkSurface(const Size& size, SkColorType type = kN32_SkColorType)
+{
+    return createSkSurface(size.Width(), size.Height(), type);
+}
+
+#ifdef DBG_UTIL
+void prefillSurface(sk_sp<SkSurface>& surface);
+void dump(const SkBitmap& bitmap, const char* file);
+void dump(const sk_sp<SkImage>& image, const char* file);
+void dump(const sk_sp<SkSurface>& surface, const char* file);
+#endif
+
 } // namespace
 
 #endif // INCLUDED_VCL_INC_SKIA_UTILS_H
diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx
index 471d47f01f69..6949c2d9fe40 100644
--- a/vcl/skia/SkiaHelper.cxx
+++ b/vcl/skia/SkiaHelper.cxx
@@ -25,8 +25,13 @@ bool isVCLSkiaEnabled() { return false; }
 
 #include <skia/utils.hxx>
 
+#include <SkSurface.h>
 #include <tools/sk_app/VulkanWindowContext.h>
 
+#ifdef DBG_UTIL
+#include <fstream>
+#endif
+
 namespace SkiaHelper
 {
 static bool supportsVCLSkia()
@@ -151,12 +156,81 @@ GrContext* getSharedGrContext()
     return nullptr;
 }
 
+sk_sp<SkSurface> createSkSurface(int width, int height, SkColorType type)
+{
+    assert(type == kN32_SkColorType || type == kAlpha_8_SkColorType);
+    sk_sp<SkSurface> surface;
+    switch (SkiaHelper::renderMethodToUse())
+    {
+        case SkiaHelper::RenderVulkan:
+        {
+            if (GrContext* grContext = getSharedGrContext())
+            {
+                surface = SkSurface::MakeRenderTarget(
+                    grContext, SkBudgeted::kNo,
+                    SkImageInfo::Make(width, height, type, kPremul_SkAlphaType));
+                assert(surface);
+#ifdef DBG_UTIL
+                prefillSurface(surface);
+#endif
+                return surface;
+            }
+            break;
+        }
+        default:
+            break;
+    }
+    // Create raster surface as a fallback.
+    surface = SkSurface::MakeRaster(SkImageInfo::Make(width, height, type, kPremul_SkAlphaType));
+    assert(surface);
+#ifdef DBG_UTIL
+    prefillSurface(surface);
+#endif
+    return surface;
+}
+
 void cleanup()
 {
     delete sharedGrContext;
     sharedGrContext = nullptr;
 }
 
+#ifdef DBG_UTIL
+void prefillSurface(sk_sp<SkSurface>& surface)
+{
+    // Pre-fill the surface with deterministic garbage.
+    SkBitmap bitmap;
+    bitmap.allocN32Pixels(2, 2);
+    SkPMColor* scanline;
+    scanline = bitmap.getAddr32(0, 0);
+    *scanline++ = SkPreMultiplyARGB(0xFF, 0xBF, 0x80, 0x40);
+    *scanline++ = SkPreMultiplyARGB(0xFF, 0x40, 0x80, 0xBF);
+    scanline = bitmap.getAddr32(0, 1);
+    *scanline++ = SkPreMultiplyARGB(0xFF, 0xE3, 0x5C, 0x13);
+    *scanline++ = SkPreMultiplyARGB(0xFF, 0x13, 0x5C, 0xE3);
+    SkPaint paint;
+    paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
+    paint.setShader(bitmap.makeShader(SkTileMode::kRepeat, SkTileMode::kRepeat));
+    surface->getCanvas()->drawPaint(paint);
+}
+
+void dump(const SkBitmap& bitmap, const char* file) { dump(SkImage::MakeFromBitmap(bitmap), file); }
+
+void dump(const sk_sp<SkSurface>& surface, const char* file)
+{
+    surface->getCanvas()->flush();
+    dump(surface->makeImageSnapshot(), file);
+}
+
+void dump(const sk_sp<SkImage>& image, const char* file)
+{
+    sk_sp<SkData> data = image->encodeToData();
+    std::ofstream ostream(file, std::ios::binary);
+    ostream.write(static_cast<const char*>(data->data()), data->size());
+}
+
+#endif
+
 } // namespace
 
 #endif // HAVE_FEATURE_SKIA
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 6a977ab6b598..ddcd9b75d30f 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -35,10 +35,6 @@
 
 #include <basegfx/polygon/b2dpolygontools.hxx>
 
-#ifdef DBG_UTIL
-#include <fstream>
-#endif
-
 namespace
 {
 // Create Skia Path from B2DPolygon
@@ -199,9 +195,6 @@ void SkiaSalGraphicsImpl::createSurface()
         createOffscreenSurface();
     else
         createWindowSurface();
-#ifdef DBG_UTIL
-    prefillSurface();
-#endif
     mSurface->getCanvas()->save(); // see SetClipRegion()
     mClipRegion = vcl::Region(tools::Rectangle(0, 0, GetWidth(), GetHeight()));
 
@@ -232,6 +225,10 @@ void SkiaSalGraphicsImpl::createWindowSurface()
                 abort(); // this should not really happen
         }
     }
+    assert((mSurface->getCanvas()->getGrContext() != nullptr) == mIsGPU);
+#ifdef DBG_UTIL
+    SkiaHelper::prefillSurface(mSurface);
+#endif
 }
 
 void SkiaSalGraphicsImpl::createOffscreenSurface()
@@ -259,14 +256,10 @@ void SkiaSalGraphicsImpl::createOffscreenSurface()
             }
             if (grContext)
             {
-                mSurface = SkSurface::MakeRenderTarget(
-                    grContext, SkBudgeted::kNo,
-                    SkImageInfo::MakeN32Premul(GetWidth(), GetHeight()));
-                mIsGPU = true;
+                mSurface = SkiaHelper::createSkSurface(GetWidth(), GetHeight());
                 assert(mSurface);
-#ifdef DBG_UTIL
-                prefillSurface();
-#endif
+                assert(mSurface->getCanvas()->getGrContext()); // is GPU-backed
+                mIsGPU = true;
                 return;
             }
             SAL_WARN("vcl.skia", "cannot create Vulkan offscreen GPU surface, disabling Vulkan");
@@ -276,9 +269,10 @@ void SkiaSalGraphicsImpl::createOffscreenSurface()
         default:
             break;
     }
-    // Create raster surface. Subclasses will create GPU-backed surfaces as appropriate.
-    mSurface = SkSurface::MakeRasterN32Premul(GetWidth(), GetHeight());
+    // Create raster surface as a fallback.
+    mSurface = SkiaHelper::createSkSurface(GetWidth(), GetHeight());
     assert(mSurface);
+    assert(!mSurface->getCanvas()->getGrContext()); // is not GPU-backed
     mIsGPU = false;
 }
 
@@ -768,10 +762,8 @@ bool SkiaSalGraphicsImpl::blendBitmap(const SalTwoRect& rPosAry, const SalBitmap
         return false;
 
     assert(dynamic_cast<const SkiaSalBitmap*>(&rBitmap));
-
     const SkiaSalBitmap& rSkiaBitmap = static_cast<const SkiaSalBitmap&>(rBitmap);
-    drawBitmap(rPosAry, rSkiaBitmap.GetSkBitmap(), SkBlendMode::kMultiply);
-
+    drawImage(rPosAry, rSkiaBitmap.GetSkImage(), SkBlendMode::kMultiply);
     return true;
 }
 
@@ -787,31 +779,27 @@ bool SkiaSalGraphicsImpl::blendAlphaBitmap(const SalTwoRect& rPosAry,
     assert(dynamic_cast<const SkiaSalBitmap*>(&rMaskBitmap));
     assert(dynamic_cast<const SkiaSalBitmap*>(&rAlphaBitmap));
 
-    SkBitmap aTempBitmap;
-    if (!aTempBitmap.tryAllocN32Pixels(rSourceBitmap.GetSize().Width(),
-                                       rSourceBitmap.GetSize().Height()))
-    {
+    sk_sp<SkSurface> tmpSurface = SkiaHelper::createSkSurface(rSourceBitmap.GetSize());
+    if (!tmpSurface)
         return false;
-    }
 
     const SkiaSalBitmap& rSkiaSourceBitmap = static_cast<const SkiaSalBitmap&>(rSourceBitmap);
     const SkiaSalBitmap& rSkiaMaskBitmap = static_cast<const SkiaSalBitmap&>(rMaskBitmap);
     const SkiaSalBitmap& rSkiaAlphaBitmap = static_cast<const SkiaSalBitmap&>(rAlphaBitmap);
 
-    SkCanvas aCanvas(aTempBitmap);
+    SkCanvas* aCanvas = tmpSurface->getCanvas();
     SkPaint aPaint;
 
     aPaint.setBlendMode(SkBlendMode::kSrc);
-    aCanvas.drawBitmap(rSkiaMaskBitmap.GetAlphaSkBitmap(), 0, 0, &aPaint);
-
-    aPaint.setBlendMode(SkBlendMode::kSrcIn);
-    aCanvas.drawBitmap(rSkiaAlphaBitmap.GetAlphaSkBitmap(), 0, 0, &aPaint);
+    aCanvas->drawImage(rSkiaSourceBitmap.GetSkImage(), 0, 0, &aPaint);
 
-    aPaint.setBlendMode(SkBlendMode::kSrcOut);
-    aCanvas.drawBitmap(rSkiaSourceBitmap.GetSkBitmap(), 0, 0, &aPaint);
-
-    drawBitmap(rPosAry, aTempBitmap);
+    // Apply cumulatively both the bitmap alpha and the device alpha.
+    aPaint.setBlendMode(SkBlendMode::kDstOut); // VCL alpha is one-minus-alpha
+    aCanvas->drawImage(rSkiaMaskBitmap.GetAlphaSkImage(), 0, 0, &aPaint);
+    aPaint.setBlendMode(SkBlendMode::kDstOut); // VCL alpha is one-minus-alpha
+    aCanvas->drawImage(rSkiaAlphaBitmap.GetAlphaSkImage(), 0, 0, &aPaint);
 
+    drawImage(rPosAry, tmpSurface->makeImageSnapshot());
     return true;
 }
 
@@ -823,7 +811,7 @@ void SkiaSalGraphicsImpl::drawBitmap(const SalTwoRect& rPosAry, const SalBitmap&
     assert(dynamic_cast<const SkiaSalBitmap*>(&rSalBitmap));
     const SkiaSalBitmap& rSkiaSourceBitmap = static_cast<const SkiaSalBitmap&>(rSalBitmap);
 
-    drawBitmap(rPosAry, rSkiaSourceBitmap.GetSkBitmap());
+    drawImage(rPosAry, rSkiaSourceBitmap.GetSkImage());
 }
 
 void SkiaSalGraphicsImpl::drawBitmap(const SalTwoRect& rPosAry, const SalBitmap& rSalBitmap,
@@ -836,26 +824,24 @@ void SkiaSalGraphicsImpl::drawMask(const SalTwoRect& rPosAry, const SalBitmap& r
                                    Color nMaskColor)
 {
     assert(dynamic_cast<const SkiaSalBitmap*>(&rSalBitmap));
-    sk_sp<SkImage> image
-        = SkImage::MakeFromBitmap(static_cast<const SkiaSalBitmap&>(rSalBitmap).GetAlphaSkBitmap());
-    drawMask(rPosAry, *image, nMaskColor);
+    drawMask(rPosAry, static_cast<const SkiaSalBitmap&>(rSalBitmap).GetAlphaSkImage(), nMaskColor);
 }
 
-void SkiaSalGraphicsImpl::drawMask(const SalTwoRect& rPosAry, const SkImage& rImage,
+void SkiaSalGraphicsImpl::drawMask(const SalTwoRect& rPosAry, const sk_sp<SkImage>& rImage,
                                    Color nMaskColor)
 {
-    SkBitmap tmpBitmap;
-    if (!tmpBitmap.tryAllocN32Pixels(rImage.width(), rImage.height()))
-        abort();
-    tmpBitmap.eraseColor(toSkColor(nMaskColor));
+    SAL_INFO("vcl.skia", "drawmask(" << this << "): " << rPosAry << ":" << nMaskColor);
+    sk_sp<SkSurface> tmpSurface = SkiaHelper::createSkSurface(rImage->width(), rImage->height());
+    assert(tmpSurface);
+    SkCanvas* canvas = tmpSurface->getCanvas();
+    canvas->clear(toSkColor(nMaskColor));
     SkPaint paint;
     // Draw the color with the given mask.
-    // TODO figure out the right blend mode to avoid the temporary bitmap
+    // TODO figure out the right blend mode to avoid the temporary surface
     paint.setBlendMode(SkBlendMode::kDstOut);
-    SkCanvas canvas(tmpBitmap);
-    canvas.drawImage(&rImage, 0, 0, &paint);
+    canvas->drawImage(rImage, 0, 0, &paint);
 
-    drawBitmap(rPosAry, tmpBitmap);
+    drawImage(rPosAry, tmpSurface->makeImageSnapshot());
 }
 
 std::shared_ptr<SalBitmap> SkiaSalGraphicsImpl::getBitmap(long nX, long nY, long nWidth,
@@ -866,7 +852,7 @@ std::shared_ptr<SalBitmap> SkiaSalGraphicsImpl::getBitmap(long nX, long nY, long
              "getbitmap(" << this << "): " << Point(nX, nY) << "/" << Size(nWidth, nHeight));
     mSurface->getCanvas()->flush();
     sk_sp<SkImage> image = mSurface->makeImageSnapshot(SkIRect::MakeXYWH(nX, nY, nWidth, nHeight));
-    return std::make_shared<SkiaSalBitmap>(*image);
+    return std::make_shared<SkiaSalBitmap>(image);
 }
 
 Color SkiaSalGraphicsImpl::getPixel(long nX, long nY)
@@ -922,7 +908,7 @@ void SkiaSalGraphicsImpl::invert(basegfx::B2DPolygon const& rPoly, SalInvert eFl
         if (eFlags == SalInvert::N50)
         {
             // This creates 4x4 checker pattern bitmap
-            // TODO Cache the bitmap
+            // TODO Use SkiaHelper::createSkSurface() and cache the image
             SkBitmap aBitmap;
             aBitmap.allocN32Pixels(4, 4);
             const SkPMColor white = SkPreMultiplyARGB(0xFF, 0xFF, 0xFF, 0xFF);
@@ -989,21 +975,37 @@ bool SkiaSalGraphicsImpl::drawAlphaBitmap(const SalTwoRect& rPosAry, const SalBi
 {
     assert(dynamic_cast<const SkiaSalBitmap*>(&rSourceBitmap));
     assert(dynamic_cast<const SkiaSalBitmap*>(&rAlphaBitmap));
-    SkBitmap tmpBitmap;
-    if (!tmpBitmap.tryAllocN32Pixels(rSourceBitmap.GetSize().Width(),
-                                     rSourceBitmap.GetSize().Height()))
+    sk_sp<SkSurface> tmpSurface = SkiaHelper::createSkSurface(rSourceBitmap.GetSize());
+    if (!tmpSurface)
         return false;
-    SkCanvas canvas(tmpBitmap);
+    SkCanvas* canvas = tmpSurface->getCanvas();
     SkPaint paint;
     paint.setBlendMode(SkBlendMode::kSrc); // copy as is, including alpha
-    canvas.drawBitmap(static_cast<const SkiaSalBitmap&>(rSourceBitmap).GetSkBitmap(), 0, 0, &paint);
-    paint.setBlendMode(SkBlendMode::kDstOut);
-    canvas.drawBitmap(static_cast<const SkiaSalBitmap&>(rAlphaBitmap).GetAlphaSkBitmap(), 0, 0,
+    canvas->drawImage(static_cast<const SkiaSalBitmap&>(rSourceBitmap).GetSkImage(), 0, 0, &paint);
+    paint.setBlendMode(SkBlendMode::kDstOut); // VCL alpha is one-minus-alpha
+    canvas->drawImage(static_cast<const SkiaSalBitmap&>(rAlphaBitmap).GetAlphaSkImage(), 0, 0,
                       &paint);
-    drawBitmap(rPosAry, tmpBitmap);
+    drawImage(rPosAry, tmpSurface->makeImageSnapshot());
     return true;
 }
 
+void SkiaSalGraphicsImpl::drawImage(const SalTwoRect& rPosAry, const sk_sp<SkImage>& aImage,
+                                    SkBlendMode eBlendMode)
+{
+    SkRect aSourceRect
+        = SkRect::MakeXYWH(rPosAry.mnSrcX, rPosAry.mnSrcY, rPosAry.mnSrcWidth, rPosAry.mnSrcHeight);
+    SkRect aDestinationRect = SkRect::MakeXYWH(rPosAry.mnDestX, rPosAry.mnDestY,
+                                               rPosAry.mnDestWidth, rPosAry.mnDestHeight);
+
+    SkPaint aPaint;
+    aPaint.setBlendMode(eBlendMode);
+
+    preDraw();
+    SAL_INFO("vcl.skia", "drawimage(" << this << "): " << rPosAry << ":" << int(eBlendMode));
+    mSurface->getCanvas()->drawImageRect(aImage, aSourceRect, aDestinationRect, &aPaint);
+    postDraw();
+}
+
 void SkiaSalGraphicsImpl::drawBitmap(const SalTwoRect& rPosAry, const SkBitmap& aBitmap,
                                      SkBlendMode eBlendMode)
 {
@@ -1033,26 +1035,19 @@ bool SkiaSalGraphicsImpl::drawTransformedBitmap(const basegfx::B2DPoint& rNull,
     const SkiaSalBitmap& rSkiaBitmap = static_cast<const SkiaSalBitmap&>(rSourceBitmap);
     const SkiaSalBitmap* pSkiaAlphaBitmap = static_cast<const SkiaSalBitmap*>(pAlphaBitmap);
 
-    long nSourceWidth = rSourceBitmap.GetSize().Width();
-    long nSourceHeight = rSourceBitmap.GetSize().Height();
-
-    SkBitmap aTemporaryBitmap;
-    if (!aTemporaryBitmap.tryAllocN32Pixels(nSourceWidth, nSourceHeight))
-    {
+    sk_sp<SkSurface> tmpSurface = SkiaHelper::createSkSurface(rSourceBitmap.GetSize());
+    if (!tmpSurface)
         return false;
-    }
 
+    // Combine bitmap + alpha bitmap into one temporary bitmap with alpha
+    SkCanvas* aCanvas = tmpSurface->getCanvas();
+    SkPaint aPaint;
+    aPaint.setBlendMode(SkBlendMode::kSrc); // copy as is, including alpha
+    aCanvas->drawImage(rSkiaBitmap.GetSkImage(), 0, 0, &aPaint);
+    if (pSkiaAlphaBitmap != nullptr)
     {
-        // Combine bitmap + alpha bitmap into one temporary bitmap with alpha
-        SkCanvas aCanvas(aTemporaryBitmap);
-        SkPaint aPaint;
-        aPaint.setBlendMode(SkBlendMode::kSrc); // copy as is, including alpha
-        aCanvas.drawBitmap(rSkiaBitmap.GetSkBitmap(), 0, 0, &aPaint);
-        if (pSkiaAlphaBitmap != nullptr)
-        {
-            aPaint.setBlendMode(SkBlendMode::kDstOut);
-            aCanvas.drawBitmap(pSkiaAlphaBitmap->GetAlphaSkBitmap(), 0, 0, &aPaint);
-        }
+        aPaint.setBlendMode(SkBlendMode::kDstOut); // VCL alpha is one-minus-alpha
+        aCanvas->drawImage(pSkiaAlphaBitmap->GetAlphaSkImage(), 0, 0, &aPaint);
     }
     // setup the image transformation
     // using the rNull, rX, rY points as destinations for the (0,0), (0,Width), (Height,0) source points
@@ -1075,7 +1070,7 @@ bool SkiaSalGraphicsImpl::drawTransformedBitmap(const basegfx::B2DPoint& rNull,
     {
         SkAutoCanvasRestore autoRestore(mSurface->getCanvas(), true);
         mSurface->getCanvas()->concat(aMatrix);
-        mSurface->getCanvas()->drawBitmap(aTemporaryBitmap, 0, 0);
+        mSurface->getCanvas()->drawImage(tmpSurface->makeImageSnapshot(), 0, 0);
     }
     postDraw();
 
@@ -1111,36 +1106,7 @@ bool SkiaSalGraphicsImpl::supportsOperation(OutDevSupportType eType) const
 void SkiaSalGraphicsImpl::dump(const char* file) const
 {
     assert(mSurface.get());
-    mSurface->getCanvas()->flush();
-    sk_sp<SkImage> image = mSurface->makeImageSnapshot();
-    sk_sp<SkData> data = image->encodeToData();
-    std::ofstream ostream(file, std::ios::binary);
-    ostream.write(static_cast<const char*>(data->data()), data->size());
-}
-
-void SkiaSalGraphicsImpl::dump(const SkBitmap& bitmap, const char* file)
-{
-    sk_sp<SkImage> image = SkImage::MakeFromBitmap(bitmap);
-    sk_sp<SkData> data = image->encodeToData();
-    std::ofstream ostream(file, std::ios::binary);
-    ostream.write(static_cast<const char*>(data->data()), data->size());
-}
-
-void SkiaSalGraphicsImpl::prefillSurface()
-{
-    // Pre-fill the surface with deterministic garbage.
-    SkBitmap bitmap;
-    bitmap.allocN32Pixels(2, 2);
-    SkPMColor* scanline;
-    scanline = bitmap.getAddr32(0, 0);
-    *scanline++ = SkPreMultiplyARGB(0xFF, 0xBF, 0x80, 0x40);
-    *scanline++ = SkPreMultiplyARGB(0xFF, 0x40, 0x80, 0xBF);
-    scanline = bitmap.getAddr32(0, 1);
-    *scanline++ = SkPreMultiplyARGB(0xFF, 0xE3, 0x5C, 0x13);
-    *scanline++ = SkPreMultiplyARGB(0xFF, 0x13, 0x5C, 0xE3);
-    SkPaint paint;
-    paint.setShader(bitmap.makeShader(SkTileMode::kRepeat, SkTileMode::kRepeat));
-    mSurface->getCanvas()->drawPaint(paint);
+    SkiaHelper::dump(mSurface, file);
 }
 #endif
 
diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx
index fb0c3608028d..d7fc3f6905c6 100644
--- a/vcl/skia/salbmp.cxx
+++ b/vcl/skia/salbmp.cxx
@@ -27,6 +27,9 @@
 #include <SkCanvas.h>
 #include <SkImage.h>
 #include <SkPixelRef.h>
+#include <SkSurface.h>
+
+#include <skia/utils.hxx>
 
 #ifdef DBG_UTIL
 #include <fstream>
@@ -43,15 +46,18 @@ static bool isValidBitCount(sal_uInt16 nBitCount)
            || (nBitCount == 32);
 }
 
-SkiaSalBitmap::SkiaSalBitmap(const SkImage& image)
+SkiaSalBitmap::SkiaSalBitmap(const sk_sp<SkImage>& image)
 {
-    if (Create(Size(image.width(), image.height()), 32, BitmapPalette()))
+    if (Create(Size(image->width(), image->height()), 32, BitmapPalette()))
     {
         SkCanvas canvas(mBitmap);
         // TODO makeNonTextureImage() ?
         SkPaint paint;
         paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
-        canvas.drawImage(&image, 0, 0, &paint);
+        canvas.drawImage(image, 0, 0, &paint);
+        // Also set up the cached image, often it will be immediately read again.
+        // TODO Maybe do the image -> bitmap conversion on demand?
+        mImage = image;
     }
 }
 
@@ -291,9 +297,12 @@ bool SkiaSalBitmap::ConvertToGreyscale()
     return false;
 }
 
-const SkBitmap& SkiaSalBitmap::GetSkBitmap() const
+SkBitmap SkiaSalBitmap::GetAsSkBitmap() const
 {
-    if (mBuffer && mBitmap.drawsNothing())
+    if (!mBitmap.drawsNothing())
+        return mBitmap;
+    SkBitmap bitmap;
+    if (mBuffer)
     {
         if (mBitCount == 24)
         {
@@ -311,12 +320,12 @@ const SkBitmap& SkiaSalBitmap::GetSkBitmap() const
                     *dest++ = 0xff;
                 }
             }
-            if (!const_cast<SkBitmap&>(mBitmap).installPixels(
+            if (!bitmap.installPixels(
                     SkImageInfo::MakeS32(mSize.Width(), mSize.Height(), kOpaque_SkAlphaType),
                     data.release(), mSize.Width() * 4,
                     [](void* addr, void*) { delete[] static_cast<sal_uInt8*>(addr); }, nullptr))
                 abort();
-            SAL_INFO("vcl.skia", "skbitmap(" << this << ")");
+            bitmap.setImmutable();
         }
         else
         {
@@ -327,90 +336,105 @@ const SkBitmap& SkiaSalBitmap::GetSkBitmap() const
                 = convertDataBitCount(mBuffer.get(), mSize.Width(), mSize.Height(), mBitCount,
                                       mScanlineSize, mPalette, GET_FORMAT);
 #undef GET_FORMAT
-            if (!const_cast<SkBitmap&>(mBitmap).installPixels(
+            if (!bitmap.installPixels(
                     SkImageInfo::MakeS32(mSize.Width(), mSize.Height(), kOpaque_SkAlphaType),
                     data.release(), mSize.Width() * 4,
                     [](void* addr, void*) { delete[] static_cast<sal_uInt8*>(addr); }, nullptr))
                 abort();
-            SAL_INFO("vcl.skia", "skbitmap(" << this << ")");
+            bitmap.setImmutable();
         }
     }
-    return mBitmap;
+    return bitmap;
+}
+
+const sk_sp<SkImage>& SkiaSalBitmap::GetSkImage() const
+{
+    if (mImage)
+        return mImage;
+    sk_sp<SkSurface> surface = SkiaHelper::createSkSurface(mSize);
+    assert(surface);
+    SkPaint paint;
+    paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
+    surface->getCanvas()->drawBitmap(GetAsSkBitmap(), 0, 0, &paint);
+    const_cast<sk_sp<SkImage>&>(mImage) = surface->makeImageSnapshot();
+    SAL_INFO("vcl.skia", "getskimage(" << this << ")");
+    return mImage;
 }
 
-const SkBitmap& SkiaSalBitmap::GetAlphaSkBitmap() const
+const sk_sp<SkImage>& SkiaSalBitmap::GetAlphaSkImage() const
 {
-    if (mAlphaBitmap.drawsNothing())
+    if (mAlphaImage)
+        return mAlphaImage;
+    SkBitmap alphaBitmap;
+    if (mBuffer && mBitCount <= 8)
     {
-        if (mBuffer && mBitCount <= 8)
+        assert(mBuffer.get());
+        verify();
+        std::unique_ptr<sal_uInt8[]> data
+            = convertDataBitCount(mBuffer.get(), mSize.Width(), mSize.Height(), mBitCount,
+                                  mScanlineSize, mPalette, BitConvert::A8);
+        if (!alphaBitmap.installPixels(
+                SkImageInfo::MakeA8(mSize.Width(), mSize.Height()), data.release(), mSize.Width(),
+                [](void* addr, void*) { delete[] static_cast<sal_uInt8*>(addr); }, nullptr))
+            abort();
+        alphaBitmap.setImmutable();
+    }
+    else
+    {
+        SkBitmap originalBitmap = GetAsSkBitmap();
+        // To make things more interesting, some LO code creates masks as 24bpp,
+        // so we first need to convert to 8bit to be able to convert that to 8bit alpha.
+        SkBitmap* convertedBitmap = nullptr;
+        const SkBitmap* bitmap8 = &originalBitmap;
+        if (originalBitmap.colorType() != kGray_8_SkColorType)
         {
-            assert(mBuffer.get());
-            verify();
-            std::unique_ptr<sal_uInt8[]> data
-                = convertDataBitCount(mBuffer.get(), mSize.Width(), mSize.Height(), mBitCount,
-                                      mScanlineSize, mPalette, BitConvert::A8);
-            if (!const_cast<SkBitmap&>(mAlphaBitmap)
-                     .installPixels(
-                         SkImageInfo::MakeA8(mSize.Width(), mSize.Height()), data.release(),
-                         mSize.Width(),
-                         [](void* addr, void*) { delete[] static_cast<sal_uInt8*>(addr); },
-                         nullptr))
+            convertedBitmap = new SkBitmap;
+            if (!convertedBitmap->tryAllocPixels(SkImageInfo::Make(
+                    mSize.Width(), mSize.Height(), kGray_8_SkColorType, kOpaque_SkAlphaType)))
                 abort();
-            SAL_INFO("vcl.skia", "skalphabitmap(" << this << ")");
-        }
-        else
-        {
-            GetSkBitmap(); // make sure we have mBitmap, in case (mBuffer && mBitCount > 8)
-            // To make things more interesting, some LO code creates masks as 24bpp,
-            // so we first need to convert to 8bit to be able to convert that to 8bit alpha.
-            SkBitmap* convertedBitmap = nullptr;
-            const SkBitmap* bitmap8 = &mBitmap;
-            if (mBitmap.colorType() != kGray_8_SkColorType)
-            {
-                convertedBitmap = new SkBitmap;
-                if (!convertedBitmap->tryAllocPixels(SkImageInfo::Make(
-                        mSize.Width(), mSize.Height(), kGray_8_SkColorType, kOpaque_SkAlphaType)))
-                    abort();
-                SkCanvas canvas(*convertedBitmap);
-                SkPaint paint;
-                paint.setBlendMode(SkBlendMode::kSrc); // copy and convert depth
-                canvas.drawBitmap(mBitmap, 0, 0, &paint);
-                bitmap8 = convertedBitmap;
-            }
-            // Skia uses a bitmap as an alpha channel only if it's set as kAlpha_8_SkColorType.
-            // But in SalBitmap::Create() it's not quite clear if the 8-bit image will be used
-            // as a mask or as a real bitmap. So mBitmap is always kGray_8_SkColorType for 8bpp
-            // and mAlphaBitmap is kAlpha_8_SkColorType that can be used as a mask.
-            // Make mAlphaBitmap share mBitmap's data.
-            const_cast<SkBitmap&>(mAlphaBitmap)
-                .setInfo(bitmap8->info().makeColorType(kAlpha_8_SkColorType), bitmap8->rowBytes());
-            const_cast<SkBitmap&>(mAlphaBitmap)
-                .setPixelRef(sk_ref_sp(bitmap8->pixelRef()), bitmap8->pixelRefOrigin().x(),
-                             bitmap8->pixelRefOrigin().y());
-            delete convertedBitmap;
-            SAL_INFO("vcl.skia", "skalphabitmap(" << this << ")");
-            return mAlphaBitmap;
+            SkCanvas canvas(*convertedBitmap);
+            SkPaint paint;
+            paint.setBlendMode(SkBlendMode::kSrc); // copy and convert depth
+            canvas.drawBitmap(originalBitmap, 0, 0, &paint);
+            convertedBitmap->setImmutable();
+            bitmap8 = convertedBitmap;
         }
+        // Skia uses a bitmap as an alpha channel only if it's set as kAlpha_8_SkColorType.
+        // But in SalBitmap::Create() it's not quite clear if the 8-bit image will be used
+        // as a mask or as a real bitmap. So mBitmap is always kGray_8_SkColorType for 8bpp
+        // and alphaBitmap is kAlpha_8_SkColorType that can be used as a mask.
+        // Make alphaBitmap share bitmap8's data.
+        alphaBitmap.setInfo(
+            bitmap8->info().makeColorType(kAlpha_8_SkColorType).makeAlphaType(kPremul_SkAlphaType),
+            bitmap8->rowBytes());
+        alphaBitmap.setPixelRef(sk_ref_sp(bitmap8->pixelRef()), bitmap8->pixelRefOrigin().x(),
+                                bitmap8->pixelRefOrigin().y());
+        delete convertedBitmap;
+        alphaBitmap.setImmutable();
     }
-    return mAlphaBitmap;
+    sk_sp<SkSurface> surface = SkiaHelper::createSkSurface(mSize, kAlpha_8_SkColorType);
+    assert(surface);
+    // https://bugs.chromium.org/p/skia/issues/detail?id=9692
+    // Raster kAlpha_8_SkColorType surfaces need empty contents for SkBlendMode::kSrc.
+    if (!surface->getCanvas()->getGrContext())
+        surface->getCanvas()->clear(SkColorSetARGB(0x00, 0x00, 0x00, 0x00));
+    SkPaint paint;
+    paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
+    surface->getCanvas()->drawBitmap(alphaBitmap, 0, 0, &paint);
+    const_cast<sk_sp<SkImage>&>(mAlphaImage) = surface->makeImageSnapshot();
+    SAL_INFO("vcl.skia", "getalphaskbitmap(" << this << ")");
+    return mAlphaImage;
 }
 
-// Reset the cached bitmap allocated in GetSkBitmap().
+// Reset the cached images allocated in GetSkImage().
 void SkiaSalBitmap::ResetCachedBitmap()
 {
-    mAlphaBitmap.reset();
-    if (mBuffer)
-        mBitmap.reset();
+    mAlphaImage.reset();
+    mImage.reset();
 }
 
 #ifdef DBG_UTIL
-void SkiaSalBitmap::dump(const char* file) const
-{
-    sk_sp<SkImage> image = SkImage::MakeFromBitmap(GetSkBitmap());
-    sk_sp<SkData> data = image->encodeToData();
-    std::ofstream ostream(file, std::ios::binary);
-    ostream.write(static_cast<const char*>(data->data()), data->size());
-}
+void SkiaSalBitmap::dump(const char* file) const { SkiaHelper::dump(GetSkImage(), file); }
 
 void SkiaSalBitmap::verify() const
 {
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index 5270a7c3af18..c680d21dde70 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -11,6 +11,7 @@
 
 #include <win/saldata.hxx>
 #include <vcl/skia/SkiaHelper.hxx>
+#include <skia/utils.hxx>
 
 #include <SkColorFilter.h>
 #include <SkPixelRef.h>
@@ -150,7 +151,7 @@ void WinSkiaSalGraphicsImpl::DrawTextMask(CompatibleDC::Texture* pTexture, Color
                                           const SalTwoRect& rPosAry)
 {
     assert(dynamic_cast<SkiaCompatibleDC::Texture*>(pTexture));
-    drawMask(rPosAry, *static_cast<const SkiaCompatibleDC::Texture*>(pTexture)->image, nMaskColor);
+    drawMask(rPosAry, static_cast<const SkiaCompatibleDC::Texture*>(pTexture)->image, nMaskColor);
 }
 
 SkiaCompatibleDC::SkiaCompatibleDC(SalGraphics& rGraphics, int x, int y, int width, int height)
@@ -192,8 +193,16 @@ sk_sp<SkImage> SkiaCompatibleDC::getAsMaskImage()
     alpha.setInfo(bitmap8.info().makeColorType(kAlpha_8_SkColorType), bitmap8.rowBytes());
     alpha.setPixelRef(sk_ref_sp(bitmap8.pixelRef()), bitmap8.pixelRefOrigin().x(),
                       bitmap8.pixelRefOrigin().y());
-    // TODO GPU?
-    return SkImage::MakeFromBitmap(alpha);
+    sk_sp<SkSurface> surface
+        = SkiaHelper::createSkSurface(alpha.width(), alpha.height(), kAlpha_8_SkColorType);
+    // https://bugs.chromium.org/p/skia/issues/detail?id=9692
+    // Raster kAlpha_8_SkColorType surfaces need empty contents for SkBlendMode::kSrc.
+    if (!surface->getCanvas()->getGrContext())
+        surface->getCanvas()->clear(SkColorSetARGB(0x00, 0x00, 0x00, 0x00));
+    SkPaint paint;
+    paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
+    surface->getCanvas()->drawBitmap(alpha, 0, 0, &paint);
+    return surface->makeImageSnapshot();
 }
 
 sk_sp<SkImage> SkiaCompatibleDC::getAsImage()
@@ -203,22 +212,21 @@ sk_sp<SkImage> SkiaCompatibleDC::getAsImage()
                                                    kBGRA_8888_SkColorType, kUnpremul_SkAlphaType),
                                  mpData, maRects.mnSrcWidth * 4))
         abort();
-    SkBitmap bitmap;
-    if (!bitmap.tryAllocPixels(tmpBitmap.info()))
-        abort();
+    sk_sp<SkSurface> surface = SkiaHelper::createSkSurface(tmpBitmap.width(), tmpBitmap.height());
     SkPaint paint;
     paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
-    SkCanvas canvas(bitmap);
+    SkCanvas* canvas = surface->getCanvas();
+    canvas->save();
     // The data we got is upside-down.
     SkMatrix matrix;
     matrix.preTranslate(0, maRects.mnSrcHeight);
     matrix.setConcat(matrix, SkMatrix::MakeScale(1, -1));
-    canvas.concat(matrix);
-    canvas.drawBitmapRect(tmpBitmap,
-                          SkRect::MakeXYWH(0, 0, maRects.mnSrcWidth, maRects.mnSrcHeight),
-                          SkRect::MakeXYWH(0, 0, maRects.mnSrcWidth, maRects.mnSrcHeight), &paint);
-    // TODO GPU
-    return SkImage::MakeFromBitmap(bitmap);
+    canvas->concat(matrix);
+    canvas->drawBitmapRect(tmpBitmap,
+                           SkRect::MakeXYWH(0, 0, maRects.mnSrcWidth, maRects.mnSrcHeight),
+                           SkRect::MakeXYWH(0, 0, maRects.mnSrcWidth, maRects.mnSrcHeight), &paint);
+    canvas->restore();
+    return surface->makeImageSnapshot();
 }
 
 SkiaControlsCache::SkiaControlsCache()
commit cd46c8710b2eb244572670d78fec1f0dcb4a577f
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Tue Dec 3 21:49:08 2019 +0100
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Dec 6 14:26:18 2019 +0100

    check also setupDrawBlend() in visualbackendtest
    
    Change-Id: I57eb88b7b6bb498253639867d4c158041d8299b7
    Reviewed-on: https://gerrit.libreoffice.org/84561
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>

diff --git a/vcl/backendtest/VisualBackendTest.cxx b/vcl/backendtest/VisualBackendTest.cxx
index 47b4149ef324..fb39156714a7 100644
--- a/vcl/backendtest/VisualBackendTest.cxx
+++ b/vcl/backendtest/VisualBackendTest.cxx
@@ -359,7 +359,7 @@ public:
         tools::Rectangle aRectangle;
         size_t index = 0;
 
-        std::vector<tools::Rectangle> aRegions = setupRegions(2, 2, nWidth, nHeight);
+        std::vector<tools::Rectangle> aRegions = setupRegions(3, 2, nWidth, nHeight);
 
         aRectangle = aRegions[index++];
         {
@@ -389,6 +389,13 @@ public:
             assertAndSetBackground(vcl::test::OutputDeviceTestBitmap::checkMask(aBitmap), aRectangle, rRenderContext);
             drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
         }
+        aRectangle = aRegions[index++];
+        {
+            vcl::test::OutputDeviceTestBitmap aOutDevTest;
+            BitmapEx aBitmap = aOutDevTest.setupDrawBlend();
+            assertAndSetBackground(vcl::test::OutputDeviceTestBitmap::checkBlend(aBitmap), aRectangle, rRenderContext);
+            drawBitmapScaledAndCentered(aRectangle, aBitmap.GetBitmap(), rRenderContext);
+        }
     }
 
     static void testInvert(vcl::RenderContext& rRenderContext, int nWidth, int nHeight)
commit ea5eb4639ac3cca4301e23655d0e7aeaed6f8bcc
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Tue Dec 3 19:16:00 2019 +0100
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Dec 6 14:25:52 2019 +0100

    keep just one shared reference to Skia shared GrContext
    
    This should make it easier to keep the reference without having
    to keep references all over the place, especially when the shared GrContext
    starts to be used also for GPU-backed surfaces elsewhere.
    
    Change-Id: Icf3f6eb849ebc5eb63b1836f9caeb6f5e5e58ca6
    Reviewed-on: https://gerrit.libreoffice.org/84560
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>

diff --git a/include/vcl/skia/SkiaHelper.hxx b/include/vcl/skia/SkiaHelper.hxx
index d27cffd650f3..8cb76c653600 100644
--- a/include/vcl/skia/SkiaHelper.hxx
+++ b/include/vcl/skia/SkiaHelper.hxx
@@ -14,26 +14,28 @@
 
 #include <config_features.h>
 
-// All member functions static and VCL_DLLPUBLIC. Basically a glorified namespace.
-struct VCL_DLLPUBLIC SkiaHelper
+namespace SkiaHelper
 {
-    SkiaHelper() = delete; // Should not be instantiated
-
-public:
-    static bool isVCLSkiaEnabled();
+VCL_DLLPUBLIC bool isVCLSkiaEnabled();
 
 #if HAVE_FEATURE_SKIA
-    // Which Skia backend to use.
-    enum RenderMethod
-    {
-        RenderRaster,
-        RenderVulkan
-    };
-    static RenderMethod renderMethodToUse();
-    static void disableRenderMethod(RenderMethod method);
-#endif
+
+// Which Skia backend to use.
+enum RenderMethod
+{
+    RenderRaster,
+    RenderVulkan
 };
 
-#endif
+VCL_DLLPUBLIC RenderMethod renderMethodToUse();
+
+// Clean up before exit.
+VCL_DLLPUBLIC void cleanup();
+
+#endif // HAVE_FEATURE_SKIA
+
+} // namespace
+
+#endif // INCLUDED_VCL_SKIA_SKIAHELPER_HXX
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index e61ee6cfb787..7365d58f9173 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -28,7 +28,7 @@
 #include <SkSurface.h>
 
 #include <prewin.h>
-#include <tools/sk_app/VulkanWindowContext.h>
+#include <tools/sk_app/WindowContext.h>
 #include <postwin.h>
 
 class SkiaFlushIdle;
@@ -263,7 +263,6 @@ protected:
     /// Pointer to the SalFrame or SalVirtualDevice
     SalGeometryProvider* mProvider;
     std::unique_ptr<sk_app::WindowContext> mWindowContext;
-    sk_app::VulkanWindowContext::SharedGrContext mOffscreenGrContext;
     // The Skia surface that is target of all the rendering.
     sk_sp<SkSurface> mSurface;
     bool mIsGPU; // whether the surface is GPU-backed
diff --git a/vcl/inc/skia/salbmp.hxx b/vcl/inc/skia/salbmp.hxx
index ed0f374162d6..0b0e1aa5439c 100644
--- a/vcl/inc/skia/salbmp.hxx
+++ b/vcl/inc/skia/salbmp.hxx
@@ -101,6 +101,6 @@ private:
     int mScanlineSize; // size of one row in mBuffer
 };
 
-#endif // INCLUDED_VCL_INC_OPENGL_SALBMP_H
+#endif // INCLUDED_VCL_INC_SKIA_SALBMP_H
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/inc/skia/utils.hxx b/vcl/inc/skia/utils.hxx
new file mode 100644
index 000000000000..5d824400030a
--- /dev/null
+++ b/vcl/inc/skia/utils.hxx
@@ -0,0 +1,38 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * This file incorporates work covered by the following license notice:
+ *
+ *   Licensed to the Apache Software Foundation (ASF) under one or more
+ *   contributor license agreements. See the NOTICE file distributed
+ *   with this work for additional information regarding copyright
+ *   ownership. The ASF licenses this file to you under the Apache
+ *   License, Version 2.0 (the "License"); you may not use this file
+ *   except in compliance with the License. You may obtain a copy of
+ *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
+ */
+
+#ifndef INCLUDED_VCL_INC_SKIA_UTILS_H
+#define INCLUDED_VCL_INC_SKIA_UTILS_H
+
+#include <vcl/skia/SkiaHelper.hxx>
+
+#include <tools/sk_app/VulkanWindowContext.h>
+
+namespace SkiaHelper
+{
+// Get the one shared GrContext instance.
+GrContext* getSharedGrContext();
+
+void disableRenderMethod(RenderMethod method);
+
+} // namespace
+
+#endif // INCLUDED_VCL_INC_SKIA_UTILS_H
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx
index 040f189be74f..471d47f01f69 100644
--- a/vcl/skia/SkiaHelper.cxx
+++ b/vcl/skia/SkiaHelper.cxx
@@ -14,10 +14,21 @@
 #include <officecfg/Office/Common.hxx>
 
 #if !HAVE_FEATURE_SKIA
-bool SkiaHelper::isVCLSkiaEnabled() { return false; }
+
+namespace SkiaHelper
+{
+bool isVCLSkiaEnabled() { return false; }
+
+} // namespace
 
 #else
 
+#include <skia/utils.hxx>
+
+#include <tools/sk_app/VulkanWindowContext.h>
+
+namespace SkiaHelper
+{
 static bool supportsVCLSkia()
 {
     static bool bDisableSkia = !!getenv("SAL_DISABLESKIA");
@@ -26,7 +37,7 @@ static bool supportsVCLSkia()
     return !bDisableSkia && !bBlacklisted;
 }
 
-bool SkiaHelper::isVCLSkiaEnabled()
+bool isVCLSkiaEnabled()
 {
     /**
      * The !bSet part should only be called once! Changing the results in the same
@@ -84,7 +95,7 @@ bool SkiaHelper::isVCLSkiaEnabled()
     return bRet;
 }
 
-static SkiaHelper::RenderMethod methodToUse = SkiaHelper::RenderRaster;
+static RenderMethod methodToUse = RenderRaster;
 
 static bool initRenderMethodToUse()
 {
@@ -92,15 +103,15 @@ static bool initRenderMethodToUse()
     {
         if (strcmp(env, "raster") == 0)
         {
-            methodToUse = SkiaHelper::RenderRaster;
+            methodToUse = RenderRaster;
             return true;
         }
     }
-    methodToUse = SkiaHelper::RenderVulkan;
+    methodToUse = RenderVulkan;
     return true;
 }
 
-SkiaHelper::RenderMethod SkiaHelper::renderMethodToUse()
+RenderMethod renderMethodToUse()
 {
     static bool methodToUseInited = initRenderMethodToUse();
     if (methodToUseInited) // Used just to ensure thread-safe one-time init.
@@ -108,7 +119,7 @@ SkiaHelper::RenderMethod SkiaHelper::renderMethodToUse()
     abort();
 }
 
-void SkiaHelper::disableRenderMethod(RenderMethod method)
+void disableRenderMethod(RenderMethod method)
 {
     if (renderMethodToUse() != method)
         return;
@@ -116,6 +127,38 @@ void SkiaHelper::disableRenderMethod(RenderMethod method)
     methodToUse = RenderRaster;
 }
 
+static sk_app::VulkanWindowContext::SharedGrContext* sharedGrContext;
+
+GrContext* getSharedGrContext()
+{
+    assert(renderMethodToUse() == RenderVulkan);
+    if (sharedGrContext)
+        return sharedGrContext->getGrContext();
+    // TODO mutex?
+    // Setup the shared GrContext from Skia's (patched) VulkanWindowContext, if it's been
+    // already set up.
+    sk_app::VulkanWindowContext::SharedGrContext context
+        = sk_app::VulkanWindowContext::getSharedGrContext();
+    GrContext* grContext = context.getGrContext();
+    if (grContext)
+    {
+        sharedGrContext = new sk_app::VulkanWindowContext::SharedGrContext(context);
+        return grContext;
+    }
+    // TODO
+    // SkiaSalGraphicsImpl::createOffscreenSurface() creates the shared context using a dummy window,
+    // but we do not have a window here. Is it worth it to try to do it here?
+    return nullptr;
+}
+
+void cleanup()
+{
+    delete sharedGrContext;
+    sharedGrContext = nullptr;
+}
+
+} // namespace
+
 #endif // HAVE_FEATURE_SKIA
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 9c8d7c497ba2..6a977ab6b598 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -25,6 +25,7 @@
 #include <vcl/svapp.hxx>
 #include <vcl/lazydelete.hxx>
 #include <vcl/skia/SkiaHelper.hxx>
+#include <skia/utils.hxx>
 
 #include <SkCanvas.h>
 #include <SkPath.h>
@@ -182,7 +183,6 @@ SkiaSalGraphicsImpl::~SkiaSalGraphicsImpl()
 {
     assert(!mSurface);
     assert(!mWindowContext);
-    assert(!mOffscreenGrContext);
 }
 
 void SkiaSalGraphicsImpl::Init() {}
@@ -243,8 +243,7 @@ void SkiaSalGraphicsImpl::createOffscreenSurface()
     {
         case SkiaHelper::RenderVulkan:
         {
-            mOffscreenGrContext = sk_app::VulkanWindowContext::getSharedGrContext();
-            GrContext* grContext = mOffscreenGrContext.getGrContext();
+            GrContext* grContext = SkiaHelper::getSharedGrContext();
             // We may not get a GrContext if called before any onscreen window is created.
             if (!grContext)
             {
@@ -253,14 +252,10 @@ void SkiaSalGraphicsImpl::createOffscreenSurface()
                 // Create temporary WindowContext with no window. That will fail,
                 // but it will initialize the shared GrContext.
                 createWindowContext();
-                // Keep a reference.
-                sk_app::VulkanWindowContext::SharedGrContext context
-                    = sk_app::VulkanWindowContext::getSharedGrContext();
+                // This will use the temporarily created context.
+                grContext = SkiaHelper::getSharedGrContext();
                 // Destroy the temporary WindowContext.
                 destroySurface();
-                // Keep a reference until the surface is destroyed.
-                mOffscreenGrContext = context;
-                grContext = mOffscreenGrContext.getGrContext();
             }
             if (grContext)
             {
@@ -309,7 +304,6 @@ void SkiaSalGraphicsImpl::destroySurface()
     mSurface.reset();
     mWindowContext.reset();
     mIsGPU = false;
-    mOffscreenGrContext.reset();
 }
 
 void SkiaSalGraphicsImpl::DeInit() { destroySurface(); }
diff --git a/vcl/unx/generic/app/salinst.cxx b/vcl/unx/generic/app/salinst.cxx
index 3c39a1addbd9..f253f70d7d93 100644
--- a/vcl/unx/generic/app/salinst.cxx
+++ b/vcl/unx/generic/app/salinst.cxx
@@ -79,6 +79,10 @@ X11SalInstance::~X11SalInstance()
     // would be done in a static destructor else which is
     // a little late
     GetGenericUnixSalData()->Dispose();
+
+#if HAVE_FEATURE_SKIA
+    SkiaHelper::cleanup();
+#endif
 }
 
 SalX11Display* X11SalInstance::CreateDisplay() const
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 43cc8e8e6a85..cd9332e0c728 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -407,6 +407,9 @@ WinSalInstance::~WinSalInstance()
 {
     ImplFreeSalGDI();
     DestroyWindow( mhComWnd );
+#if HAVE_FEATURE_SKIA
+    SkiaHelper::cleanup();
+#endif
 }
 
 static LRESULT ImplSalDispatchMessage( const MSG* pMsg )


More information about the Libreoffice-commits mailing list