diff --git a/include/svx/diagram/datamodel_svx.hxx b/include/svx/diagram/datamodel_svx.hxx index 352eae2104d0..d2990e000f61 100644 --- a/include/svx/diagram/datamodel_svx.hxx +++ b/include/svx/diagram/datamodel_svx.hxx @@ -154,7 +154,6 @@ typedef std::shared_ptr< DiagramDataState > DiagramDataStatePtr; class SVXCORE_DLLPUBLIC DiagramData_svx { public: - typedef std::map< OUString, Point* > PointNameMap; typedef std::map< OUString, std::vector< Point* > > PointsNameMap; typedef std::map< OUString, const Connection* > ConnectionNameMap; @@ -193,7 +192,6 @@ public: Points& getPoints() { return maPoints; } const Points& getPoints() const { return maPoints; } StringMap& getPresOfNameMap() { return maPresOfNameMap; } - PointNameMap& getPointNameMap() { return maPointNameMap; } PointsNameMap& getPointsPresNameMap() { return maPointsPresNameMap; } ::std::vector& getExtDrawings() { return maExtDrawings; } const Point* getRootPoint() const; @@ -217,6 +215,7 @@ public: css::uno::Reference getMasterXShapeForPoint(const Point& rPoint) const; OUString getTextForPoint(const Point& rPoint) const; css::uno::Reference getXShapeByModelID(std::u16string_view rModelID) const; + const Point* getPointByModelID(std::u16string_view rModelID) const; protected: // helpers @@ -246,7 +245,6 @@ protected: css::uno::Reference< css::xml::dom::XDocument > mxThemeDocument; // temporary processing data, partially deleted when using build() - PointNameMap maPointNameMap; PointsNameMap maPointsPresNameMap; ConnectionNameMap maConnectionNameMap; StringMap maPresOfNameMap; diff --git a/include/svx/svdobj.hxx b/include/svx/svdobj.hxx index 4fd5a4773499..dcf53c3d357c 100644 --- a/include/svx/svdobj.hxx +++ b/include/svx/svdobj.hxx @@ -234,6 +234,9 @@ public: const OUString& getDiagramDataModelID() const; private: + // check if a ObjectName is set and it is in DiagramModelID syntax + bool ObjectNameIsDiagramModelID(); + friend class SdrObjListIter; friend class SdrObjList; friend class SdrVirtObj; diff --git a/oox/source/drawingml/diagram/diagram.cxx b/oox/source/drawingml/diagram/diagram.cxx index dc78b6d46511..c6d33ee4df8b 100644 --- a/oox/source/drawingml/diagram/diagram.cxx +++ b/oox/source/drawingml/diagram/diagram.cxx @@ -146,7 +146,12 @@ void SmartArtDiagram::createShapeHierarchyFromModel( const ShapePtr & pParentSha pBackground->setSubType(XML_rect); pBackground->getCustomShapeProperties()->setShapePresetType(XML_rect); pBackground->setSize(pParentShape->getSize()); - pBackground->getFillProperties() = *mpData->getBackgroundShapeFillProperties(); + if (mpData->getBackgroundShapeFillProperties()) + pBackground->getFillProperties() = *mpData->getBackgroundShapeFillProperties(); + // MoveProtect/SizeProtect..? Keep for now, but mnay be removed + // when IA needs change - the Diagram will be a closed GroupObject. + // If it gets 'broken' (un-grouped) the BGShape will keep that attributes, + // despite main reason to break that Diagram is probably to edit it. pBackground->setLocked(true); // create and set ModelID for BackgroundShape to allow later association diff --git a/oox/source/drawingml/diagram/diagramhelper_oox.cxx b/oox/source/drawingml/diagram/diagramhelper_oox.cxx index 4f3ad43d84fe..8c1aa2036d0e 100644 --- a/oox/source/drawingml/diagram/diagramhelper_oox.cxx +++ b/oox/source/drawingml/diagram/diagramhelper_oox.cxx @@ -143,6 +143,14 @@ void DiagramHelper_oox::reLayout() } } + // reset temporary buffered ModelData association lists & rebuild them + // and the Diagram DataModel + mpDiagramPtr->getData()->buildDiagramDataModel(true); + + // also reset temporary buffered layout data - that might + // still refer to changed oox::Shape data + mpDiagramPtr->getLayout()->getPresPointShapeMap().clear(); + // Re-create the oox::Shapes for the diagram content mpDiagramPtr->createShapeHierarchyFromModel(pShapePtr, true); diff --git a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx index d70c59cb1ece..1e1a17c14299 100644 --- a/oox/source/drawingml/diagram/diagramlayoutatoms.cxx +++ b/oox/source/drawingml/diagram/diagramlayoutatoms.cxx @@ -909,12 +909,10 @@ bool ConditionAtom::getDecision(const svx::diagram::Point* pPresPoint) const // point, ask the parent. OUString aParent = navigate(mrLayoutNode, svx::diagram::TypeConstant::XML_presParOf, pPresPoint->msModelId, /*bSourceToDestination*/ false); - DiagramData_oox::PointNameMap& rPointNameMap - = mrLayoutNode.getDiagram().getData()->getPointNameMap(); - auto it = rPointNameMap.find(aParent); - if (it != rPointNameMap.end()) + const svx::diagram::Point* it(mrLayoutNode.getDiagram().getData()->getPointByModelID(aParent)); + if (nullptr != it) { - const svx::diagram::Point* pParent = it->second; + const svx::diagram::Point* pParent = it; if (pParent->moHierarchyBranch.has_value()) nHierarchyBranch = pParent->moHierarchyBranch.value(); } @@ -1077,8 +1075,6 @@ bool HasCustomText(const ShapePtr& rShape, LayoutNode& rLayoutNode) = rLayoutNode.getDiagram().getLayout()->getPresPointShapeMap(); const DiagramData_oox::StringMap& rPresOfNameMap = rLayoutNode.getDiagram().getData()->getPresOfNameMap(); - const DiagramData_oox::PointNameMap& rPointNameMap - = rLayoutNode.getDiagram().getData()->getPointNameMap(); // Get the first presentation node of the shape. const svx::diagram::Point* pPresNode = nullptr; for (const auto& rPair : rPresPointShapeMap) @@ -1090,7 +1086,7 @@ bool HasCustomText(const ShapePtr& rShape, LayoutNode& rLayoutNode) } } // Get the first data node of the presentation node. - svx::diagram::Point* pDataNode = nullptr; + const svx::diagram::Point* pDataNode = nullptr; if (pPresNode) { auto itPresToData = rPresOfNameMap.find(pPresNode->msModelId); @@ -1099,10 +1095,10 @@ bool HasCustomText(const ShapePtr& rShape, LayoutNode& rLayoutNode) for (const auto& rPair : itPresToData->second) { const DiagramData_oox::SourceIdAndDepth& rItem = rPair.second; - auto it = rPointNameMap.find(rItem.msSourceId); - if (it != rPointNameMap.end()) + const svx::diagram::Point* it(rLayoutNode.getDiagram().getData()->getPointByModelID(rItem.msSourceId)); + if (nullptr != it) { - pDataNode = it->second; + pDataNode = it; break; } } @@ -1887,23 +1883,22 @@ bool LayoutNode::setupShape( const ShapePtr& rShape, const svx::diagram::Point* for (const auto& rPair : aNodeName->second) { const DiagramData_oox::SourceIdAndDepth& rItem = rPair.second; - DiagramData_oox::PointNameMap& rMap = mrDgm.getData()->getPointNameMap(); // pPresNode is the presentation node of the aDataNode2 data node. - DiagramData_oox::PointNameMap::const_iterator aDataNode2 = rMap.find(rItem.msSourceId); - if (aDataNode2 == rMap.end()) + const svx::diagram::Point* pDataNode2(mrDgm.getData()->getPointByModelID(rItem.msSourceId)); + if (nullptr == pDataNode2) { //busted, skip it continue; } - Shape* pDataNode2Shape(mrDgm.getData()->getOrCreateAssociatedShape(*aDataNode2->second)); + Shape* pDataNode2Shape(mrDgm.getData()->getOrCreateAssociatedShape(*pDataNode2)); if (nullptr == pDataNode2Shape) { //busted, skip it continue; } - rShape->setDataNodeType(aDataNode2->second->mnXMLType); + rShape->setDataNodeType(pDataNode2->mnXMLType); if (rItem.mnDepth == 0) { diff --git a/sd/qa/unit/data/xml/n819614_0.xml b/sd/qa/unit/data/xml/n819614_0.xml index 665ce867cedf..fe53b6ccc3a6 100644 --- a/sd/qa/unit/data/xml/n819614_0.xml +++ b/sd/qa/unit/data/xml/n819614_0.xml @@ -16,7 +16,7 @@ - + @@ -79,7 +79,7 @@ - + @@ -134,7 +134,7 @@ - + @@ -189,7 +189,7 @@ - + @@ -244,7 +244,7 @@ - + @@ -303,7 +303,7 @@ - + @@ -358,7 +358,7 @@ - + @@ -413,7 +413,7 @@ - + @@ -468,7 +468,7 @@ - + @@ -523,7 +523,7 @@ - + @@ -578,7 +578,7 @@ - + @@ -633,7 +633,7 @@ - + @@ -692,7 +692,7 @@ - + @@ -747,7 +747,7 @@ - + @@ -802,7 +802,7 @@ - + @@ -857,7 +857,7 @@ - + @@ -916,7 +916,7 @@ - + @@ -971,7 +971,7 @@ - + @@ -1026,7 +1026,7 @@ - + @@ -1081,7 +1081,7 @@ - + @@ -1136,7 +1136,7 @@ - + @@ -1191,7 +1191,7 @@ - + @@ -1246,7 +1246,7 @@ - + @@ -1301,7 +1301,7 @@ - + @@ -1356,7 +1356,7 @@ - + @@ -1411,7 +1411,7 @@ - + @@ -1466,7 +1466,7 @@ - + @@ -1525,7 +1525,7 @@ - + @@ -1580,7 +1580,7 @@ - + @@ -1635,7 +1635,7 @@ - + @@ -1690,7 +1690,7 @@ - + @@ -1745,7 +1745,7 @@ - + @@ -1800,7 +1800,7 @@ - + @@ -1855,7 +1855,7 @@ - + @@ -1914,7 +1914,7 @@ - + @@ -1969,7 +1969,7 @@ - + @@ -2024,7 +2024,7 @@ - + @@ -2079,7 +2079,7 @@ - + @@ -2134,7 +2134,7 @@ - + @@ -2189,7 +2189,7 @@ - + @@ -2244,7 +2244,7 @@ - + @@ -2299,7 +2299,7 @@ - + @@ -2354,7 +2354,7 @@ - + @@ -2409,7 +2409,7 @@ - + @@ -2464,7 +2464,7 @@ - + @@ -2523,7 +2523,7 @@ - + @@ -2578,7 +2578,7 @@ - + @@ -2633,7 +2633,7 @@ - + @@ -2688,7 +2688,7 @@ - + @@ -2743,7 +2743,7 @@ - + @@ -2798,7 +2798,7 @@ - + @@ -2853,7 +2853,7 @@ - + @@ -2908,7 +2908,7 @@ - + @@ -2963,7 +2963,7 @@ - + @@ -3018,7 +3018,7 @@ - + @@ -3073,7 +3073,7 @@ - + @@ -3132,7 +3132,7 @@ - + @@ -3195,7 +3195,7 @@ - + @@ -3258,7 +3258,7 @@ - + @@ -3321,7 +3321,7 @@ - + @@ -3384,7 +3384,7 @@ - + @@ -3447,7 +3447,7 @@ - + @@ -3510,7 +3510,7 @@ - + @@ -3573,7 +3573,7 @@ - + @@ -3636,7 +3636,7 @@ - + @@ -3699,7 +3699,7 @@ - + @@ -3762,7 +3762,7 @@ - + @@ -3825,7 +3825,7 @@ - + @@ -3888,7 +3888,7 @@ - + @@ -3951,7 +3951,7 @@ - + @@ -4014,7 +4014,7 @@ - + @@ -4077,7 +4077,7 @@ - + @@ -4140,7 +4140,7 @@ - + @@ -4203,7 +4203,7 @@ - + @@ -4266,7 +4266,7 @@ - + @@ -4329,7 +4329,7 @@ - + @@ -4392,7 +4392,7 @@ - + @@ -4455,7 +4455,7 @@ - + @@ -4518,7 +4518,7 @@ - + @@ -4581,7 +4581,7 @@ - + @@ -4644,7 +4644,7 @@ - + @@ -4707,7 +4707,7 @@ - + @@ -4770,7 +4770,7 @@ - + @@ -4833,7 +4833,7 @@ - + @@ -4896,7 +4896,7 @@ - + @@ -4959,7 +4959,7 @@ - + @@ -5022,7 +5022,7 @@ - + @@ -5085,7 +5085,7 @@ - + @@ -5148,7 +5148,7 @@ - + @@ -5211,7 +5211,7 @@ - + @@ -5274,7 +5274,7 @@ - + @@ -5337,7 +5337,7 @@ - + @@ -5400,7 +5400,7 @@ - + @@ -5463,7 +5463,7 @@ - + @@ -5526,7 +5526,7 @@ - + @@ -5589,7 +5589,7 @@ - + @@ -5652,7 +5652,7 @@ - + @@ -5715,7 +5715,7 @@ - + @@ -5778,7 +5778,7 @@ - + @@ -5841,7 +5841,7 @@ - + @@ -5904,7 +5904,7 @@ - + @@ -5967,7 +5967,7 @@ - + @@ -6030,7 +6030,7 @@ - + @@ -6093,7 +6093,7 @@ - + @@ -6156,7 +6156,7 @@ - + @@ -6219,7 +6219,7 @@ - + @@ -6282,7 +6282,7 @@ - + @@ -6345,7 +6345,7 @@ - + @@ -6408,7 +6408,7 @@ - + @@ -6471,7 +6471,7 @@ - + @@ -6534,7 +6534,7 @@ - + @@ -6597,7 +6597,7 @@ - + diff --git a/svx/source/diagram/datamodel_svx.cxx b/svx/source/diagram/datamodel_svx.cxx index a4fba56d989a..3f5caf5d8b40 100644 --- a/svx/source/diagram/datamodel_svx.cxx +++ b/svx/source/diagram/datamodel_svx.cxx @@ -204,7 +204,6 @@ DiagramData_svx::DiagramData_svx() , maConnections() , maPoints() , mxThemeDocument() -, maPointNameMap() , maPointsPresNameMap() , maConnectionNameMap() , maPresOfNameMap() @@ -215,14 +214,16 @@ DiagramData_svx::DiagramData_svx() DiagramData_svx::DiagramData_svx(DiagramData_svx const& rSource) : mxRootShape() , maExtDrawings() +// copy all Connections , maConnections(rSource.maConnections) +// copy all Points , maPoints(rSource.maPoints) , mxThemeDocument() -, maPointNameMap() , maPointsPresNameMap() , maConnectionNameMap() , maPresOfNameMap() -, msBackgroundShapeModelID() +// copy BackgroundShapeModelID, the BGSgape will need to be identified on reLayout +, msBackgroundShapeModelID(rSource.msBackgroundShapeModelID) { } @@ -359,9 +360,9 @@ void DiagramData_svx::getDiagramChildrenString( { if (rCxn.mnSourceOrder >= static_cast(aChildren.size())) aChildren.resize(rCxn.mnSourceOrder + 1); - const auto pChild = maPointNameMap.find(rCxn.msDestId); - if (pChild != maPointNameMap.end()) - aChildren[rCxn.mnSourceOrder] = pChild->second; + const Point* pChild(getPointByModelID(rCxn.msDestId)); + if (nullptr != pChild) + aChildren[rCxn.mnSourceOrder] = pChild; } for (auto pChild : aChildren) @@ -389,6 +390,15 @@ uno::Reference DiagramData_svx::getXShapeByModelID(std::u16stri return xRetval; } +const Point* DiagramData_svx::getPointByModelID(std::u16string_view rModelID) const +{ + for (const auto& rCandidate : getPoints()) + if (rModelID == rCandidate.msModelId) + return &rCandidate; + + return nullptr; +} + uno::Reference DiagramData_svx::getMasterXShapeForPoint(const Point& rPoint) const { for (auto& rCandidate : getPoints()) @@ -426,12 +436,12 @@ std::vector> DiagramData_svx::getDiagramChildren(c { if (rCxn.mnSourceOrder >= static_cast(aChildren.size())) aChildren.resize(rCxn.mnSourceOrder + 1); - const auto pChild = maPointNameMap.find(rCxn.msDestId); - if (pChild != maPointNameMap.end()) + const Point* pChild(getPointByModelID(rCxn.msDestId)); + if (nullptr != pChild) { - const OUString aText(getTextForPoint(*pChild->second)); + const OUString aText(getTextForPoint(*pChild)); aChildren[rCxn.mnSourceOrder] = std::make_pair( - pChild->second->msModelId, + pChild->msModelId, aText); } } @@ -479,7 +489,7 @@ std::pair DiagramData_svx::addDiagramNode() if (!sPresSibling.isEmpty()) { // no idea where to get these values from, so copy from previous sibling - const svx::diagram::Point* pSiblingPoint = maPointNameMap[sPresSibling]; + const svx::diagram::Point* pSiblingPoint(getPointByModelID(sPresSibling)); aPresPoint.msPresentationLayoutName = pSiblingPoint->msPresentationLayoutName; aPresPoint.msPresentationLayoutStyleLabel = pSiblingPoint->msPresentationLayoutStyleLabel; aPresPoint.mnLayoutStyleIndex = pSiblingPoint->mnLayoutStyleIndex; @@ -563,7 +573,6 @@ static sal_Int32 calcDepth( std::u16string_view rNodeName, void DiagramData_svx::buildDiagramDataModel(bool /*bClearOoxShapes*/) { // build name-object maps - maPointNameMap.clear(); maPointsPresNameMap.clear(); maConnectionNameMap.clear(); maPresOfNameMap.clear(); @@ -628,9 +637,7 @@ void DiagramData_svx::buildDiagramDataModel(bool /*bClearOoxShapes*/) #endif } - const bool bInserted1 = getPointNameMap().insert( - std::make_pair(point.msModelId,&point)).second; - + const bool bInserted1(nullptr != getPointByModelID(point.msModelId)); SAL_WARN_IF(!bInserted1, "oox.drawingml", "DiagramData_svx::build(): non-unique point model id"); if( !point.msPresentationLayoutName.isEmpty() ) diff --git a/svx/source/svdraw/svdobj.cxx b/svx/source/svdraw/svdobj.cxx index 625af3a3a664..4d49f442e302 100644 --- a/svx/source/svdraw/svdobj.cxx +++ b/svx/source/svdraw/svdobj.cxx @@ -202,17 +202,45 @@ struct SdrObject::Impl meRelativeHeightRelation(text::RelOrientation::PAGE_FRAME) {} }; +bool SdrObject::ObjectNameIsDiagramModelID() +{ + if (!m_pPlusData) + return false; + + const OUString& rStr(m_pPlusData->aObjName); + if (rStr.isEmpty()) + return false; + + // length is fixed (38 chars) + if (38 != rStr.getLength()) + return false; + + // starts and ends with curly braces + const sal_Unicode* pStr(rStr.getStr()); + if (pStr[0] != '{' || pStr[37] != '}') + return false; + + // has 'minus' signs at positions 9, 14, 19 and 24 + const char minus('-'); + if (pStr[9] != minus || pStr[14] != minus || pStr[19] != minus || pStr[24] != minus) + return false; + + // we could also check for upper-case hex alphabet (0..9, A..F) for + // the rest of characters, but should be safe enough already + return true; +} + void SdrObject::setDiagramDataModelID(const OUString& rID) { if (!m_pPlusData) ImpForcePlusData(); - m_pPlusData->aObjTitle = rID; + m_pPlusData->aObjName = rID; } const OUString& SdrObject::getDiagramDataModelID() const { if(m_pPlusData) - return m_pPlusData->aObjTitle; + return m_pPlusData->aObjName; return EMPTY_OUSTRING; } @@ -3228,6 +3256,10 @@ void SdrObject::MakeNameUnique(std::unordered_set& rNameSet) if (GetName().isEmpty()) return; + if (ObjectNameIsDiagramModelID()) + // do not make name unique for DiagramModelID's + return; + OUString sName(GetName().trim()); OUString sRootName(sName); diff --git a/test/source/diff/diff.cxx b/test/source/diff/diff.cxx index fa801ab6c67f..36697879b08b 100644 --- a/test/source/diff/diff.cxx +++ b/test/source/diff/diff.cxx @@ -368,13 +368,38 @@ bool XMLDiff::compareAttributes(xmlNodePtr node1, xmlNodePtr node2) } else { + bool bIgnoreGUIDStrings(false); -#if USE_CPPUNIT - cppunitAssertEqual(node1, attr1, val1, val2); -#else if(!xmlStrEqual( val1, val2 )) - return false; + { + // there might be GuidString's used, these may get created + // at every export or internal usage, so these cannot be + // compared to a saved pattern. To identify, use their + // format. An example is: + // "{662825F2-FD0A-477C-81F6-D00196403932}" + // We can use here their specific length of 38, that + // should be good for now. If needed this may be extended + // to check for + // - curly braces at start/end + // - minus ('-') at positions 9, 14, 19 and 24 + // - upper-case hex alphabet (0..9, A..F) for the characters + // also see ObjectNameIsDiagramModelID() which is similar + const int len1(xmlStrlen(val1)); + const int len2(xmlStrlen(val2)); + + if (len1 == len2 && 38 == len1) + bIgnoreGUIDStrings = true; + } + + if (!bIgnoreGUIDStrings) + { +#if USE_CPPUNIT + cppunitAssertEqual(node1, attr1, val1, val2); +#else + if(!xmlStrEqual( val1, val2 )) + return false; #endif + } } xmlFree(val1);