oox/source/drawingml/diagram/diagramlayoutatoms.cxx |   88 ++++++++++++++++++--
 sd/qa/unit/data/pptx/smartart-linear-rule.pptx      |binary
 sd/qa/unit/import-tests-smartart.cxx                |   13 ++
 3 files changed, 93 insertions(+), 8 deletions(-)

New commits:
commit d5bff7f5e4b10ec42395efa3cd0520c40c06a356
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Mon Aug 3 09:57:57 2020 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Aug 5 11:26:17 2020 +0200

    oox smartart, linear layout: fix scaling of spacing without rules
    
    With this, finally the arrow shape has the correct horizontal position
    and width, even if the markup is as complex as the PowerPoint UI
    generates it (the previous version was a more minimal version).
    
    (cherry picked from commit 880673412143a7db7ea1bf4766040662dfc085dc)
    
    Change-Id: I59f237c582053067e890180a1ae40471e5f46dea
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/100104
    Tested-by: Jenkins
    Reviewed-by: Gülşah Köse <gulsah.k...@collabora.com>

diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx 
b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index e18e34e5c12a..24e5422ce654 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -963,6 +963,8 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const 
std::vector<Constraint>&
                         {
                             fCount -= 1.0;
 
+                            bool bIsDependency = false;
+                            double fFactor = 0;
                             for (const auto& rConstraint : rConstraints)
                             {
                                 if (rConstraint.msForName != 
aCurrShape->getInternalName())
@@ -970,16 +972,25 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const 
std::vector<Constraint>&
                                     continue;
                                 }
 
-                                if 
(aChildrenToShrink.find(rConstraint.msRefForName) == aChildrenToShrink.end())
+                                if ((nDir == XML_fromL || nDir == XML_fromR) 
&& rConstraint.mnType != XML_w)
                                 {
                                     continue;
                                 }
+                                if ((nDir == XML_fromL || nDir == XML_fromR) 
&& rConstraint.mnType == XML_w)
+                                {
+                                    fFactor = rConstraint.mfFactor;
+                                }
 
-                                if ((nDir == XML_fromL || nDir == XML_fromR) 
&& rConstraint.mnType != XML_w)
+                                if ((nDir == XML_fromT || nDir == XML_fromB) 
&& rConstraint.mnType != XML_h)
                                 {
                                     continue;
                                 }
-                                if ((nDir == XML_fromT || nDir == XML_fromB) 
&& rConstraint.mnType != XML_h)
+                                if ((nDir == XML_fromT || nDir == XML_fromB) 
&& rConstraint.mnType == XML_h)
+                                {
+                                    fFactor = rConstraint.mfFactor;
+                                }
+
+                                if 
(aChildrenToShrink.find(rConstraint.msRefForName) == aChildrenToShrink.end())
                                 {
                                     continue;
                                 }
@@ -988,8 +999,29 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const 
std::vector<Constraint>&
                                 // other child which will be scaled.
                                 fCount += rConstraint.mfFactor;
                                 
aChildrenToShrinkDeps.insert(aCurrShape->getInternalName());
+                                bIsDependency = true;
                                 break;
                             }
+
+                            if (!bIsDependency && aCurrShape->getServiceName() 
== "com.sun.star.drawing.GroupShape")
+                            {
+                                bool bScaleDownEmptySpacing = false;
+                                if (nDir == XML_fromL || nDir == XML_fromR)
+                                {
+                                    oox::OptValue<sal_Int32> oWidth = 
findProperty(aProperties, aCurrShape->getInternalName(), XML_w);
+                                    bScaleDownEmptySpacing = oWidth.has() && 
oWidth.get() > 0;
+                                }
+                                if (!bScaleDownEmptySpacing && (nDir == 
XML_fromT || nDir == XML_fromB))
+                                {
+                                    oox::OptValue<sal_Int32> oHeight = 
findProperty(aProperties, aCurrShape->getInternalName(), XML_h);
+                                    bScaleDownEmptySpacing = oHeight.has() && 
oHeight.get() > 0;
+                                }
+                                if (bScaleDownEmptySpacing && 
aCurrShape->getChildren().empty())
+                                {
+                                    fCount += fFactor;
+                                    
aChildrenToShrinkDeps.insert(aCurrShape->getInternalName());
+                                }
+                            }
                         }
                     }
                 }
@@ -1083,6 +1115,14 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const 
std::vector<Constraint>&
                     aCurrPos.Y = (rShape->getSize().Height - aSize.Height) / 2;
                 if (nIncY)
                     aCurrPos.X = (rShape->getSize().Width - aSize.Width) / 2;
+                if (aCurrPos.X < 0)
+                {
+                    aCurrPos.X = 0;
+                }
+                if (aCurrPos.Y < 0)
+                {
+                    aCurrPos.Y = 0;
+                }
 
                 aCurrShape->setPosition(aCurrPos);
 
diff --git a/sd/qa/unit/data/pptx/smartart-linear-rule.pptx 
b/sd/qa/unit/data/pptx/smartart-linear-rule.pptx
index e76dfd9ee770..05905299ed27 100644
Binary files a/sd/qa/unit/data/pptx/smartart-linear-rule.pptx and 
b/sd/qa/unit/data/pptx/smartart-linear-rule.pptx differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx 
b/sd/qa/unit/import-tests-smartart.cxx
index 556b1562c584..1a4818e8474a 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -1529,6 +1529,11 @@ void SdImportTestSmartArt::testLinearRule()
     // - Expected: 3160
     // - Actual  : 8770
     // i.e. there was unexpected spacing on the left of the arrow.
+    // Then the imporoved version of the test document failed with:
+    // - Expected: 3160
+    // - Actual  : 19828
+    // i.e. the spacing on the left of the arrow was so large that the shape 
was mostly outside the
+    // slide.
     sal_Int32 nGroupLeft = xGroup->getPosition().X;
     sal_Int32 nArrowLeft = xShape->getPosition().X;
     CPPUNIT_ASSERT_EQUAL(nGroupLeft, nArrowLeft);
commit 14e664db5211f8c398ee58dded853e12ac9142de
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri Jul 31 15:59:10 2020 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Aug 5 11:26:01 2020 +0200

    oox smartart, linear layout: correctly scale spacings wrt constraints and 
rules
    
    When constraints request a width which is larger than 100%, we scale
    down. Then rules decide which children should be scaled down and which
    ones stay as-is.
    
    This commit adjusts the size of children which have no rule, but their
    size has a constraint that they're a fraction of a scaled down child.
    
    (cherry picked from commit 91f0f7e5e0a55cb1dbd729a1d7de52388b1cfb15)
    
    Change-Id: I0a007d82f49f18951215afb1bfe8c0f1328ecd41
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/100103
    Tested-by: Jenkins
    Reviewed-by: Gülşah Köse <gulsah.k...@collabora.com>

diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx 
b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index 42508f5984de..e18e34e5c12a 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -905,7 +905,7 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const 
std::vector<Constraint>&
             const sal_Int32 nIncX = nDir==XML_fromL ? 1 : (nDir==XML_fromR ? 
-1 : 0);
             const sal_Int32 nIncY = nDir==XML_fromT ? 1 : (nDir==XML_fromB ? 
-1 : 0);
 
-            sal_Int32 nCount = rShape->getChildren().size();
+            double fCount = rShape->getChildren().size();
             sal_Int32 nConnectorAngle = 0;
             switch (nDir)
             {
@@ -952,18 +952,50 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const 
std::vector<Constraint>&
             if (!aChildrenToShrink.empty())
             {
                 // Have scaling info from rules: then only count scaled 
children.
+                // Also count children which are a fraction of a scaled child.
+                std::set<OUString> aChildrenToShrinkDeps;
                 for (auto& aCurrShape : rShape->getChildren())
                 {
                     if (aChildrenToShrink.find(aCurrShape->getInternalName())
                         == aChildrenToShrink.end())
                     {
-                        if (nCount > 1)
+                        if (fCount > 1.0)
                         {
-                            --nCount;
+                            fCount -= 1.0;
+
+                            for (const auto& rConstraint : rConstraints)
+                            {
+                                if (rConstraint.msForName != 
aCurrShape->getInternalName())
+                                {
+                                    continue;
+                                }
+
+                                if 
(aChildrenToShrink.find(rConstraint.msRefForName) == aChildrenToShrink.end())
+                                {
+                                    continue;
+                                }
+
+                                if ((nDir == XML_fromL || nDir == XML_fromR) 
&& rConstraint.mnType != XML_w)
+                                {
+                                    continue;
+                                }
+                                if ((nDir == XML_fromT || nDir == XML_fromB) 
&& rConstraint.mnType != XML_h)
+                                {
+                                    continue;
+                                }
+
+                                // At this point we have a child with a size 
which is a factor of an
+                                // other child which will be scaled.
+                                fCount += rConstraint.mfFactor;
+                                
aChildrenToShrinkDeps.insert(aCurrShape->getInternalName());
+                                break;
+                            }
                         }
                     }
                 }
 
+                aChildrenToShrink.insert(aChildrenToShrinkDeps.begin(), 
aChildrenToShrinkDeps.end());
+
                 // No manual spacing: spacings are children as well.
                 aSpaceSize = awt::Size();
             }
@@ -978,13 +1010,13 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const 
std::vector<Constraint>&
                                               && aChild->getChildren().empty();
                                    }),
                     rShape->getChildren().end());
-                nCount = rShape->getChildren().size();
+                fCount = rShape->getChildren().size();
             }
             awt::Size aChildSize = rShape->getSize();
             if (nDir == XML_fromL || nDir == XML_fromR)
-                aChildSize.Width /= nCount;
+                aChildSize.Width /= fCount;
             else if (nDir == XML_fromT || nDir == XML_fromB)
-                aChildSize.Height /= nCount;
+                aChildSize.Height /= fCount;
 
             awt::Point aCurrPos(0, 0);
             if (nIncX == -1)
@@ -1008,8 +1040,8 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const 
std::vector<Constraint>&
                 aTotalSize.Height += aSize.Height;
             }
 
-            aTotalSize.Width += (nCount-1) * aSpaceSize.Width;
-            aTotalSize.Height += (nCount-1) * aSpaceSize.Height;
+            aTotalSize.Width += (fCount-1) * aSpaceSize.Width;
+            aTotalSize.Height += (fCount-1) * aSpaceSize.Height;
 
             double fWidthScale = 1.0;
             double fHeightScale = 1.0;
diff --git a/sd/qa/unit/data/pptx/smartart-linear-rule.pptx 
b/sd/qa/unit/data/pptx/smartart-linear-rule.pptx
index f5fbb5c87a54..e76dfd9ee770 100644
Binary files a/sd/qa/unit/data/pptx/smartart-linear-rule.pptx and 
b/sd/qa/unit/data/pptx/smartart-linear-rule.pptx differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx 
b/sd/qa/unit/import-tests-smartart.cxx
index 84d6450b0df7..556b1562c584 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -1525,6 +1525,14 @@ void SdImportTestSmartArt::testLinearRule()
     // i.e. the width of the background arrow was too small.
     CPPUNIT_ASSERT_GREATER(static_cast<sal_Int32>(17500), 
xShape->getSize().Width);
 
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 3160
+    // - Actual  : 8770
+    // i.e. there was unexpected spacing on the left of the arrow.
+    sal_Int32 nGroupLeft = xGroup->getPosition().X;
+    sal_Int32 nArrowLeft = xShape->getPosition().X;
+    CPPUNIT_ASSERT_EQUAL(nGroupLeft, nArrowLeft);
+
     xDocShRef->DoClose();
 }
 
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to