pool/libreoffice
SHA256
3
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
3311affae0
libreoffice/bsc1146025.diff
Tomáš Chvátal 2b68d5ba44 Accepting request 810676 from LibreOffice:6.4 
- Fix bsc#1146025 - LO-L3: Colored textboxes in PPTX look very odd (SmartArt)
  * bsc1146025.diff

OBS-URL: https://build.opensuse.org/request/show/810676
OBS-URL: https://build.opensuse.org/package/show/LibreOffice:Factory/libreoffice?expand=0&rev=883
2020-06-02 06:03:32 +00:00

502 lines
21 KiB
Diff
Raw Blame History

From bc15a8241b1c8e113bcbb83343652b2c84820e87 Mon Sep 17 00:00:00 2001
From: Miklos Vajna <vmiklos@collabora.com>
Date: Fri, 22 May 2020 17:58:22 +0200
Subject: [PATCH] bsc1146025.diff
smartart import: handle multiple <a:schemeClr> in <dgm:fillClrLst>
The TODO in the ColorFragmentHandler ctor was right: we only handled the
last <a:schemeClr> child, but there can be multiple one.
Use them based on the index of a shape in a <dgm:forEach> loop.
Move the TODO to the only place which still assumes a single color in
the color list.
(cherry picked from commit 12bea6c897822964ad4705418da54411cb15749e)
Conflicts:
oox/source/drawingml/colorchoicecontext.cxx
oox/source/drawingml/diagram/diagram.cxx
sd/qa/unit/import-tests-smartart.cxx
oox smartart import: fix aspect ratio of shape with composite algo
The layout node has alg=composite, then a parTx and a desTx child layout
nodes. No matter what order is used (parent first, child first), the
result will be wrong, as the constraints refer to each other. I did not
spot any description in ISO 29500-1 that would describe what is the
expected behavior.
Researching this, found "One other consideration when specifying
composite constraints is that the constraints must be specified in the
same order as the nested layout nodes." at
<http://web.archive.org/web/20111015151600/http://msdn.microsoft.com/en-us/magazine/cc163470.aspx>,
which suggests to handle constraints for each shape in a parent -> child
order, but keep a shared state when iterating over the children which
gives us:
- parent node, all direct constraints
- for each child node:
- child's constraints from parent
- child's own constraints
This way the desTx top value can depend on the parTx's height, and it's
supported to define parTx's height only in the parTx layout node, not in
the composite parent.
And after all, it matches what PowerPoint does, so the column headings
in the bugdoc have a 4:10 height:width aspect ratio.
(cherry picked from commit 414586649582e182b2603702f4f586f4beeed8a9)
oox smartart import, composite alg: implement vertical centering
The bugdoc's case was that the total height would be used by 2 shapes,
but then a constraint decreases the height of one shape, so not all
vertical space is used.
We used to just count from the top, need to center vertically, as
PowerPoint does it.
(cherry picked from commit acdde3c643fde015214c546b1567727272ea799e)
Change-Id: I1c5c4f82e621f1110ef06b0490ff79f82f60f214
deb76c1ddd1ffff8d2a217cddf81106d1bb97eb9
436019e9e837b73130e387c9bcd309e20045b0f9
---
oox/inc/drawingml/colorchoicecontext.hxx | 15 ++
oox/source/drawingml/colorchoicecontext.cxx | 25 +++
oox/source/drawingml/diagram/diagram.cxx | 17 +-
oox/source/drawingml/diagram/diagram.hxx | 16 +-
.../diagram/diagramfragmenthandler.cxx | 15 +-
.../drawingml/diagram/diagramlayoutatoms.cxx | 162 ++++++++++++++----
.../drawingml/diagram/diagramlayoutatoms.hxx | 3 +-
.../drawingml/diagram/layoutatomvisitors.cxx | 4 +-
8 files changed, 208 insertions(+), 49 deletions(-)
diff --git a/oox/inc/drawingml/colorchoicecontext.hxx b/oox/inc/drawingml/colorchoicecontext.hxx
index 29889f494e3d..dae8d7fb8e25 100644
--- a/oox/inc/drawingml/colorchoicecontext.hxx
+++ b/oox/inc/drawingml/colorchoicecontext.hxx
@@ -22,6 +22,8 @@
#include <oox/core/contexthandler2.hxx>
+#include <vector>
+
namespace oox {
namespace drawingml {
@@ -65,6 +67,19 @@ private:
Color& mrColor;
};
+/// Same as ColorContext, but handles multiple colors.
+class ColorsContext : public ::oox::core::ContextHandler2
+{
+public:
+ explicit ColorsContext(::oox::core::ContextHandler2Helper const& rParent,
+ std::vector<Color>& rColors);
+
+ virtual ::oox::core::ContextHandlerRef
+ onCreateContext(sal_Int32 nElement, const ::oox::AttributeList& rAttribs) override;
+
+private:
+ std::vector<Color>& mrColors;
+};
} // namespace drawingml
} // namespace oox
diff --git a/oox/source/drawingml/colorchoicecontext.cxx b/oox/source/drawingml/colorchoicecontext.cxx
index cf6c17ecd3b4..a9e0d91ef32e 100644
--- a/oox/source/drawingml/colorchoicecontext.cxx
+++ b/oox/source/drawingml/colorchoicecontext.cxx
@@ -149,6 +149,31 @@ ColorContext::ColorContext( ContextHandler2Helper const & rParent, Color& rColor
return nullptr;
}
+ColorsContext::ColorsContext(ContextHandler2Helper const& rParent, std::vector<Color>& rColors)
+ : ContextHandler2(rParent)
+ , mrColors(rColors)
+{
+}
+
+::oox::core::ContextHandlerRef ColorsContext::onCreateContext(sal_Int32 nElement,
+ const AttributeList&)
+{
+ switch (nElement)
+ {
+ case A_TOKEN(scrgbClr):
+ case A_TOKEN(srgbClr):
+ case A_TOKEN(hslClr):
+ case A_TOKEN(sysClr):
+ case A_TOKEN(schemeClr):
+ case A_TOKEN(prstClr):
+ {
+ mrColors.emplace_back();
+ return new ColorValueContext(*this, mrColors.back());
+ }
+ }
+ return nullptr;
+}
+
} // namespace drawingml
} // namespace oox
diff --git a/oox/source/drawingml/diagram/diagram.cxx b/oox/source/drawingml/diagram/diagram.cxx
index b2f3373ad113..a03a06c39125 100644
--- a/oox/source/drawingml/diagram/diagram.cxx
+++ b/oox/source/drawingml/diagram/diagram.cxx
@@ -321,9 +321,11 @@ void loadDiagram( ShapePtr const & pShape,
if( !pData->getExtDrawings().empty() )
{
const DiagramColorMap::const_iterator aColor = pDiagram->getColors().find("node0");
- if( aColor != pDiagram->getColors().end() )
+ if( aColor != pDiagram->getColors().end() && !aColor->second.maTextFillColors.empty())
{
- pShape->setFontRefColorForNodes(aColor->second.maTextFillColor);
+ // TODO(F1): well, actually, there might be *several* color
+ // definitions in it, after all it's called list.
+ pShape->setFontRefColorForNodes(DiagramColor::getColorByIndex(aColor->second.maTextFillColors, -1));
}
}
@@ -425,6 +427,17 @@ void reloadDiagram(SdrObject* pObj, core::XmlFilterBase& rFilter)
child->addShape(rFilter, rFilter.getCurrentTheme(), xShapes, aTransformation, pShape->getFillProperties());
}
+const oox::drawingml::Color&
+DiagramColor::getColorByIndex(const std::vector<oox::drawingml::Color>& rColors, sal_Int32 nIndex)
+{
+ assert(!rColors.empty());
+ if (nIndex == -1)
+ {
+ return rColors[rColors.size() - 1];
+ }
+
+ return rColors[nIndex % rColors.size()];
+}
} }
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/oox/source/drawingml/diagram/diagram.hxx b/oox/source/drawingml/diagram/diagram.hxx
index 576c4007e29f..a674e248961e 100644
--- a/oox/source/drawingml/diagram/diagram.hxx
+++ b/oox/source/drawingml/diagram/diagram.hxx
@@ -22,6 +22,7 @@
#include <map>
#include <memory>
+#include <vector>
#include <rtl/ustring.hxx>
@@ -111,12 +112,15 @@ typedef std::map<OUString,DiagramStyle> DiagramQStyleMap;
struct DiagramColor
{
- oox::drawingml::Color maFillColor;
- oox::drawingml::Color maLineColor;
- oox::drawingml::Color maEffectColor;
- oox::drawingml::Color maTextFillColor;
- oox::drawingml::Color maTextLineColor;
- oox::drawingml::Color maTextEffectColor;
+ std::vector<oox::drawingml::Color> maFillColors;
+ std::vector<oox::drawingml::Color> maLineColors;
+ std::vector<oox::drawingml::Color> maEffectColors;
+ std::vector<oox::drawingml::Color> maTextFillColors;
+ std::vector<oox::drawingml::Color> maTextLineColors;
+ std::vector<oox::drawingml::Color> maTextEffectColors;
+
+ static const oox::drawingml::Color&
+ getColorByIndex(const std::vector<oox::drawingml::Color>& rColors, sal_Int32 nIndex);
};
typedef std::map<OUString,DiagramColor> DiagramColorMap;
diff --git a/oox/source/drawingml/diagram/diagramfragmenthandler.cxx b/oox/source/drawingml/diagram/diagramfragmenthandler.cxx
index 6e1000af3627..7eae543dc6f9 100644
--- a/oox/source/drawingml/diagram/diagramfragmenthandler.cxx
+++ b/oox/source/drawingml/diagram/diagramfragmenthandler.cxx
@@ -196,21 +196,18 @@ ColorFragmentHandler::ColorFragmentHandler( ::oox::core::XmlFilterBase& rFilter,
{
// the actual colors - defer to color fragment handlers.
- // TODO(F1): well, actually, there might be *several* color
- // definitions in it, after all it's called list. But
- // apparently ColorContext doesn't handle that anyway...
case DGM_TOKEN(fillClrLst):
- return new ColorContext( *this, maColorEntry.maFillColor );
+ return new ColorsContext( *this, maColorEntry.maFillColors );
case DGM_TOKEN(linClrLst):
- return new ColorContext( *this, maColorEntry.maLineColor );
+ return new ColorsContext( *this, maColorEntry.maLineColors );
case DGM_TOKEN(effectClrLst):
- return new ColorContext( *this, maColorEntry.maEffectColor );
+ return new ColorsContext( *this, maColorEntry.maEffectColors );
case DGM_TOKEN(txFillClrLst):
- return new ColorContext( *this, maColorEntry.maTextFillColor );
+ return new ColorsContext( *this, maColorEntry.maTextFillColors );
case DGM_TOKEN(txLinClrLst):
- return new ColorContext( *this, maColorEntry.maTextLineColor );
+ return new ColorsContext( *this, maColorEntry.maTextLineColors );
case DGM_TOKEN(txEffectClrLst):
- return new ColorContext( *this, maColorEntry.maTextEffectColor );
+ return new ColorsContext( *this, maColorEntry.maTextEffectColors );
}
break;
}
diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
index ff83dde63fa3..19b1d10679f4 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -430,6 +430,42 @@ sal_Int32 AlgAtom::getVerticalShapesCount(const ShapePtr& rShape)
return nCount;
}
+namespace
+{
+/**
+ * Apply rConstraint to the rProperties shared layout state.
+ *
+ * Note that the order in which constraints are applied matters, given that constraints can refer to
+ * each other, and in case A depends on B and A is applied before B, the effect of A won't be
+ * updated when B is applied.
+ */
+void ApplyConstraintToLayout(const Constraint& rConstraint, LayoutPropertyMap& rProperties)
+{
+ const LayoutPropertyMap::const_iterator aRef = rProperties.find(rConstraint.msRefForName);
+ if (aRef != rProperties.end())
+ {
+ const LayoutProperty::const_iterator aRefType = aRef->second.find(rConstraint.mnRefType);
+ if (aRefType != aRef->second.end())
+ rProperties[rConstraint.msForName][rConstraint.mnType]
+ = aRefType->second * rConstraint.mfFactor;
+ else
+ {
+ // Values are never in EMU, while oox::drawingml::Shape position and size are always in
+ // EMU.
+ double fUnitFactor = 0;
+ if (isFontUnit(rConstraint.mnRefType))
+ // Points -> EMU.
+ fUnitFactor = EMU_PER_PT;
+ else
+ // Millimeters -> EMU.
+ fUnitFactor = EMU_PER_HMM * 100;
+ rProperties[rConstraint.msForName][rConstraint.mnType]
+ = rConstraint.mfValue * fUnitFactor;
+ }
+ }
+}
+}
+
void AlgAtom::layoutShape( const ShapePtr& rShape,
const std::vector<Constraint>& rConstraints )
{
@@ -443,6 +479,11 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
LayoutProperty& rParent = aProperties[""];
sal_Int32 nParentXOffset = 0;
+
+ // Track min/max vertical positions, so we can center everything at the end, if needed.
+ sal_Int32 nVertMin = std::numeric_limits<sal_Int32>::max();
+ sal_Int32 nVertMax = 0;
+
if (mfAspectRatio != 1.0)
{
rParent[XML_w] = rShape->getSize().Width;
@@ -467,31 +508,74 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
for (const auto & rConstr : rConstraints)
{
- const LayoutPropertyMap::const_iterator aRef = aProperties.find(rConstr.msRefForName);
- if (aRef != aProperties.end())
+ // Apply direct constraints for all layout nodes.
+ ApplyConstraintToLayout(rConstr, aProperties);
+ }
+
+ for (auto& aCurrShape : rShape->getChildren())
+ {
+ // Apply constraints from the current layout node for this child shape.
+ // Previous child shapes may have changed aProperties.
+ for (const auto& rConstr : rConstraints)
{
- const LayoutProperty::const_iterator aRefType = aRef->second.find(rConstr.mnRefType);
- if (aRefType != aRef->second.end())
- aProperties[rConstr.msForName][rConstr.mnType] = aRefType->second * rConstr.mfFactor;
- else
+ if (rConstr.msForName != aCurrShape->getInternalName())
{
- // Values are never in EMU, while oox::drawingml::Shape
- // position and size are always in EMU.
- double fUnitFactor = 0;
- if (isFontUnit(rConstr.mnRefType))
- // Points -> EMU.
- fUnitFactor = EMU_PER_PT;
- else
- // Millimeters -> EMU.
- fUnitFactor = EMU_PER_HMM * 100;
- aProperties[rConstr.msForName][rConstr.mnType]
- = rConstr.mfValue * fUnitFactor;
+ continue;
+ }
+
+ ApplyConstraintToLayout(rConstr, aProperties);
+ }
+
+ // Apply constraints from the child layout node for this child shape.
+ // This builds on top of the own parent state + the state of previous shapes in the
+ // same composite algorithm.
+ const LayoutNode& rLayoutNode = getLayoutNode();
+ for (const auto& pDirectChild : rLayoutNode.getChildren())
+ {
+ auto pLayoutNode = dynamic_cast<LayoutNode*>(pDirectChild.get());
+ if (!pLayoutNode)
+ {
+ continue;
+ }
+
+ if (pLayoutNode->getName() != aCurrShape->getInternalName())
+ {
+ continue;
+ }
+
+ for (const auto& pChild : pLayoutNode->getChildren())
+ {
+ auto pConstraintAtom = dynamic_cast<ConstraintAtom*>(pChild.get());
+ if (!pConstraintAtom)
+ {
+ continue;
+ }
+
+ const Constraint& rConstraint = pConstraintAtom->getConstraint();
+ if (!rConstraint.msForName.isEmpty())
+ {
+ continue;
+ }
+
+ if (!rConstraint.msRefForName.isEmpty())
+ {
+ continue;
+ }
+
+ // Either an absolute value or a factor of a property.
+ if (rConstraint.mfValue == 0.0 && rConstraint.mnRefType == XML_none)
+ {
+ continue;
+ }
+
+ Constraint aConstraint(rConstraint);
+ aConstraint.msForName = pLayoutNode->getName();
+ aConstraint.msRefForName = pLayoutNode->getName();
+
+ ApplyConstraintToLayout(aConstraint, aProperties);
}
}
- }
- for (auto & aCurrShape : rShape->getChildren())
- {
awt::Size aSize = rShape->getSize();
awt::Point aPos(0, 0);
@@ -535,6 +619,24 @@ void AlgAtom::layoutShape( const ShapePtr& rShape,
aCurrShape->setSize(aSize);
aCurrShape->setChildSize(aSize);
aCurrShape->setPosition(aPos);
+
+ nVertMin = std::min(aPos.Y, nVertMin);
+ nVertMax = std::max(aPos.Y + aSize.Height, nVertMax);
+ }
+
+ // See if all vertical space is used or we have to center the content.
+ if (nVertMin >= 0 && nVertMax <= rParent[XML_h])
+ {
+ sal_Int32 nDiff = rParent[XML_h] - (nVertMax - nVertMin);
+ if (nDiff > 0)
+ {
+ for (auto& aCurrShape : rShape->getChildren())
+ {
+ awt::Point aPosition = aCurrShape->getPosition();
+ aPosition.Y += nDiff / 2;
+ aCurrShape->setPosition(aPosition);
+ }
+ }
}
break;
}
@@ -1275,7 +1377,7 @@ void LayoutNode::accept( LayoutAtomVisitor& rVisitor )
rVisitor.visit(*this);
}
-bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode ) const
+bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode, sal_Int32 nCurrIdx ) const
{
SAL_INFO(
"oox.drawingml",
@@ -1413,15 +1515,17 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const dgm::Point* pPresNode
const DiagramColorMap::const_iterator aColor = mrDgm.getColors().find(aStyleLabel);
if( aColor != mrDgm.getColors().end() )
{
+ // Take the nth color from the color list in case we are the nth shape in a
+ // <dgm:forEach> loop.
const DiagramColor& rColor=aColor->second;
- if( rColor.maFillColor.isUsed() )
- rShape->getShapeStyleRefs()[XML_fillRef].maPhClr = rColor.maFillColor;
- if( rColor.maLineColor.isUsed() )
- rShape->getShapeStyleRefs()[XML_lnRef].maPhClr = rColor.maLineColor;
- if( rColor.maEffectColor.isUsed() )
- rShape->getShapeStyleRefs()[XML_effectRef].maPhClr = rColor.maEffectColor;
- if( rColor.maTextFillColor.isUsed() )
- rShape->getShapeStyleRefs()[XML_fontRef].maPhClr = rColor.maTextFillColor;
+ if( !rColor.maFillColors.empty() )
+ rShape->getShapeStyleRefs()[XML_fillRef].maPhClr = DiagramColor::getColorByIndex(rColor.maFillColors, nCurrIdx);
+ if( !rColor.maLineColors.empty() )
+ rShape->getShapeStyleRefs()[XML_lnRef].maPhClr = DiagramColor::getColorByIndex(rColor.maLineColors, nCurrIdx);
+ if( !rColor.maEffectColors.empty() )
+ rShape->getShapeStyleRefs()[XML_effectRef].maPhClr = DiagramColor::getColorByIndex(rColor.maEffectColors, nCurrIdx);
+ if( !rColor.maTextFillColors.empty() )
+ rShape->getShapeStyleRefs()[XML_fontRef].maPhClr = DiagramColor::getColorByIndex(rColor.maTextFillColors, nCurrIdx);
}
}
diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.hxx b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
index 91028971473e..2e4551642389 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
@@ -260,7 +260,8 @@ public:
{ mpNodeShapes.push_back(pShape); }
bool setupShape( const ShapePtr& rShape,
- const dgm::Point* pPresNode ) const;
+ const dgm::Point* pPresNode,
+ sal_Int32 nCurrIdx ) const;
const LayoutNode* getParentLayoutNode() const;
diff --git a/oox/source/drawingml/diagram/layoutatomvisitors.cxx b/oox/source/drawingml/diagram/layoutatomvisitors.cxx
index 4bfadc3affe8..c616ca3a9010 100644
--- a/oox/source/drawingml/diagram/layoutatomvisitors.cxx
+++ b/oox/source/drawingml/diagram/layoutatomvisitors.cxx
@@ -73,7 +73,7 @@ void ShapeCreationVisitor::visit(LayoutNode& rAtom)
{
// reuse existing shape
ShapePtr pShape = rAtom.getExistingShape();
- if (rAtom.setupShape(pShape, pNewNode))
+ if (rAtom.setupShape(pShape, pNewNode, mnCurrIdx))
{
pShape->setInternalName(rAtom.getName());
rAtom.addNodeShape(pShape);
@@ -92,7 +92,7 @@ void ShapeCreationVisitor::visit(LayoutNode& rAtom)
"oox.drawingml",
"processing shape type " << (pShape->getCustomShapeProperties()->getShapePresetType()));
- if (rAtom.setupShape(pShape, pNewNode))
+ if (rAtom.setupShape(pShape, pNewNode, mnCurrIdx))
{
pShape->setInternalName(rAtom.getName());
pCurrParent->addChild(pShape);
--
2.26.1
Reference in New Issue View Git Blame Copy Permalink