[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - 2 commits - oox/source sd/qa

Miklos Vajna (via logerrit) logerrit at kemper.freedesktop.org
Wed Aug 5 09:26:37 UTC 2020


 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 <vmiklos at collabora.com>
AuthorDate: Mon Aug 3 09:57:57 2020 +0200
Commit:     Miklos Vajna <vmiklos at 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.kose at 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 <vmiklos at collabora.com>
AuthorDate: Fri Jul 31 15:59:10 2020 +0200
Commit:     Miklos Vajna <vmiklos at 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.kose at 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();
 }
 


More information about the Libreoffice-commits mailing list