diff --git a/vcl/inc/skia/salbmp.hxx b/vcl/inc/skia/salbmp.hxx index 0509ed3381b6..012594169132 100644 --- a/vcl/inc/skia/salbmp.hxx +++ b/vcl/inc/skia/salbmp.hxx @@ -105,8 +105,6 @@ private: void ResetToBuffer(); // Sets the data only as SkImage (will be converted as needed). void ResetToSkImage(sk_sp image); - // Resets all data that does not match mSize. - void ResetCachedDataBySize(); // Resets all data (buffer and images). void ResetAllData(); // Call to ensure mBuffer has data (will convert from mImage if necessary). @@ -119,6 +117,8 @@ private: void CreateBitmapData(); // Should be called whenever mPixelsSize or mBitCount is set/changed. bool ComputeScanlineSize(); + // Resets information about pending scaling. To be called when mBuffer is resized or created. + void ResetPendingScaling(); // Sets bitmap to be erased on demand. void EraseInternal(const Color& color); // Sets pixels to the erase color. diff --git a/vcl/qa/cppunit/skia/skia.cxx b/vcl/qa/cppunit/skia/skia.cxx index 8f288d813404..d8e103d1a431 100644 --- a/vcl/qa/cppunit/skia/skia.cxx +++ b/vcl/qa/cppunit/skia/skia.cxx @@ -20,6 +20,8 @@ #include #include +using namespace SkiaHelper; + // This tests backends that use Skia (i.e. intentionally not the svp one, which is the default.) // Note that you still may need to actually set for Skia to be used (see vcl/README.vars). // If Skia is not enabled, all tests will be silently skipped. @@ -39,6 +41,7 @@ public: void testAlphaBlendWith(); void testBitmapCopyOnWrite(); void testMatrixQuality(); + void testDelayedScale(); void testTdf137329(); CPPUNIT_TEST_SUITE(SkiaTest); @@ -48,6 +51,7 @@ public: CPPUNIT_TEST(testAlphaBlendWith); CPPUNIT_TEST(testBitmapCopyOnWrite); CPPUNIT_TEST(testMatrixQuality); + CPPUNIT_TEST(testDelayedScale); CPPUNIT_TEST(testTdf137329); CPPUNIT_TEST_SUITE_END(); @@ -329,6 +333,45 @@ void SkiaTest::testMatrixQuality() #endif } +void SkiaTest::testDelayedScale() +{ + if (!SkiaHelper::isVCLSkiaEnabled()) + return; + Bitmap bitmap1(Size(10, 10), vcl::PixelFormat::N24_BPP); + SkiaSalBitmap* skiaBitmap1 = dynamic_cast(bitmap1.ImplGetSalBitmap().get()); + CPPUNIT_ASSERT(skiaBitmap1); + // Do scaling based on mBuffer. + (void)BitmapReadAccess(bitmap1); // allocates mBuffer + CPPUNIT_ASSERT(skiaBitmap1->unittestHasBuffer()); + CPPUNIT_ASSERT(!skiaBitmap1->unittestHasImage()); + CPPUNIT_ASSERT(bitmap1.Scale(2, 2, BmpScaleFlag::Default)); + skiaBitmap1 = dynamic_cast(bitmap1.ImplGetSalBitmap().get()); + CPPUNIT_ASSERT(skiaBitmap1); + CPPUNIT_ASSERT(skiaBitmap1->unittestHasBuffer()); + CPPUNIT_ASSERT(!skiaBitmap1->unittestHasImage()); + CPPUNIT_ASSERT_EQUAL(Size(20, 20), bitmap1.GetSizePixel()); + CPPUNIT_ASSERT_EQUAL(Size(20, 20), imageSize(skiaBitmap1->GetSkImage())); + BitmapBuffer* buffer1 = skiaBitmap1->AcquireBuffer(BitmapAccessMode::Read); + CPPUNIT_ASSERT(buffer1); + CPPUNIT_ASSERT_EQUAL(tools::Long(20), buffer1->mnWidth); + CPPUNIT_ASSERT_EQUAL(tools::Long(20), buffer1->mnHeight); + skiaBitmap1->ReleaseBuffer(buffer1, BitmapAccessMode::Read); + // Do scaling based on mImage. + SkiaSalBitmap skiaBitmap2(skiaBitmap1->GetSkImage()); + CPPUNIT_ASSERT(!skiaBitmap2.unittestHasBuffer()); + CPPUNIT_ASSERT(skiaBitmap2.unittestHasImage()); + CPPUNIT_ASSERT(skiaBitmap2.Scale(2, 3, BmpScaleFlag::Default)); + CPPUNIT_ASSERT(!skiaBitmap2.unittestHasBuffer()); + CPPUNIT_ASSERT(skiaBitmap2.unittestHasImage()); + CPPUNIT_ASSERT_EQUAL(Size(40, 60), skiaBitmap2.GetSize()); + CPPUNIT_ASSERT_EQUAL(Size(40, 60), imageSize(skiaBitmap2.GetSkImage())); + BitmapBuffer* buffer2 = skiaBitmap2.AcquireBuffer(BitmapAccessMode::Read); + CPPUNIT_ASSERT(buffer2); + CPPUNIT_ASSERT_EQUAL(tools::Long(40), buffer2->mnWidth); + CPPUNIT_ASSERT_EQUAL(tools::Long(60), buffer2->mnHeight); + skiaBitmap2.ReleaseBuffer(buffer2, BitmapAccessMode::Read); +} + void SkiaTest::testTdf137329() { if (!SkiaHelper::isVCLSkiaEnabled()) diff --git a/vcl/skia/salbmp.cxx b/vcl/skia/salbmp.cxx index c8abfb64252b..3b4ec7104af4 100644 --- a/vcl/skia/salbmp.cxx +++ b/vcl/skia/salbmp.cxx @@ -85,7 +85,8 @@ bool SkiaSalBitmap::Create(const Size& rSize, sal_uInt16 nBitCount, const Bitmap return false; mPalette = rPal; mBitCount = nBitCount; - mSize = mPixelsSize = rSize; + mSize = rSize; + ResetPendingScaling(); if (!ComputeScanlineSize()) { mBitCount = 0; @@ -387,8 +388,8 @@ bool SkiaSalBitmap::Scale(const double& rScaleX, const double& rScaleY, BmpScale if (mEraseColorSet) { // Simple. - mSize = mPixelsSize = newSize; - ComputeScanlineSize(); + mSize = newSize; + ResetPendingScaling(); EraseInternal(mEraseColor); return true; } @@ -408,7 +409,14 @@ bool SkiaSalBitmap::Scale(const double& rScaleX, const double& rScaleY, BmpScale // That means it can be GPU-accelerated, while done here directly it would need // to be either done by CPU, or with the CPU->GPU->CPU roundtrip required // by GPU-accelerated scaling. - // Pending scaling is detected by 'mSize != mPixelsSize'. + // Pending scaling is detected by 'mSize != mPixelsSize' for mBuffer, + // and 'imageSize(mImage) != mSize' for mImage. It is not intended to have 3 different + // sizes though, code below keeps only mBuffer or mImage. Note that imageSize(mImage) + // may or may not be equal to mPixelsSize, depending on whether mImage is set here + // (sizes will be equal) or whether it's set in GetSkImage() (will not be equal). + // Pending scaling is considered "done" by the time mBuffer is resized (or created). + // Resizing of mImage is somewhat independent of this, since mImage is primarily + // considered to be a cached object (although sometimes it's the only data available). // If there is already one scale() pending, use the lowest quality of all requested. switch (nScaleFlag) @@ -427,10 +435,11 @@ bool SkiaSalBitmap::Scale(const double& rScaleX, const double& rScaleY, BmpScale SAL_INFO("vcl.skia.trace", "scale(" << this << "): unsupported scale algorithm"); return false; } - // scaling will be actually done on-demand when needed, the need will be recognized - // by mSize != mPixelsSize mSize = newSize; - // Do not reset cached data if mImage is possibly the only data we have. + // If we have both mBuffer and mImage, prefer mImage, since it likely will be drawn later. + // We could possibly try to keep the buffer as well, but that would complicate things + // with two different data structures to be scaled on-demand, and it's a question + // if that'd realistically help with anything. if (mImage) ResetToSkImage(mImage); else @@ -456,11 +465,12 @@ bool SkiaSalBitmap::ConvertToGreyscale() // Normally this would need to convert contents of mBuffer for all possible formats, // so just let the VCL algorithm do it. // Avoid the costly SkImage->buffer->SkImage conversion. - if (!mBuffer && mImage) + if (!mBuffer && mImage && !mEraseColorSet) { if (mBitCount == 8 && mPalette.IsGreyPalette8Bit()) return true; - sk_sp surface = createSkSurface(mPixelsSize, mImage->imageInfo().alphaType()); + sk_sp surface + = createSkSurface(imageSize(mImage), mImage->imageInfo().alphaType()); SkPaint paint; paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha // VCL uses different coefficients for conversion to gray than Skia, so use the VCL @@ -763,7 +773,7 @@ const sk_sp& SkiaSalBitmap::GetSkImage() const } if (mImage) { - if (mImage->width() != mSize.Width() || mImage->height() != mSize.Height()) + if (imageSize(mImage) != mSize) { assert(!mBuffer); // This code should be only called if only mImage holds data. SkiaZone zone; @@ -829,7 +839,7 @@ const sk_sp& SkiaSalBitmap::GetAlphaSkImage() const if (mImage) { SkiaZone zone; - bool scaling = mImage->width() != mSize.Width() || mImage->height() != mSize.Height(); + const bool scaling = imageSize(mImage) != mSize; SkPixmap pixmap; // Note: We cannot do this when 'scaling' because SkCanvas::drawImageRect() // with kAlpha_8_SkColorType as source and destination would act as SkBlendMode::kSrcOver @@ -1026,7 +1036,6 @@ void SkiaSalBitmap::EnsureBitmapData() SkiaZone zone; assert(mPixelsSize == mSize); assert(!mBuffer); - mScaleQuality = BmpScaleFlag::BestQuality; CreateBitmapData(); // Unset now, so that repeated call will return mBuffer. mEraseColorSet = false; @@ -1054,7 +1063,8 @@ void SkiaSalBitmap::EnsureBitmapData() } // Convert from alpha image, if the conversion is simple. - if (mAlphaImage && mSize == mPixelsSize && mBitCount == 8 && mPalette.IsGreyPalette8Bit()) + if (mAlphaImage && imageSize(mAlphaImage) == mSize && mBitCount == 8 + && mPalette.IsGreyPalette8Bit()) { assert(mAlphaImage->colorType() == kAlpha_8_SkColorType); SkiaZone zone; @@ -1073,6 +1083,7 @@ void SkiaSalBitmap::EnsureBitmapData() canvas.flush(); } bitmap.setImmutable(); + ResetPendingScaling(); CreateBitmapData(); assert(mBuffer != nullptr); assert(mPixelsSize == mSize); @@ -1123,7 +1134,7 @@ void SkiaSalBitmap::EnsureBitmapData() #endif SkBitmap bitmap; SkPixmap pixmap; - if (mSize == mPixelsSize && mImage->imageInfo().alphaType() == alphaType + if (imageSize(mImage) == mSize && mImage->imageInfo().alphaType() == alphaType && mImage->peekPixels(&pixmap)) { bitmap.installPixels(pixmap); @@ -1135,27 +1146,20 @@ void SkiaSalBitmap::EnsureBitmapData() SkCanvas canvas(bitmap); SkPaint paint; paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha - if (mSize != mPixelsSize) // pending scaling? + if (imageSize(mImage) != mSize) // pending scaling? { - assert(mImage->width() == mPixelsSize.getWidth() - && mImage->height() == mPixelsSize.getHeight()); canvas.drawImageRect(mImage, SkRect::MakeWH(mSize.getWidth(), mSize.getHeight()), makeSamplingOptions(mScaleQuality), &paint); - SAL_INFO("vcl.skia.trace", "ensurebitmapdata(" << this << "): image scaled " - << mPixelsSize << "->" << mSize << ":" - << static_cast(mScaleQuality)); - mPixelsSize = mSize; - ComputeScanlineSize(); - mScaleQuality = BmpScaleFlag::BestQuality; - // Information about the pending scaling has been discarded, so make sure we do not - // keep around any cached images that would still need scaling. - ResetCachedDataBySize(); + SAL_INFO("vcl.skia.trace", + "ensurebitmapdata(" << this << "): image scaled " << imageSize(mImage) << "->" + << mSize << ":" << static_cast(mScaleQuality)); } else canvas.drawImage(mImage, 0, 0, SkSamplingOptions(), &paint); canvas.flush(); } bitmap.setImmutable(); + ResetPendingScaling(); CreateBitmapData(); assert(mBuffer != nullptr); assert(mPixelsSize == mSize); @@ -1286,15 +1290,19 @@ void SkiaSalBitmap::ResetAllData() mEraseColorSet = false; } -void SkiaSalBitmap::ResetCachedDataBySize() +void SkiaSalBitmap::ResetPendingScaling() { + if (mPixelsSize == mSize) + return; SkiaZone zone; - assert(mSize == mPixelsSize); - assert(!mEraseColorSet); - if (mImage && (mImage->width() != mSize.getWidth() || mImage->height() != mSize.getHeight())) + mScaleQuality = BmpScaleFlag::BestQuality; + mPixelsSize = mSize; + ComputeScanlineSize(); + // Information about the pending scaling has been discarded, so make sure we do not + // keep around any cached images that would still need scaling. + if (mImage && imageSize(mImage) != mSize) mImage.reset(); - if (mAlphaImage - && (mAlphaImage->width() != mSize.getWidth() || mAlphaImage->height() != mSize.getHeight())) + if (mAlphaImage && imageSize(mAlphaImage) != mSize) mAlphaImage.reset(); }