summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMike Kaganski <mike.kaganski@collabora.com>2022-06-27 15:29:25 +0300
committerXisco Fauli <xiscofauli@libreoffice.org>2022-06-28 18:18:26 +0200
commitc0c1145d9f7aa82417bfa8194de0a27c2c556aab (patch)
tree40f65bf71074db4dcee3d45f156f33b9375173cc
parenttdf#134210: Reimplement cropping from srcRect and fillRect (diff)
downloadcore-c0c1145d9f7aa82417bfa8194de0a27c2c556aab.tar.gz
core-c0c1145d9f7aa82417bfa8194de0a27c2c556aab.zip
Integer division could cancel small values of wrong sign
... as seen at some documents where the values are like -1. There the checks in pushToPropMap may pass (the division result would be 0), but the original small negative values would fail the asserts that were introduced in commit 5772cef244dbee5834efbc693bc714d89ae6301d Author Mike Kaganski <mike.kaganski@collabora.com> Date Wed Jun 15 18:33:38 2022 +0300 tdf#134210: Reimplement cropping from srcRect and fillRect Change-Id: I114588862b5cfd2b2e4491424430cc139bdbaae9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136492 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolanm@redhat.com> (cherry picked from commit 2d9f3c066a065d6aa98f1e594dcf8a091fec2bde) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136471 Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org> Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136556
-rw-r--r--oox/source/drawingml/fillproperties.cxx12
-rw-r--r--sd/qa/unit/data/pptx/croppedTo0.pptxbin32741 -> 12974 bytes
-rw-r--r--sd/qa/unit/import-tests2.cxx1
3 files changed, 9 insertions, 4 deletions
diff --git a/oox/source/drawingml/fillproperties.cxx b/oox/source/drawingml/fillproperties.cxx
index 773e08287368..7cb497767c93 100644
--- a/oox/source/drawingml/fillproperties.cxx
+++ b/oox/source/drawingml/fillproperties.cxx
@@ -97,8 +97,10 @@ Quotients getQuotients(geometry::IntegerRectangle2D aRelRect, double hDiv, doubl
// 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);
+ aSrcRect.X1 = std::max(aSrcRect.X1, sal_Int32(0));
+ aSrcRect.X2 = std::max(aSrcRect.X2, sal_Int32(0));
+ aSrcRect.Y1 = std::max(aSrcRect.Y1, sal_Int32(0));
+ aSrcRect.Y2 = std::max(aSrcRect.Y2, sal_Int32(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);
@@ -107,8 +109,10 @@ std::optional<Quotients> CropQuotientsFromSrcRect(geometry::IntegerRectangle2D a
// 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);
+ aFillRect.X1 = std::min(aFillRect.X1, sal_Int32(0));
+ aFillRect.X2 = std::min(aFillRect.X2, sal_Int32(0));
+ aFillRect.Y1 = std::min(aFillRect.Y1, sal_Int32(0));
+ aFillRect.Y2 = std::min(aFillRect.Y2, sal_Int32(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);
diff --git a/sd/qa/unit/data/pptx/croppedTo0.pptx b/sd/qa/unit/data/pptx/croppedTo0.pptx
index fecf53559b1f..081661f48601 100644
--- a/sd/qa/unit/data/pptx/croppedTo0.pptx
+++ b/sd/qa/unit/data/pptx/croppedTo0.pptx
Binary files differ
diff --git a/sd/qa/unit/import-tests2.cxx b/sd/qa/unit/import-tests2.cxx
index 29ef8d7c3ce9..142599fa945d 100644
--- a/sd/qa/unit/import-tests2.cxx
+++ b/sd/qa/unit/import-tests2.cxx
@@ -1996,6 +1996,7 @@ void SdImportTest2::testDefaultTabStop()
void SdImportTest2::testCropToZero()
{
// Must not crash because of division by zero
+ // Also must not fail assertions because of passing negative value to CropQuotientsFromSrcRect
loadURL(m_directories.getURLFromSrc(u"/sd/qa/unit/data/pptx/croppedTo0.pptx"), PPTX);
}