summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMiklos Vajna <vmiklos@collabora.com>2020-05-22 17:58:22 +0200
committerMiklos Vajna <vmiklos@collabora.com>2020-05-27 16:23:47 +0200
commitd7161f2b8e65185fc0c98e7f3e5e00b0fc87bdab (patch)
tree1bf139344663f2608e86bec07d8ca681003c22e2
parent14ef4875b3b4ccacd4846d564de08d368e9fabbd (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 Change-Id: I1c5c4f82e621f1110ef06b0490ff79f82f60f214
-rw-r--r--oox/inc/drawingml/colorchoicecontext.hxx15
-rw-r--r--oox/source/drawingml/colorchoicecontext.cxx25
-rw-r--r--oox/source/drawingml/diagram/diagram.cxx17
-rw-r--r--oox/source/drawingml/diagram/diagram.hxx16
-rw-r--r--oox/source/drawingml/diagram/diagramfragmenthandler.cxx15
-rw-r--r--oox/source/drawingml/diagram/diagramlayoutatoms.cxx20
-rw-r--r--oox/source/drawingml/diagram/diagramlayoutatoms.hxx3
-rw-r--r--oox/source/drawingml/diagram/layoutatomvisitors.cxx4
-rw-r--r--sd/qa/unit/data/pptx/fill-color-list.pptxbin0 -> 41044 bytes
-rw-r--r--sd/qa/unit/import-tests-smartart.cxx19
10 files changed, 105 insertions, 29 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..0832bb1bfe7b 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -1275,7 +1275,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 +1413,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);
diff --git a/sd/qa/unit/data/pptx/fill-color-list.pptx b/sd/qa/unit/data/pptx/fill-color-list.pptx
new file mode 100644
index 000000000000..341233ad5f78
--- /dev/null
+++ b/sd/qa/unit/data/pptx/fill-color-list.pptx
Binary files differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx b/sd/qa/unit/import-tests-smartart.cxx
index d14c4bc6950a..273056c868d7 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -105,6 +105,7 @@ public:
void testRecursion();
void testDataFollow();
void testOrgChart2();
+ void testFillColorList();
CPPUNIT_TEST_SUITE(SdImportTestSmartArt);
@@ -149,6 +150,7 @@ public:
CPPUNIT_TEST(testRecursion);
CPPUNIT_TEST(testDataFollow);
CPPUNIT_TEST(testOrgChart2);
+ CPPUNIT_TEST(testFillColorList);
CPPUNIT_TEST_SUITE_END();
};
@@ -1454,6 +1456,23 @@ void SdImportTestSmartArt::testOrgChart2()
xDocShRef->DoClose();
}
+void SdImportTestSmartArt::testFillColorList()
+{
+ sd::DrawDocShellRef xDocShRef
+ = loadURL(m_directories.getURLFromSrc("/sd/qa/unit/data/pptx/fill-color-list.pptx"), PPTX);
+ uno::Reference<drawing::XShape> xGroup(getShapeFromPage(0, 0, xDocShRef), uno::UNO_QUERY);
+ uno::Reference<drawing::XShape> xShape = getChildShape(getChildShape(xGroup, 1), 0);
+ uno::Reference<beans::XPropertySet> xPropertySet(xShape, uno::UNO_QUERY_THROW);
+ sal_Int32 nFillColor = 0;
+ xPropertySet->getPropertyValue("FillColor") >>= nFillColor;
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 12603469 (0xc0504d)
+ // - Actual : 16225862 (0xf79646)
+ // i.e. the background of the "A" shape was orange-ish, rather than red-ish.
+ CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0xC0504D), nFillColor);
+ xDocShRef->DoClose();
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(SdImportTestSmartArt);
CPPUNIT_PLUGIN_IMPLEMENT();