oox/source/drawingml/diagram/diagramlayoutatoms.cxx | 87 ++++++++++++++++---- sd/qa/unit/import-tests-smartart.cxx | 11 +- 2 files changed, 80 insertions(+), 18 deletions(-)
New commits: commit 39792891f4e4c1331f7e532e1645166412016e30 Author: Miklos Vajna <[email protected]> AuthorDate: Wed Sep 30 12:41:15 2020 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Wed Sep 30 17:26:00 2020 +0200 oox smartart: snake algo: make sure child shape height stays within parent 1) When applying double outside spacing, introduced with commit 0a29c928afa74123bca05dc089c751603d368467 (oox smartart, picture strip: fix lack of spacing around the picture list, 2019-02-26), make sure that is only applied in the direction of a signle row: i.e. the bugdoc case is left/right outer spacing, but no top/bottom spacing. 2) If a child shape has an aspect ratio request, make sure that it only decreases what would be allocated by default, so the children never leave the parent's rectangle. 3) Fix a mis-match between the first and second row, the unexpected small left padding in the second row was because code assumed that all child shapes have the same width; which is not true, when widths come from constraints. With this in place, we finally do a good rendering of the bugdoc, and child shapes are always within the bounds of the background. (cherry picked from commit 71303c5c23bdb385e9f12c0dbe5d2a0818b836ec) Change-Id: Ia2606dcd945402f7dfe17c6e2f261bfd98667022 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103703 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx index 2807a8a4ea3c..d3c2033f74f1 100644 --- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx +++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx @@ -1385,12 +1385,13 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& const sal_Int32 nDir = maMap.count(XML_grDir) ? maMap.find(XML_grDir)->second : XML_tL; sal_Int32 nIncX = 1; sal_Int32 nIncY = 1; + bool bHorizontal = true; switch (nDir) { case XML_tL: nIncX = 1; nIncY = 1; break; case XML_tR: nIncX = -1; nIncY = 1; break; - case XML_bL: nIncX = 1; nIncY = -1; break; - case XML_bR: nIncX = -1; nIncY = -1; break; + case XML_bL: nIncX = 1; nIncY = -1; bHorizontal = false; break; + case XML_bR: nIncX = -1; nIncY = -1; bHorizontal = false; break; } sal_Int32 nCount = rShape->getChildren().size(); @@ -1454,6 +1455,8 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& static_cast<sal_Int32>(nHeight * fChildAspectRatio)); aChildSize = awt::Size(nWidth, nHeight); } + + bHorizontal = false; } awt::Point aCurrPos(0, 0); @@ -1462,8 +1465,13 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& 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; + { + if (!bHorizontal) + { + // 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; @@ -1482,7 +1490,8 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& // aShapeWidths items are a portion of nMaxRowWidth. We want the same ratio, // based on the original parent width, ignoring the aspect ratio request. double fWidthFactor = static_cast<double>(aShapeWidths[index]) / nMaxRowWidth; - if (nCount >= 2 && rShape->getChildren()[1]->getDataNodeType() == XML_sibTrans) + bool bWidthsFromConstraints = nCount >= 2 && rShape->getChildren()[1]->getDataNodeType() == XML_sibTrans; + if (bWidthsFromConstraints) { // We can only work from constraints if spacing is represented by a real // child shape. @@ -1491,6 +1500,9 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& if (fChildAspectRatio) { aCurrSize.Height = aCurrSize.Width / fChildAspectRatio; + + // Child shapes are not allowed to leave their parent. + aCurrSize.Height = std::min<sal_Int32>(aCurrSize.Height, rShape->getSize().Height / (nRow + (nRow-1)*fSpace)); } if (aCurrSize.Height > nRowHeight) { @@ -1508,8 +1520,18 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& { // if last row, then position children according to number of shapes. if((index+1)%nCol!=0 && (index+1)>=3 && ((index+1)/nCol+1)==nRow && nCount!=nRow*nCol) + { // position first child of last row - aCurrPos.X = nStartX + (nIncX * (aCurrSize.Width + fSpace*aCurrSize.Width))/2; + if (bWidthsFromConstraints) + { + aCurrPos.X = nStartX; + } + else + { + // Can assume that all child shape has the same width. + aCurrPos.X = nStartX + (nIncX * (aCurrSize.Width + fSpace*aCurrSize.Width))/2; + } + } else // if not last row, positions first child of that row aCurrPos.X = nStartX; diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx index 20c679a0bc33..6af86e6a5343 100644 --- a/sd/qa/unit/import-tests-smartart.cxx +++ b/sd/qa/unit/import-tests-smartart.cxx @@ -1581,14 +1581,12 @@ void SdImportTestSmartArt::testSnakeRows() m_directories.getURLFromSrc("/sd/qa/unit/data/pptx/smartart-snake-rows.pptx"), PPTX); uno::Reference<drawing::XShapes> xDiagram(getShapeFromPage(0, 0, xDocShRef), uno::UNO_QUERY); + // Collect position of the background and the real child shapes. First row and background has + // the same top position, unless some unexpected spacing happens, since this is a + // "left-to-right, then top-to-bottom" snake direction. std::set<sal_Int32> aYPositions; for (sal_Int32 nChild = 0; nChild < xDiagram->getCount(); ++nChild) { - if (nChild == 0) - { - // Ignore background shape, we check how many rows actual children use. - continue; - } uno::Reference<drawing::XShape> xChild(xDiagram->getByIndex(nChild), uno::UNO_QUERY); aYPositions.insert(xChild->getPosition().Y); } commit 2f00ae052c7f76329f341c8e5d56da3c940885a6 Author: Miklos Vajna <[email protected]> AuthorDate: Tue Sep 29 17:41:12 2020 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Wed Sep 30 17:25:45 2020 +0200 oox smartart: snake algo: apply constraints on child shape widths This requires tracking what is the total of the width request of child shapes, then scaling them according to what is the total available width. Additionally, the height of child shapes should be adjusted based on their aspect ratio requests. A related trap is when an (invisible) spacing shape is at the end of the row, that would result in smaller spacing between the rows, so track the max height of shapes inside a single row. With this, finally the 6 child shapes are arranged on 2 rows, not 3 ones. (cherry picked from commit 5d899bf3ee59a226f855c8c56389344862efaa95) Change-Id: I4eb2f06676df11c1432e0934ca3a0ec8891c5843 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103702 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx index 4e0870d51ee5..2807a8a4ea3c 100644 --- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx +++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx @@ -1309,7 +1309,7 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& fShapeWidth = fShapeHeight * fChildAspectRatio; } - double fSpaceFromConstraint = 0; + double fSpaceFromConstraint = 1.0; LayoutPropertyMap aPropertiesByName; std::map<sal_Int32, LayoutProperty> aPropertiesByType; LayoutProperty& rParent = aPropertiesByName[""]; @@ -1317,7 +1317,7 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& rParent[XML_h] = fShapeHeight; for (const auto& rConstr : rConstraints) { - if (rConstr.mnRefType == XML_h) + if (rConstr.mnRefType == XML_w || rConstr.mnRefType == XML_h) { if (rConstr.mnType == XML_sp && rConstr.msForName.isEmpty()) fSpaceFromConstraint = rConstr.mfFactor; @@ -1380,7 +1380,7 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& aShapeWidths[i] = it->second; } - bool bSpaceFromConstraints = fSpaceFromConstraint != 0; + bool bSpaceFromConstraints = fSpaceFromConstraint != 1.0; const sal_Int32 nDir = maMap.count(XML_grDir) ? maMap.find(XML_grDir)->second : XML_tL; sal_Int32 nIncX = 1; @@ -1400,6 +1400,7 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& sal_Int32 nCol = 1; sal_Int32 nRow = 1; + sal_Int32 nMaxRowWidth = 0; 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. @@ -1409,8 +1410,22 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& for ( ; nRow<nCount; nRow++) { nCol = std::ceil(static_cast<double>(nCount) / nRow); - if ((fShapeHeight * nRow) / (fShapeWidth * nCol) >= fAspectRatio) + sal_Int32 nRowWidth = 0; + for (sal_Int32 i = 0; i < nCol; ++i) { + if (i >= nCount) + { + break; + } + + nRowWidth += aShapeWidths[i]; + } + if ((fShapeHeight * nRow) / nRowWidth >= fAspectRatio) + { + if (nRowWidth > nMaxRowWidth) + { + nMaxRowWidth = nRowWidth; + } break; } } @@ -1458,35 +1473,57 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector<Constraint>& switch(aContDir) { case XML_sameDir: + { + sal_Int32 nRowHeight = 0; for (auto & aCurrShape : rShape->getChildren()) { aCurrShape->setPosition(aCurrPos); - aCurrShape->setSize(aChildSize); - aCurrShape->setChildSize(aChildSize); + awt::Size aCurrSize(aChildSize); + // aShapeWidths items are a portion of nMaxRowWidth. We want the same ratio, + // based on the original parent width, ignoring the aspect ratio request. + double fWidthFactor = static_cast<double>(aShapeWidths[index]) / nMaxRowWidth; + if (nCount >= 2 && rShape->getChildren()[1]->getDataNodeType() == XML_sibTrans) + { + // We can only work from constraints if spacing is represented by a real + // child shape. + aCurrSize.Width = rShape->getSize().Width * fWidthFactor; + } + if (fChildAspectRatio) + { + aCurrSize.Height = aCurrSize.Width / fChildAspectRatio; + } + if (aCurrSize.Height > nRowHeight) + { + nRowHeight = aCurrSize.Height; + } + aCurrShape->setSize(aCurrSize); + aCurrShape->setChildSize(aCurrSize); index++; // counts index of child, helpful for positioning. if(index%nCol==0 || ((index/nCol)+1)!=nRow) - aCurrPos.X += nIncX * (aChildSize.Width + fSpace*aChildSize.Width); + aCurrPos.X += nIncX * (aCurrSize.Width + fSpace*aCurrSize.Width); if(++nColIdx == nCol) // condition for next row { // if last row, then position children according to number of shapes. if((index+1)%nCol!=0 && (index+1)>=3 && ((index+1)/nCol+1)==nRow && nCount!=nRow*nCol) // position first child of last row - aCurrPos.X = nStartX + (nIncX * (aChildSize.Width + fSpace*aChildSize.Width))/2; + aCurrPos.X = nStartX + (nIncX * (aCurrSize.Width + fSpace*aCurrSize.Width))/2; else // if not last row, positions first child of that row aCurrPos.X = nStartX; - aCurrPos.Y += nIncY * (aChildSize.Height + fSpace*aChildSize.Height); + aCurrPos.Y += nIncY * (nRowHeight + fSpace*nRowHeight); nColIdx = 0; + nRowHeight = 0; } // positions children in the last row. if(index%nCol!=0 && index>=3 && ((index/nCol)+1)==nRow) - aCurrPos.X += (nIncX * (aChildSize.Width + fSpace*aChildSize.Width)); + aCurrPos.X += (nIncX * (aCurrSize.Width + fSpace*aCurrSize.Width)); } break; + } case XML_revDir: for (auto & aCurrShape : rShape->getChildren()) { diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx index 1746b7c58f4a..20c679a0bc33 100644 --- a/sd/qa/unit/import-tests-smartart.cxx +++ b/sd/qa/unit/import-tests-smartart.cxx @@ -1584,15 +1584,20 @@ void SdImportTestSmartArt::testSnakeRows() std::set<sal_Int32> aYPositions; for (sal_Int32 nChild = 0; nChild < xDiagram->getCount(); ++nChild) { + if (nChild == 0) + { + // Ignore background shape, we check how many rows actual children use. + continue; + } uno::Reference<drawing::XShape> xChild(xDiagram->getByIndex(nChild), uno::UNO_QUERY); aYPositions.insert(xChild->getPosition().Y); } // Without the accompanying fix in place, this test would have failed with: - // - Expected: 3 - // - Actual : 4 - // i.e. one more unwanted row appeared. This is better, but the ideal would be just 2 rows. - CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), aYPositions.size()); + // - Expected: 2 + // - Actual : 3 + // i.e. an unwanted row appeared. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aYPositions.size()); xDocShRef->DoClose(); } _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
