From 1a875d46c58d72ffd6654689229fcd35a9cb16e6 Mon Sep 17 00:00:00 2001
From: Markus Frank <markus.frank@cern.ch>
Date: Tue, 2 Jul 2013 11:47:22 +0000
Subject: [PATCH] Add utility function to BitField64- Use BitField64 in the
 IDDescriptor class- Remove implossible lookup functions using PlacedVolumes
 from VolumeManager.

---
 DDCore/include/DD4hep/BitField64.h     |  16 +-
 DDCore/include/DD4hep/IDDescriptor.h   |  21 +-
 DDCore/include/DD4hep/VolumeManager.h  |  32 +--
 DDCore/src/BitField64.cpp              |  21 +-
 DDCore/src/IDDescriptor.cpp            |  35 ++-
 DDCore/src/VolumeManager.cpp           | 377 +++++++++++++++++--------
 DDCore/src/plugins/LCDDConverter.cpp   |   8 +
 DDCore/src/plugins/StandardPlugins.cpp |   2 +-
 8 files changed, 359 insertions(+), 153 deletions(-)

diff --git a/DDCore/include/DD4hep/BitField64.h b/DDCore/include/DD4hep/BitField64.h
index 7eb0d2a7e..00a8c42a1 100644
--- a/DDCore/include/DD4hep/BitField64.h
+++ b/DDCore/include/DD4hep/BitField64.h
@@ -48,11 +48,7 @@ namespace DD4hep {
     typedef std::map<std::string, unsigned int> IndexMap ;
 
 
-    ~BitField64() {  // clean up
-      for(unsigned i=0;i<_fields.size();i++){
-	delete _fields[i] ;
-      }
-    }
+    virtual ~BitField64();  // clean up
   
     /** The c'tor takes an initialization string of the form:<br>
      *  <fieldDesc>[,<fieldDesc>...]<br>
@@ -234,6 +230,9 @@ namespace DD4hep {
      */
     long64 value() const ;
   
+    /// Calculate Field value given an external 64 bit bitmap value
+    long64 value(long64 id) const;
+
     /** Assignment operator for user convenience 
      */
     BitFieldValue& operator=(long64 in) ;
@@ -279,8 +278,11 @@ namespace DD4hep {
   };
 
 
-
-
+  inline BitField64::~BitField64() {  // clean up
+    for(unsigned i=0;i<_fields.size();i++){
+      delete _fields[i] ;
+    }
+  }
 
 
 } // end namespace
diff --git a/DDCore/include/DD4hep/IDDescriptor.h b/DDCore/include/DD4hep/IDDescriptor.h
index 62692ee8f..88c588f6c 100644
--- a/DDCore/include/DD4hep/IDDescriptor.h
+++ b/DDCore/include/DD4hep/IDDescriptor.h
@@ -12,6 +12,7 @@
 
 // Framework include files
 #include "DD4hep/Handle.h"
+#include "DD4hep/BitField64.h"
 
 // C++ include files
 #include <string>
@@ -37,6 +38,7 @@ namespace DD4hep {
     struct IDDescriptor : public Ref_t  {
     public:
       typedef unsigned long long int VolumeID;
+#if 0
       //typedef std::pair<int,int>          Field;
       struct Field  {
 	int first, second;
@@ -52,6 +54,9 @@ namespace DD4hep {
 	  //xx return (~mask&value)>>(64-second-first);
 	}
       };
+#endif
+      typedef std::pair<std::string,int> VolID;
+      typedef BitFieldValue* Field;
       typedef std::vector<std::pair<std::string,Field> >  FieldMap;
       typedef std::vector<std::pair<size_t,std::string> > FieldIDs;
 
@@ -61,13 +66,13 @@ namespace DD4hep {
        *  @version 1.0
        *  @date    2012/07/31
        */
-      struct Object : public TNamed {
-	std::string description;
-	FieldMap    fieldMap;
-	FieldIDs    fieldIDs;
-	int         maxBit;
+      struct Object : public TNamed, public BitField64 {
+	FieldMap      fieldMap;
+	FieldIDs      fieldIDs;
+	std::string   description;
+	//unsigned      maxBit;
 	/// Standard constructor
-	Object();
+	Object(const std::string& initString);
 	/// Default destructor
 	virtual ~Object();
       };
@@ -80,7 +85,7 @@ namespace DD4hep {
       /// Initializing constructor
       IDDescriptor(const std::string& description);
       /// The total number of encoding bits for this descriptor
-      int maxBit() const;
+      unsigned maxBit() const;
       /// Access the field-id container 
       const FieldIDs& ids() const;
       /// Access the fieldmap container 
@@ -91,6 +96,8 @@ namespace DD4hep {
       size_t fieldID(const std::string& field_name)  const;
       /// Get the field descriptor of one field by its identifier
       Field field(size_t identifier)  const;
+      /// Encoede a set of volume identifiers (corresponding to this description of course!) to a volumeID.
+      VolumeID encode(const std::vector<VolID>& ids)  const;
       /// Acces string representation
       std::string toString() const;
     };
diff --git a/DDCore/include/DD4hep/VolumeManager.h b/DDCore/include/DD4hep/VolumeManager.h
index 0a96fb6df..e6732bf82 100644
--- a/DDCore/include/DD4hep/VolumeManager.h
+++ b/DDCore/include/DD4hep/VolumeManager.h
@@ -26,6 +26,9 @@ namespace DD4hep {
    */
   namespace Geometry  {
 
+    // Forward declarations
+    class LCDD;
+
     /** @class VolumeManager  VolumeManager.h DD4hep/lcdd/VolumeManager.h
      * 
      *  The VolumeManager manages the repository of sensitive physical 
@@ -119,6 +122,9 @@ namespace DD4hep {
        */
       struct Context   {
       public:
+	typedef std::vector<const TGeoNode*> Path;
+	typedef PlacedVolume::VolIDs::Base VolIDs;
+
 	/// Placement identifier
 	VolumeID      identifier;
 	/// Ignore mask of the placement identifier
@@ -133,8 +139,10 @@ namespace DD4hep {
 	TGeoHMatrix   toDetector;
 	/// The transformation of space-points to the world corrdinate system 
 	TGeoHMatrix   toWorld;
-	
-	PlacedVolume::VolIDs::Base volID;
+	/// Volume IDS corresponding to this element
+	VolIDs        volID;
+	/// Path of placements to this sensitive volume
+	Path          path;
       public:
 	/// Default constructor
 	Context();
@@ -158,6 +166,8 @@ namespace DD4hep {
       public:
 	typedef IDDescriptor::Field Field;
       public:
+	/// Reference to the LCDD instance
+	LCDD&         lcdd;
 	/// The container of subdetector elements
 	Detectors     subdetectors;
 	/// The volume managers for the individual subdetector elements
@@ -180,7 +190,7 @@ namespace DD4hep {
 	int           flags;
       public:
 	/// Default constructor
-	Object();
+	Object(LCDD& lcdd);
 	/// Default destructor
 	virtual ~Object();
 	/// Search the locally cached volumes for a matching ID
@@ -204,7 +214,7 @@ namespace DD4hep {
        *  Please see enum PopulateFlags for further info.
        *  No action whatsoever is performed here, if the detector element is not valid.
        */
-      VolumeManager(const std::string& name, DetElement world=DetElement(), Readout ro=Readout(), int flags=NONE);
+      VolumeManager(LCDD& lcdd, const std::string& name, DetElement world=DetElement(), Readout ro=Readout(), int flags=NONE);
       /// Add a new Volume manager section according to a new subdetector
       VolumeManager addSubdetector(DetElement detector, Readout ro);
       /// Access the volume manager by cell id
@@ -237,20 +247,6 @@ namespace DD4hep {
       DetElement   lookupDetElement(VolumeID volume_id)  const;
       /// Access the transformation of a physical volume to the world coordinate system
       const TGeoMatrix& worldTransformation(VolumeID volume_id)  const;
-
-      /** This set of functions is required when reading/analyzing 
-       *  already created hits which have a VolumeID attached.
-       */
-      /// Lookup the context, which belongs to a registered physical volume.
-      Context*     lookupContext(PlacedVolume vol) const throw();
-      /// Access the physical volume identifier from the placed volume
-      VolumeID     lookupID(PlacedVolume vol) const;
-      /// Lookup a top level subdetector detector element according to a contained 64 bit hit ID
-      DetElement   lookupDetector(PlacedVolume vol)  const;
-      /// Lookup the closest subdetector detector element in the hierarchy according to a contained 64 bit hit ID
-      DetElement   lookupDetElement(PlacedVolume vol)  const;
-      /// Access the transformation of a physical volume to the world coordinate system
-      const TGeoMatrix& worldTransformation(PlacedVolume vol)  const;
     };
 
     /// Enable printouts for debugging
diff --git a/DDCore/src/BitField64.cpp b/DDCore/src/BitField64.cpp
index 515450ab6..268a92cd0 100644
--- a/DDCore/src/BitField64.cpp
+++ b/DDCore/src/BitField64.cpp
@@ -71,7 +71,26 @@ namespace DD4hep{
     }
   }
 
-  BitFieldValue& BitFieldValue::operator=(long64 in) {
+  long64 BitFieldValue::value(long64 id) const { 
+      
+    if(  _isSigned   ) {
+
+      long64 val = ( id & _mask ) >> _offset ;
+      
+      if( ( val  & ( 1LL << ( _width - 1 ) ) ) != 0 ) { // negative value
+	  
+	val -= ( 1LL << _width );
+      }
+	
+      return val ;
+
+    } else { 
+      
+      return  ( id & _mask ) >> _offset ;
+    }
+  }
+
+   BitFieldValue& BitFieldValue::operator=(long64 in) {
     
     // check range 
     if( in < _minVal || in > _maxVal  ) {
diff --git a/DDCore/src/IDDescriptor.cpp b/DDCore/src/IDDescriptor.cpp
index c0b65afa0..726642dd9 100644
--- a/DDCore/src/IDDescriptor.cpp
+++ b/DDCore/src/IDDescriptor.cpp
@@ -30,6 +30,7 @@ namespace {
   }
 
   void _construct(IDDescriptor::Object* o, const string& dsc) {
+#if 0
     typedef vector<string>  Elements;
     Elements elements, f;
     IDDescriptor::Field field;
@@ -59,13 +60,23 @@ namespace {
       o->fieldIDs.push_back(make_pair(o->fieldMap.size(),f[0]));
       o->fieldMap.push_back(make_pair(f[0],field));
     }
+#endif
+
+    BitField64& bf = *o;
+    o->fieldIDs.clear();
+    o->fieldMap.clear();
+    o->description = dsc ;
+    for(size_t i=0; i < bf.size(); ++i)    {
+      IDDescriptor::Field f = &bf[i];
+      o->fieldIDs.push_back(make_pair(i,f->name()));
+      o->fieldMap.push_back(make_pair(f->name(),f));
+    }
   }
 }
 
 /// Standard constructor
-IDDescriptor::Object::Object() : TNamed(), maxBit(0) {
+IDDescriptor::Object::Object(const std::string& desc) : TNamed(), BitField64(desc) /*, maxBit(0) */ {
   InstanceCount::increment(this);
-
 }
 /// Default destructor
 IDDescriptor::Object::~Object()  {
@@ -75,7 +86,7 @@ IDDescriptor::Object::~Object()  {
 /// Initializing constructor
 IDDescriptor::IDDescriptor(const string& description) 
 {
-  Object* obj = new Object();
+  Object* obj = new Object(description);
   assign(obj,description,"iddescriptor");
   _construct(obj, description);
 }
@@ -83,14 +94,14 @@ IDDescriptor::IDDescriptor(const string& description)
 /// Acces string representation
 string IDDescriptor::toString() const  {
   if ( isValid() )  {
-    return data<Object>()->description;
+    return m_element->GetName();
   }
   return "----";
 }
 
 /// The total number of encoding bits for this descriptor
-int IDDescriptor::maxBit() const  { 
-  return data<Object>()->maxBit;
+unsigned IDDescriptor::maxBit() const  { 
+  return data<Object>()->highestBit();
 }
 
 /// Access the field-id container 
@@ -130,3 +141,15 @@ size_t IDDescriptor::fieldID(const string& field_name)  const   {
     if ( (*i).second == field_name ) return (*i).first;
   throw runtime_error(string(name())+": This ID descriptor has no field with the name:"+field_name);
 }
+
+/// Encoede a set of volume identifiers (corresponding to this description of course!) to a volumeID.
+IDDescriptor::VolumeID IDDescriptor::encode(const std::vector<VolID>& ids)  const   {
+  typedef std::vector<VolID> VolIds;
+  VolumeID id = 0;
+  for(VolIds::const_iterator i=ids.begin(); i!=ids.end(); ++i)   {
+    Field f = field((*i).first);
+    id |= f->value((*i).second<<f->offset())<<f->offset();
+  }
+  return id;
+}
+
diff --git a/DDCore/src/VolumeManager.cpp b/DDCore/src/VolumeManager.cpp
index f6245fd88..b19bb8062 100644
--- a/DDCore/src/VolumeManager.cpp
+++ b/DDCore/src/VolumeManager.cpp
@@ -9,8 +9,10 @@
 // Framework include files
 #include "DD4hep/VolumeManager.h"
 #include "DD4hep/Printout.h"
+#include "DD4hep/LCDD.h"
 
 // C/C++ includes
+#include <cmath>
 #include <sstream>
 #include <iomanip>
 
@@ -22,25 +24,69 @@ using namespace DD4hep::Geometry;
 //#define VOLUME_IDENTIFIER(id,mask)  id,mask
 
 namespace {
+
   struct Populator  {
     typedef VolumeManager::VolumeID VolumeID;
     typedef vector<const TGeoNode*> Chain;
+
+    struct DetCtxt  {
+      DetElement        detector;
+      Chain             chain;
+      SensitiveDetector sd;
+      DetCtxt() {}
+      ~DetCtxt() {}
+    };
+    typedef map<TNamed*,DetCtxt*> DetContext;
+    /// Reference to the LCDD instance
+    LCDD&         m_lcdd;
     /// Reference to the volume manager to be populated
     VolumeManager m_volManager;
+    /// Flag
+    bool m_add;
+
+    DetContext m_detContext;
     /// Default constructor
-    Populator(VolumeManager vm) : m_volManager(vm) {}
+    Populator(LCDD& lcdd, VolumeManager vm) : m_lcdd(lcdd), m_volManager(vm), m_add(true)  {}
 
     /// Populate the Volume manager
-    void populate(DetElement e)   {
+    void populate(DetElement e, bool add=true)   {
+      m_add = add;
       const DetElement::Children& c = e.children();
       for(DetElement::Children::const_iterator i=c.begin(); i!=c.end(); ++i)   {
 	DetElement   de = (*i).second;
 	PlacedVolume pv = de.placement();
 	if ( pv.isValid() )  {
+	  Chain                chain;
 	  PlacedVolume::VolIDs ids;
-	  Chain node_chain, elt_nodes;
-	  //cout << "Scanning " << de.name() << endl;
-	  scanPhysicalVolume(de, de, pv, ids, node_chain, elt_nodes);
+	  m_detContext.clear();
+	  scanPhysicalVolume(de, de, pv, ids, chain);
+	  if ( add )  {
+	    for(DetContext::const_iterator di=m_detContext.begin(); di != m_detContext.end(); ++di)  {	
+	      Chain det_chain;
+	      PlacedVolume::VolIDs ids;
+	      DetElement det = (*di).second->detector;
+	      const Chain& chain = (*di).second->chain;
+	      if ( !det.parent().isValid() ) continue;
+	      for(Chain::const_iterator ci=chain.begin(); ci!=chain.end(); ++ci)  {
+		PlacedVolume pv = Ref_t(*ci);
+		det_chain.push_back(*ci);
+		ids.PlacedVolume::VolIDs::Base::insert(ids.end(),pv.volIDs().begin(),pv.volIDs().end());
+		if ( pv.ptr() == det.placement().ptr() )  break;
+	      }
+	      if ( !ids.empty() )  {
+		stringstream log;
+		IDDescriptor            iddesc = (*di).second->sd.readout().idSpec();
+		pair<VolumeID,VolumeID> code   = encoding(iddesc, ids);
+		log << det.path() << " encoding:" << code.first << "," << code.second << " ids:";
+		for(size_t m=0; m<ids.size(); ++m)
+		  log << ids[m].first << "=" << ids[m].second << "  ";
+		printout(DEBUG,"VolumeManager","++ Add DetElements:%s",log.str().c_str());
+		add_entry((*di).second->sd, de, det, det.placement().ptr(), ids, det_chain);
+	      }
+	    }
+	  }
+	  for(DetContext::const_iterator di=m_detContext.begin(); di != m_detContext.end(); ++di)
+	    delete (*di).second;
 	  continue;
 	}
 	printout(WARNING,"VolumeManager","++ Detector element %s of type %s has no placement.",
@@ -62,69 +108,85 @@ namespace {
       return DetElement();
     }
     /// Scan a single physical volume and look for sensitive elements below
-    void scanPhysicalVolume(DetElement parent, DetElement e, PlacedVolume pv, 
-		  PlacedVolume::VolIDs ids, Chain& node_chain, Chain& elt_nodes)
+    size_t scanPhysicalVolume(DetElement parent, DetElement e, PlacedVolume pv, 
+			      PlacedVolume::VolIDs ids, Chain& chain)
     {
       const TGeoNode* node = pv.ptr();
+      size_t count = 0;
       if ( node )  {
 	Volume vol = pv.volume();
-	node_chain.push_back(node);
-	elt_nodes.push_back(node);
-#if 0
-	const PlacedVolume::VolIDs::Base& vids = pv.volIDs();
-	for(PlacedVolume::VolIDs::Base::const_iterator i=vids.begin(); i!=vids.end(); ++i)
-	  ids.push_back(*i);
-#endif
+	chain.push_back(node);
 	ids.PlacedVolume::VolIDs::Base::insert(ids.end(),pv.volIDs().begin(),pv.volIDs().end());
 	if ( vol.isSensitive() )  {
 	  SensitiveDetector sd = vol.sensitiveDetector();
 	  Readout           ro = sd.readout();
-	  //Readout ro = parent.readout();
 	  if ( ro.isValid() )  {
-	    add_entry(parent, e, node, ids, node_chain, elt_nodes);
+	    if ( m_add )  {
+	      add_entry(parent, e, node, ids, chain);
+	      ++count;
+	    }
+	    else  {
+	      find_entry(parent, e, node, ids, chain);
+	    }
 	  }
-	  else   {
+	  else  {
 	    printout(WARNING,"VolumeManager",
-		     "%s: Strange constellation volume %s is sensitive, but has no readout! sd:%p ro:%p",
-		     parent.name(), pv.volume().name(), sd.ptr(), ro.ptr());
+		     "%s: Strange constellation volume %s is sensitive, but has no readout! sd:%p",
+		     parent.name(), pv.volume().name(), sd.ptr());
 	  }
 	}
 	for(Int_t idau=0, ndau=node->GetNdaughters(); idau<ndau; ++idau)  {
 	  TGeoNode* daughter = node->GetDaughter(idau);
 	  if ( dynamic_cast<const PlacedVolume::Object*>(daughter) ) {
-	    Chain dau_nodes;
+	    size_t cnt;
 	    PlacedVolume pv_dau = Ref_t(daughter);
 	    DetElement   de_dau = findElt(e,daughter);
-	    //cout << " ---> Scanning " << pv_dau.name() << endl;
-	    if ( de_dau.isValid() )
-	      scanPhysicalVolume(parent, de_dau, pv_dau, ids, node_chain, dau_nodes);
-	    else
-	      scanPhysicalVolume(parent, e, pv_dau, ids, node_chain, elt_nodes);
+	    if ( de_dau.isValid() )  {
+	      Chain dau_chain;
+	      cnt = scanPhysicalVolume(parent, de_dau, pv_dau, ids, dau_chain);
+	    }
+	    else  {
+	      cnt = scanPhysicalVolume(parent, e, pv_dau, ids, chain);
+	    }
+	    count += cnt;
 	  }
 	}
+	chain.pop_back();
       }
-      elt_nodes.pop_back();
-      node_chain.pop_back();
+      return count;
     }
     pair<VolumeID,VolumeID> encoding(const IDDescriptor iddesc, const PlacedVolume::VolIDs& ids )  const  {
-      VolumeID volume_id = ~0x0ull, mask = 0;
-      //fg: default value 0 for volids does not seem to work (!?):  VolumeID volume_id = 0, mask = 0;
+      //VolumeID volume_id = ~0x0ULL, mask = 0;
+      VolumeID volume_id = 0, mask = 0;
       for(PlacedVolume::VolIDs::const_iterator i=ids.begin(); i!=ids.end(); ++i) {
 	const PlacedVolume::VolID& id = (*i);
 	IDDescriptor::Field f = iddesc.field(id.first);
-	volume_id &= f.encode(id.second);
-	mask |= f.mask;
+	VolumeID msk = f->mask();
+	int offset = f->offset();
+	// This pads the unused bits with '1' instead of '0': 
+	// volume_id &= ~msk;
+	volume_id |= f->value(id.second<<offset)<<offset;
+	mask      |= msk;
+	//volume_id &= f.encode(id.second);
+	//mask |= f.mask;
       }
       return make_pair(volume_id,mask);
     }
 
     void add_entry(DetElement parent, DetElement e,const TGeoNode* n, 
-		  const PlacedVolume::VolIDs& ids, const Chain& nodes, const Chain& elt_nodes)
+		  const PlacedVolume::VolIDs& ids, const Chain& nodes)
     {
-      Volume vol = PlacedVolume(n).volume();
-      SensitiveDetector sd = vol.sensitiveDetector();
-      Readout           ro = sd.readout();
-      IDDescriptor      iddesc = ro.idSpec();
+      Volume            vol     = PlacedVolume(n).volume();
+      SensitiveDetector sd      = vol.sensitiveDetector();
+      add_entry(sd, parent,e,n,ids,nodes);
+    }
+
+    void add_entry(SensitiveDetector sd, 
+		   DetElement parent, DetElement e,const TGeoNode* n, 
+		   const PlacedVolume::VolIDs& ids, const Chain& nodes)
+    {
+      Readout           ro      = sd.readout();
+      IDDescriptor      iddesc  = ro.idSpec();
       VolumeManager     section = m_volManager.addSubdetector(parent,ro);
       pair<VolumeID,VolumeID> code = encoding(iddesc, ids);
 
@@ -137,6 +199,7 @@ namespace {
       context->element    = e;
       context->placement  = Ref_t(n);
       context->volID      = ids;
+      context->path       = nodes;
       for(size_t i=nodes.size(); i>1; --i)  {  // Omit the placement of the parent DetElement
 	TGeoMatrix* m = nodes[i-1]->GetMatrix();
 	context->toWorld.MultiplyLeft(m);
@@ -144,62 +207,113 @@ namespace {
       context->toDetector = context->toWorld;
       context->toDetector.MultiplyLeft(nodes[0]->GetMatrix());
       context->toWorld.MultiplyLeft(o.worldTransformation());
+      if ( parent.path().find("HcalBarrel") != string::npos ) print_node(sd,parent,e,n,ids,nodes);
       if ( !section.adoptPlacement(context) )  {
-	print_node(parent,e,n,ids,nodes,elt_nodes);
+	print_node(sd,parent,e,n,ids,nodes);
+      }
+      if ( e.placement().ptr() != n )  {
+	DetContext::const_iterator di = m_detContext.find(e.ptr());
+	if ( di == m_detContext.end() )  {
+	  DetCtxt* c  = new DetCtxt();
+	  c->detector = e;
+	  c->chain    = nodes;
+	  c->sd       = sd;
+	  m_detContext[e.ptr()] = c;
+	}
       }
     }
+    void find_entry(DetElement parent, DetElement e,const TGeoNode* n, 
+		  const PlacedVolume::VolIDs& ids, const Chain& nodes)
+    {
+      Volume            vol     = PlacedVolume(n).volume();
+      SensitiveDetector sd      = vol.sensitiveDetector();
+      Readout           ro      = sd.readout();
+      IDDescriptor      iddesc  = ro.idSpec();
+      pair<VolumeID,VolumeID> code = encoding(iddesc, ids);
+#if 0
+      VolumeID id  = (VolumeID(rand())<<32) + VolumeID(rand());
+      id &= ~code.second;
+      id |=  code.second;
+      VolumeID volID = id&code.first;
+#else
+      VolumeID volID = code.first;
+#endif
+
+      VolumeManager::Context* context = m_volManager.lookupContext(volID);
+      stringstream log;
+      if ( !context )  {
+	log  << "CANNOT FIND volume:" 
+	     << " Ptr:"  << (void*)n
+	     << " ["     << n->GetName() << "]"
+	     << " ID:"   << (void*)volID
+	     << " Mask:" << (void*)code.second;
+	printout(ERROR,"VolumeManager",log.str().c_str());
+	return;
+      }
+      else if ( context->placement.ptr() != n )  {
+	log  << "FOUND WRONG volume:" 
+	     << " Ptr:"  << (void*)context->placement.ptr() 
+	     << " instead of " << (void*)n
+	     << " ["     << context->placement.name()
+	     << " instead of " << n->GetName() << "]"
+	     << " ID:"   << (void*)context->identifier 
+	     << " instead of " << (void*)volID;
+	printout(ERROR,"VolumeManager",log.str().c_str());
+	return;
+      }
+      log  << "Found volume:" 
+	   << " Ptr:"  << (void*)context->placement.ptr()
+	   << " ["     << context->placement.name() << "]"
+	   << " ID:"   << (void*)context->identifier 
+	   << " Mask:" << (void*)context->mask;
+      printout(DEBUG,"VolumeManager",log.str().c_str());
+    }
     void print_node(DetElement parent, DetElement e, const TGeoNode* n, 
-		    const PlacedVolume::VolIDs& ids, const Chain& nodes, const Chain& elt_nodes)
+		    const PlacedVolume::VolIDs& ids, const Chain& nodes)
+    {
+      Volume              vol = PlacedVolume(n).volume();
+      SensitiveDetector   sd = vol.sensitiveDetector();
+      print_node(sd, parent, e, n, ids, nodes);
+    }
+
+    void print_node(SensitiveDetector sd, DetElement parent, DetElement e, const TGeoNode* n, 
+		    const PlacedVolume::VolIDs& ids, const Chain& nodes)
     {
       static int s_count = 0;
-      Volume vol = PlacedVolume(n).volume();
-      SensitiveDetector sd = vol.sensitiveDetector();
-      Readout ro = sd.readout();
+      Readout             ro = sd.readout();
       const IDDescriptor& en = ro.idSpec();
-      PlacedVolume pv = Ref_t(n);
-      bool sensitive  = pv.volume().isSensitive();
+      PlacedVolume        pv = Ref_t(n);
+      bool                sensitive  = pv.volume().isSensitive();
       pair<VolumeID,VolumeID> code = encoding(en, ids);
       IDDescriptor::VolumeID volume_id = code.first;
 
       if ( !sensitive ) return;
       ++s_count;
-      cout << s_count << ": " << e.name() << " de:" << e.ptr() << " ro:" << ro.ptr() 
-	   << " pv:" << n->GetName() << " id:" << (void*)volume_id << " : ";
+      stringstream log;
+      log << s_count << ": " << e.name() << " de:" << e.ptr() << " ro:" << ro.ptr() 
+	  << " pv:" << n->GetName() << " id:" << (void*)volume_id << " : ";
       for(PlacedVolume::VolIDs::const_iterator i=ids.begin(); i!=ids.end(); ++i) {
 	const PlacedVolume::VolID& id = (*i);
 	IDDescriptor::Field f = ro.idSpec().field(id.first);
-	int value = en.field(id.first).decode(volume_id);
-	cout << id.first << "=" << id.second << "," << value 
-	     << " [" << f.first << "," << f.second << "] ";
-      }
-      cout << " Sensitive:" << (sensitive ? "YES" : "NO");
-      if ( sensitive )   {
-	VolumeManager section = m_volManager.subdetector(volume_id);
-#if 0
-	VolumeManager::Context* ctxt = section.lookupContext(volume_id|0xDEAD);
-	if ( ctxt->placement.ptr() != pv.ptr() )  {
-	  cout << " !!!!! No Volume found!" << endl;
-	}
-	cout << " Pv:" << pv.ptr() << " <> " << ctxt->placement.ptr();
-#endif
+	VolumeID value = f->value(volume_id);
+	log << id.first << "=" << id.second << "," << value 
+	    << " [" << f->offset() << "," << f->width() << "] ";
       }
-      cout << endl;
+      log << " Sensitive:" << (sensitive ? "YES" : "NO");
+      printout(INFO,"VolumeManager",log.str().c_str());
 #if 0
-      cout << s_count << ": " << e.name() << " Detector GeoNodes:";
+      log.str("");
+      log << s_count << ": " << e.name() << " Detector GeoNodes:";
       for(vector<const TGeoNode*>::const_iterator j=nodes.begin(); j!=nodes.end();++j)
-	cout << (void*)(*j) << " ";
-      cout << endl;
-      cout << s_count << ": " << e.name() << " Element  GeoNodes:";
-      for(vector<const TGeoNode*>::const_iterator j=elt_nodes.begin(); j!=elt_nodes.end();++j)
-	cout << (void*)(*j) << " ";
-      cout << endl;
+	log << (void*)(*j) << " ";
+      printout(INFO,"VolumeManager",log.str().c_str());
 #endif
     }
   };
 }
 
 /// Default constructor
-VolumeManager::Context::Context() : identifier(0), mask(~0x0ull) {
+VolumeManager::Context::Context() : identifier(0), mask(~0x0ULL) {
 }
 
 /// Default destructor
@@ -207,7 +321,7 @@ VolumeManager::Context::~Context()   {
 }
 
 /// Default constructor
-VolumeManager::Object::Object() : top(0), sysID(0), flags(VolumeManager::NONE)  {
+VolumeManager::Object::Object(LCDD& l) : lcdd(l), top(0), sysID(0), flags(VolumeManager::NONE)  {
 }
 
 /// Default destructor
@@ -216,12 +330,8 @@ VolumeManager::Object::~Object()   {
   bool   isTop =  obj == this;
   bool  hasTop = (flags&VolumeManager::ONE)==VolumeManager::ONE;
   bool  isSdet = (flags&VolumeManager::TREE)==VolumeManager::TREE && obj != this;
-  //cout << "Delete Volume manager: " << this << " " << detector.name() << endl;
-
   /// Cleanup volume tree
-  if ( (isTop && hasTop) || (isSdet && !hasTop) )
-    for_each(volumes.begin(),volumes.end(),DestroyObjects<VolIdentifier,Context*>());
-
+  for_each(volumes.begin(),volumes.end(),DestroyObjects<VolIdentifier,Context*>());
   volumes.clear();
   /// Cleanup dependent managers
   for_each(managers.begin(),managers.end(),DestroyHandles<VolumeID,VolumeManager>());
@@ -247,17 +357,18 @@ VolumeManager::Context* VolumeManager::Object::search(const PlacedVolume pv)  co
 }
 
 /// Initializing constructor to create a new object
-VolumeManager::VolumeManager(const string& nam, DetElement elt, Readout ro, int flags)
+VolumeManager::VolumeManager(LCDD& lcdd, const string& nam, DetElement elt, Readout ro, int flags)
 {
-  Object* ptr = new Object();
+  Object* ptr = new Object(lcdd);
   assign(ptr,nam,"VolumeManager");
   if ( elt.isValid() )   {
-    Populator p(*this);
+    Populator p(lcdd, *this);
     Object& o = _data();
     setDetector(elt, ro);
     o.top   = ptr;
     o.flags = flags;
     p.populate(elt);
+    p.populate(elt,false);
   }
 }
 
@@ -284,10 +395,13 @@ VolumeManager VolumeManager::addSubdetector(DetElement detector, Readout ro)  {
 			    "valid placement VolIDs are allowed. [Invalid DetElement:"+det_name+"]");
       }
 
-      i = o.subdetectors.insert(make_pair(detector,VolumeManager(detector.name()))).first;
+      i = o.subdetectors.insert(make_pair(detector,VolumeManager(o.lcdd,detector.name()))).first;
       const PlacedVolume::VolID& id = (*vit);
       VolumeManager m = (*i).second;
       IDDescriptor::Field field = ro.idSpec().field(id.first);
+      if ( !field )   {
+	throw runtime_error("VolumeManager::addSubdetector: IdDescriptor of "+string(detector.name())+" has no field "+id.first);
+      }
       Object& mo = m._data();
       m.setDetector(detector,ro);
       mo.top    = o.top;
@@ -308,7 +422,8 @@ VolumeManager VolumeManager::subdetector(VolumeID id)  const   {
     /// Need to perform a linear search, because the "system" tag width may vary between subdetectors
     for(Detectors::const_iterator j=o.subdetectors.begin(); j != o.subdetectors.end(); ++j)  {
       const Object& mo = (*j).second._data();
-      VolumeID sys_id = mo.system.decode(id);
+      //VolumeID sys_id = mo.system.decode(id);
+      VolumeID sys_id = mo.system->value(id<<mo.system->offset());
       if ( sys_id == mo.sysID )
 	return (*j).second;
     }
@@ -362,12 +477,14 @@ bool VolumeManager::adoptPlacement(VolumeID sys_id, Context* context)   {
   PlacedVolume pv = context->placement;
   VolIdentifier vid(VOLUME_IDENTIFIER(context->identifier,context->mask));
   Volumes::iterator i = o.volumes.find(vid);
+#if 0
   if ( (context->identifier&context->mask) != context->identifier )   {
     err << "Bad context mask:" << (void*)context->mask << " id:" << (void*)context->identifier
 	<< " pv:" << pv.name() << " Sensitive:" 
 	<< (pv.volume().isSensitive() ? "YES" : "NO") << endl;
     goto Fail;
   }
+#endif
   if ( i == o.volumes.end() )   {
     o.volumes[vid] = context;
     o.phys_volumes[pv.ptr()] = context;
@@ -383,8 +500,8 @@ bool VolumeManager::adoptPlacement(VolumeID sys_id, Context* context)   {
       << (void*)context->identifier << " Mask:" << (void*)context->mask
       << " to detector " << o.detector.name()
       << " ptr:" << (void*)pv.ptr() << " -- " << (*i).second->placement.ptr()
-      << " pv:" << pv.name() << " Sensitive:" 
-      << (pv.volume().isSensitive() ? "YES" : "NO") << endl;
+      << " pv:" << pv.name() << " clashes with " << (*i).second->placement.name()
+      << " Sensitive:" << (pv.volume().isSensitive() ? "YES" : "NO") << endl;
   goto Fail;
  Fail:
   {
@@ -393,7 +510,7 @@ bool VolumeManager::adoptPlacement(VolumeID sys_id, Context* context)   {
     for(PlacedVolume::VolIDs::Base::const_iterator vit = ids.begin(); vit != ids.end(); ++vit)
       err << (*vit).first << "=" << (*vit).second << "; ";
   }
-  printout(DEBUG,"VolumeManager","++++[!!!] adoptPlacement: %s",err.str().c_str());
+  printout(ERROR,"VolumeManager","++++[!!!] adoptPlacement: %s",err.str().c_str());
   // throw runtime_error(err.str());
   return false;
 }
@@ -404,25 +521,31 @@ bool VolumeManager::adoptPlacement(Context* context)   {
   if ( isValid() )  {
     Object& o = _data();
     if ( context )   {
-      VolumeID sys_id = o.system.decode(context->identifier);
-      if ( sys_id == o.sysID )  {
+      if ( (o.flags&ONE) == ONE )  {
+	VolumeManager top(Ref_t(o.top));
+	return top.adoptPlacement(context);
+      }
+      if ( (o.flags&TREE) == TREE )  {
 	bool isTop = ptr() == o.top;
-	if ( !isTop && (o.flags&ONE) == ONE )  {
+	if ( !isTop )   {
+	  VolumeID sys_id = o.system->value(context->identifier);
+	  if ( sys_id == o.sysID )  {
+	    return adoptPlacement(sys_id,context);
+	  }
 	  VolumeManager top(Ref_t(o.top));
-	  if ( top.adoptPlacement(sys_id,context) )
-	    return true;
+	  return top.adoptPlacement(context);
 	}
-	if ( (o.flags&TREE) == TREE )  {
-	  if ( adoptPlacement(sys_id,context) )
-	    return true;
+	for(Managers::iterator j=o.managers.begin(); j != o.managers.end(); ++j)  {
+	  Object& m = (*j).second._data();
+	  VolumeID sid = m.system->value(context->identifier);
+	  if ( (*j).first == sid )  {
+	    return (*j).second.adoptPlacement(sid,context);
+	  }
 	}
-	return false;
       }
-      PlacedVolume pv = context->placement;
-      err << "The placement " << pv.name() << " does not belong to detector:" << o.detector.name();
-      goto Fail;
+      return false;
     }
-    err << "Failed to add new physical volume to detector:" << o.detector.name() << " [Invalid object]";
+    err << "Failed to add new physical volume to detector:" << o.detector.name() << " [Invalid Context]";
     goto Fail;
   }
   err << "Failed to add new physical volume [Invalid Manager Handle]";
@@ -437,17 +560,21 @@ VolumeManager::Context* VolumeManager::lookupContext(VolumeID volume_id) const
   if ( isValid() )  {
     Context* c = 0;
     const Object& o = _data();
-    if ( o.top != ptr() && (o.flags&ONE) == ONE )  {
+    bool is_top = o.top == ptr();
+    bool one_tree = (o.flags&ONE) == ONE;
+    if ( !is_top && one_tree )  {
       return VolumeManager(Ref_t(o.top)).lookupContext(volume_id);
     }
-    VolIdentifier id(VOLUME_IDENTIFIER(volume_id,~0x0ull));
+    VolIdentifier id(VOLUME_IDENTIFIER(volume_id,~0x0ULL));
     /// First look in our own volume cache if the entry is found.
     c = o.search(id);
     if ( c ) return c;
     /// Second: look in the subdetector volume cache if the entry is found.
-    for(Detectors::const_iterator j=o.subdetectors.begin(); j != o.subdetectors.end(); ++j)  {
-      if ( (c=(*j).second._data().search(id)) != 0 )
-	return c;
+    if ( !one_tree )  {
+      for(Detectors::const_iterator j=o.subdetectors.begin(); j != o.subdetectors.end(); ++j)  {
+	if ( (c=(*j).second._data().search(id)) != 0 )
+	  return c;
+      }
     }
     throw runtime_error("VolumeManager::lookupContext: Failed to search Volume context [Unknown identifier]");
   }
@@ -486,27 +613,50 @@ std::ostream& DD4hep::Geometry::operator<<(std::ostream& os, const VolumeManager
   bool  hasTop = (o.flags&VolumeManager::ONE)==VolumeManager::ONE;
   bool  isSdet = (o.flags&VolumeManager::TREE)==VolumeManager::TREE && top != &o;
   string prefix(isTop ? "" : "++  ");
-  cout << prefix 
-       << (isTop ? "TOP Level " : "Secondary ") 
-       << "Volume manager:" << &o  << " " << o.detector.name()
-       << " IDD:"   << o.id.toString()
-       << " SysID:" << (void*)o.sysID << " "
-       << o.managers.size() << " subsections "
-       << o.volumes.size()  << " placements ";
-  if ( !(o.managers.empty() && o.volumes.empty()) ) cout << endl;
+  os << prefix 
+     << (isTop ? "TOP Level " : "Secondary ") 
+     << "Volume manager:" << &o  << " " << o.detector.name()
+     << " IDD:"   << o.id.toString()
+     << " SysID:" << (void*)o.sysID << " "
+     << o.managers.size() << " subsections "
+     << o.volumes.size()  << " placements ";
+  if ( !(o.managers.empty() && o.volumes.empty()) ) os << endl;
   for(VolumeManager::Volumes::const_iterator i = o.volumes.begin(); i!=o.volumes.end(); ++i)  {
     const VolumeManager::Context* c = (*i).second;
     PlacedVolume pv = c->placement;
-    cout << prefix
-	 << "PV:"    << setw(32) << left << pv.name() 
-	 << " id:"   << setw(18) << left << (void*)c->identifier 
-	 << " mask:" << setw(18) << left << (void*) c->mask << endl;
+    os << prefix
+       << "PV:"    << setw(32) << left << pv.name() 
+       << " id:"   << setw(18) << left << (void*)c->identifier 
+       << " mask:" << setw(18) << left << (void*) c->mask << endl;
   }
   for(VolumeManager::Managers::const_iterator i = o.managers.begin(); i!=o.managers.end(); ++i)
-    cout << prefix << (*i).second << endl;
+    os << prefix << (*i).second << endl;
   return os;
 }
 
+
+#if 0
+
+      It was wishful thinking, the implementation of the reverse lookups would be as simple.
+      Hence the folling calls are removed for the time being.
+
+      Markus Frank
+
+
+      /** This set of functions is required when reading/analyzing 
+       *  already created hits which have a VolumeID attached.
+       */
+      /// Lookup the context, which belongs to a registered physical volume.
+      Context*     lookupContext(PlacedVolume vol) const throw();
+      /// Access the physical volume identifier from the placed volume
+      VolumeID     lookupID(PlacedVolume vol) const;
+      /// Lookup a top level subdetector detector element according to a contained 64 bit hit ID
+      DetElement   lookupDetector(PlacedVolume vol)  const;
+      /// Lookup the closest subdetector detector element in the hierarchy according to a contained 64 bit hit ID
+      DetElement   lookupDetElement(PlacedVolume vol)  const;
+      /// Access the transformation of a physical volume to the world coordinate system
+      const TGeoMatrix& worldTransformation(PlacedVolume vol)  const;
+
 /// Lookup the context, which belongs to a registered physical volume.
 VolumeManager::Context* VolumeManager::lookupContext(PlacedVolume pv) const throw()  {
   if ( isValid() )  {
@@ -552,3 +702,4 @@ const TGeoMatrix& VolumeManager::worldTransformation(PlacedVolume vol)  const  {
   return c->toWorld;
 }
 
+#endif
diff --git a/DDCore/src/plugins/LCDDConverter.cpp b/DDCore/src/plugins/LCDDConverter.cpp
index 7cce61301..92c4fc155 100644
--- a/DDCore/src/plugins/LCDDConverter.cpp
+++ b/DDCore/src/plugins/LCDDConverter.cpp
@@ -854,6 +854,7 @@ xml_h LCDDConverter::handleIdSpec(const std::string& name, const TNamed* id_spec
     const IDDescriptor::FieldMap& m = desc.fields();
     for(IDDescriptor::FieldMap::const_iterator i=m.begin(); i!=m.end(); ++i) {
       xml_h idfield = xml_elt_t(geo.doc,_U(idfield));
+#if 0
       const IDDescriptor::Field& f = (*i).second;
       start = f.first;
       length = f.second<0 ? -f.second : f.second;
@@ -861,6 +862,13 @@ xml_h LCDDConverter::handleIdSpec(const std::string& name, const TNamed* id_spec
       idfield.setAttr(_U(label),(*i).first);
       idfield.setAttr(_U(length),length);
       idfield.setAttr(_U(start),start);
+#else
+      IDDescriptor::Field f = (*i).second;
+      idfield.setAttr(_U(signed),f->isSigned() ? true : false);
+      idfield.setAttr(_U(label),f->name());
+      idfield.setAttr(_U(length),(int)f->width());
+      idfield.setAttr(_U(start),(int)f->offset());
+#endif
       id.append(idfield);
     }
     id.setAttr(_U(length),length+start);
diff --git a/DDCore/src/plugins/StandardPlugins.cpp b/DDCore/src/plugins/StandardPlugins.cpp
index 16cfb5929..2bb20cd3b 100644
--- a/DDCore/src/plugins/StandardPlugins.cpp
+++ b/DDCore/src/plugins/StandardPlugins.cpp
@@ -64,7 +64,7 @@ DECLARE_APPLY(DD4hepXMLLoader,load_xml);
 static long load_volmgr(LCDD& lcdd,int,char**)    {
   try {
     LCDDImp* imp = dynamic_cast<LCDDImp*>(&lcdd);
-    imp->m_volManager = VolumeManager("World", imp->world(), Readout(), VolumeManager::TREE);
+    imp->m_volManager = VolumeManager(lcdd, "World", imp->world(), Readout(), VolumeManager::TREE);
     cout << "++ Volume manager populated and loaded." << endl;
   }
   catch(const exception& e)  {
-- 
GitLab