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/68370Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

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)

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();
 }