oox smartart import: fix aspect ratio of shape with composite algo

The layout node has alg=composite, then a parTx and a desTx child layout
nodes. No matter what order is used (parent first, child first), the
result will be wrong, as the constraints refer to each other. I did not
spot any description in ISO 29500-1 that would describe what is the
expected behavior.

Researching this, found "One other consideration when specifying
composite constraints is that the constraints must be specified in the
same order as the nested layout nodes." at
<http://web.archive.org/web/20111015151600/http://msdn.microsoft.com/en-us/magazine/cc163470.aspx>,
which suggests to handle constraints for each shape in a parent -> child
order, but keep a shared state when iterating over the children which
gives us:

- parent node, all direct constraints
- for each child node:
  - child's constraints from parent
  - child's own constraints

This way the desTx top value can depend on the parTx's height, and it's
supported to define parTx's height only in the parTx layout node, not in
the composite parent.

And after all, it matches what PowerPoint does, so the column headings
in the bugdoc have a 4:10 height:width aspect ratio.

(cherry picked from commit 414586649582e182b2603702f4f586f4beeed8a9)

Change-Id: Ideb76c1ddd1ffff8d2a217cddf81106d1bb97eb9

3 files changed, 109 insertions, 20 deletions

diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index 0832bb1bfe7b..33f858386999 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -430,6 +430,42 @@ sal_Int32 AlgAtom::getVerticalShapesCount(const ShapePtr& rShape)
     return nCount;
 }
 
+namespace
+{
+/**
+ * Apply rConstraint to the rProperties shared layout state.
+ *
+ * Note that the order in which constraints are applied matters, given that constraints can refer to
+ * each other, and in case A depends on B and A is applied before B, the effect of A won't be
+ * updated when B is applied.
+ */
+void ApplyConstraintToLayout(const Constraint& rConstraint, LayoutPropertyMap& rProperties)
+{
+    const LayoutPropertyMap::const_iterator aRef = rProperties.find(rConstraint.msRefForName);
+    if (aRef != rProperties.end())
+    {
+        const LayoutProperty::const_iterator aRefType = aRef->second.find(rConstraint.mnRefType);
+        if (aRefType != aRef->second.end())
+            rProperties[rConstraint.msForName][rConstraint.mnType]
+                = aRefType->second * rConstraint.mfFactor;
+        else
+        {
+            // Values are never in EMU, while oox::drawingml::Shape position and size are always in
+            // EMU.
+            double fUnitFactor = 0;
+            if (isFontUnit(rConstraint.mnRefType))
+                // Points -> EMU.
+                fUnitFactor = EMU_PER_PT;
+            else
+                // Millimeters -> EMU.
+                fUnitFactor = EMU_PER_HMM * 100;
+            rProperties[rConstraint.msForName][rConstraint.mnType]
+                = rConstraint.mfValue * fUnitFactor;
+        }
+    }
+}
+}
+
 void AlgAtom::layoutShape( const ShapePtr& rShape,
                            const std::vector<Constraint>& rConstraints )
 {
@@ -467,31 +503,74 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
 
             for (const auto & rConstr : rConstraints)
             {
-                const LayoutPropertyMap::const_iterator aRef = aProperties.find(rConstr.msRefForName);
-                if (aRef != aProperties.end())
+                // Apply direct constraints for all layout nodes.
+                ApplyConstraintToLayout(rConstr, aProperties);
+            }
+
+            for (auto& aCurrShape : rShape->getChildren())
+            {
+                // Apply constraints from the current layout node for this child shape.
+                // Previous child shapes may have changed aProperties.
+                for (const auto& rConstr : rConstraints)
                 {
-                    const LayoutProperty::const_iterator aRefType = aRef->second.find(rConstr.mnRefType);
-                    if (aRefType != aRef->second.end())
-                        aProperties[rConstr.msForName][rConstr.mnType] = aRefType->second * rConstr.mfFactor;
-                    else
+                    if (rConstr.msForName != aCurrShape->getInternalName())
                     {
-                        // Values are never in EMU, while oox::drawingml::Shape
-                        // position and size are always in EMU.
-                        double fUnitFactor = 0;
-                        if (isFontUnit(rConstr.mnRefType))
-                            // Points -> EMU.
-                            fUnitFactor = EMU_PER_PT;
-                        else
-                            // Millimeters -> EMU.
-                            fUnitFactor = EMU_PER_HMM * 100;
-                        aProperties[rConstr.msForName][rConstr.mnType]
-                            = rConstr.mfValue * fUnitFactor;
+                        continue;
+                    }
+
+                    ApplyConstraintToLayout(rConstr, aProperties);
+                }
+
+                // Apply constraints from the child layout node for this child shape.
+                // This builds on top of the own parent state + the state of previous shapes in the
+                // same composite algorithm.
+                const LayoutNode& rLayoutNode = getLayoutNode();
+                for (const auto& pDirectChild : rLayoutNode.getChildren())
+                {
+                    auto pLayoutNode = dynamic_cast<LayoutNode*>(pDirectChild.get());
+                    if (!pLayoutNode)
+                    {
+                        continue;
+                    }
+
+                    if (pLayoutNode->getName() != aCurrShape->getInternalName())
+                    {
+                        continue;
+                    }
+
+                    for (const auto& pChild : pLayoutNode->getChildren())
+                    {
+                        auto pConstraintAtom = dynamic_cast<ConstraintAtom*>(pChild.get());
+                        if (!pConstraintAtom)
+                        {
+                            continue;
+                        }
+
+                        const Constraint& rConstraint = pConstraintAtom->getConstraint();
+                        if (!rConstraint.msForName.isEmpty())
+                        {
+                            continue;
+                        }
+
+                        if (!rConstraint.msRefForName.isEmpty())
+                        {
+                            continue;
+                        }
+
+                        // Either an absolute value or a factor of a property.
+                        if (rConstraint.mfValue == 0.0 && rConstraint.mnRefType == XML_none)
+                        {
+                            continue;
+                        }
+
+                        Constraint aConstraint(rConstraint);
+                        aConstraint.msForName = pLayoutNode->getName();
+                        aConstraint.msRefForName = pLayoutNode->getName();
+
+                        ApplyConstraintToLayout(aConstraint, aProperties);
                     }
                 }
-            }
 
-            for (auto & aCurrShape : rShape->getChildren())
-            {
                 awt::Size aSize = rShape->getSize();
                 awt::Point aPos(0, 0);
 
diff --git a/sd/qa/unit/data/pptx/fill-color-list.pptx b/sd/qa/unit/data/pptx/fill-color-list.pptx
index 341233ad5f78..c88434952631 100644
--- a/sd/qa/unit/data/pptx/fill-color-list.pptx
+++ b/sd/qa/unit/data/pptx/fill-color-list.pptx
diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx
index 273056c868d7..327645fddc3f 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -1470,6 +1470,16 @@ void SdImportTestSmartArt::testFillColorList()
     // - Actual  : 16225862 (0xf79646)
     // i.e. the background of the "A" shape was orange-ish, rather than red-ish.
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0xC0504D), nFillColor);
+
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 2239
+    // - Actual  : 5199
+    // i.e. the "A" shape's height/width aspect ratio was not 0.4 but rather close to 1.0, even if
+    // ppt/diagrams/layout1.xml's <dgm:constr type="h" refType="w" op="lte" fact="0.4"/> requested
+    // 0.4.
+    awt::Size aActualSize = xShape->getSize();
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2239), aActualSize.Height);
+
     xDocShRef->DoClose();
 }