From 2f00ae052c7f76329f341c8e5d56da3c940885a6 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Tue, 29 Sep 2020 17:41:12 +0200 Subject: [PATCH] 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 Reviewed-by: Miklos Vajna --- .../drawingml/diagram/diagramlayoutatoms.cxx | 57 ++++++++++++++++++---- sd/qa/unit/import-tests-smartart.cxx | 13 +++-- 2 files changed, 56 insertions(+), 14 deletions(-) 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& fShapeWidth = fShapeHeight * fChildAspectRatio; } - double fSpaceFromConstraint = 0; + double fSpaceFromConstraint = 1.0; LayoutPropertyMap aPropertiesByName; std::map aPropertiesByType; LayoutProperty& rParent = aPropertiesByName[""]; @@ -1317,7 +1317,7 @@ void AlgAtom::layoutShape(const ShapePtr& rShape, const std::vector& 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& 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& 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& for ( ; nRow(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& 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(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 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 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(3), aYPositions.size()); + // - Expected: 2 + // - Actual : 3 + // i.e. an unwanted row appeared. + CPPUNIT_ASSERT_EQUAL(static_cast(2), aYPositions.size()); xDocShRef->DoClose(); } -- 2.11.4.GIT