Skia alpha handling improvements

CppunitTest_vcl_bitmap_render_test now passes.

Change-Id: I88863c63de84f28b5dfeeaf73d3879bc7cbba1b2

7 files changed, 190 insertions, 115 deletions

diff --git a/vcl/inc/salbmp.hxx b/vcl/inc/salbmp.hxx
index ecb1a1b7d7bf..a526e2b6ee65 100644
--- a/vcl/inc/salbmp.hxx
+++ b/vcl/inc/salbmp.hxx
@@ -102,11 +102,18 @@ protected:
 
 protected:
     virtual void updateChecksum() const;
-    // helper function to convert data in 1,2,4 bpp formats to a 24bpp format
-    static std::unique_ptr< sal_uInt8[] > convertDataTo24Bpp( const sal_uInt8* src,
-        int width, int height, int bitCount, int bytesPerRow, const BitmapPalette& palette, bool toBgr );
-    static std::unique_ptr< sal_uInt8[] > convertDataTo32Bpp( const sal_uInt8* src,
-        int width, int height, int bitCount, int bytesPerRow, const BitmapPalette& palette, bool toBgra );
+    // helper function to convert data in 1,2,4 bpp formats to a 8/24/32bpp format
+    enum class BitConvert
+    {
+        A8,
+        RGB,
+        BGR,
+        RGBA,
+        BGRA
+    };
+    static std::unique_ptr< sal_uInt8[] > convertDataBitCount( const sal_uInt8* src,
+        int width, int height, int bitCount, int bytesPerRow, const BitmapPalette& palette,
+        BitConvert type );
 };
 
 #endif
diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index 6c99669ba541..0b08c3c81c5f 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -184,6 +184,7 @@ public:
 
 #ifdef DBG_UTIL
     void dump(const char* file) const;
+    void dump(const SkBitmap& bitmap, const char* file) const;
 #endif
 
 protected:
diff --git a/vcl/inc/skia/salbmp.hxx b/vcl/inc/skia/salbmp.hxx
index 63a54a8555be..7c8ea900c241 100644
--- a/vcl/inc/skia/salbmp.hxx
+++ b/vcl/inc/skia/salbmp.hxx
@@ -64,14 +64,22 @@ public:
     // to a 32bpp SkBitmap.
     const SkBitmap& GetSkBitmap() const;
 
+    const SkBitmap& GetAlphaSkBitmap() const;
+
 #ifdef DBG_UTIL
     void dump(const char* file) const;
 #endif
 
 private:
     void ResetCachedBitmap();
+#ifdef DBG_UTIL
+    void verify() const;
+#else
+    void verify() const {};
+#endif
 
     SkBitmap mBitmap;
+    SkBitmap mAlphaBitmap; // TODO for use as an alpha channel or mask
     BitmapPalette mPalette;
     int mBitCount; // bpp
     Size mSize;
diff --git a/vcl/opengl/salbmp.cxx b/vcl/opengl/salbmp.cxx
index f3147cdcc101..3a033c4afef0 100644
--- a/vcl/opengl/salbmp.cxx
+++ b/vcl/opengl/salbmp.cxx
@@ -403,8 +403,9 @@ GLuint OpenGLSalBitmap::CreateTexture()
             VCL_GL_INFO( "::CreateTexture - convert from " << mnBits << " to 24 bits" );
             // convert to 24 bits RGB using palette
             determineTextureFormat(24, nFormat, nType);
-            pData = convertDataTo24Bpp( mpUserBuffer.get(), mnWidth, mnHeight,
-                mnBits, mnBytesPerRow, maPalette, nFormat == GL_BGR ).release();
+            pData = convertDataBitCount( mpUserBuffer.get(), mnWidth, mnHeight,
+                mnBits, mnBytesPerRow, maPalette,
+                nFormat == GL_BGR ? BitConvert::BGR : BitConvert::RGB ).release();
             bAllocated = true;
         }
     }
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index a80bf73c0cbe..dfdd6a054cf8 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -90,6 +90,8 @@ void SkiaSalGraphicsImpl::drawPixel(long nX, long nY, Color nColor)
     SkCanvas* canvas = mSurface->getCanvas();
     SkPaint paint(mPaint);
     paint.setColor(toSkColor(nColor));
+    // Apparently drawPixel() is actually expected to set the pixel and not draw it.
+    paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
     canvas->drawPoint(nX, nY, paint);
 }
 
@@ -307,16 +309,16 @@ bool SkiaSalGraphicsImpl::drawAlphaBitmap(const SalTwoRect& rPosAry, const SalBi
     assert(dynamic_cast<const SkiaSalBitmap*>(&rSourceBitmap));
     assert(dynamic_cast<const SkiaSalBitmap*>(&rAlphaBitmap));
     SkBitmap tmpBitmap;
-    if (!tmpBitmap.tryAllocPixels(SkImageInfo::Make(rSourceBitmap.GetSize().Width(),
-                                                    rSourceBitmap.GetSize().Height(),
-                                                    kN32_SkColorType, kUnpremul_SkAlphaType)))
+    if (!tmpBitmap.tryAllocN32Pixels(rSourceBitmap.GetSize().Width(),
+                                     rSourceBitmap.GetSize().Height()))
         return false;
     SkCanvas canvas(tmpBitmap);
     SkPaint paint;
-    paint.setBlendMode(SkBlendMode::kDst);
-    canvas.drawBitmap(static_cast<const SkiaSalBitmap&>(rSourceBitmap).GetSkBitmap(), 0, 0, &paint);
+    paint.setBlendMode(SkBlendMode::kSrc); // copy as is, including alpha
+    canvas.drawBitmap(static_cast<const SkiaSalBitmap&>(rAlphaBitmap).GetAlphaSkBitmap(), 0, 0,
+                      &paint);
     paint.setBlendMode(SkBlendMode::kSrcIn);
-    canvas.drawBitmap(static_cast<const SkiaSalBitmap&>(rAlphaBitmap).GetSkBitmap(), 0, 0, &paint);
+    canvas.drawBitmap(static_cast<const SkiaSalBitmap&>(rSourceBitmap).GetSkBitmap(), 0, 0, &paint);
     mSurface->getCanvas()->drawBitmapRect(
         tmpBitmap,
         SkRect::MakeXYWH(rPosAry.mnSrcX, rPosAry.mnSrcY, rPosAry.mnSrcWidth, rPosAry.mnSrcHeight),
@@ -367,6 +369,14 @@ void SkiaSalGraphicsImpl::dump(const char* file) const
     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) const
+{
+    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());
+}
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx
index 88677b6132ad..68ea005e5784 100644
--- a/vcl/skia/salbmp.cxx
+++ b/vcl/skia/salbmp.cxx
@@ -26,21 +26,23 @@
 
 #include <SkCanvas.h>
 #include <SkImage.h>
+#include <SkPixelRef.h>
 
 #ifdef DBG_UTIL
 #include <fstream>
+#define CANARY "skia-canary"
 #endif
 
 SkiaSalBitmap::SkiaSalBitmap() {}
 
 SkiaSalBitmap::~SkiaSalBitmap() {}
 
-static SkColorType getSkColorType(int bitCount, const BitmapPalette& palette)
+static SkColorType getSkColorType(int bitCount)
 {
     switch (bitCount)
     {
         case 8:
-            return palette.IsGreyPalette() ? kGray_8_SkColorType : kAlpha_8_SkColorType;
+            return kGray_8_SkColorType; // see GetAlphaSkBitmap()
         case 24:
             return kRGB_888x_SkColorType;
         case 32:
@@ -72,18 +74,18 @@ bool SkiaSalBitmap::Create(const Size& rSize, sal_uInt16 nBitCount, const Bitmap
     Destroy();
     if (!isValidBitCount(nBitCount))
         return false;
-    if (nBitCount >= 8)
+    // Skia does not support paletted images, so convert only types Skia supports.
+    if (nBitCount > 8 || (nBitCount == 8 && (!rPal || rPal.IsGreyPalette())))
     {
-        if (!mBitmap.tryAllocPixels(
-                SkImageInfo::Make(rSize.Width(), rSize.Height(), getSkColorType(nBitCount, rPal),
-                                  nBitCount == 32 ? kPremul_SkAlphaType : kOpaque_SkAlphaType)))
+        if (!mBitmap.tryAllocPixels(SkImageInfo::Make(
+                rSize.Width(), rSize.Height(), getSkColorType(nBitCount), kPremul_SkAlphaType)))
         {
             return false;
         }
     }
     else
     {
-        // Skia doesn't support the (ancient) low bpp bit counts, so handle them manually
+        // Paletted images are stored in a buffer and converted as necessary.
         int bitScanlineWidth;
         if (o3tl::checked_multiply<int>(rSize.Width(), nBitCount, bitScanlineWidth))
         {
@@ -93,7 +95,16 @@ bool SkiaSalBitmap::Create(const Size& rSize, sal_uInt16 nBitCount, const Bitmap
         mScanlineSize = AlignedWidth4Bytes(bitScanlineWidth);
         sal_uInt8* buffer = nullptr;
         if (mScanlineSize != 0 && rSize.Height() != 0)
-            buffer = new sal_uInt8[mScanlineSize * rSize.Height()];
+        {
+            size_t allocate = mScanlineSize * rSize.Height();
+#ifdef DBG_UTIL
+            allocate += sizeof(CANARY);
+#endif
+            buffer = new sal_uInt8[allocate];
+#if OSL_DEBUG_LEVEL > 0
+            memcpy(buffer + allocate - sizeof(CANARY), CANARY, sizeof(CANARY));
+#endif
+        }
         mBuffer.reset(buffer);
     }
     mPalette = rPal;
@@ -123,9 +134,13 @@ bool SkiaSalBitmap::Create(const SalBitmap& rSalBmp, sal_uInt16 nNewBitCount)
         mSize = src.mSize;
         if (src.mBuffer != nullptr)
         {
-            sal_uInt32 dataSize = src.mScanlineSize * src.mSize.Height();
-            sal_uInt8* newBuffer = new sal_uInt8[dataSize];
-            memcpy(newBuffer, src.mBuffer.get(), dataSize);
+            sal_uInt32 allocate = src.mScanlineSize * src.mSize.Height();
+#ifdef DBG_UTIL
+            assert(memcmp(src.mBuffer.get() + allocate, CANARY, sizeof(CANARY)) == 0);
+            allocate += sizeof(CANARY);
+#endif
+            sal_uInt8* newBuffer = new sal_uInt8[allocate];
+            memcpy(newBuffer, src.mBuffer.get(), allocate);
             mBuffer.reset(newBuffer);
             mScanlineSize = src.mScanlineSize;
         }
@@ -167,61 +182,58 @@ BitmapBuffer* SkiaSalBitmap::AcquireBuffer(BitmapAccessMode nMode)
     buffer->mnHeight = mSize.Height();
     buffer->mnBitCount = mBitCount;
     buffer->maPalette = mPalette;
-    if (mBitCount >= 8)
+    if (mBuffer)
     {
-        buffer->mpBits = static_cast<sal_uInt8*>(mBitmap.getPixels());
-        buffer->mnScanlineSize = mBitmap.rowBytes();
+        buffer->mpBits = mBuffer.get();
+        buffer->mnScanlineSize = mScanlineSize;
     }
     else
     {
-        buffer->mpBits = mBuffer.get();
-        buffer->mnScanlineSize = mScanlineSize;
+        buffer->mpBits = static_cast<sal_uInt8*>(mBitmap.getPixels());
+        buffer->mnScanlineSize = mBitmap.rowBytes();
     }
     switch (mBitCount)
     {
         case 1:
-#ifdef OSL_BIGENDIAN
-            buffer->mnFormat = ScanlineFormat::N1BitMsbPal | ScanlineFormat::TopDown;
-#else
-            buffer->mnFormat = ScanlineFormat::N1BitLsbPal | ScanlineFormat::TopDown;
-#endif
+            buffer->mnFormat = ScanlineFormat::N1BitMsbPal;
             break;
         case 4:
-#ifdef OSL_BIGENDIAN
-            buffer->mnFormat = ScanlineFormat::N4BitMsnPal | ScanlineFormat::TopDown;
-#else
-            buffer->mnFormat = ScanlineFormat::N4BitLsnPal | ScanlineFormat::TopDown;
-#endif
+            buffer->mnFormat = ScanlineFormat::N4BitMsnPal;
             break;
         case 8:
-            buffer->mnFormat = ScanlineFormat::N8BitPal | ScanlineFormat::TopDown;
+            // TODO or always N8BitPal?
+            //            buffer->mnFormat = !mPalette ? ScanlineFormat::N8BitTcMask : ScanlineFormat::N8BitPal;
+            buffer->mnFormat = ScanlineFormat::N8BitPal;
             break;
         case 24:
-            buffer->mnFormat = ScanlineFormat::N24BitTcRgb | ScanlineFormat::TopDown;
+            buffer->mnFormat = ScanlineFormat::N24BitTcRgb;
             break;
         case 32:
             // TODO are these correct?
             buffer->mnFormat = mBitmap.colorType() == kRGBA_8888_SkColorType
-                                   ? ScanlineFormat::N32BitTcBgra
-                                   : ScanlineFormat::N32BitTcArgb;
-            buffer->mnFormat |= ScanlineFormat::TopDown;
+                                   ? ScanlineFormat::N32BitTcRgba
+                                   : ScanlineFormat::N32BitTcBgra;
             break;
         default:
             abort();
     }
+    buffer->mnFormat |= ScanlineFormat::TopDown;
     return buffer;
 }
 
 void SkiaSalBitmap::ReleaseBuffer(BitmapBuffer* pBuffer, BitmapAccessMode nMode)
 {
-    mPalette = pBuffer->maPalette;
+    if (nMode == BitmapAccessMode::Write) // TODO something more?
+    {
+        mPalette = pBuffer->maPalette;
+        ResetCachedBitmap();
+    }
     // Are there any more ground movements underneath us ?
     assert(pBuffer->mnWidth == mSize.Width());
     assert(pBuffer->mnHeight == mSize.Height());
     assert(pBuffer->mnBitCount == mBitCount);
+    verify();
     delete pBuffer;
-    if (nMode == BitmapAccessMode::Write) // TODO something more?
-        ResetCachedBitmap();
 }
 
 bool SkiaSalBitmap::GetSystemData(BitmapSystemData& rData)
@@ -254,10 +266,10 @@ const SkBitmap& SkiaSalBitmap::GetSkBitmap() const
 {
     if (mBuffer && mBitmap.drawsNothing())
     {
-        assert(mBitCount == 1 || mBitCount == 2 || mBitCount == 4);
-        std::unique_ptr<sal_uInt8[]> data = convertDataTo32Bpp(
+        std::unique_ptr<sal_uInt8[]> data = convertDataBitCount(
             mBuffer.get(), mSize.Width(), mSize.Height(), mBitCount, mScanlineSize, mPalette,
-            kN32_SkColorType == kBGRA_8888_SkColorType); // TODO
+            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,
@@ -267,9 +279,49 @@ const SkBitmap& SkiaSalBitmap::GetSkBitmap() const
     return mBitmap;
 }
 
-// Reset the cached bitmap allocatd in GetSkBitmap().
+const SkBitmap& SkiaSalBitmap::GetAlphaSkBitmap() const
+{
+    assert(mBitCount <= 8);
+    if (mAlphaBitmap.drawsNothing())
+    {
+        if (mBuffer)
+        {
+            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))
+                abort();
+        }
+        else
+        {
+            assert(mBitmap.colorType() == kGray_8_SkColorType);
+            // 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
+            // and mAlphaBitmap is kAlpha_8_SkColorType that can be used as a mask.
+            // Make mAlphaBitmap share mBitmap's data.
+            const_cast<SkBitmap&>(mAlphaBitmap)
+                .setInfo(mBitmap.info().makeColorType(kAlpha_8_SkColorType), mBitmap.rowBytes());
+            const_cast<SkBitmap&>(mAlphaBitmap)
+                .setPixelRef(sk_ref_sp(mBitmap.pixelRef()), mBitmap.pixelRefOrigin().x(),
+                             mBitmap.pixelRefOrigin().y());
+            return mAlphaBitmap;
+        }
+    }
+    return mAlphaBitmap;
+}
+
+// Reset the cached bitmap allocated in GetSkBitmap().
 void SkiaSalBitmap::ResetCachedBitmap()
 {
+    mAlphaBitmap.reset();
     if (mBuffer)
         mBitmap.reset();
 }
@@ -282,6 +334,15 @@ void SkiaSalBitmap::dump(const char* file) const
     std::ofstream ostream(file, std::ios::binary);
     ostream.write(static_cast<const char*>(data->data()), data->size());
 }
+
+void SkiaSalBitmap::verify() const
+{
+    if (!mBuffer)
+        return;
+    size_t canary = mScanlineSize * mSize.Height();
+    assert(memcmp(mBuffer.get() + canary, CANARY, sizeof(CANARY)) == 0);
+}
+
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/bitmap/salbmp.cxx b/vcl/source/bitmap/salbmp.cxx
index c7183e117598..59ff8ef2e5ec 100644
--- a/vcl/source/bitmap/salbmp.cxx
+++ b/vcl/source/bitmap/salbmp.cxx
@@ -143,10 +143,12 @@ ImplPixelFormat* ImplPixelFormat::GetFormat( sal_uInt16 nBits, const BitmapPalet
 
 } // namespace
 
-std::unique_ptr< sal_uInt8[] > SalBitmap::convertDataTo24Bpp( const sal_uInt8* src,
-    int width, int height, int bitCount, int bytesPerRow, const BitmapPalette& palette, bool toBgr )
+std::unique_ptr< sal_uInt8[] > SalBitmap::convertDataBitCount( const sal_uInt8* src,
+    int width, int height, int bitCount, int bytesPerRow, const BitmapPalette& palette, BitConvert type )
 {
-    std::unique_ptr< sal_uInt8[] > data( new sal_uInt8[width * height * 3] );
+    assert( bitCount == 1 || bitCount == 4 || bitCount == 8 );
+    static const int bpp[] = { 1, 3, 3, 4, 4 };
+    std::unique_ptr< sal_uInt8[] > data( new sal_uInt8[width * height * bpp[ (int)type ]] );
     std::unique_ptr<ImplPixelFormat> pSrcFormat(ImplPixelFormat::GetFormat(bitCount, palette));
 
     const sal_uInt8* pSrcData = src;
@@ -158,68 +160,53 @@ std::unique_ptr< sal_uInt8[] > SalBitmap::convertDataTo24Bpp( const sal_uInt8* s
         pSrcFormat->StartLine( pSrcData );
 
         sal_uInt32 nX = width;
-        if (toBgr)
+        switch( type )
         {
-            while( nX-- )
-            {
-                const BitmapColor& c = pSrcFormat->ReadPixel();
-                *pDstData++ = c.GetBlue();
-                *pDstData++ = c.GetGreen();
-                *pDstData++ = c.GetRed();
-            }
-        }
-        else // RGB
-        {
-            while( nX-- )
-            {
-                const BitmapColor& c = pSrcFormat->ReadPixel();
-                *pDstData++ = c.GetRed();
-                *pDstData++ = c.GetGreen();
-                *pDstData++ = c.GetBlue();
-            }
-        }
-
-        pSrcData += bytesPerRow;
-    }
-    return data;
-}
-
-std::unique_ptr< sal_uInt8[] > SalBitmap::convertDataTo32Bpp( const sal_uInt8* src,
-    int width, int height, int bitCount, int bytesPerRow, const BitmapPalette& palette, bool toBgra )
-{
-    std::unique_ptr< sal_uInt8[] > data( new sal_uInt8[width * height * 4] );
-    std::unique_ptr<ImplPixelFormat> pSrcFormat(ImplPixelFormat::GetFormat(bitCount, palette));
-
-    const sal_uInt8* pSrcData = src;
-    sal_uInt8* pDstData = data.get();
-
-    sal_uInt32 nY = height;
-    while( nY-- )
-    {
-        pSrcFormat->StartLine( pSrcData );
-
-        sal_uInt32 nX = width;
-        if (toBgra)
-        {
-            while( nX-- )
-            {
-                const BitmapColor& c = pSrcFormat->ReadPixel();
-                *pDstData++ = c.GetBlue();
-                *pDstData++ = c.GetGreen();
-                *pDstData++ = c.GetRed();
-                *pDstData++ = 0xff;
-            }
-        }
-        else // RGBA
-        {
-            while( nX-- )
-            {
-                const BitmapColor& c = pSrcFormat->ReadPixel();
-                *pDstData++ = c.GetRed();
-                *pDstData++ = c.GetGreen();
-                *pDstData++ = c.GetBlue();
-                *pDstData++ = 0xff;
-            }
+            case BitConvert::A8 :
+                while( nX-- )
+                {
+                    const BitmapColor& c = pSrcFormat->ReadPixel();
+                    *pDstData++ = 0xff - c.GetBlue();
+                }
+                break;
+            case BitConvert::BGR :
+                while( nX-- )
+                {
+                    const BitmapColor& c = pSrcFormat->ReadPixel();
+                    *pDstData++ = c.GetBlue();
+                    *pDstData++ = c.GetGreen();
+                    *pDstData++ = c.GetRed();
+                }
+                break;
+            case BitConvert::RGB :
+                while( nX-- )
+                {
+                    const BitmapColor& c = pSrcFormat->ReadPixel();
+                    *pDstData++ = c.GetRed();
+                    *pDstData++ = c.GetGreen();
+                    *pDstData++ = c.GetBlue();
+                }
+                break;
+            case BitConvert::BGRA :
+                while( nX-- )
+                {
+                    const BitmapColor& c = pSrcFormat->ReadPixel();
+                    *pDstData++ = c.GetBlue();
+                    *pDstData++ = c.GetGreen();
+                    *pDstData++ = c.GetRed();
+                    *pDstData++ = 0xff;
+                }
+                break;
+            case BitConvert::RGBA :
+                while( nX-- )
+                {
+                    const BitmapColor& c = pSrcFormat->ReadPixel();
+                    *pDstData++ = c.GetRed();
+                    *pDstData++ = c.GetGreen();
+                    *pDstData++ = c.GetBlue();
+                    *pDstData++ = 0xff;
+                }
+                break;
         }
 
         pSrcData += bytesPerRow;