oox/source/drawingml/fillproperties.cxx | 92 +++++++++++++-------------------
sd/qa/unit/data/pptx/croppedTo0.pptx |binary
sd/qa/unit/import-tests2.cxx | 10 +++
3 files changed, 48 insertions(+), 54 deletions(-)
New commits:
commit 5772cef244dbee5834efbc693bc714d89ae6301d
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Wed Jun 15 18:33:38 2022 +0300
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Fri Jun 17 14:25:54 2022 +0200
tdf#134210: Reimplement cropping from srcRect and fillRect
This avoids the scaling after the crop, since scaling is performed
anyway when applying BitmapMode_STRETCH. This improves resulting
bitmap quality.
Also consider the "crop to zero" case (when the sum of cropped
parts is equal to 100%). In that case, just use an empty graphic
as the fill bitmap.
This makes the differences between srcRect and fillRect processing
explicit, simplifies the code, avoids extra rounding inaccuracies,
and takes care of the edge cases that were considered in commit
2859ec288f2c1323ea3123d82cb1684b349ff598
Author Miklos Vajna <vmik...@collabora.com>
Date Wed Jun 15 15:52:18 2022 +0200
oox: fix div by zero in lclCalculateCropPercentage()
The change in SdImportTest2::testTdf134210 is because we now don't
scale the cropped image. The previous value was an interpolated
color, while the new value is the actual color of pixel [0, 41] of
the original image.
Change-Id: I24fa9928cff32bcaa6a7b3e34def14700fddd7ca
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135917
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/oox/source/drawingml/fillproperties.cxx
b/oox/source/drawingml/fillproperties.cxx
index 93fea51194ef..9270201920b3 100644
--- a/oox/source/drawingml/fillproperties.cxx
+++ b/oox/source/drawingml/fillproperties.cxx
@@ -88,67 +88,55 @@ Reference< XGraphic >
lclRotateGraphic(uno::Reference<graphic::XGraphic> const &
return aReturnGraphic.GetXGraphic();
}
-void lclCalculateCropPercentage(uno::Reference<graphic::XGraphic> const &
xGraphic, geometry::IntegerRectangle2D &aFillRect)
+using Quotients = std::tuple<double, double, double, double>;
+Quotients getQuotients(geometry::IntegerRectangle2D aRelRect, double hDiv,
double vDiv)
{
- ::Graphic aGraphic(xGraphic);
- assert (aGraphic.GetType() == GraphicType::Bitmap);
-
- BitmapEx aBitmapEx(aGraphic.GetBitmapEx());
-
- sal_Int32 nScaledWidth = aBitmapEx.GetSizePixel().Width();
- sal_Int32 nScaledHeight = aBitmapEx.GetSizePixel().Height();
-
- sal_Int32 nOrigWidth = (nScaledWidth * (100000 - aFillRect.X1 -
aFillRect.X2)) / 100000;
- if (nOrigWidth == 0)
- {
- nOrigWidth = 1;
- }
- sal_Int32 nOrigHeight = (nScaledHeight * (100000 - aFillRect.Y1 -
aFillRect.Y2)) / 100000;
- if (nOrigHeight == 0)
- {
- nOrigHeight = 1;
- }
+ return { aRelRect.X1 / hDiv, aRelRect.Y1 / vDiv, aRelRect.X2 / hDiv,
aRelRect.Y2 / vDiv };
+}
- sal_Int32 nLeftPercentage = nScaledWidth * aFillRect.X1 / nOrigWidth;
- sal_Int32 nRightPercentage = nScaledWidth * aFillRect.X2 / nOrigWidth;
- sal_Int32 nTopPercentage = nScaledHeight * aFillRect.Y1 / nOrigHeight;
- sal_Int32 nBottomPercentage = nScaledHeight * aFillRect.Y2 / nOrigHeight;
+// ECMA-376 Part 1 20.1.8.55 srcRect (Source Rectangle)
+std::optional<Quotients> CropQuotientsFromSrcRect(geometry::IntegerRectangle2D
aSrcRect)
+{
+ // Currently the following precondition is guaranteed in
GraphicProperties::pushToPropMap
+ assert(aSrcRect.X1 >= 0 && aSrcRect.X2 >= 0 && aSrcRect.Y1 >= 0 &&
aSrcRect.Y2 >= 0);
+ if (aSrcRect.X1 + aSrcRect.X2 >= 100'000 || aSrcRect.Y1 + aSrcRect.Y2 >=
100'000)
+ return {}; // Cropped everything
+ return getQuotients(aSrcRect, 100'000.0, 100'000.0);
+}
- aFillRect.X1 = -nLeftPercentage;
- aFillRect.X2 = -nRightPercentage;
- aFillRect.Y1 = -nTopPercentage;
- aFillRect.Y2 = -nBottomPercentage;
+// ECMA-376 Part 1 20.1.8.30 fillRect (Fill Rectangle)
+std::optional<Quotients>
CropQuotientsFromFillRect(geometry::IntegerRectangle2D aFillRect)
+{
+ // Currently the following precondition is guaranteed in
FillProperties::pushToPropMap
+ assert(aFillRect.X1 <= 0 && aFillRect.X2 <= 0 && aFillRect.Y1 <= 0 &&
aFillRect.Y2 <= 0);
+ // Negative divisor and negative relative offset give positive value
wanted in lclCropGraphic
+ return getQuotients(aFillRect, -100'000.0 + aFillRect.X1 + aFillRect.X2,
+ -100'000.0 + aFillRect.Y1 + aFillRect.Y2);
}
-// Crops a piece of the bitmap. Takes negative aFillRect values. Negative
values means "crop",
-// positive values means "grow" bitmap with empty spaces. lclCropGraphic
doesn't handle growing.
-Reference< XGraphic > lclCropGraphic(uno::Reference<graphic::XGraphic> const &
xGraphic, geometry::IntegerRectangle2D aFillRect)
+// Crops a piece of the bitmap. lclCropGraphic doesn't handle growing.
+Reference<XGraphic> lclCropGraphic(uno::Reference<graphic::XGraphic> const&
xGraphic,
+ std::optional<Quotients> quotients)
{
::Graphic aGraphic(xGraphic);
- ::Graphic aReturnGraphic;
-
assert (aGraphic.GetType() == GraphicType::Bitmap);
- BitmapEx aBitmapEx(aGraphic.GetBitmapEx());
-
- sal_Int32 nOrigHeight = aBitmapEx.GetSizePixel().Height();
- sal_Int32 nHeight = nOrigHeight;
- sal_Int32 nTopCorr = nOrigHeight * -1 * static_cast<double>(aFillRect.Y1)
/ 100000;
- nHeight += nTopCorr;
- sal_Int32 nBottomCorr = nOrigHeight * -1 *
static_cast<double>(aFillRect.Y2) / 100000;
- nHeight += nBottomCorr;
+ BitmapEx aBitmapEx;
+ if (quotients)
+ {
+ aBitmapEx = aGraphic.GetBitmapEx();
- sal_Int32 nOrigWidth = aBitmapEx.GetSizePixel().Width();
- sal_Int32 nWidth = nOrigWidth;
- sal_Int32 nLeftCorr = nOrigWidth * -1 * static_cast<double>(aFillRect.X1)
/ 100000;
- nWidth += nLeftCorr;
- sal_Int32 nRightCorr = nOrigWidth * -1 * static_cast<double>(aFillRect.X2)
/ 100000;
- nWidth += nRightCorr;
+ const Size bmpSize = aBitmapEx.GetSizePixel();
+ const auto& [qx1, qy1, qx2, qy2] = *quotients;
+ const tools::Long l = std::round(bmpSize.Width() * qx1);
+ const tools::Long t = std::round(bmpSize.Height() * qy1);
+ const tools::Long r = std::round(bmpSize.Width() * qx2);
+ const tools::Long b = std::round(bmpSize.Height() * qy2);
- aBitmapEx.Scale(Size(nWidth, nHeight));
- aBitmapEx.Crop(tools::Rectangle(Point(nLeftCorr, nTopCorr),
Size(nOrigWidth, nOrigHeight)));
+ aBitmapEx.Crop({ l, t, bmpSize.Width() - r - 1, bmpSize.Height() - b -
1 });
+ }
- aReturnGraphic = ::Graphic(aBitmapEx);
+ ::Graphic aReturnGraphic(aBitmapEx);
aReturnGraphic.setOriginURL(aGraphic.getOriginURL());
return aReturnGraphic.GetXGraphic();
@@ -818,7 +806,7 @@ void FillProperties::pushToPropMap( ShapePropertyMap&
rPropMap,
if(bIsCustomShape && bHasCropValues && bNeedCrop)
{
- xGraphic = lclCropGraphic(xGraphic, aFillRect);
+ xGraphic = lclCropGraphic(xGraphic,
CropQuotientsFromFillRect(aFillRect));
rPropMap.setProperty(ShapeProperty::FillBitmap, xGraphic);
}
}
@@ -932,9 +920,7 @@ void GraphicProperties::pushToPropMap( PropertyMap&
rPropMap, const GraphicHelpe
if(mbIsCustomShape && bHasCropValues && bNeedCrop)
{
- geometry::IntegerRectangle2D aCropRect = oClipRect;
- lclCalculateCropPercentage(xGraphic, aCropRect);
- xGraphic = lclCropGraphic(xGraphic, aCropRect);
+ xGraphic = lclCropGraphic(xGraphic,
CropQuotientsFromSrcRect(oClipRect));
}
}
}
diff --git a/sd/qa/unit/data/pptx/croppedTo0.pptx
b/sd/qa/unit/data/pptx/croppedTo0.pptx
new file mode 100644
index 000000000000..fecf53559b1f
Binary files /dev/null and b/sd/qa/unit/data/pptx/croppedTo0.pptx differ
diff --git a/sd/qa/unit/import-tests2.cxx b/sd/qa/unit/import-tests2.cxx
index 3bddc580c07d..8ada9e682f39 100644
--- a/sd/qa/unit/import-tests2.cxx
+++ b/sd/qa/unit/import-tests2.cxx
@@ -135,6 +135,7 @@ public:
void testTdf112209();
void testTdf128596();
void testDefaultTabStop();
+ void testCropToZero();
CPPUNIT_TEST_SUITE(SdImportTest2);
@@ -204,6 +205,7 @@ public:
CPPUNIT_TEST(testTdf112209);
CPPUNIT_TEST(testTdf128596);
CPPUNIT_TEST(testDefaultTabStop);
+ CPPUNIT_TEST(testCropToZero);
CPPUNIT_TEST_SUITE_END();
};
@@ -1293,7 +1295,7 @@ void SdImportTest2::testTdf134210()
Graphic aGraphic(xGraphic);
BitmapEx aBitmap(aGraphic.GetBitmapEx());
- CPPUNIT_ASSERT_EQUAL(Color(0x60563e), aBitmap.GetPixelColor(0, 0));
+ CPPUNIT_ASSERT_EQUAL(Color(0x605741), aBitmap.GetPixelColor(0, 0));
xDocShRef->DoClose();
}
@@ -2035,6 +2037,12 @@ void SdImportTest2::testDefaultTabStop()
xDocShRef->DoClose();
}
+void SdImportTest2::testCropToZero()
+{
+ // Must not crash because of division by zero
+
loadURL(m_directories.getURLFromSrc(u"/sd/qa/unit/data/pptx/croppedTo0.pptx"),
PPTX);
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(SdImportTest2);
CPPUNIT_PLUGIN_IMPLEMENT();
