better handling of bit depths in skia code

Change-Id: Ifb3103cc3494bc55a562d4b6a16b59a044782416

2 files changed, 71 insertions, 39 deletions

diff --git a/vcl/qa/cppunit/BitmapTest.cxx b/vcl/qa/cppunit/BitmapTest.cxx
index d3ed8c621b99..c920cf463c13 100644
--- a/vcl/qa/cppunit/BitmapTest.cxx
+++ b/vcl/qa/cppunit/BitmapTest.cxx
@@ -403,11 +403,6 @@ void BitmapTest::testConvert()
             CPPUNIT_ASSERT_EQUAL(sal_uInt32(30), pReadAccess->GetScanlineSize());
         else
 #endif
-#if HAVE_FEATURE_SKIA
-            if (SkiaHelper::isVCLSkiaEnabled())
-            CPPUNIT_ASSERT_EQUAL(sal_uInt32(40), pReadAccess->GetScanlineSize());
-        else
-#endif
             CPPUNIT_ASSERT_EQUAL(sal_uInt32(32), pReadAccess->GetScanlineSize());
 #else
 #if defined(_WIN32)
diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx
index 5fce7d7073c7..332a5972bab2 100644
--- a/vcl/skia/salbmp.cxx
+++ b/vcl/skia/salbmp.cxx
@@ -37,23 +37,6 @@ SkiaSalBitmap::SkiaSalBitmap() {}
 
 SkiaSalBitmap::~SkiaSalBitmap() {}
 
-static SkColorType getSkColorType(int bitCount)
-{
-    switch (bitCount)
-    {
-        case 8:
-            return kGray_8_SkColorType; // see GetAlphaSkBitmap()
-        case 16:
-            return kRGB_565_SkColorType;
-        case 24:
-            return kRGB_888x_SkColorType;
-        case 32:
-            return kN32_SkColorType;
-        default:
-            abort();
-    }
-}
-
 static bool isValidBitCount(sal_uInt16 nBitCount)
 {
     return (nBitCount == 1) || (nBitCount == 4) || (nBitCount == 8) || (nBitCount == 16)
@@ -76,18 +59,37 @@ bool SkiaSalBitmap::Create(const Size& rSize, sal_uInt16 nBitCount, const Bitmap
     Destroy();
     if (!isValidBitCount(nBitCount))
         return false;
-    // Skia does not support paletted images, so convert only types Skia supports.
-    if (nBitCount > 8 || (nBitCount == 8 && (!rPal || rPal.IsGreyPalette())))
+    // Skia only supports 8bit gray, 16bit and 32bit formats (e.g. 24bpp is actually stored as 32bpp).
+    // But some of our code accessing the bitmap assumes that when it asked for 24bpp, the format
+    // really will be 24bpp (e.g. the png loader).
+    // TODO what is the performance impact of handling 24bpp ourselves instead of in Skia?
+    SkColorType colorType = kUnknown_SkColorType;
+    switch (nBitCount)
     {
-        if (!mBitmap.tryAllocPixels(SkImageInfo::Make(
-                rSize.Width(), rSize.Height(), getSkColorType(nBitCount), kPremul_SkAlphaType)))
+        case 8:
+            if (rPal.IsGreyPalette()) // see GetAlphaSkBitmap()
+                colorType = kGray_8_SkColorType;
+            break;
+        case 16:
+            colorType = kRGB_565_SkColorType;
+            break;
+        case 32:
+            colorType = kN32_SkColorType;
+            break;
+        default:
+            break;
+    }
+    if (colorType != kUnknown_SkColorType)
+    {
+        if (!mBitmap.tryAllocPixels(
+                SkImageInfo::Make(rSize.Width(), rSize.Height(), colorType, kPremul_SkAlphaType)))
         {
             return false;
         }
     }
     else
     {
-        // Paletted images are stored in a buffer and converted as necessary.
+        // Image formats not supported by Skia are stored in a buffer and converted as necessary.
         int bitScanlineWidth;
         if (o3tl::checked_multiply<int>(rSize.Width(), nBitCount, bitScanlineWidth))
         {
@@ -203,8 +205,6 @@ BitmapBuffer* SkiaSalBitmap::AcquireBuffer(BitmapAccessMode nMode)
             buffer->mnFormat = ScanlineFormat::N4BitMsnPal;
             break;
         case 8:
-            // TODO or always N8BitPal?
-            //            buffer->mnFormat = !mPalette ? ScanlineFormat::N8BitTcMask : ScanlineFormat::N8BitPal;
             buffer->mnFormat = ScanlineFormat::N8BitPal;
             break;
         case 16:
@@ -220,8 +220,17 @@ BitmapBuffer* SkiaSalBitmap::AcquireBuffer(BitmapAccessMode nMode)
             break;
         }
         case 24:
-            buffer->mnFormat = ScanlineFormat::N24BitTcRgb;
+        {
+// Make the RGB/BGR format match the default Skia 32bpp format, to allow
+// easy conversion later.
+// Use a macro to hide an unreachable code warning.
+#define GET_FORMAT                                                                                 \
+    (kN32_SkColorType == kBGRA_8888_SkColorType ? ScanlineFormat::N24BitTcBgr                      \
+                                                : ScanlineFormat::N24BitTcRgb)
+            buffer->mnFormat = GET_FORMAT;
+#undef GET_FORMAT
             break;
+        }
         case 32:
             // TODO are these correct?
             buffer->mnFormat = mBitmap.colorType() == kRGBA_8888_SkColorType
@@ -280,15 +289,43 @@ const SkBitmap& SkiaSalBitmap::GetSkBitmap() const
 {
     if (mBuffer && mBitmap.drawsNothing())
     {
-        std::unique_ptr<sal_uInt8[]> data = convertDataBitCount(
-            mBuffer.get(), mSize.Width(), mSize.Height(), mBitCount, mScanlineSize, mPalette,
-            kN32_SkColorType == kBGRA_8888_SkColorType ? BitConvert::BGRA
-                                                       : BitConvert::RGBA); // TODO
-        if (!const_cast<SkBitmap&>(mBitmap).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();
+        if (mBitCount == 24)
+        {
+            // Convert 24bpp RGB/BGR to 32bpp RGBA/BGRA.
+            std::unique_ptr<sal_uInt8[]> data(new sal_uInt8[mSize.Height() * mSize.Width() * 4]);
+            sal_uInt8* dest = data.get();
+            for (int y = 0; y < mSize.Height(); ++y)
+            {
+                const sal_uInt8* src = mBuffer.get() + mScanlineSize * y;
+                for (int x = 0; x < mSize.Width(); ++x)
+                {
+                    *dest++ = *src++;
+                    *dest++ = *src++;
+                    *dest++ = *src++;
+                    *dest++ = 0xff;
+                }
+            }
+            if (!const_cast<SkBitmap&>(mBitmap).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();
+        }
+        else
+        {
+// Use a macro to hide an unreachable code warning.
+#define GET_FORMAT                                                                                 \
+    (kN32_SkColorType == kBGRA_8888_SkColorType ? BitConvert::BGRA : BitConvert::RGBA)
+            std::unique_ptr<sal_uInt8[]> data
+                = convertDataBitCount(mBuffer.get(), mSize.Width(), mSize.Height(), mBitCount,
+                                      mScanlineSize, mPalette, GET_FORMAT);
+#undef GET_FORMAT
+            if (!const_cast<SkBitmap&>(mBitmap).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();
+        }
     }
     return mBitmap;
 }