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 ++++++----
oox/source/drawingml/diagram/diagramfragmenthandler.cxx | 15 +++------
oox/source/drawingml/diagram/diagramlayoutatoms.cxx | 20 +++++++-----
oox/source/drawingml/diagram/diagramlayoutatoms.hxx | 3 +
oox/source/drawingml/diagram/layoutatomvisitors.cxx | 4 +-
sd/qa/unit/data/pptx/fill-color-list.pptx |binary
sd/qa/unit/import-tests-smartart.cxx | 19 ++++++++++++
10 files changed, 105 insertions(+), 29 deletions(-)
New commits:
commit 12bea6c897822964ad4705418da54411cb15749e
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri May 22 17:58:22 2020 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri May 22 18:46:16 2020 +0200
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 <vmik...@collabora.com>
Tested-by: Jenkins
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);
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
Binary files /dev/null and b/sd/qa/unit/data/pptx/fill-color-list.pptx differ
diff --git a/sd/qa/unit/import-tests-smartart.cxx
b/sd/qa/unit/import-tests-smartart.cxx
index e3b71703293f..76659c0cf680 100644
--- a/sd/qa/unit/import-tests-smartart.cxx
+++ b/sd/qa/unit/import-tests-smartart.cxx
@@ -108,6 +108,7 @@ public:
void testDataFollow();
void testOrgChart2();
void testTdf131553();
+ void testFillColorList();
CPPUNIT_TEST_SUITE(SdImportTestSmartArt);
@@ -153,6 +154,7 @@ public:
CPPUNIT_TEST(testDataFollow);
CPPUNIT_TEST(testOrgChart2);
CPPUNIT_TEST(testTdf131553);
+ CPPUNIT_TEST(testFillColorList);
CPPUNIT_TEST_SUITE_END();
};
@@ -1472,6 +1474,23 @@ void SdImportTestSmartArt::testTdf131553()
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();
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
