oox/source/drawingml/fillproperties.cxx | 6 +++++- sw/qa/extras/ooxmlexport/data/crop-roundtrip.docx |binary sw/qa/extras/ooxmlexport/ooxmlexport18.cxx | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-)
New commits: commit dc62b698f7a57b244dc80e0b607922505af233e9 Author: Jan Holesovsky <ke...@collabora.com> AuthorDate: Thu Nov 24 11:22:49 2022 +0100 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Fri Nov 25 10:50:58 2022 +0100 tdf#152199: Don't crop twice We have a "GraphicCrop" property that is supposed to roundtrip the cropping in OOXML, but there is no core feature backing it (ie. the image is not shown cropped when this is imported and set). Instead, to show the image "cropped", we crop the image physically on import (throw away pixels that are 'outside' of the cropped area). But - the "GraphicCrop" is then saved on export, together with the image already physically cropped, which leads to garbled DOCX on re-import. Given that the core feature to show image cropped when the "GraphicCrop" is set, let's avoid setting it when we physically crop the image. Change-Id: Ia1090ea9c6d22e60c77d52bf65281f6588d07d4e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/143214 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Miklos Vajna <vmik...@collabora.com> diff --git a/oox/source/drawingml/fillproperties.cxx b/oox/source/drawingml/fillproperties.cxx index 3fb6b4ca372e..80ba3ba7664d 100644 --- a/oox/source/drawingml/fillproperties.cxx +++ b/oox/source/drawingml/fillproperties.cxx @@ -818,7 +818,6 @@ void FillProperties::pushToPropMap( ShapePropertyMap& rPropMap, aGraphCrop.Right = static_cast< sal_Int32 >( ( static_cast< double >( aOriginalSize.Width ) * aFillRect.X2 ) / 100000 ); if ( aFillRect.Y2 ) aGraphCrop.Bottom = static_cast< sal_Int32 >( ( static_cast< double >( aOriginalSize.Height ) * aFillRect.Y2 ) / 100000 ); - rPropMap.setProperty(PROP_GraphicCrop, aGraphCrop); bool bHasCropValues = aGraphCrop.Left != 0 || aGraphCrop.Right !=0 || aGraphCrop.Top != 0 || aGraphCrop.Bottom != 0; // Negative GraphicCrop values means "crop" here. @@ -826,12 +825,17 @@ void FillProperties::pushToPropMap( ShapePropertyMap& rPropMap, if(bIsCustomShape && bHasCropValues && bNeedCrop) { + // Physically crop the image + // In this case, don't set the PROP_GraphicCrop because that + // would lead to applying the crop twice after roundtrip xGraphic = lclCropGraphic(xGraphic, CropQuotientsFromFillRect(aFillRect)); if (rPropMap.supportsProperty(ShapeProperty::FillBitmapName)) rPropMap.setProperty(ShapeProperty::FillBitmapName, xGraphic); else rPropMap.setProperty(ShapeProperty::FillBitmap, xGraphic); } + else + rPropMap.setProperty(PROP_GraphicCrop, aGraphCrop); } } } diff --git a/sw/qa/extras/ooxmlexport/data/crop-roundtrip.docx b/sw/qa/extras/ooxmlexport/data/crop-roundtrip.docx new file mode 100644 index 000000000000..6db60d0e8c60 Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/crop-roundtrip.docx differ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx index bc48cc158975..2d5cca60eb27 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx @@ -14,6 +14,7 @@ #include <com/sun/star/beans/NamedValue.hpp> #include <com/sun/star/drawing/XShapes.hpp> #include <com/sun/star/frame/XStorable.hpp> +#include <com/sun/star/text/GraphicCrop.hpp> #include <com/sun/star/text/XFootnotesSupplier.hpp> #include <com/sun/star/text/XTextDocument.hpp> #include <com/sun/star/text/XTextFieldsSupplier.hpp> @@ -77,6 +78,20 @@ CPPUNIT_TEST_FIXTURE(Test, testSdtDuplicatedId) assertXPath(pXmlDoc, "//w:sdt", 2); } +CPPUNIT_TEST_FIXTURE(Test, testImageCropping) +{ + loadAndReload("crop-roundtrip.docx"); + + // the image has no cropping after roundtrip, because it has been physically cropped + // NB: this test should be fixed when the core feature to show image cropped when it + // has the "GraphicCrop" is set is implemented + auto aGraphicCropStruct = getProperty<text::GraphicCrop>(getShape(1), "GraphicCrop"); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0), aGraphicCropStruct.Left); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0), aGraphicCropStruct.Right); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0), aGraphicCropStruct.Top); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0), aGraphicCropStruct.Bottom); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */