clarifications on the use of SKIA_USE_BITMAP32

Change-Id: Ia2f80c3dc6ac3e0b16993dde588a4987ce98aa81
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103235
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

2 files changed, 15 insertions, 4 deletions

diff --git a/config_host/config_skia.h.in b/config_host/config_skia.h.in
index ba3d3c6c0a07..db103238d4fc 100644
--- a/config_host/config_skia.h.in
+++ b/config_host/config_skia.h.in
@@ -8,19 +8,24 @@ are the same.
 #ifndef CONFIG_SKIA_H
 #define CONFIG_SKIA_H
 
-// This a setting that should be set manually and it affects LO
+// This is a setting that should be set manually and it affects LO
 // code rather than Skia itself. It basically controls setting
-// BackendCapabilities::mbSupportsBitmap32.
+// BackendCapabilities::mbSupportsBitmap32, i.e. whether one LO bitmap
+// contains all the 32bits of an image including the alpha (premultiplied).
+//
 // 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.
+// with 32bpp keep this disabled in order to avoid such breakages.
+
 //#define SKIA_USE_BITMAP32 1
 #define SKIA_USE_BITMAP32 0
 
diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx
index d7399855a3ec..379f53bb032d 100644
--- a/vcl/skia/salbmp.cxx
+++ b/vcl/skia/salbmp.cxx
@@ -466,10 +466,12 @@ SkBitmap SkiaSalBitmap::GetAsSkBitmap() const
             const size_t bytes = mPixelsSize.Height() * mScanlineSize;
             std::unique_ptr<sal_uInt8[]> data(new sal_uInt8[bytes]);
             memcpy(data.get(), mBuffer.get(), bytes);
+            // The bitmap's alpha matters only if SKIA_USE_BITMAP32 is set, otherwise
+            // the alpha is in a separate bitmap.
 #if SKIA_USE_BITMAP32
             SkAlphaType alphaType = kPremul_SkAlphaType;
 #else
-            SkAlphaType alphaType = kUnpremul_SkAlphaType;
+            SkAlphaType alphaType = kOpaque_SkAlphaType;
 #endif
             if (!bitmap.installPixels(
                     SkImageInfo::MakeS32(mPixelsSize.Width(), mPixelsSize.Height(), alphaType),
@@ -917,6 +919,10 @@ void SkiaSalBitmap::EnsureBitmapData()
     // Try to fill mBuffer from mImage.
     assert(mImage->colorType() == kN32_SkColorType);
     SkiaZone zone;
+    // Use kUnpremul_SkAlphaType to make Skia convert from premultiplied alpha when reading
+    // from the SkImage, in case there is any alpha involved. If converting to bpp<32 formats,
+    // we will ignore the alpha when converting to mBuffer. Unless bpp==32 and SKIA_USE_BITMAP32,
+    // in which case keep the format, since SKIA_USE_BITMAP32 implies premultiplied alpha.
     SkAlphaType alphaType = kUnpremul_SkAlphaType;
 #if SKIA_USE_BITMAP32
     if (mBitCount == 32)