[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.4' - 2 commits - oox/source sd/qa
Miklos Vajna (via logerrit)
logerrit at kemper.freedesktop.org
Wed Sep 30 15:26:36 UTC 2020
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 <vmiklos at collabora.com>
AuthorDate: Wed Sep 30 12:41:15 2020 +0200
Commit: Miklos Vajna <vmiklos at collabora.com>
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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
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 <vmiklos at collabora.com>
AuthorDate: Tue Sep 29 17:41:12 2020 +0200
Commit: Miklos Vajna <vmiklos at collabora.com>
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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
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();
}
More information about the Libreoffice-commits
mailing list