[Libreoffice-commits] core.git: config_host/config_skia.h.in vcl/skia vcl/unx vcl/win

[Luboš Luňák (via logerrit)]

 config_host/config_skia.h.in    |   17 +++++++++++++++++
 vcl/skia/salbmp.cxx             |   16 ++++++++++------
 vcl/unx/generic/app/salinst.cxx |    3 +++
 vcl/win/app/salinst.cxx         |    3 +++
 4 files changed, 33 insertions(+), 6 deletions(-)

New commits:
commit 994b8e52fc02c7468a243efd5e3ba7ffcd81e283
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Thu Jan 9 14:10:00 2020 +0100
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Thu Jan 9 15:26:31 2020 +0100

    add setting to config_skia.h whether to use 32bpp bitmaps
    
    As said in previous Skia commits about BackendCapabilities::mbSupportsBitmap32
    this should be preferrably enabled, but there are still parts of LO code
    that have a problem with it, so make it easier to enable/disable this.
    
    Change-Id: Iae7a8d5fc77a27372c00f6745317d168e8b2a05b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86489
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>

diff --git a/config_host/config_skia.h.in b/config_host/config_skia.h.in
index 75b1e099ce8e..1a8b64a1dfd6 100644
--- a/config_host/config_skia.h.in
+++ b/config_host/config_skia.h.in
@@ -8,6 +8,23 @@ are the same.
 #ifndef CONFIG_SKIA_H
 #define CONFIG_SKIA_H
 
+// This a setting that should be set manually and it affects LO
+// code rather than Skia itself. It basically controls setting
+// BackendCapabilities::mbSupportsBitmap32.
+// Since Skia does not natively support 24bpp, the preferred setup is
+// that the setting should be enabled, it makes the code faster and cleaner.
+// Unfortunately VCL historically splits alpha into a whole separate
+// bitmap and works with 24bpp+8bpp, which is generally more complicated,
+// more error-prone and just worse, but that's how LO code has been
+// written and so there are many places in LO that expect this and
+// do not work correctly with true 32bpp bitmaps.
+// So ultimately the 24+8 split should be dumped (preferably in all of LO,
+// not just the Skia-related code), but until all of LO works correctly
+// with 32bpp disabling this will avoid such breakages.
+#define SKIA_USE_BITMAP32 1
+//#define SKIA_USE_BITMAP32 0
+
+
 /* TODO SKIA check all these */
 
 #define SK_SUPPORT_GPU 1
diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx
index 172b7c37ef92..568996723011 100644
--- a/vcl/skia/salbmp.cxx
+++ b/vcl/skia/salbmp.cxx
@@ -111,15 +111,19 @@ bool SkiaSalBitmap::CreateBitmapData()
     }
     if (colorType != kUnknown_SkColorType)
     {
-        // If vcl::BackendCapabilities::mbSupportsBitmap32 is set,
-        // BitmapReadAccess::ImplSetAccessPointers() uses functions that use premultiplied
-        // alpha. If not set, it would use functions that would read just RGB, so using
-        // premultiplied alpha here would change those values.
-        // Using kPremul_SkAlphaType should be better for performance, so ensure
-        // the flag is set.
+    // If vcl::BackendCapabilities::mbSupportsBitmap32 is set,
+    // BitmapReadAccess::ImplSetAccessPointers() uses functions that use premultiplied
+    // alpha. If not set, it would use functions that would read just RGB, so using
+    // premultiplied alpha here would change those values.
+#if SKIA_USE_BITMAP32
         assert(ImplGetSVData()->mpDefInst->GetBackendCapabilities()->mbSupportsBitmap32);
         if (!mBitmap.tryAllocPixels(
                 SkImageInfo::Make(mSize.Width(), mSize.Height(), colorType, kPremul_SkAlphaType)))
+#else
+        assert(!ImplGetSVData()->mpDefInst->GetBackendCapabilities()->mbSupportsBitmap32);
+        if (!mBitmap.tryAllocPixels(
+                SkImageInfo::Make(mSize.Width(), mSize.Height(), colorType, kUnpremul_SkAlphaType)))
+#endif
         {
             return false;
         }
diff --git a/vcl/unx/generic/app/salinst.cxx b/vcl/unx/generic/app/salinst.cxx
index e5b2a92b89ce..bf4a71d5f6fc 100644
--- a/vcl/unx/generic/app/salinst.cxx
+++ b/vcl/unx/generic/app/salinst.cxx
@@ -34,6 +34,7 @@
 
 #include <config_features.h>
 #include <vcl/skia/SkiaHelper.hxx>
+#include <config_skia.h>
 
 // plugin factory function
 extern "C"
@@ -228,8 +229,10 @@ std::shared_ptr<vcl::BackendCapabilities> X11SalInstance::GetBackendCapabilities
 {
     auto pBackendCapabilities = SalInstance::GetBackendCapabilities();
 #if HAVE_FEATURE_SKIA
+#if SKIA_USE_BITMAP32
     if( SkiaHelper::isVCLSkiaEnabled())
         pBackendCapabilities->mbSupportsBitmap32 = true;
+#endif
 #endif
     return pBackendCapabilities;
 }
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 4b97215b6dac..26ce820a1b67 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -50,6 +50,7 @@
 #include <config_features.h>
 #include <vcl/skia/SkiaHelper.hxx>
 #if HAVE_FEATURE_SKIA
+#include <config_skia.h>
 #include <skia/salbmp.hxx>
 #include <skia/win/gdiimpl.hxx>
 #endif
@@ -1078,8 +1079,10 @@ std::shared_ptr<vcl::BackendCapabilities> WinSalInstance::GetBackendCapabilities
 {
     auto pBackendCapabilities = SalInstance::GetBackendCapabilities();
 #if HAVE_FEATURE_SKIA
+#if SKIA_USE_BITMAP32
     if( SkiaHelper::isVCLSkiaEnabled())
         pBackendCapabilities->mbSupportsBitmap32 = true;
+#endif
 #endif
     return pBackendCapabilities;
 }