[Libreoffice-commits] core.git: vcl/qa vcl/skia

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

 vcl/qa/cppunit/skia/skia.cxx |   40 +++++++++++++++++++++++++++++++++++-----
 vcl/skia/salbmp.cxx          |   13 +++++++++++--
 2 files changed, 46 insertions(+), 7 deletions(-)

New commits:
commit c57010def9e2e62b1d3d60f13ec58967c09fbf91
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Fri Oct 2 15:29:34 2020 +0200
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Sun Oct 4 11:10:51 2020 +0200

    optimize and unittest SkiaSalBitmap::InterpretAs8Bit()
    
    Reinterpreting a color is even simpler, and do it ourselves even if
    there is pixel buffer, as the SkImage is preferred (it'll be more
    likely used afterwards).
    
    Change-Id: I3ea7ea0abb3133dce18ee51f1aae324e7bb51196
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103886
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>

diff --git a/vcl/qa/cppunit/skia/skia.cxx b/vcl/qa/cppunit/skia/skia.cxx
index 439440962b44..7fa4a9e3f00e 100644
--- a/vcl/qa/cppunit/skia/skia.cxx
+++ b/vcl/qa/cppunit/skia/skia.cxx
@@ -32,10 +32,12 @@ public:
 
     void testBitmapErase();
     void testDrawShaders();
+    void testInterpretAs8Bit();
 
     CPPUNIT_TEST_SUITE(SkiaTest);
     CPPUNIT_TEST(testBitmapErase);
     CPPUNIT_TEST(testDrawShaders);
+    CPPUNIT_TEST(testInterpretAs8Bit);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -76,8 +78,7 @@ void SkiaTest::testBitmapErase()
     CPPUNIT_ASSERT(!skiaBitmap->unittestHasAlphaImage());
     CPPUNIT_ASSERT(skiaBitmap->unittestHasEraseColor());
     // Reading a pixel will create pixel data.
-    BitmapReadAccess access(bitmap);
-    CPPUNIT_ASSERT_EQUAL(BitmapColor(COL_RED), access.GetColor(0, 0));
+    CPPUNIT_ASSERT_EQUAL(BitmapColor(COL_RED), BitmapReadAccess(bitmap).GetColor(0, 0));
     skiaBitmap = dynamic_cast<SkiaSalBitmap*>(bitmap.ImplGetSalBitmap().get());
     CPPUNIT_ASSERT(skiaBitmap->unittestHasBuffer());
     CPPUNIT_ASSERT(!skiaBitmap->unittestHasImage());
@@ -97,12 +98,10 @@ void SkiaTest::testDrawShaders()
     Bitmap bitmap(Size(10, 10), 24);
     bitmap.Erase(COL_RED);
     SkiaSalBitmap* skiaBitmap = dynamic_cast<SkiaSalBitmap*>(bitmap.ImplGetSalBitmap().get());
-    CPPUNIT_ASSERT(skiaBitmap);
     CPPUNIT_ASSERT(skiaBitmap->PreferSkShader());
     AlphaMask alpha(Size(10, 10));
     alpha.Erase(64);
     SkiaSalBitmap* skiaAlpha = dynamic_cast<SkiaSalBitmap*>(alpha.ImplGetSalBitmap().get());
-    CPPUNIT_ASSERT(skiaAlpha);
     CPPUNIT_ASSERT(skiaAlpha->PreferSkShader());
 
     device->DrawBitmap(Point(5, 5), bitmap);
@@ -143,7 +142,6 @@ void SkiaTest::testDrawShaders()
     bitmapLarge.Erase(COL_RED);
     SkiaSalBitmap* skiaBitmapLarge
         = dynamic_cast<SkiaSalBitmap*>(bitmapLarge.ImplGetSalBitmap().get());
-    CPPUNIT_ASSERT(skiaBitmapLarge);
     CPPUNIT_ASSERT(skiaBitmapLarge->PreferSkShader());
     AlphaMask alphaLarge(Size(100, 100));
     alphaLarge.Erase(64);
@@ -171,6 +169,38 @@ void SkiaTest::testDrawShaders()
     device->Erase();
 }
 
+void SkiaTest::testInterpretAs8Bit()
+{
+    if (!SkiaHelper::isVCLSkiaEnabled())
+        return;
+    Bitmap bitmap(Size(10, 10), 24);
+    // Test with erase color.
+    bitmap.Erase(Color(33, 33, 33));
+    SkiaSalBitmap* skiaBitmap = dynamic_cast<SkiaSalBitmap*>(bitmap.ImplGetSalBitmap().get());
+    CPPUNIT_ASSERT(skiaBitmap->unittestHasEraseColor());
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(24), bitmap.GetBitCount());
+    bitmap.Convert(BmpConversion::N8BitNoConversion);
+    skiaBitmap = dynamic_cast<SkiaSalBitmap*>(bitmap.ImplGetSalBitmap().get());
+    CPPUNIT_ASSERT(skiaBitmap->unittestHasEraseColor());
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(8), bitmap.GetBitCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt8>(33), BitmapReadAccess(bitmap).GetPixelIndex(0, 0));
+
+    // Test with image.
+    bitmap = Bitmap(Size(10, 10), 24);
+    bitmap.Erase(Color(34, 34, 34));
+    BitmapReadAccess(bitmap).GetColor(0, 0); // Create pixel data, reset erase color.
+    skiaBitmap = dynamic_cast<SkiaSalBitmap*>(bitmap.ImplGetSalBitmap().get());
+    skiaBitmap->GetSkImage();
+    CPPUNIT_ASSERT(!skiaBitmap->unittestHasEraseColor());
+    CPPUNIT_ASSERT(skiaBitmap->unittestHasImage());
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(24), bitmap.GetBitCount());
+    bitmap.Convert(BmpConversion::N8BitNoConversion);
+    skiaBitmap = dynamic_cast<SkiaSalBitmap*>(bitmap.ImplGetSalBitmap().get());
+    CPPUNIT_ASSERT(skiaBitmap->unittestHasImage());
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt16>(8), bitmap.GetBitCount());
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_uInt8>(34), BitmapReadAccess(bitmap).GetPixelIndex(0, 0));
+}
+
 } // namespace
 
 CPPUNIT_TEST_SUITE_REGISTRATION(SkiaTest);
diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx
index b6b85f60d9f3..e8f67b0585ed 100644
--- a/vcl/skia/salbmp.cxx
+++ b/vcl/skia/salbmp.cxx
@@ -412,6 +412,14 @@ bool SkiaSalBitmap::InterpretAs8Bit()
 #endif
     if (mBitCount == 8 && mPalette.IsGreyPalette8Bit())
         return true;
+    if (mEraseColorSet)
+    {
+        mBitCount = 8;
+        ComputeScanlineSize();
+        mPalette = Bitmap::GetGreyPalette(256);
+        SAL_INFO("vcl.skia.trace", "interpretas8bit(" << this << ") with erase color");
+        return true;
+    }
     // This is usually used by AlphaMask, the point is just to treat
     // the content as an alpha channel. This is often used
     // by the horrible separate-alpha-outdev hack, where the bitmap comes
@@ -421,15 +429,16 @@ bool SkiaSalBitmap::InterpretAs8Bit()
     // just treat the SkImage as being for 8bit bitmap. EnsureBitmapData()
     // will do the conversion if needed, but the normal case will be
     // GetAlphaSkImage() creating kAlpha_8_SkColorType SkImage from it.
-    if (!mBuffer && mImage)
+    if (mImage)
     {
         mBitCount = 8;
         ComputeScanlineSize();
         mPalette = Bitmap::GetGreyPalette(256);
         ResetToSkImage(mImage); // keep mImage, it will be interpreted as 8bit if needed
-        SAL_INFO("vcl.skia.trace", "interpretas8bit(" << this << ")");
+        SAL_INFO("vcl.skia.trace", "interpretas8bit(" << this << ") with image");
         return true;
     }
+    SAL_INFO("vcl.skia.trace", "interpretas8bit(" << this << ") with pixel data, ignoring");
     return false;
 }