make sure Skia bitmap checksum is invalidated properly

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

3 files changed, 56 insertions, 1 deletions

diff --git a/vcl/inc/skia/salbmp.hxx b/vcl/inc/skia/salbmp.hxx
index 2959952a3442..05e489643f88 100644
--- a/vcl/inc/skia/salbmp.hxx
+++ b/vcl/inc/skia/salbmp.hxx
@@ -107,8 +107,12 @@ public:
     const sal_uInt8* unittestGetBuffer() const { return mBuffer.get(); }
     const SkImage* unittestGetImage() const { return mImage.get(); }
     const SkImage* unittestGetAlphaImage() const { return mAlphaImage.get(); }
+    void unittestResetToImage() { ResetToSkImage(GetSkImage()); }
 
 private:
+    // This should be called whenever the contents have (possibly) changed.
+    // It may reset some cached data such as the checksum.
+    void DataChanged();
     // Reset the state to pixel data (resets cached images allocated in GetSkImage()/GetAlphaSkImage()).
     void ResetToBuffer();
     // Sets the data only as SkImage (will be converted as needed).
diff --git a/vcl/qa/cppunit/skia/skia.cxx b/vcl/qa/cppunit/skia/skia.cxx
index b94e2ede7eba..21e3d0baf696 100644
--- a/vcl/qa/cppunit/skia/skia.cxx
+++ b/vcl/qa/cppunit/skia/skia.cxx
@@ -44,6 +44,7 @@ public:
     void testDelayedScale();
     void testDelayedScaleAlphaImage();
     void testDrawDelayedScaleImage();
+    void testChecksum();
     void testTdf137329();
     void testTdf140848();
     void testTdf132367();
@@ -58,6 +59,7 @@ public:
     CPPUNIT_TEST(testDelayedScale);
     CPPUNIT_TEST(testDelayedScaleAlphaImage);
     CPPUNIT_TEST(testDrawDelayedScaleImage);
+    CPPUNIT_TEST(testChecksum);
     CPPUNIT_TEST(testTdf137329);
     CPPUNIT_TEST(testTdf140848);
     CPPUNIT_TEST(testTdf132367);
@@ -468,6 +470,45 @@ void SkiaTest::testDrawDelayedScaleImage()
     CPPUNIT_ASSERT_EQUAL(Size(10, 10), SkiaHelper::imageSize(image3));
 }
 
+void SkiaTest::testChecksum()
+{
+    if (!SkiaHelper::isVCLSkiaEnabled())
+        return;
+    Bitmap bitmap(Size(10, 10), vcl::PixelFormat::N24_BPP);
+    bitmap.Erase(COL_RED);
+    BitmapChecksum checksum1 = bitmap.GetChecksum();
+    // Set a pixel to create pixel data, that should change checksum.
+    BitmapWriteAccess(bitmap).SetPixel(0, 0, COL_BLUE);
+    BitmapChecksum checksum2 = bitmap.GetChecksum();
+    CPPUNIT_ASSERT(checksum2 != checksum1);
+    SkiaSalBitmap* skiaBitmap1 = dynamic_cast<SkiaSalBitmap*>(bitmap.ImplGetSalBitmap().get());
+    // Creating an image should not change the checksum.
+    sk_sp<SkImage> image1 = skiaBitmap1->GetSkImage();
+    BitmapChecksum checksum3 = bitmap.GetChecksum();
+    CPPUNIT_ASSERT_EQUAL(checksum2, checksum3);
+    // Delayed scaling should change checksum even if the scaling has not taken place.
+    bitmap.Scale(Size(20, 20));
+    BitmapChecksum checksum4 = bitmap.GetChecksum();
+    CPPUNIT_ASSERT(checksum4 != checksum3);
+    // Setting back to the original red content should have the original checksum.
+    // (This also makes sure this next step is not affected by the delayed scaling
+    // above possibly taking place now.)
+    bitmap = Bitmap(Size(10, 10), vcl::PixelFormat::N24_BPP);
+    bitmap.Erase(COL_RED);
+    BitmapChecksum checksum5 = bitmap.GetChecksum();
+    CPPUNIT_ASSERT_EQUAL(checksum1, checksum5);
+    // The optimized changing of images to greyscale should change the checksum.
+    SkiaSalBitmap* skiaBitmap2 = dynamic_cast<SkiaSalBitmap*>(bitmap.ImplGetSalBitmap().get());
+    skiaBitmap2->unittestResetToImage();
+    BitmapChecksum checksum6;
+    skiaBitmap2->GetChecksum(checksum6);
+    CPPUNIT_ASSERT_EQUAL(checksum5, checksum6);
+    CPPUNIT_ASSERT(skiaBitmap2->ConvertToGreyscale());
+    BitmapChecksum checksum7;
+    skiaBitmap2->GetChecksum(checksum7);
+    CPPUNIT_ASSERT(checksum7 != checksum6);
+}
+
 void SkiaTest::testTdf137329()
 {
     if (!SkiaHelper::isVCLSkiaEnabled())
diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx
index 7b5d05007129..c18ad18f8428 100644
--- a/vcl/skia/salbmp.cxx
+++ b/vcl/skia/salbmp.cxx
@@ -158,6 +158,8 @@ bool SkiaSalBitmap::Create(const SalBitmap& rSalBmp, SalGraphics* pGraphics)
 bool SkiaSalBitmap::Create(const SalBitmap& rSalBmp, vcl::PixelFormat eNewPixelFormat)
 {
     assert(mAnyAccessCount == 0);
+    assert(&rSalBmp != this);
+    ResetAllData();
     const SkiaSalBitmap& src = static_cast<const SkiaSalBitmap&>(rSalBmp);
     mImage = src.mImage;
     mAlphaImage = src.mAlphaImage;
@@ -296,7 +298,7 @@ void SkiaSalBitmap::ReleaseBuffer(BitmapBuffer* pBuffer, BitmapAccessMode nMode,
 #endif
         mPalette = pBuffer->maPalette;
         ResetToBuffer();
-        InvalidateChecksum();
+        DataChanged();
     }
     assert(mAnyAccessCount > 0);
     --mAnyAccessCount;
@@ -451,6 +453,7 @@ bool SkiaSalBitmap::Scale(const double& rScaleX, const double& rScaleY, BmpScale
         ResetToSkImage(mImage);
     else
         ResetToBuffer();
+    DataChanged();
     // The rest will be handled when the scaled bitmap is actually needed,
     // such as in EnsureBitmapData() or GetSkImage().
     return true;
@@ -494,6 +497,7 @@ bool SkiaSalBitmap::ConvertToGreyscale()
         ComputeScanlineSize();
         mPalette = Bitmap::GetGreyPalette(256);
         ResetToSkImage(makeCheckedImageSnapshot(surface));
+        DataChanged();
         SAL_INFO("vcl.skia.trace", "converttogreyscale(" << this << ")");
         return true;
     }
@@ -531,6 +535,7 @@ bool SkiaSalBitmap::InterpretAs8Bit()
         ComputeScanlineSize();
         mPalette = Bitmap::GetGreyPalette(256);
         ResetToSkImage(mImage); // keep mImage, it will be interpreted as 8bit if needed
+        DataChanged();
         SAL_INFO("vcl.skia.trace", "interpretas8bit(" << this << ") with image");
         return true;
     }
@@ -584,6 +589,7 @@ bool SkiaSalBitmap::AlphaBlendWith(const SalBitmap& rSalBmp)
         const sal_uInt16 nGrey2 = otherBitmap->mEraseColor.GetRed();
         const sal_uInt8 nGrey = static_cast<sal_uInt8>(nGrey1 + nGrey2 - nGrey1 * nGrey2 / 255);
         mEraseColor = Color(nGrey, nGrey, nGrey);
+        DataChanged();
         SAL_INFO("vcl.skia.trace",
                  "alphablendwith(" << this << ") : with erase color " << otherBitmap);
         return true;
@@ -603,6 +609,7 @@ bool SkiaSalBitmap::AlphaBlendWith(const SalBitmap& rSalBmp)
     paint.setBlendMode(SkBlendMode::kScreen); // src+dest - src*dest/255 (in 0..1)
     surface->getCanvas()->drawImage(otherBitmap->GetSkImage(), 0, 0, SkSamplingOptions(), &paint);
     ResetToSkImage(makeCheckedImageSnapshot(surface));
+    DataChanged();
     SAL_INFO("vcl.skia.trace", "alphablendwith(" << this << ") : with image " << otherBitmap);
     return true;
 }
@@ -1318,8 +1325,11 @@ void SkiaSalBitmap::ResetAllData()
     mEraseColorSet = false;
     mPixelsSize = mSize;
     ComputeScanlineSize();
+    DataChanged();
 }
 
+void SkiaSalBitmap::DataChanged() { InvalidateChecksum(); }
+
 void SkiaSalBitmap::ResetPendingScaling()
 {
     if (mPixelsSize == mSize)