From 98eae52c511b35aef84882e155425d909bec58ea Mon Sep 17 00:00:00 2001
From: Johannes Junggeburth <johannes.josef.junggeburth@cern.ch>
Date: Thu, 15 Feb 2024 18:34:50 +0100
Subject: [PATCH] Use GeoDeDuplicator in the Shape processors

---
 .../GeoModelHelpers/GeoShapeSorter.h          |  9 ---
 .../GeoModelHelpers/src/GeoShapeSorter.cxx    | 10 +--
 .../GeoModelHelpers/src/GeoShapeUtils.cxx     | 11 ++-
 .../GeoModelXml/GeoModelXml/shape/AddPlane.h  |  4 +-
 .../GeoModelXml/GeoModelXml/shape/MakeBox.h   |  6 +-
 .../GeoModelXml/GeoModelXml/shape/MakeCons.h  |  9 ++-
 .../GeoModelXml/shape/MakeEllipticalTube.h    |  5 +-
 .../GeoModelXml/shape/MakeGenericTrap.h       |  5 +-
 .../GeoModelXml/shape/MakeIntersection.h      |  6 +-
 .../GeoModelXml/GeoModelXml/shape/MakePara.h  |  7 +-
 .../GeoModelXml/GeoModelXml/shape/MakePcon.h  |  7 +-
 .../GeoModelXml/GeoModelXml/shape/MakePgon.h  | 10 ++-
 .../GeoModelXml/shape/MakeSimplePolygonBrep.h |  4 +-
 .../GeoModelXml/shape/MakeSubtraction.h       |  4 +-
 .../GeoModelXml/GeoModelXml/shape/MakeTorus.h |  4 +-
 .../GeoModelXml/GeoModelXml/shape/MakeTrap.h  |  2 +-
 .../GeoModelXml/GeoModelXml/shape/MakeTrd.h   |  2 +-
 .../GeoModelXml/GeoModelXml/shape/MakeTube.h  |  6 +-
 .../GeoModelXml/GeoModelXml/shape/MakeTubs.h  |  3 +-
 .../GeoModelXml/shape/MakeTwistedTrap.h       |  4 +-
 .../GeoModelXml/GeoModelXml/shape/MakeUnion.h |  3 +-
 .../GeoModelXML/GeoModelXml/src/MakeBox.cxx   | 23 +++---
 .../GeoModelXML/GeoModelXml/src/MakeCons.cxx  | 15 ++--
 .../GeoModelXml/src/MakeEllipticalTube.cxx    | 14 ++--
 .../GeoModelXml/src/MakeGenericTrap.cxx       | 31 ++++---
 .../GeoModelXml/src/MakeIntersection.cxx      | 80 ++++++++++---------
 .../GeoModelXML/GeoModelXml/src/MakePara.cxx  | 14 ++--
 .../GeoModelXML/GeoModelXml/src/MakePcon.cxx  | 36 ++++-----
 .../GeoModelXML/GeoModelXml/src/MakePgon.cxx  | 24 +++---
 .../GeoModelXml/src/MakeRotation.cxx          |  6 +-
 .../GeoModelXml/src/MakeSimplePolygonBrep.cxx | 30 ++++---
 .../GeoModelXml/src/MakeSubtraction.cxx       | 78 +++++++++---------
 .../GeoModelXML/GeoModelXml/src/MakeTorus.cxx | 21 +++--
 .../GeoModelXml/src/MakeTransformation.cxx    |  6 +-
 .../GeoModelXml/src/MakeTranslation.cxx       |  6 +-
 .../GeoModelXML/GeoModelXml/src/MakeTrap.cxx  | 20 ++---
 .../GeoModelXML/GeoModelXml/src/MakeTrd.cxx   | 13 ++-
 .../GeoModelXML/GeoModelXml/src/MakeTube.cxx  | 13 ++-
 .../GeoModelXML/GeoModelXml/src/MakeTubs.cxx  | 13 ++-
 .../GeoModelXml/src/MakeTwistedTrap.cxx       | 19 ++---
 .../GeoModelXML/GeoModelXml/src/MakeUnion.cxx | 80 +++++++++----------
 .../GeoModelXml/src/MulticopyProcessor.cxx    |  2 +-
 .../GeoModelXml/src/ReplicaRPhiProcessor.cxx  |  4 +-
 .../GeoModelXml/src/ReplicaXProcessor.cxx     |  4 +-
 .../GeoModelXml/src/ReplicaXYArrays.cxx       |  4 +-
 .../GeoModelXml/src/ReplicaYProcessor.cxx     |  4 +-
 .../GeoModelXml/src/ReplicaZProcessor.cxx     |  4 +-
 47 files changed, 348 insertions(+), 337 deletions(-)

diff --git a/GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoShapeSorter.h b/GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoShapeSorter.h
index a18006c36..a7214d78f 100644
--- a/GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoShapeSorter.h
+++ b/GeoModelCore/GeoModelHelpers/GeoModelHelpers/GeoShapeSorter.h
@@ -28,15 +28,6 @@ struct GeoShapeSorter {
 
 };
 
-struct GeoComposedShapeSorter {
-    template<class ShapeType1, class ShapeType2>
-    bool operator()(const GeoIntrusivePtr<ShapeType1>& a, 
-                    const GeoIntrusivePtr<ShapeType2>& b) const {
-            return (*this)(a.get(), b.get());
-    }
-    bool operator()(const GeoShape* a, const GeoShape* b) const;
-
-};
 /// @brief 
 /// @tparam ShapeType 
 template<class ShapeType>
diff --git a/GeoModelCore/GeoModelHelpers/src/GeoShapeSorter.cxx b/GeoModelCore/GeoModelHelpers/src/GeoShapeSorter.cxx
index 5d1aee762..b103d3ef9 100644
--- a/GeoModelCore/GeoModelHelpers/src/GeoShapeSorter.cxx
+++ b/GeoModelCore/GeoModelHelpers/src/GeoShapeSorter.cxx
@@ -83,6 +83,7 @@ int GeoShapeSorter::compare(const GeoShape* objA, const GeoShape* objB) const {
     
     std::pair<const GeoShape*, const GeoShape*> shapeOpsA{getOps(objA)};
     std::pair<const GeoShape*, const GeoShape*> shapeOpsB{getOps(objB)};
+    
     CALL_SORTER(shapeOpsA.first, shapeOpsB.first);
     CALL_SORTER(shapeOpsA.second, shapeOpsB.second);
   } 
@@ -156,7 +157,8 @@ int GeoShapeSorter::compare(const GeoShape* objA, const GeoShape* objB) const {
       CHECK_PROPERTY(trapA, trapB, getZHalfLength);
       CHECK_PROPERTY(trapA, trapB, getVertices().size);
       for (size_t v = 0; v < trapA->getVertices().size(); ++v) {
-          if (transCmp(trapA->getVertices()[v], trapB->getVertices()[v])) return true;
+          const int compare = transCmp.compare(trapA->getVertices()[v], trapB->getVertices()[v]);
+          if (compare) return compare;
       }
    } else if (typeID == GeoPcon::getClassTypeID()) {
      const GeoPcon* pconA = dynamic_cast<const GeoPcon*>(objA);
@@ -226,12 +228,6 @@ int GeoShapeSorter::compare(const GeoShape* objA, const GeoShape* objB) const {
   return 0;
 }
 
-bool GeoComposedShapeSorter::operator()(const GeoShape* a, const GeoShape* b) const {
-    const unsigned int  aComposed{countComposedShapes(a)};
-    const unsigned int  bComposed{countComposedShapes(b)};
-    if (aComposed != bComposed) return aComposed < bComposed;
-    return a->typeID() < b->typeID();
-}
 #undef CHECK_PROPERTY
 #undef CHECK_VEC_PROPERTY
 #undef COMPARE_GEOVEC
diff --git a/GeoModelCore/GeoModelHelpers/src/GeoShapeUtils.cxx b/GeoModelCore/GeoModelHelpers/src/GeoShapeUtils.cxx
index 2117315dd..b0a6f8d6f 100644
--- a/GeoModelCore/GeoModelHelpers/src/GeoShapeUtils.cxx
+++ b/GeoModelCore/GeoModelHelpers/src/GeoShapeUtils.cxx
@@ -21,6 +21,7 @@
 #include "GeoModelKernel/GeoPara.h"
 #include "GeoModelKernel/GeoTorus.h"
 #include "GeoModelKernel/GeoSimplePolygonBrep.h"
+#include "GeoModelKernel/GeoGenericTrap.h"
 
 #include "GeoModelKernel/Units.h"
 
@@ -33,7 +34,7 @@ std::pair<const GeoShape* , const GeoShape*> getOps(const GeoShape* composed) {
     } else if (composed->typeID() == GeoShapeSubtraction::getClassTypeID()) {
         const GeoShapeSubtraction* shapeSubtract = dynamic_cast<const GeoShapeSubtraction*>(composed);
         return std::make_pair(shapeSubtract->getOpA(), shapeSubtract->getOpB());
-    } else if (composed->typeID() == GeoShapeUnion::getClassTypeID()) {
+    } else if (composed->typeID() == GeoShapeIntersection::getClassTypeID()) {
         const GeoShapeIntersection* shapeIntersect = dynamic_cast<const GeoShapeIntersection*>(composed);
         return std::make_pair(shapeIntersect->getOpA(), shapeIntersect->getOpB());
     } else if (composed->typeID() == GeoShapeShift::getClassTypeID()) {
@@ -82,7 +83,7 @@ std::vector<const GeoShape*> getBooleanComponents(const GeoShape* booleanShape)
 std::string printGeoShape(const GeoShape* shape) {
     std::stringstream ostr{};
     constexpr double toDeg{1./GeoModelKernelUnits::deg};
-    ostr<<shape->type()<<" ("<<shape<<") ";
+    ostr<<shape->type()<<" ("<<shape<<"/"<<shape->refCount()<<") ";
     const int typeID = shape->typeID();
     if (typeID == GeoShapeUnion::getClassTypeID()) {
         const GeoShapeUnion* shapeUnion = dynamic_cast<const GeoShapeUnion*>(shape);
@@ -138,6 +139,12 @@ std::string printGeoShape(const GeoShape* shape) {
       ostr<<"Torus R="<<torus->getRTor()<<", ";
       ostr<<"sPhi="<<torus->getSPhi()*toDeg<<", ";
       ostr<<"dPhi="<<torus->getDPhi()*toDeg;
+   } else if (typeID == GeoGenericTrap::getClassTypeID()) {
+        const GeoGenericTrap* trap = dynamic_cast<const GeoGenericTrap*>(shape);
+        ostr<<"half Z: "<<trap->getZHalfLength()<<" n Vertices: "<<trap->getVertices().size()<<std::endl;
+        for ( const GeoTrf::Vector2D& vec : trap->getVertices()) {
+            ostr<<"  **** "<<GeoTrf::toString(vec)<<std::endl;
+        }
    }
     return ostr.str();
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/AddPlane.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/AddPlane.h
index 5bdf2cf62..b604566e8 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/AddPlane.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/AddPlane.h
@@ -14,8 +14,8 @@ class GmxUtil;
 
 class AddPlane {
 public:
-    AddPlane() {};
-    ~AddPlane() {};
+    AddPlane() = default;
+    ~AddPlane() = default;
     void process(const xercesc::DOMElement *element, double &zPlane, double &rMinPlane, double &rMaxPlane);
     GmxUtil* gmxUtil=nullptr;
 };
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeBox.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeBox.h
index 968bc4806..220b875c6 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeBox.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeBox.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -12,10 +12,12 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+#include "GeoModelKernel/GeoBox.h"
 class MakeBox: public Element2GeoItem {
 public:
-    MakeBox();
+    MakeBox() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_BOX_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeCons.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeCons.h
index 7de87d2d0..7fb7d0ebf 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeCons.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeCons.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -13,9 +13,10 @@
 #include "GeoModelXml/Element2GeoItem.h"
 
 class MakeCons: public Element2GeoItem {
-public:
-    MakeCons();
-    virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+  public:
+      MakeCons() = default;
+      virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_CONS_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeEllipticalTube.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeEllipticalTube.h
index 11d894ca7..9609bc2e8 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeEllipticalTube.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeEllipticalTube.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 #ifndef GEO_MODEL_XML_MAKE_ELLIPTICAL_TUBE_H
@@ -10,8 +10,9 @@
 
 class MakeEllipticalTube: public Element2GeoItem {
 public:
-    MakeEllipticalTube();
+    MakeEllipticalTube() = default;;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_ELLIPTICAL_TUBE_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeGenericTrap.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeGenericTrap.h
index 83b011534..4e33e9e08 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeGenericTrap.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeGenericTrap.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -12,9 +12,10 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
 class MakeGenericTrap: public Element2GeoItem {
 public:
-    MakeGenericTrap();
+    MakeGenericTrap() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
 };
 
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeIntersection.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeIntersection.h
index c8d7b71d9..f3ebfdf3a 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeIntersection.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeIntersection.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -12,10 +12,12 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
 class MakeIntersection: public Element2GeoItem {
 public:
-    MakeIntersection();
+    MakeIntersection() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_INTERSECTION_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePara.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePara.h
index c340ca705..3cf210eb0 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePara.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePara.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -13,9 +13,10 @@
 #include "GeoModelXml/Element2GeoItem.h"
 
 class MakePara: public Element2GeoItem {
-public:
-    MakePara();
+  public:
+    MakePara() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+   
 };
 
 #endif // GEO_MODEL_XML_MAKE_PARA_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePcon.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePcon.h
index 190badf08..623383296 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePcon.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePcon.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -12,10 +12,13 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
+
 class MakePcon: public Element2GeoItem {
 public:
-    MakePcon();
+    MakePcon() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_PCON_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePgon.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePgon.h
index cc48599aa..2b36bb38a 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePgon.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakePgon.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -12,10 +12,12 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
 class MakePgon: public Element2GeoItem {
-public:
-    MakePgon();
-    virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+  public:
+      MakePgon() = default;
+      virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_PGON_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeSimplePolygonBrep.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeSimplePolygonBrep.h
index 92382bef9..9a4f2897b 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeSimplePolygonBrep.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeSimplePolygonBrep.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 #include <xercesc/util/XercesDefs.hpp>
 //
@@ -13,7 +13,7 @@
 
 class MakeSimplePolygonBrep: public Element2GeoItem {
 public:
-    MakeSimplePolygonBrep();
+    MakeSimplePolygonBrep() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
 };
 
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeSubtraction.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeSubtraction.h
index 359b43037..cb9ac7391 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeSubtraction.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeSubtraction.h
@@ -12,10 +12,12 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
 class MakeSubtraction: public Element2GeoItem {
 public:
-    MakeSubtraction();
+    MakeSubtraction() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_SUBTRACTION_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTorus.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTorus.h
index 5d269cb7e..2425818b0 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTorus.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTorus.h
@@ -9,10 +9,12 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
 class MakeTorus: public Element2GeoItem {
 public:
-    MakeTorus();
+    MakeTorus() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_TORUS_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTrap.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTrap.h
index 283281459..3c9d4435d 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTrap.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTrap.h
@@ -14,7 +14,7 @@
 
 class MakeTrap: public Element2GeoItem {
 public:
-    MakeTrap();
+    MakeTrap() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
 };
 
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTrd.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTrd.h
index ae05777cd..cf758804b 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTrd.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTrd.h
@@ -14,7 +14,7 @@
 
 class MakeTrd: public Element2GeoItem {
 public:
-    MakeTrd();
+    MakeTrd() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
 };
 
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTube.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTube.h
index bdf81d3c2..eebad477c 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTube.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTube.h
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -12,10 +12,12 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
 class MakeTube: public Element2GeoItem {
 public:
-    MakeTube();
+    MakeTube() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_TUBE_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTubs.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTubs.h
index a75b17263..7d3520c18 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTubs.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTubs.h
@@ -12,9 +12,10 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
 class MakeTubs: public Element2GeoItem {
 public:
-    MakeTubs();
+    MakeTubs() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
 };
 
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTwistedTrap.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTwistedTrap.h
index 984ae165a..54a88ac91 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTwistedTrap.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeTwistedTrap.h
@@ -8,10 +8,12 @@
 
 #include "GeoModelXml/Element2GeoItem.h"
 
+
 class MakeTwistedTrap: public Element2GeoItem {
 public:
-    MakeTwistedTrap();
+    MakeTwistedTrap() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_TWISTED_TRAP_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeUnion.h b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeUnion.h
index cb5f32fc1..2606df773 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeUnion.h
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/GeoModelXml/shape/MakeUnion.h
@@ -14,8 +14,9 @@
 
 class MakeUnion: public Element2GeoItem {
 public:
-    MakeUnion();
+    MakeUnion() = default;
     virtual RCBase * make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const override;
+
 };
 
 #endif // GEO_MODEL_XML_MAKE_UNION_H
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeBox.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeBox.cxx
index bd8cf3e8b..854c5cd44 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeBox.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeBox.cxx
@@ -10,21 +10,20 @@
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
 
+#include <array>
 using namespace xercesc;
 
-MakeBox::MakeBox() {}
-
 RCBase * MakeBox::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 3; 
-char const *parName[nParams] = {"xhalflength", "yhalflength", "zhalflength"};
-double p[nParams];
-char *toRelease;
+  constexpr int nParams = 3; 
+  static const std::array<std::string, nParams> parName{"xhalflength", "yhalflength", "zhalflength"};
+  std::array<double, nParams> p{};
+  char *toRelease;
 
-    for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
-        p[i] = gmxUtil.evaluate(toRelease);
-        XMLString::release(&toRelease);
-    }
+  for (int i = 0; i < nParams; ++i) {
+      toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
+      p[i] = gmxUtil.evaluate(toRelease);
+      XMLString::release(&toRelease);
+  }
 
-    return new GeoBox(p[0], p[1], p[2]);
+  return const_cast<GeoShape*>(cacheShape(new GeoBox(p[0], p[1], p[2])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeCons.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeCons.cxx
index 092261f6d..799b735d4 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeCons.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeCons.cxx
@@ -10,21 +10,22 @@
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
 
+#include <array>
+
 using namespace xercesc;
 
-MakeCons::MakeCons() {}
 
 RCBase * MakeCons::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 7; 
-char const *parName[nParams] = {"rmin1", "rmin2", "rmax1", "rmax2", "dz", "sphi", "dphi"};
-double p[nParams];
-char *toRelease;
+    constexpr int nParams = 7; 
+    static const std::array<std::string, nParams> parName{"rmin1", "rmin2", "rmax1", "rmax2", "dz", "sphi", "dphi"};
+    std::array<double, nParams> p{};
+    char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    return new GeoCons(p[0], p[1], p[2], p[3], p[4], p[5], p[6]);
+    return const_cast<GeoShape*>(cacheShape(new GeoCons(p[0], p[1], p[2], p[3], p[4], p[5], p[6])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeEllipticalTube.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeEllipticalTube.cxx
index 6b874b879..178f4b022 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeEllipticalTube.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeEllipticalTube.cxx
@@ -8,22 +8,22 @@
 #include "GeoModelKernel/GeoEllipticalTube.h"
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
+#include <array>
 
 using namespace xercesc;
 
-MakeEllipticalTube::MakeEllipticalTube() {}
 
 RCBase * MakeEllipticalTube::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 3; 
-char const *parName[nParams] = {"xhalflength", "yhalflength", "zhalflength"};
-double p[nParams];
-char *toRelease;
+    constexpr int nParams = 3; 
+    static const std::array<std::string, nParams> parName{"xhalflength", "yhalflength", "zhalflength"};
+    std::array<double, nParams> p{};
+    char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    return new GeoEllipticalTube(p[0], p[1], p[2]);
+    return  const_cast<GeoShape*>(cacheShape(new GeoEllipticalTube(p[0], p[1], p[2])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeGenericTrap.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeGenericTrap.cxx
index 4bf1df752..5f9e1723c 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeGenericTrap.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeGenericTrap.cxx
@@ -16,33 +16,30 @@
 
 using namespace xercesc;
 
-MakeGenericTrap::MakeGenericTrap() {}
 
 RCBase * MakeGenericTrap::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 17; 
-char const *parName[nParams] = {"x0", "y0", "x1", "y1", "x2", "y2", "x3", "y3", 
-                                "x4", "y4", "x5", "y5", "x6", "y6", "x7", "y7", "zhalflength"};
-double p;
-char *toRelease;
-std::vector<GeoTwoVector> vertices;
-double x;
-double y;
+    constexpr int nParams = 17; 
+    std::array<std::string, 17> parName{"x0", "y0", "x1", "y1", "x2", "y2", "x3", "y3", 
+                                        "x4", "y4", "x5", "y5", "x6", "y6", "x7", "y7", "zhalflength"};
+    char *toRelease;
+    std::vector<GeoTwoVector> vertices;
+    double x{0.}, y{0.}, p{0.};
 
     for (int i = 0; i < (nParams - 1) / 2; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[2 * i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[2 * i].data())));
         x = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[2 * i + 1])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[2 * i + 1].data())));
         y = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
-        vertices.push_back(GeoTwoVector(x, y));
+        vertices.emplace_back(x, y);
     }
-//
-//    z-half-length
-//
-    toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[16])));
+    //
+    //    z-half-length
+    //
+    toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[16].data())));
     p = gmxUtil.evaluate(toRelease);
     XMLString::release(&toRelease);
 
-    return new GeoGenericTrap(p, vertices);
+    return const_cast<GeoShape*>(cacheShape(new GeoGenericTrap(p, vertices)).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeIntersection.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeIntersection.cxx
index e787255e4..8348b666f 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeIntersection.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeIntersection.cxx
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 // Automatically generated code from /home/hessey/prog/gmx2geo/makeshape
@@ -9,60 +9,64 @@
 #include "OutputDirector.h"
 #include <xercesc/dom/DOM.hpp>
 #include "GeoModelKernel/GeoDefinitions.h"
-#include "GeoModelKernel/RCBase.h"
-#include "GeoModelKernel/GeoShape.h"
-#include "GeoModelKernel/GeoTransform.h"
+#include "GeoModelKernel/GeoShapeIntersection.h"
+#include "GeoModelKernel/GeoShapeShift.h"
 
+#include "GeoModelHelpers/TransformSorter.h"
+#include "GeoModelHelpers/GeoShapeUtils.h"
+#include "GeoModelHelpers/throwExcept.h"
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
 
 using namespace xercesc;
 using namespace std;
 
-MakeIntersection::MakeIntersection() {}
-
 RCBase * MakeIntersection::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
 // 
 //    Process child elements; first is first shaperef; then transform; then second shaperef.
 //
-    const GeoShape *first = 0;
-    const GeoShape *second = 0;
+    GeoIntrusivePtr<const GeoShape> first{}, second{};
     GeoTrf::Transform3D hepXf=GeoTrf::Transform3D::Identity(); 
     int elementIndex = 0;
     for (DOMNode *child = element->getFirstChild(); child != 0; child = child->getNextSibling()) {
         if (child->getNodeType() == DOMNode::ELEMENT_NODE) { // Skips text nodes
-	  switch (elementIndex) {
-	  case 0: { // First element is first shaperef
-	    first = static_cast<const GeoShape *>(gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
-	    break;
-	  }
-	  case 1:  { // Second element is transformation or transformationref
-	    char *toRelease = XMLString::transcode(child->getNodeName());
-	    string nodeName(toRelease);
-	    XMLString::release(&toRelease);
-	    const GeoTransform *geoXf = (nodeName == "transformation")
-	      ? static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformation.process(dynamic_cast<DOMElement *>(child), gmxUtil))
-	      : static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformationref.process(dynamic_cast<DOMElement *>(child), gmxUtil));
-	    hepXf = geoXf->getTransform();
-	    break;
-	  }
-	  case 2: { // Third element is second shaperef
-	    second = static_cast<const GeoShape *>( gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
-	    break;
-	  }
-	  default: // More than 3 elements?
-	    msglog << MSG::FATAL << "MakeIntersection: Incompatible DTD? got more than 3 child elements\n";
-	  }
-	  elementIndex++;
+      switch (elementIndex) {
+      case 0: { // First element is first shaperef
+        first.reset(static_cast<const GeoShape *>(gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil)));
+        break;
+      }
+      case 1:  { // Second element is transformation or transformationref
+        char *toRelease = XMLString::transcode(child->getNodeName());
+        string nodeName(toRelease);
+        XMLString::release(&toRelease);
+        const GeoTransform *geoXf = (nodeName == "transformation")
+          ? static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformation.process(dynamic_cast<DOMElement *>(child), gmxUtil))
+          : static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformationref.process(dynamic_cast<DOMElement *>(child), gmxUtil));
+        hepXf = geoXf->getTransform();
+        break;
+      }
+      case 2: { // Third element is second shaperef
+        second.reset(static_cast<const GeoShape *>(gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil)));
+        break;
+      }
+      default: // More than 3 elements?
+        THROW_EXCEPTION("MakeIntersection: Incompatible DTD? got more than 3 child elements\n");
+      }
+      elementIndex++;
         }
     }
 
-    if (!first || !second) std::abort();
-
-    // FIXME: intersect() returns a new'd object --- should really be
-    // returning a `unique_ptr<GeoShapeIntersection>' not a
-    // `const GeoShapeIntersection'
-    GeoShapeIntersection *temp = const_cast<GeoShapeIntersection*>(&(first->intersect(*(GeoShape *) &(*(second) << hepXf))));
+    if (!first || !second) {
+        THROW_EXCEPTION("Not both shapes were defined");
+    } 
+    GeoIntrusivePtr<GeoShapeIntersection> isect{};
+    static const GeoTrf::TransformSorter trfSorter{};
+    if (trfSorter.compare(hepXf, GeoTrf::Transform3D::Identity())) {
+        isect = new GeoShapeIntersection(first, new GeoShapeShift(second, hepXf));
+    } else {
+        isect = new GeoShapeIntersection(first, second);
 
-    return (RCBase *) temp;
+    }
+    std::cout<<isect->refCount()<<std::endl;
+    return const_cast<GeoShape*>(cacheShape(isect).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePara.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePara.cxx
index c68e18761..edaf9c774 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePara.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePara.cxx
@@ -10,21 +10,21 @@
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
 
+#include <array>
 using namespace xercesc;
 
-MakePara::MakePara() {}
 
 RCBase * MakePara::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 6; 
-char const *parName[nParams] = {"xhalflength", "yhalflength", "zhalflength", "alpha", "theta", "phi"};
-double p[nParams];
-char *toRelease;
+    constexpr int nParams = 6; 
+    static const std::array<std::string, nParams> parName{"xhalflength", "yhalflength", "zhalflength", "alpha", "theta", "phi"};
+    std::array<double, nParams> p{};
+    char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    return new GeoPara(p[0], p[1], p[2], p[3], p[4], p[5]);
+    return const_cast<GeoShape*>(cacheShape(new GeoPara(p[0], p[1], p[2], p[3], p[4], p[5])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePcon.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePcon.cxx
index 4b98472a3..ec658425a 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePcon.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePcon.cxx
@@ -1,39 +1,37 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 // Automatically generated code from /home/hessey/prog/gmx2geo/makeshape
 // But then edited for AddPlane stuff
 #include "GeoModelXml/shape/MakePcon.h"
 #include <xercesc/dom/DOM.hpp>
-#include "GeoModelKernel/RCBase.h"
 #include "GeoModelKernel/GeoPcon.h"
+#include "GeoModelHelpers/GeoShapeUtils.h"
+#include "GeoModelHelpers/throwExcept.h"
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
 
+#include <array>
 using namespace xercesc;
 
-MakePcon::MakePcon() {}
 
 RCBase * MakePcon::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 2; 
-char const *parName[nParams] = {"sphi", "dphi"};
-double p[nParams];
-char *toRelease;
-
+    constexpr int nParams = 2; 
+    static const std::array<std::string, nParams> parName{"sphi", "dphi"};
+    std::array<double, nParams> p{};
+    char *toRelease;
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    GeoPcon *pcon = new GeoPcon(p[0], p[1]);
-//
-//    Add planes
-//
-    double zPlane = 0.;
-    double rMinPlane = 0.;
-    double rMaxPlane = 0.;
+    GeoIntrusivePtr<GeoPcon> pcon{new GeoPcon(p[0], p[1])};
+    //
+    //    Add planes
+    //
+    double zPlane{0.}, rMinPlane{0.}, rMaxPlane{0.};
     for (DOMNode *child = element->getFirstChild(); child != 0; child = child->getNextSibling()) {
         if (child->getNodeType() == DOMNode::ELEMENT_NODE) {
             toRelease = XMLString::transcode(child->getNodeName());
@@ -45,6 +43,8 @@ char *toRelease;
             }
         }
     }
-
-    return pcon;
+    if (!pcon->isValid()) {
+        THROW_EXCEPTION("Invalid Pcon defined "<<printGeoShape(pcon));
+    }
+    return const_cast<GeoShape*>(cacheShape(pcon).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePgon.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePgon.cxx
index a73b002c7..36d277c87 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePgon.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakePgon.cxx
@@ -5,35 +5,32 @@
 // Automatically generated code from /home/hessey/prog/gmx2geo/makeshape
 // But then edited for AddPlane stuff
 #include "GeoModelXml/shape/MakePgon.h"
-#include <xercesc/dom/DOM.hpp>
-#include "GeoModelKernel/RCBase.h"
 #include "GeoModelKernel/GeoPgon.h"
+#include <xercesc/dom/DOM.hpp>
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
 
+#include <array>
 using namespace xercesc;
 
-MakePgon::MakePgon() {}
 
 RCBase * MakePgon::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 3; 
-char const *parName[nParams] = {"sphi", "dphi", "nsides"};
-double p[nParams];
-char *toRelease;
+    constexpr int nParams = 3; 
+    static const std::array<std::string, nParams> parName {"sphi", "dphi", "nsides"};
+    std::array<double, nParams> p{};
+    char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    GeoPgon *pgon = new GeoPgon(p[0], p[1], p[2]);
+    GeoIntrusivePtr<GeoPgon> pgon{new GeoPgon(p[0], p[1], p[2])};
 //
 //    Add planes
 //
-    double zPlane = 0.;
-    double rMinPlane = 0.;
-    double rMaxPlane = 0.;
+    double zPlane{0.}, rMinPlane{0.}, rMaxPlane{0.};
     for (DOMNode *child = element->getFirstChild(); child != 0; child = child->getNextSibling()) {
         if (child->getNodeType() == DOMNode::ELEMENT_NODE) {
             toRelease = XMLString::transcode(child->getNodeName());
@@ -45,6 +42,5 @@ char *toRelease;
             }
         }
     }
-
-    return pgon;
+    return const_cast<GeoShape*>(cacheShape(pgon).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeRotation.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeRotation.cxx
index db97f3f01..5473cbfaf 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeRotation.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeRotation.cxx
@@ -18,12 +18,12 @@ using namespace xercesc;
 GeoTrf::Rotation3D MakeRotation::getTransform(const DOMElement *rotation, GmxUtil &gmxUtil) {
 
 const int nParams = 4; 
-char const *parName[nParams] = {"angle", "xcos", "ycos", "zcos"};
-double p[nParams];
+static const std::array<std::string, nParams> parName {"angle", "xcos", "ycos", "zcos"};
+std::array<double, nParams> p{};
 char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(rotation->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(rotation->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeSimplePolygonBrep.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeSimplePolygonBrep.cxx
index 80943dc73..4c6f22bc7 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeSimplePolygonBrep.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeSimplePolygonBrep.cxx
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 #include <string>
 #include <vector>
@@ -7,37 +7,35 @@
 #include <stdexcept>
 
 #include "GeoModelXml/shape/MakeSimplePolygonBrep.h"
-#include <xercesc/dom/DOM.hpp>
-#include "GeoModelKernel/RCBase.h"
 #include "GeoModelKernel/GeoSimplePolygonBrep.h"
+#include <xercesc/dom/DOM.hpp>
+#include "GeoModelHelpers/throwExcept.h"
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
 using namespace xercesc;
 
-MakeSimplePolygonBrep::MakeSimplePolygonBrep() {}
 
 RCBase * MakeSimplePolygonBrep::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 3; 
-char const *parName[nParams] = {"xpoints", "ypoints", "zhalflength"};
-double z;
-std::vector <double> x;
-std::vector <double> y;
+    constexpr int nParams = 3; 
+    static const std::array<std::string, nParams> parName {"xpoints", "ypoints", "zhalflength"};
+    double z{0.};
+    std::vector <double> x{}, y{};
 
-char *toRelease;
+    char *toRelease;
 
-    toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[0])));
+    toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[0].data())));
     std::string xPoints(toRelease);
     XMLString::release(&toRelease);
 
-    toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[1])));
+    toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[1].data())));
     std::string yPoints(toRelease);
     XMLString::release(&toRelease);
 
-    toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[2])));
+    toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[2].data())));
     z = gmxUtil.evaluate(toRelease);
     XMLString::release(&toRelease);
 
-    GeoSimplePolygonBrep * poly = new GeoSimplePolygonBrep(z);
+    GeoIntrusivePtr<GeoSimplePolygonBrep> poly{new GeoSimplePolygonBrep(z)};
 
     std::istringstream xSS(xPoints);
     while (!xSS.eof()) {
@@ -57,7 +55,7 @@ char *toRelease;
     int ny = y.size();
   
     if (nx < 3 || ny < 3 || nx != ny) {
-        throw std::runtime_error(std::string("MakeSimplePolygonBrep: Unequal number of x and y points, or less than 3\n\n") +
+        THROW_EXCEPTION("Unequal number of x and y points, or less than 3\n\n"<<
          "xpoints was:\n" + xPoints + "\nypoints was:\n" + yPoints + "\n\n");
     }
 
@@ -65,6 +63,6 @@ char *toRelease;
         poly->addVertex(x[i], y[i]);
     }
 
-    return poly;
+    return const_cast<GeoShape*>(cacheShape(poly).get());
 
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeSubtraction.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeSubtraction.cxx
index 1e16c5897..4ee9be38a 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeSubtraction.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeSubtraction.cxx
@@ -8,62 +8,64 @@
 #include "OutputDirector.h"
 #include <string>
 #include <xercesc/dom/DOM.hpp>
-//   #include <CLHEP/Geometry/Transform3D.h>
-#include "GeoModelKernel/GeoDefinitions.h"
-#include "GeoModelKernel/RCBase.h"
-#include "GeoModelKernel/GeoShape.h"
+#include "GeoModelKernel/GeoShapeSubtraction.h"
+#include "GeoModelKernel/GeoShapeShift.h"
 #include "GeoModelKernel/GeoTransform.h"
+#include "GeoModelHelpers/throwExcept.h"
 
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
-
+#include <array>
 using namespace xercesc;
 using namespace std;
 
-MakeSubtraction::MakeSubtraction() {}
 
 RCBase * MakeSubtraction::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
 // 
 //    Process child elements; first is first shaperef; then transformation; then second shaperef.
 //
-    const GeoShape *first = 0;
-    const GeoShape *second = 0;
+    GeoIntrusivePtr<const GeoShape> first{}, second{};
     GeoTrf::Transform3D hepXf=GeoTrf::Transform3D::Identity();
     int elementIndex = 0;
     for (DOMNode *child = element->getFirstChild(); child != 0; child = child->getNextSibling()) {
         if (child->getNodeType() == DOMNode::ELEMENT_NODE) { // Skips text nodes
-	  switch (elementIndex) {
-	  case 0: { // First element is first shaperef
-	    first = static_cast<const GeoShape *>(gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
-	    break;
-	  }
-	  case 1: { // Second element is transformation or transformationref
-	    char *toRelease = XMLString::transcode(child->getNodeName());
-	    string nodeName(toRelease);
-	    XMLString::release(&toRelease);
-	    const GeoTransform *geoXf = (nodeName == "transformation")
-	      ? static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformation.process(dynamic_cast<DOMElement *>(child), gmxUtil))
-	      : static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformationref.process(dynamic_cast<DOMElement *>(child), gmxUtil));
-	    hepXf = geoXf->getTransform();
-	    break;
-	  }
-	  case 2: { // Third element is second shaperef
-	    second = static_cast<const GeoShape *>( gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
-	    break;
-	  }
-	  default: // More than 3 elements?
-	    msglog << MSG::FATAL  << "MakeSubtraction: Incompatible DTD? got more than 3 child elements" << endmsg;
-	  }
-	  elementIndex++;
+      switch (elementIndex) {
+      case 0: { // First element is first shaperef
+        first = static_cast<const GeoShape *>(gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
+        break;
+      }
+      case 1: { // Second element is transformation or transformationref
+        char *toRelease = XMLString::transcode(child->getNodeName());
+        string nodeName(toRelease);
+        XMLString::release(&toRelease);
+        const GeoTransform *geoXf = (nodeName == "transformation")
+          ? static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformation.process(dynamic_cast<DOMElement *>(child), gmxUtil))
+          : static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformationref.process(dynamic_cast<DOMElement *>(child), gmxUtil));
+        hepXf = geoXf->getTransform();
+        break;
+      }
+      case 2: { // Third element is second shaperef
+        second = static_cast<const GeoShape *>( gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
+        break;
+      }
+      default: // More than 3 elements?
+        THROW_EXCEPTION("MakeSubtraction: Incompatible DTD? got more than 3 child elements");
+      }
+      elementIndex++;
         }
     }
 
-    if (!first || !second) std::abort();
-
-    // FIXME: subtract() returns a new'd object --- should really be
-    // returning a `unique_ptr<GeoShapeSubtraction>' not a
-    // `const GeoShapeSubtraction'
-    GeoShapeSubtraction *temp = const_cast<GeoShapeSubtraction*>(&(first->subtract(*(GeoShape *) &(*(second) << hepXf))));
+    if (!first || !second) {
+        THROW_EXCEPTION("Only one of the two subtraction operands was defined.");
+    }
 
-    return (RCBase *) temp;
+  
+    GeoIntrusivePtr<GeoShapeSubtraction> subtract{};
+    static const GeoTrf::TransformSorter sorter{};
+    if (sorter.compare(hepXf, GeoTrf::Transform3D::Identity())) {
+        subtract = new GeoShapeSubtraction(first, new GeoShapeShift(second, hepXf));
+    } else {
+        subtract = new GeoShapeSubtraction(first, second);
+    }
+    return const_cast<GeoShape*>(cacheShape(subtract).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTorus.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTorus.cxx
index 205a260e5..99bfa5dd6 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTorus.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTorus.cxx
@@ -13,19 +13,18 @@
 
 using namespace xercesc;
 
-MakeTorus::MakeTorus() {}
 
 RCBase * MakeTorus::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 5; 
-char const *parName[nParams] = {"rmin", "rmax", "rtor", "sphi", "dphi"};
-double p[nParams];
-char *toRelease;
+  constexpr int nParams = 5; 
+  static const std::array<std::string, nParams> parName {"rmin", "rmax", "rtor", "sphi", "dphi"};
+  std::array<double, nParams> p{};
+  char *toRelease;
 
-    for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
-        p[i] = gmxUtil.evaluate(toRelease);
-        XMLString::release(&toRelease);
-    }
+  for (int i = 0; i < nParams; ++i) {
+      toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
+      p[i] = gmxUtil.evaluate(toRelease);
+      XMLString::release(&toRelease);
+  }
 
-    return new GeoTorus(p[0], p[1], p[2], p[3], p[4]);
+  return  const_cast<GeoShape*>(cacheShape(new GeoTorus(p[0], p[1], p[2], p[3], p[4])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTransformation.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTransformation.cxx
index 5c1e1cf30..200c1d7be 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTransformation.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTransformation.cxx
@@ -55,7 +55,7 @@ char *name2release;
     if (alignable.compare(string("true")) == 0) {
         return (RCBase *) new GeoAlignableTransform(hepTransform);
     }
-    else {
-        return (RCBase *) new GeoTransform(hepTransform);
-    }
+    
+    return makeTransform(hepTransform);
+    
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTranslation.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTranslation.cxx
index 7068dd222..3d73786d4 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTranslation.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTranslation.cxx
@@ -19,12 +19,12 @@ using namespace xercesc;
 GeoTrf::Translate3D MakeTranslation::getTransform(const DOMElement *translation, GmxUtil &gmxUtil) {
 
 const int nParams = 3; 
-char const *parName[nParams] = {"x", "y", "z"};
-double p[nParams];
+static const std::array<std::string, nParams> parName {"x", "y", "z"};
+std::array<double, nParams> p{};
 char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(translation->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(translation->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTrap.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTrap.cxx
index 6e881629f..577f729d5 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTrap.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTrap.cxx
@@ -1,30 +1,30 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 // Automatically generated code from /home/hessey/prog/gmx2geo/makeshape
 #include "GeoModelXml/shape/MakeTrap.h"
 #include <xercesc/dom/DOM.hpp>
 #include "GeoModelKernel/RCBase.h"
-#include "GeoModelKernel/GeoTrap.h"
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
-
+#include "GeoModelKernel/GeoTrap.h"
 using namespace xercesc;
 
-MakeTrap::MakeTrap() {}
 
 RCBase * MakeTrap::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 11; 
-char const *parName[nParams] = {"zhalflength", "theta", "phi", "dydzn", "dxdyndzn", "dxdypdzn", "angleydzn", "dydzp", "dxdyndzp", "dxdypdzp", "angleydzp"};
-double p[nParams];
-char *toRelease;
+  constexpr int nParams = 11; 
+  static const std::array<std::string, nParams> parName {"zhalflength", "theta", "phi", "dydzn", "dxdyndzn", "dxdypdzn", "angleydzn", "dydzp", "dxdyndzp", "dxdypdzp", "angleydzp"};
+  std::array<double, nParams> p{};
+  char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    return new GeoTrap(p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7], p[8], p[9], p[10]);
+    return  const_cast<GeoShape*>(cacheShape(new GeoTrap(p[0], p[1], p[2], p[3], 
+                                                         p[4], p[5], p[6], p[7], 
+                                                         p[8], p[9], p[10])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTrd.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTrd.cxx
index 7b326216d..5d8d64ff1 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTrd.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTrd.cxx
@@ -12,19 +12,18 @@
 
 using namespace xercesc;
 
-MakeTrd::MakeTrd() {}
 
 RCBase * MakeTrd::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 5; 
-char const *parName[nParams] = {"xhalflength1", "xhalflength2", "yhalflength1", "yhalflength2", "zhalflength"};
-double p[nParams];
-char *toRelease;
+    constexpr int nParams = 5; 
+    static const std::array<std::string, nParams> parName {"xhalflength1", "xhalflength2", "yhalflength1", "yhalflength2", "zhalflength"};
+    std::array<double, nParams> p{};
+    char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    return new GeoTrd(p[0], p[1], p[2], p[3], p[4]);
+    return  const_cast<GeoShape*>(cacheShape(new GeoTrd(p[0], p[1], p[2], p[3], p[4])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTube.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTube.cxx
index e916cd98a..00806d89a 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTube.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTube.cxx
@@ -12,19 +12,18 @@
 
 using namespace xercesc;
 
-MakeTube::MakeTube() {}
 
 RCBase * MakeTube::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 3; 
-char const *parName[nParams] = {"rmin", "rmax", "zhalflength"};
-double p[nParams];
-char *toRelease;
+    constexpr int nParams = 3; 
+    static const std::array<std::string, nParams> parName {"rmin", "rmax", "zhalflength"};
+    std::array<double, nParams> p{};
+    char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    return new GeoTube(p[0], p[1], p[2]);
+    return const_cast<GeoShape*>(cacheShape(new GeoTube(p[0], p[1], p[2])).get());;
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTubs.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTubs.cxx
index f6c76a6a9..c8fe9cad9 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTubs.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTubs.cxx
@@ -12,19 +12,18 @@
 
 using namespace xercesc;
 
-MakeTubs::MakeTubs() {}
 
 RCBase * MakeTubs::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 5; 
-char const *parName[nParams] = {"rmin", "rmax", "zhalflength", "sphi", "dphi"};
-double p[nParams];
-char *toRelease;
+    constexpr int nParams = 5; 
+    static const std::array<std::string, nParams> parName {"rmin", "rmax", "zhalflength", "sphi", "dphi"};
+    std::array<double, nParams> p{};
+    char *toRelease;
 
     for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    return new GeoTubs(p[0], p[1], p[2], p[3], p[4]);
+    return  const_cast<GeoShape*>(cacheShape(new GeoTubs(p[0], p[1], p[2], p[3], p[4])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTwistedTrap.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTwistedTrap.cxx
index 786e506a8..ef1b47976 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTwistedTrap.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeTwistedTrap.cxx
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 /*
@@ -31,19 +31,20 @@
 
 using namespace xercesc;
 
-MakeTwistedTrap::MakeTwistedTrap() {}
 
 RCBase * MakeTwistedTrap::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
-const int nParams = 11; 
-char const *parName[nParams] = {"twist", "dz", "theta","phi","dy1","dx1","dx2","dy2","dx3","dx4","alpha"};
-double p[nParams];
-char *toRelease;
+  constexpr int nParams = 11; 
+  static const std::array<std::string, nParams> parName {"twist", "dz", "theta","phi","dy1","dx1","dx2","dy2","dx3","dx4","alpha"};
+  std::array<double, nParams> p{};
+  char *toRelease;
 
-    for (int i = 0; i < nParams; ++i) {
-        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i])));
+  for (int i = 0; i < nParams; ++i) {
+        toRelease = XMLString::transcode(element->getAttribute(XMLString::transcode(parName[i].data())));
         p[i] = gmxUtil.evaluate(toRelease);
         XMLString::release(&toRelease);
     }
 
-    return new GeoTwistedTrap(p[0], p[1], p[2],p[3], p[4], p[5],p[6], p[7], p[8],p[9], p[10]);
+    return const_cast<GeoShape*>(cacheShape(new GeoTwistedTrap(p[0], p[1], p[2], p[3], 
+                                                               p[4], p[5], p[6], p[7], 
+                                                               p[8], p[9], p[10])).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeUnion.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeUnion.cxx
index 2bcb0d530..fc949f0b2 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeUnion.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MakeUnion.cxx
@@ -1,67 +1,67 @@
 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2024 CERN for the benefit of the ATLAS collaboration
 */
 
 #include "GeoModelXml/shape/MakeUnion.h"
 #include <string>
 #include "OutputDirector.h"
 #include <xercesc/dom/DOM.hpp>
-//   #include <CLHEP/Geometry/Transform3D.h>
-#include "GeoModelKernel/RCBase.h"
-#include "GeoModelKernel/GeoShape.h"
-#include "GeoModelKernel/GeoTransform.h"
-#include "GeoModelKernel/GeoDefinitions.h"
-
+#include "GeoModelKernel/GeoShapeUnion.h"
+#include "GeoModelKernel/GeoShapeShift.h"
+#include "GeoModelHelpers/throwExcept.h"
 #include "xercesc/util/XMLString.hpp"
 #include "GeoModelXml/GmxUtil.h"
 
 using namespace xercesc;
 using namespace std;
 
-MakeUnion::MakeUnion() {}
 
 RCBase * MakeUnion::make(const xercesc::DOMElement *element, GmxUtil &gmxUtil) const {
 // 
 //    Process child elements; first is first shaperef; then transform; then second shaperef.
 //
-    const GeoShape *first = 0;
-    const GeoShape *second = 0;
+    GeoIntrusivePtr<const GeoShape> first{}, second{};
     GeoTrf::Transform3D hepXf=GeoTrf::Transform3D::Identity();
     int elementIndex = 0;
     for (DOMNode *child = element->getFirstChild(); child != 0; child = child->getNextSibling()) {
         if (child->getNodeType() == DOMNode::ELEMENT_NODE) { // Skips text nodes
-	  switch (elementIndex) {
-	  case 0: { // First element is first shaperef
-	    first = static_cast<GeoShape *>( gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
-	    break;
-	  }
-	  case 1: { // Second element is transformation or transformationref
-	    char *toRelease = XMLString::transcode(child->getNodeName());
-	    string nodeName(toRelease);
-	    XMLString::release(&toRelease);
-	    const GeoTransform *geoXf = (nodeName == "transformation")
-	      ? static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformation.process(dynamic_cast<DOMElement *>(child), gmxUtil))
-	      : static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformationref.process(dynamic_cast<DOMElement *>(child), gmxUtil));
-	    hepXf = geoXf->getTransform();
-	    break;
-	  }
-	  case 2: { // Third element is second shaperef
-	    second = static_cast<const GeoShape *>( gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
-	    break;
-	  }
-	  default: // More than 3 elements?
-	    msglog << MSG::FATAL << "MakeUnion: Incompatible DTD? got more than 3 child elements" << endmsg;
-	  }
-	  elementIndex++;
+      switch (elementIndex) {
+      case 0: { // First element is first shaperef
+        first = static_cast<GeoShape *>( gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
+        break;
+      }
+      case 1: { // Second element is transformation or transformationref
+        char *toRelease = XMLString::transcode(child->getNodeName());
+        string nodeName(toRelease);
+        XMLString::release(&toRelease);
+        const GeoTransform *geoXf = (nodeName == "transformation")
+          ? static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformation.process(dynamic_cast<DOMElement *>(child), gmxUtil))
+          : static_cast<const GeoTransform *>( gmxUtil.tagHandler.transformationref.process(dynamic_cast<DOMElement *>(child), gmxUtil));
+        hepXf = geoXf->getTransform();
+        break;
+      }
+      case 2: { // Third element is second shaperef
+        second = static_cast<const GeoShape *>( gmxUtil.tagHandler.shaperef.process(dynamic_cast<DOMElement *> (child), gmxUtil));
+        break;
+      }
+      default: // More than 3 elements?
+        THROW_EXCEPTION("MakeUnion: Incompatible DTD? got more than 3 child elements");
+      }
+      elementIndex++;
         }
     }
 
-    if (!first || !second) std::abort();
-
-    // FIXME: add() returns a new'd object --- should really be
-    // returning a `unique_ptr<GeoShapeUnion>' not a
-    // `const GeoShapeUnion'
-    GeoShapeUnion *temp = const_cast<GeoShapeUnion*>(&(first->add(*(GeoShape *) &(*(second) << hepXf))));
+    if (!first || !second) {
+		THROW_EXCEPTION("One of the two operands is not defined");
+    }
+    GeoIntrusivePtr<const GeoShapeUnion> unionShape{};
 
-    return (RCBase *) temp;
+   static const GeoTrf::TransformSorter sorter{};
+    if (sorter.compare(hepXf, GeoTrf::Transform3D::Identity())) {
+        unionShape = new GeoShapeUnion(first, new GeoShapeShift(second,hepXf));
+    } else {
+        unionShape = new GeoShapeUnion(first, second);
+    }
+ 
+    return  const_cast<GeoShape*>(cacheShape(unionShape).get());
 }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/MulticopyProcessor.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/MulticopyProcessor.cxx
index ff5ad62d3..c332ead28 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/MulticopyProcessor.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/MulticopyProcessor.cxx
@@ -151,7 +151,7 @@ DOMDocument *doc = element->getOwnerDocument();
             }
             GeoTrf::Transform3D hepXf=GeoTrf::Transform3D::Identity(); // Identity initially
             for (int i = 0; i < nCopies; ++i) {
-                xfList->push_back((GeoGraphNode *) new GeoTransform(hepXf));
+                xfList->push_back(makeTransform(hepXf));
                 hepXf = hepXf0 * hepXf;
             }
         }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaRPhiProcessor.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaRPhiProcessor.cxx
index 8a9596ba8..dc7601ee3 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaRPhiProcessor.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaRPhiProcessor.cxx
@@ -162,14 +162,14 @@ DOMDocument *doc = element->getOwnerDocument();
                 hepXf0 = geoAXf->getTransform();
             }
             else {
-                geoXf = new GeoTransform (hepXf0);
+                geoXf = makeTransform (hepXf0);
                 hepXf0 = geoXf->getTransform();
             }
 	    double angle=offsetPhi;
             GeoTrf::Transform3D hepXf=hepXf0; 
             for (int i = 0; i < nCopies; ++i) {
 	    	hepXf=hepXf0*GeoTrf::TranslateX3D(radius*cos(angle))*GeoTrf::TranslateY3D(radius*sin(angle))*GeoTrf::RotateZ3D(angle);
-                xfList->push_back((GeoGraphNode *) new GeoTransform(hepXf));
+                xfList->push_back(makeTransform(hepXf));
                 // hepXf = hepXf * GeoTrf::RotateZ3D(stepPhi) ;
 		angle+=stepPhi;
             }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaXProcessor.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaXProcessor.cxx
index 56c8d3fdf..1f2845d9f 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaXProcessor.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaXProcessor.cxx
@@ -141,12 +141,12 @@ DOMDocument *doc = element->getOwnerDocument();
                 hepXf0 = geoAXf->getTransform();
             }
             else {
-                geoXf = new GeoTransform (hepXf0);
+                geoXf = makeTransform (hepXf0);
                 hepXf0 = geoXf->getTransform();
             }
             GeoTrf::Transform3D hepXf=hepXf0; 
             for (int i = 0; i < nCopies; ++i) {
-                xfList->push_back((GeoGraphNode *) new GeoTransform(hepXf));
+                xfList->push_back(makeTransform(hepXf));
                 hepXf = hepXf * GeoTrf::TranslateX3D(stepX) ;
             }
     }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaXYArrays.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaXYArrays.cxx
index 3b52151ce..7918f27fe 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaXYArrays.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaXYArrays.cxx
@@ -160,13 +160,13 @@ DOMDocument *doc = element->getOwnerDocument();
                 hepXf0 = geoAXf->getTransform();
             }
             else {
-                geoXf = new GeoTransform (hepXf0);
+                geoXf = makeTransform (hepXf0);
                 hepXf0 = geoXf->getTransform();
             }
             GeoTrf::Transform3D hepXf=hepXf0; 
             for (int i = 0; i < nCopies; ++i) {
 	     	hepXf = GeoTrf::TranslateZ3D(zVal)*GeoTrf::TranslateX3D(xPos[i]) * GeoTrf::TranslateY3D(yPos[i]);
-                xfList->push_back((GeoGraphNode *) new GeoTransform(hepXf));
+                xfList->push_back(makeTransform(hepXf));
             }
     }
     else {
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaYProcessor.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaYProcessor.cxx
index 438c0e620..1004de4b2 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaYProcessor.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaYProcessor.cxx
@@ -142,12 +142,12 @@ DOMDocument *doc = element->getOwnerDocument();
                 hepXf0 = geoAXf->getTransform();
             }
             else {
-                geoXf = new GeoTransform (hepXf0);
+                geoXf = makeTransform (hepXf0);
                 hepXf0 = geoXf->getTransform();
             }
             GeoTrf::Transform3D hepXf=hepXf0; 
             for (int i = 0; i < nCopies; ++i) {
-                xfList->push_back((GeoGraphNode *) new GeoTransform(hepXf));
+                xfList->push_back(makeTransform(hepXf));
                 hepXf = hepXf * GeoTrf::TranslateY3D(stepY) ;
             }
     }
diff --git a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaZProcessor.cxx b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaZProcessor.cxx
index b9c247c21..b547f6e2e 100644
--- a/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaZProcessor.cxx
+++ b/GeoModelTools/GeoModelXML/GeoModelXml/src/ReplicaZProcessor.cxx
@@ -143,12 +143,12 @@ DOMDocument *doc = element->getOwnerDocument();
                 hepXf0 = geoAXf->getTransform();
             }
             else {
-                geoXf = new GeoTransform (hepXf0);
+                geoXf = makeTransform (hepXf0);
                 hepXf0 = geoXf->getTransform();
             }
             GeoTrf::Transform3D hepXf=hepXf0; 
             for (int i = 0; i < nCopies; ++i) {
-                xfList->push_back((GeoGraphNode *) new GeoTransform(hepXf));
+                xfList->push_back(makeTransform(hepXf));
                 hepXf = hepXf * GeoTrf::TranslateZ3D(stepZ) ;
             }
     }
-- 
GitLab