From df9f0e3cc57fb69ee38918b25ed91d97d1972685 Mon Sep 17 00:00:00 2001 From: Luboš Luňák Date: Fri, 3 Jul 2020 12:29:47 +0200 Subject: prefer 8bit bitmap masks to 1bpp masks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's much simpler and more performant to work with 8bpp bytes rather than fiddle with 1bpp bits, and the memory savings don't really matter nowadays. E.g. the soft edges feature performs much worse when operating on 1bpp bitmaps (tdf#134237). I've looked and it seems nothing in LO code actually requires bitmap masks to really be 1bpp. As a next step we could perhaps also leave antiquity and embrace the past by dropping 1bpp masks entirely and using just alpha masks. The change also moves the workaround for #i75531, it was breaking CppunitTest_vcl_svm_test and X11-related hacks belong to the X11 code (especially with the X11 backend becoming more and more niche). Change-Id: I7848f06c8b83bbad2ea35f17f2b65855f8d1f456 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/97839 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl Reviewed-by: Luboš Luňák --- canvas/source/directx/dx_vcltools.cxx | 4 ++-- vcl/source/bitmap/bitmappaint.cxx | 36 +++++++++++++++++++++++++++++++---- vcl/source/gdi/bitmapex.cxx | 11 +++-------- vcl/unx/generic/gdi/gdiimpl.cxx | 5 +++++ 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/canvas/source/directx/dx_vcltools.cxx b/canvas/source/directx/dx_vcltools.cxx index d88c586c3245..287ed7beda54 100644 --- a/canvas/source/directx/dx_vcltools.cxx +++ b/canvas/source/directx/dx_vcltools.cxx @@ -168,9 +168,9 @@ namespace dxcanvas "::dxcanvas::tools::bitmapFromVCLBitmapEx(): " "Unable to acquire read access to bitmap" ); - if( rBmpEx.IsAlpha() ) + if( rBmpEx.IsAlpha() || rBmpEx.GetMask().GetBitCount() == 8 ) { - Bitmap aAlpha( rBmpEx.GetAlpha().GetBitmap() ); + Bitmap aAlpha( rBmpEx.IsAlpha() ? rBmpEx.GetAlpha().GetBitmap() : rBmpEx.GetMask()); Bitmap::ScopedReadAccess pAlphaReadAccess( aAlpha ); diff --git a/vcl/source/bitmap/bitmappaint.cxx b/vcl/source/bitmap/bitmappaint.cxx index 046b0e3ba3b5..0f4c929480f9 100644 --- a/vcl/source/bitmap/bitmappaint.cxx +++ b/vcl/source/bitmap/bitmappaint.cxx @@ -367,6 +367,12 @@ Bitmap Bitmap::CreateMask(const Color& rTransColor, sal_uInt8 nTol) const { ScopedReadAccess pReadAcc(const_cast(*this)); + // Historically LO used 1bpp masks, but 8bpp masks are much faster, + // better supported by hardware, and the memory savings are not worth + // it anymore. + // TODO: Possibly remove the 1bpp code later. + constexpr bool use8BitMask = true; + if (!nTol && pReadAcc && (pReadAcc->GetScanlineFormat() == ScanlineFormat::N1BitLsbPal || pReadAcc->GetScanlineFormat() == ScanlineFormat::N1BitMsbPal) @@ -377,7 +383,8 @@ Bitmap Bitmap::CreateMask(const Color& rTransColor, sal_uInt8 nTol) const return *this; } - Bitmap aNewBmp(GetSizePixel(), 1); + Bitmap aNewBmp(GetSizePixel(), use8BitMask ? 8 : 1, + use8BitMask ? &Bitmap::GetGreyPalette(256) : nullptr); BitmapScopedWriteAccess pWriteAcc(aNewBmp); bool bRet = false; @@ -392,8 +399,9 @@ Bitmap Bitmap::CreateMask(const Color& rTransColor, sal_uInt8 nTol) const { const BitmapColor aTest(pReadAcc->GetBestMatchingColor(rTransColor)); - if (pReadAcc->GetScanlineFormat() == ScanlineFormat::N4BitMsnPal - || pReadAcc->GetScanlineFormat() == ScanlineFormat::N4BitLsnPal) + if (pWriteAcc->GetBitCount() == 1 + && (pReadAcc->GetScanlineFormat() == ScanlineFormat::N4BitMsnPal + || pReadAcc->GetScanlineFormat() == ScanlineFormat::N4BitLsnPal)) { // optimized for 4Bit-MSN/LSN source palette const sal_uInt8 cTest = aTest.GetIndex(); @@ -433,7 +441,8 @@ Bitmap Bitmap::CreateMask(const Color& rTransColor, sal_uInt8 nTol) const } } } - else if (pReadAcc->GetScanlineFormat() == ScanlineFormat::N8BitPal) + else if (pWriteAcc->GetBitCount() == 1 + && pReadAcc->GetScanlineFormat() == ScanlineFormat::N8BitPal) { // optimized for 8Bit source palette const sal_uInt8 cTest = aTest.GetIndex(); @@ -486,6 +495,25 @@ Bitmap Bitmap::CreateMask(const Color& rTransColor, sal_uInt8 nTol) const pDst[nX] = ~pSrc[nX]; } } + else if (use8BitMask && pWriteAcc->GetBitCount() == 8 + && pReadAcc->GetScanlineFormat() == ScanlineFormat::N8BitPal) + { + // optimized for 8Bit source palette + const sal_uInt8 cTest = aTest.GetIndex(); + + for (long nY = 0; nY < nHeight; ++nY) + { + Scanline pSrc = pReadAcc->GetScanline(nY); + Scanline pDst = pWriteAcc->GetScanline(nY); + for (long nX = 0; nX < nWidth; ++nX) + { + if (cTest == pSrc[nX]) + pDst[nX] = aWhite.GetIndex(); + else + pDst[nX] = aBlack.GetIndex(); + } + } + } else { // not optimized diff --git a/vcl/source/gdi/bitmapex.cxx b/vcl/source/gdi/bitmapex.cxx index 80c6879af00c..3f65117392a5 100644 --- a/vcl/source/gdi/bitmapex.cxx +++ b/vcl/source/gdi/bitmapex.cxx @@ -126,8 +126,9 @@ BitmapEx::BitmapEx( const Bitmap& rBmp, const Bitmap& rMask ) : meTransparent ( !rMask ? TransparentType::NONE : TransparentType::Bitmap ), mbAlpha ( false ) { - // Ensure a mask is exactly one bit deep - if( !!maMask && maMask.GetBitCount() != 1 ) + // Ensure a mask is exactly one bit deep, + // alternatively also allow 8bpp masks. + if( !!maMask && maMask.GetBitCount() != 1 && !(maMask.GetBitCount() == 8 && maMask.HasGreyPalette8Bit())) { SAL_WARN( "vcl", "BitmapEx: forced mask to monochrome"); BitmapEx aMaskEx(maMask); @@ -154,12 +155,6 @@ BitmapEx::BitmapEx( const Bitmap& rBmp, const AlphaMask& rAlphaMask ) : OSL_ENSURE(false, "Alpha size differs from Bitmap size, corrected Mask (!)"); maMask.Scale(rBmp.GetSizePixel()); } - - // #i75531# the workaround below can go when - // X11SalGraphics::drawAlphaBitmap()'s render acceleration - // can handle the bitmap depth mismatch directly - if( maBitmap.GetBitCount() < maMask.GetBitCount() ) - maBitmap.Convert( BmpConversion::N24Bit ); } BitmapEx::BitmapEx( const Bitmap& rBmp, const Color& rTransparentColor ) : diff --git a/vcl/unx/generic/gdi/gdiimpl.cxx b/vcl/unx/generic/gdi/gdiimpl.cxx index ee45815a7c83..6310b6ba99fa 100644 --- a/vcl/unx/generic/gdi/gdiimpl.cxx +++ b/vcl/unx/generic/gdi/gdiimpl.cxx @@ -714,6 +714,11 @@ bool X11SalGraphicsImpl::drawAlphaBitmap( const SalTwoRect& rTR, // non 8-bit alpha not implemented yet if( rAlphaBmp.GetBitCount() != 8 ) return false; + // #i75531# the workaround below can go when + // X11SalGraphics::drawAlphaBitmap()'s render acceleration + // can handle the bitmap depth mismatch directly + if( rSrcBitmap.GetBitCount() < rAlphaBmp.GetBitCount() ) + return false; // horizontal mirroring not implemented yet if( rTR.mnDestWidth < 0 ) -- cgit