[Libreoffice-commits] core.git: oox/source sd/qa

[Libreoffice Gerrit user]

 oox/source/drawingml/diagram/diagramlayoutatoms.cxx |   29 +++++++++++++++-----
 sd/qa/unit/data/pptx/smartart-picture-strip.pptx    |binary
 sd/qa/unit/import-tests-smartart.cxx                |   15 ++++++++++
 3 files changed, 37 insertions(+), 7 deletions(-)

New commits:
commit 159e33ec661b2ce038b2642b2f30600ce7901d1b
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Tue Feb 26 09:44:00 2019 +0100
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Tue Feb 26 10:44:31 2019 +0100

    oox smartart, picture strip: fix too many columns with aspect ratio request
    
    The bugdoc has 3 items in the picture strip and PowerPoint laid this out
    as a single column with 3 rows (as a snake algorithm). We used to put
    the first two items to the first row and the third item to the second
    row.
    
    Improve out layout by taking into account what aspect ratio the child
    algorithms request: this way it's obvious that we should use a single
    column in case we have a large enough aspect ratio and few enough items.
    
    (PowerPoint also uses multiple columns without the aspect ratio
    request.)
    
    Change-Id: I9f1158c04c665fc6a2c85e4ac3a1ed363b1c75fb
    Reviewed-on: https://gerrit.libreoffice.org/68370
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
    Tested-by: Jenkins

diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index 5af87e0851b8..d06661e4f7e0 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -908,19 +908,34 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
 
             sal_Int32 nCol = 1;
             sal_Int32 nRow = 1;
-            for ( ; nRow<nCount; nRow++)
+            double fChildAspectRatio = rShape->getChildren()[0]->getAspectRatio();
+            if (nCount <= fChildAspectRatio)
+                // Child aspect ratio request (width/height) is N, and we have at most N shapes.
+                // This means we don't need multiple columns.
+                nRow = nCount;
+            else
             {
-                nCol = (nCount+nRow-1) / nRow;
-                const double fShapeHeight = rShape->getSize().Height;
-                const double fShapeWidth = rShape->getSize().Width;
-                if ((fShapeHeight / nCol) / (fShapeWidth / nRow) >= fAspectRatio)
-                    break;
+                for ( ; nRow<nCount; nRow++)
+                {
+                    nCol = (nCount+nRow-1) / nRow;
+                    const double fShapeHeight = rShape->getSize().Height;
+                    const double fShapeWidth = rShape->getSize().Width;
+                    if ((fShapeHeight / nCol) / (fShapeWidth / nRow) >= fAspectRatio)
+                        break;
+                }
             }
 
             SAL_INFO("oox.drawingml", "Snake layout grid: " << nCol << "x" << nRow);
 
             sal_Int32 nWidth = rShape->getSize().Width / (nCol + (nCol-1)*fSpace);
-            const awt::Size aChildSize(nWidth, nWidth * fAspectRatio);
+            awt::Size aChildSize(nWidth, nWidth * fAspectRatio);
+            if (nCol == 1 && nRow > 1)
+            {
+                // We have a single column, so count the height based on the parent height, not
+                // based on width.
+                sal_Int32 nHeight = rShape->getSize().Height / (nRow + (nRow - 1) * fSpace);
+                aChildSize = awt::Size(rShape->getSize().Width, nHeight);
+            }
 
             awt::Point aCurrPos(0, 0);
             if (nIncX == -1)
diff --git a/sd/qa/unit/data/pptx/smartart-picture-strip.pptx b/sd/qa/unit/data/pptx/smartart-picture-strip.pptx
index 4c379dfd396c..b117e4e70cf6 100644
Binary files a/sd/qa/unit/data/pptx/smartart-picture-strip.pptx and b/sd/qa/unit/data/pptx/smartart-picture-strip.pptx differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx
index 22d781950176..0d47ad679edb 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -940,6 +940,21 @@ void SdImportTestSmartArt::testPictureStrip()
     // xSecondImage had the bitmap fill from the second shape.
     CPPUNIT_ASSERT(aFirstGraphic.GetChecksum() != aSecondGraphic.GetChecksum());
 
+    // Test that the 3 images are in a single column, in 3 rows.
+    uno::Reference<drawing::XShape> xFirstImageShape(xFirstImage, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xFirstImage.is());
+    uno::Reference<drawing::XShape> xSecondImageShape(xSecondImage, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xSecondImage.is());
+    uno::Reference<drawing::XShape> xThirdImageShape(getChildShape(getChildShape(xGroup, 2), 1),
+                                                     uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xThirdImageShape.is());
+    // Without the accompanying fix in place, this test would have failed: the first and the second
+    // image were in the same row.
+    CPPUNIT_ASSERT_EQUAL(xFirstImageShape->getPosition().X, xSecondImageShape->getPosition().X);
+    CPPUNIT_ASSERT_EQUAL(xSecondImageShape->getPosition().X, xThirdImageShape->getPosition().X);
+    CPPUNIT_ASSERT_GREATER(xFirstImageShape->getPosition().Y, xSecondImageShape->getPosition().Y);
+    CPPUNIT_ASSERT_GREATER(xSecondImageShape->getPosition().Y, xThirdImageShape->getPosition().Y);
+
     xDocShRef->DoClose();
 }