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

[Libreoffice Gerrit user]

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

New commits:
commit 0a29c928afa74123bca05dc089c751603d368467
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Tue Feb 26 18:25:00 2019 +0100
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Tue Feb 26 20:32:26 2019 +0100

    oox smartart, picture strip: fix lack of spacing around the picture list
    
    The snake algorithm in PowerPoint seem to interpret spacing as follows:
    if you have N elements, then there should be the requested amount of
    spacing between the elements, and also double amount of spacing around
    the actual list of elements.
    
    With this, the SmartArt and the title shape in the bugdoc no longer
    overlaps.
    
    Change-Id: I5d6885b434bfaff9de9aac595a298a5346524e19
    Reviewed-on: https://gerrit.libreoffice.org/68397
    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 d06661e4f7e0..6893f04d63f7 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -446,6 +446,10 @@ void ConstraintAtom::accept( LayoutAtomVisitor& rVisitor )
 void ConstraintAtom::parseConstraint(std::vector<Constraint>& rConstraints,
                                      bool bRequireForName) const
 {
+    // The snake algorithm do want a space constraint even without a name.
+    if (bRequireForName && maConstraint.mnType == XML_sp)
+        bRequireForName = false;
+
     if (bRequireForName && maConstraint.msForName.isEmpty())
         return;
 
@@ -890,6 +894,18 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
             if (rShape->getChildren().empty() || rShape->getSize().Width == 0 || rShape->getSize().Height == 0)
                 break;
 
+            // Parse constraints, only self spacing from height as a start.
+            double fSpaceFromConstraint = 0;
+            for (const auto& rConstr : rConstraints)
+            {
+                if (rConstr.mnRefType == XML_h)
+                {
+                    if (rConstr.mnType == XML_sp && rConstr.msForName.isEmpty())
+                        fSpaceFromConstraint = rConstr.mfFactor;
+                }
+            }
+            bool bSpaceFromConstraints = fSpaceFromConstraint != 0;
+
             const sal_Int32 nDir = maMap.count(XML_grDir) ? maMap.find(XML_grDir)->second : XML_tL;
             sal_Int32 nIncX = 1;
             sal_Int32 nIncY = 1;
@@ -901,9 +917,9 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
                 case XML_bR: nIncX = -1; nIncY = -1; break;
             }
 
-            // TODO: get values from constraints
             sal_Int32 nCount = rShape->getChildren().size();
-            double fSpace = 0.3;
+            // Defaults in case not provided by constraints.
+            double fSpace = bSpaceFromConstraints ? fSpaceFromConstraint : 0.3;
             double fAspectRatio = 0.54; // diagram should not spill outside, earlier it was 0.6
 
             sal_Int32 nCol = 1;
@@ -933,7 +949,13 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
             {
                 // 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);
+                // Space occurs inside children; also double amount of space is needed outside (on
+                // both sides), if the factor comes from a constraint.
+                sal_Int32 nNumSpaces = -1;
+                if (bSpaceFromConstraints)
+                    nNumSpaces += 4;
+                sal_Int32 nHeight
+                    = rShape->getSize().Height / (nRow + (nRow + nNumSpaces) * fSpace);
                 aChildSize = awt::Size(rShape->getSize().Width, nHeight);
             }
 
@@ -942,6 +964,9 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
                 aCurrPos.X = rShape->getSize().Width - aChildSize.Width;
             if (nIncY == -1)
                 aCurrPos.Y = rShape->getSize().Height - aChildSize.Height;
+            else if (bSpaceFromConstraints)
+                // Initial vertical offset to have upper spacing (outside, so double amount).
+                aCurrPos.Y = aChildSize.Height * fSpace * 2;
 
             sal_Int32 nStartX = aCurrPos.X;
             sal_Int32 nColIdx = 0,index = 0;
diff --git a/sd/qa/unit/data/pptx/smartart-picture-strip.pptx b/sd/qa/unit/data/pptx/smartart-picture-strip.pptx
index b117e4e70cf6..bfb8d4f2c1bd 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 0d47ad679edb..3d30f576cce0 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -955,6 +955,16 @@ void SdImportTestSmartArt::testPictureStrip()
     CPPUNIT_ASSERT_GREATER(xFirstImageShape->getPosition().Y, xSecondImageShape->getPosition().Y);
     CPPUNIT_ASSERT_GREATER(xSecondImageShape->getPosition().Y, xThirdImageShape->getPosition().Y);
 
+    // Make sure that the title shape doesn't overlap with the diagram.
+    // Note that real "no overlap" is asserted here, though in fact what we want is a less strict
+    // condition: that no text part of the title shape and the diagram overlaps.
+    uno::Reference<drawing::XShape> xTitle(getShapeFromPage(1, 0, xDocShRef), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xTitle.is());
+    // Without the accompanying fix in place, this test would have failed with 'Expected greater
+    // than: 2873; Actual  : 2320', i.e. the title shape and the diagram overlapped.
+    CPPUNIT_ASSERT_GREATER(xTitle->getPosition().Y + xTitle->getSize().Height,
+                           xGroup->getPosition().Y);
+
     xDocShRef->DoClose();
 }