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.

Change-Id: I1c5c4f82e621f1110ef06b0490ff79f82f60f214
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/94697
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

8 files changed, 86 insertions, 29 deletions

diff --git a/oox/inc/drawingml/colorchoicecontext.hxx b/oox/inc/drawingml/colorchoicecontext.hxx
index 433c94b3addb..093c832cb1e6 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 0a4c627833ee..fc93b460a8de 100644
--- a/oox/source/drawingml/colorchoicecontext.cxx
+++ b/oox/source/drawingml/colorchoicecontext.cxx
@@ -148,6 +148,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 oox::drawingml
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/oox/source/drawingml/diagram/diagram.cxx b/oox/source/drawingml/diagram/diagram.cxx
index efe03fd6ff2f..5a4fef99cfcb 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 e3964c76f86c..e7f5cc68378b 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 9a7a8f3de760..de00181069a4 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 71e9f1970aa6..800c9b1f64af 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx
@@ -1274,7 +1274,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",
@@ -1412,15 +1412,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 4a24e5071301..65bfe5975a67 100644
--- a/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
+++ b/oox/source/drawingml/diagram/diagramlayoutatoms.hxx
@@ -259,7 +259,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 4a2bf97a034f..52ae12b2a592 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);