oox/source/drawingml/fillproperties.cxx | 24 ++++++++ vcl/qa/cppunit/GraphicTest.cxx | 33 ++++++++++++ vcl/qa/cppunit/data/testColorChange-red-linear-gradient.png |binary vcl/source/graphic/UnoGraphic.cxx | 4 - 4 files changed, 58 insertions(+), 3 deletions(-)
New commits: commit 2b902b6203a87bdca7856e17a9c0fcc403de4264 Author: Sarper Akdemir <[email protected]> AuthorDate: Thu Sep 1 14:59:51 2022 +0300 Commit: Miklos Vajna <[email protected]> CommitDate: Mon Sep 5 10:01:40 2022 +0200 tdf#149670 fix color change api and adjust tolerance for ooxml It appears at the start of Graphic::colorChange aBmpColorFrom & aBmpColorTo gets initialized with wrong colors. Instead of {R,G,B}, they get initialized with {B,G,R}. Instead of bitshifting use the ::Color constructor so that it is initialized correctly. For ooxml import adapt tolerance values of the image format in an attempt to get similar results on how the results appear in PowerPoint. Change-Id: I1fa901691512de82936dba0e47158b7e0ca2223e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/139203 Tested-by: Jenkins Reviewed-by: Miklos Vajna <[email protected]> diff --git a/oox/source/drawingml/fillproperties.cxx b/oox/source/drawingml/fillproperties.cxx index fef43b0ca238..1ef7579a7725 100644 --- a/oox/source/drawingml/fillproperties.cxx +++ b/oox/source/drawingml/fillproperties.cxx @@ -197,9 +197,31 @@ Reference< XGraphic > lclCheckAndApplyChangeColorTransform(const BlipFillPropert sal_Int16 nToTransparence = aBlipProps.maColorChangeTo.getTransparency(); sal_Int8 nToAlpha = static_cast< sal_Int8 >( (100 - nToTransparence) * 2.55 ); + sal_uInt8 nTolerance = 9; + Graphic aGraphic{ xGraphic }; + if( aGraphic.IsGfxLink() ) + { + // tdf#149670: Try to guess tolerance depending on image format + switch (aGraphic.GetGfxLink().GetType()) + { + case GfxLinkType::NativeJpg: + nTolerance = 15; + break; + case GfxLinkType::NativePng: + case GfxLinkType::NativeTif: + nTolerance = 1; + break; + case GfxLinkType::NativeBmp: + nTolerance = 0; + break; + default: + break; + } + } + uno::Reference<graphic::XGraphicTransformer> xTransformer(aBlipProps.mxFillGraphic, uno::UNO_QUERY); if (xTransformer.is()) - return xTransformer->colorChange(xGraphic, sal_Int32(nFromColor), 9, sal_Int32(nToColor), nToAlpha); + return xTransformer->colorChange(xGraphic, sal_Int32(nFromColor), nTolerance, sal_Int32(nToColor), nToAlpha); } } return xGraphic; diff --git a/vcl/qa/cppunit/GraphicTest.cxx b/vcl/qa/cppunit/GraphicTest.cxx index 378bc7a7b55b..97ef23eac15d 100644 --- a/vcl/qa/cppunit/GraphicTest.cxx +++ b/vcl/qa/cppunit/GraphicTest.cxx @@ -9,6 +9,8 @@ #include <sal/config.h> #include <config_oox.h> +#include <com/sun/star/graphic/XGraphic.hpp> +#include <com/sun/star/graphic/XGraphicTransformer.hpp> #include <cppunit/TestAssert.h> #include <cppunit/TestFixture.h> #include <cppunit/extensions/HelperMacros.h> @@ -84,6 +86,7 @@ private: void testLoadWEBP(); void testAvailableThreaded(); + void testColorChangeToTransparent(); CPPUNIT_TEST_SUITE(GraphicTest); CPPUNIT_TEST(testUnloadedGraphic); @@ -122,6 +125,7 @@ private: CPPUNIT_TEST(testLoadWEBP); CPPUNIT_TEST(testAvailableThreaded); + CPPUNIT_TEST(testColorChangeToTransparent); CPPUNIT_TEST_SUITE_END(); }; @@ -1345,6 +1349,35 @@ void GraphicTest::testAvailableThreaded() } } +void GraphicTest::testColorChangeToTransparent() +{ + Graphic aGraphic = importUnloadedGraphic(u"testColorChange-red-linear-gradient.png"); + + auto xGraphic = aGraphic.GetXGraphic(); + uno::Reference<graphic::XGraphicTransformer> xGraphicTransformer{ xGraphic, uno::UNO_QUERY }; + ::Color nColorFrom{ ColorTransparency, 0x00, 0xFF, 0x00, 0x00 }; + ::Color nColorTo{ ColorTransparency, 0xFF, 0xFF, 0x00, 0x00 }; + sal_uInt8 nTolerance{ 15 }; + + auto xGraphicAfter = xGraphicTransformer->colorChange( + xGraphic, static_cast<sal_Int32>(nColorFrom), nTolerance, static_cast<sal_Int32>(nColorTo), + static_cast<sal_Int8>(nColorTo.GetAlpha())); + + Graphic aGraphicAfter{ xGraphicAfter }; + const BitmapEx& rBitmapAfter = aGraphicAfter.GetBitmapExRef(); + const BitmapEx& rBitmapBefore = aGraphic.GetBitmapExRef(); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: rgba[ff000000] + // - Actual : rgba[f00000ff] + // i.e. the color change to transparent didn't apply correctly + CPPUNIT_ASSERT_EQUAL(nColorTo, rBitmapAfter.GetPixelColor(386, 140)); + + // Test if color stayed same on 410,140 + // colorChange with nTolerance 15 shouldn't change this pixel. + CPPUNIT_ASSERT_EQUAL(rBitmapBefore.GetPixelColor(410, 140), + rBitmapAfter.GetPixelColor(410, 140)); +} + } // namespace CPPUNIT_TEST_SUITE_REGISTRATION(GraphicTest); diff --git a/vcl/qa/cppunit/data/testColorChange-red-linear-gradient.png b/vcl/qa/cppunit/data/testColorChange-red-linear-gradient.png new file mode 100644 index 000000000000..6f2a9d0abfbe Binary files /dev/null and b/vcl/qa/cppunit/data/testColorChange-red-linear-gradient.png differ diff --git a/vcl/source/graphic/UnoGraphic.cxx b/vcl/source/graphic/UnoGraphic.cxx index e415de1523d3..4d5f42dfcf82 100644 --- a/vcl/source/graphic/UnoGraphic.cxx +++ b/vcl/source/graphic/UnoGraphic.cxx @@ -195,8 +195,8 @@ uno::Reference< graphic::XGraphic > SAL_CALL Graphic::colorChange( ::Graphic aGraphic(rxGraphic); ::Graphic aReturnGraphic; - BitmapColor aBmpColorFrom(static_cast< sal_uInt8 >(nColorFrom), static_cast< sal_uInt8 >(nColorFrom >> 8), static_cast< sal_uInt8 >(nColorFrom >> 16)); - BitmapColor aBmpColorTo( static_cast< sal_uInt8 >(nColorTo), static_cast< sal_uInt8 >(nColorTo >> 8), static_cast< sal_uInt8 >(nColorTo >> 16)); + BitmapColor aBmpColorFrom(Color(ColorTransparency, static_cast<sal_uInt32>(nColorFrom))); + BitmapColor aBmpColorTo(Color(ColorTransparency, static_cast<sal_uInt32>(nColorTo))); Color aColorFrom(aBmpColorFrom); Color aColorTo(aBmpColorTo);
