From a6164ae6c40181e69542747d0170cfcd961a90a4 Mon Sep 17 00:00:00 2001
From: Markus Frank <Markus.Frank@cern.ch>
Date: Mon, 15 Oct 2018 18:38:46 +0200
Subject: [PATCH] Fix bug in transformation creation and subdetector assemblies
 (introduced in last commit...)

---
 DDCore/src/XML/Utilities.cpp                  | 42 ++++++++-----------
 DDDetectors/src/SubdetectorAssembly_geo.cpp   | 12 +++---
 examples/DDUpgrade/compact/Bcm/BcmUp.xml      |  2 +-
 examples/DDUpgrade/compact/Bcm/detector.xml   |  2 +-
 examples/DDUpgrade/compact/Bls/detector.xml   | 10 ++---
 examples/DDUpgrade/compact/Bls/parameters.xml | 18 ++++----
 examples/DDUpgrade/compact/test.xml           |  8 ++--
 7 files changed, 44 insertions(+), 50 deletions(-)

diff --git a/DDCore/src/XML/Utilities.cpp b/DDCore/src/XML/Utilities.cpp
index eccb27da2..5d5259a13 100644
--- a/DDCore/src/XML/Utilities.cpp
+++ b/DDCore/src/XML/Utilities.cpp
@@ -29,40 +29,34 @@ using namespace dd4hep::xml::tools;
 
 /// Create layered transformation from xml information
 Transform3D dd4hep::xml::createTransformation(xml::Element e)   {
-  int cnt = 0;
-  Position pos;
-  RotationZYX rot;
-  Transform3D result;
+  int flag = 0;
+  Transform3D position, rotation, result;
   for( xml_coll_t c(e,_U(star)); c; ++c )    {
     string tag = c.tag();
     xml_dim_t x_elt = c;
     if ( tag == "positionRPhiZ" )   {
-      Transform3D trafo = Transform3D(pos);
-      if ( cnt > 1 ) trafo *= Transform3D(rot);
-      result = trafo * result;
-      ROOT::Math::Polar2D<double> dim(x_elt.r(0.0), x_elt.phi(0.0));
-      pos.SetXYZ(dim.X(), dim.Y(), x_elt.z(0.0));
-      rot.SetComponents(0,0,0);
-      cnt = 1;
+      if      ( flag == 1 ) result = position  * result;
+      else if ( flag == 2 ) result = (position * rotation) * result;
+      ROOT::Math::RhoZPhiVector pos(x_elt.r(0), x_elt.z(0), x_elt.phi(0));
+      position = Transform3D(pos);
+      rotation = Transform3D();
+      flag = 1;
     }
     else if ( tag == "position" )   {
-      Transform3D trafo = Transform3D(pos);
-      if ( cnt > 1 ) trafo *= Transform3D(rot);
-      result = trafo * result;
-      pos.SetXYZ(x_elt.x(0), x_elt.y(0), x_elt.z(0));
-      rot.SetComponents(0,0,0);
-      cnt = 1;
+      if      ( flag == 1 ) result = position  * result;
+      else if ( flag == 2 ) result = (position * rotation) * result;
+      Position pos(x_elt.x(0), x_elt.y(0), x_elt.z(0));
+      position = Transform3D(pos);
+      rotation = Transform3D();
+      flag = 1;
     }
     else if ( tag == "rotation" )   {
-      rot = RotationZYX(x_elt.z(0), x_elt.y(0), x_elt.x(0));
-      cnt = 2;
+      rotation = Transform3D(RotationZYX(x_elt.z(0), x_elt.y(0), x_elt.x(0)));
+      flag = 2;
     }
   }
-  if ( cnt > 0 )   {
-    Transform3D trafo = Transform3D(pos);
-    if ( cnt > 1 ) trafo *= Transform3D(rot);
-    result = trafo * result;
-  }
+  if      ( flag == 1 ) result = position  * result;
+  else if ( flag == 2 ) result = (position * rotation) * result;
   return result;
 }
 
diff --git a/DDDetectors/src/SubdetectorAssembly_geo.cpp b/DDDetectors/src/SubdetectorAssembly_geo.cpp
index f1c876a14..b47a61c27 100644
--- a/DDDetectors/src/SubdetectorAssembly_geo.cpp
+++ b/DDDetectors/src/SubdetectorAssembly_geo.cpp
@@ -56,12 +56,6 @@ static Ref_t create_element(Detector& description, xml_h e, Ref_t)  {
     vol = Assembly(det_name);
   }
 
-  for(xml_coll_t c(x_det,_U(composite)); c; ++c)  {
-    xml_dim_t component = c;
-    string nam = component.nameStr();
-    description.declareParent(nam, sdet);
-  }
-
   vol.setAttributes(description,x_det.regionStr(),x_det.limitsStr(),x_det.visStr());
 
   Volume mother = description.pickMotherVolume(sdet);
@@ -75,8 +69,12 @@ static Ref_t create_element(Detector& description, xml_h e, Ref_t)  {
   } else {
     pv = mother.placeVolume(vol);
   }
-
   sdet.setPlacement(pv);
+  for(xml_coll_t c(x_det,_U(composite)); c; ++c)  {
+    xml_dim_t component = c;
+    string nam = component.nameStr();
+    description.declareParent(nam, sdet);
+  }
   return sdet;
 }
 
diff --git a/examples/DDUpgrade/compact/Bcm/BcmUp.xml b/examples/DDUpgrade/compact/Bcm/BcmUp.xml
index 230f8654f..8e30f692b 100644
--- a/examples/DDUpgrade/compact/Bcm/BcmUp.xml
+++ b/examples/DDUpgrade/compact/Bcm/BcmUp.xml
@@ -57,7 +57,7 @@
         <positionRPhiZ r="Bcm:UpWireRad" phi="270.0*degree" z="Bcm:UpWirePosZ"/>
         <rotation x="90.0*degree"/>
         <position/>
-        <rotation x="270.0*degree"/>
+        <rotation z="270.0*degree"/>
       </physvol>
       <physvol logvol="lvBcmUpMount" element="Mount"/>
       <physvol logvol="lvBcmUpContact" element="Contact">
diff --git a/examples/DDUpgrade/compact/Bcm/detector.xml b/examples/DDUpgrade/compact/Bcm/detector.xml
index d7b5ef0d3..93f3ad8ea 100644
--- a/examples/DDUpgrade/compact/Bcm/detector.xml
+++ b/examples/DDUpgrade/compact/Bcm/detector.xml
@@ -15,8 +15,8 @@
 <lccdd>
   <!--
        XML description of the 2 BCM stations
-  <include ref="BcmDown.xml"/>
   -->
+  <include ref="BcmDown.xml"/>
   <include ref="BcmUp.xml"/>
 
 </lccdd>
diff --git a/examples/DDUpgrade/compact/Bls/detector.xml b/examples/DDUpgrade/compact/Bls/detector.xml
index 93e788e2f..34c8f5139 100644
--- a/examples/DDUpgrade/compact/Bls/detector.xml
+++ b/examples/DDUpgrade/compact/Bls/detector.xml
@@ -29,7 +29,7 @@
       </volume>
       <volume name="lvBlsFiber" material="Bls/Resin" vis="Bls:FiberVis">
         <shape type="Trapezoid" z="Bls:FiberCoverH/2"
-               x1="Bls:ScintLSizeX" y1="Bls:ScintSSizeZ" x2="Bls:ScintSSizeX" y2="Bls:ScintSSizeZ"/>
+               x1="Bls:ScintLSizeX/2" y1="Bls:ScintSSizeZ/2" x2="Bls:ScintSSizeX/2" y2="Bls:ScintSSizeZ/2"/>
       </volume>
       <volume name="lvBlsRing" material="Bls/Vinyp" vis="Bls:PassiveVis">
         <shape type="Tube" dz="Bls:RingH/2" rmin="Bls:RingInnerD / 2.0 + Bls:GeometryPrecision" rmax="Bls:RingOuterD / 2.0"/>
@@ -104,15 +104,15 @@
 <!--
 -->
      <physvol element="Bls" id="3" logvol="lvBls34" >
-       <position z="-Bls:OffsetZ" x="Bls:OffsetX34" y="Bls:OffsetY34" />
+       <position z="-Bls:OffsetZ" x="Bls:OffsetX34"   y="Bls:OffsetY34" />
        <rotation z="-90.0*degree" />
      </physvol>
      <physvol element="Bls" id="4" logvol="lvBls34">
-       <position z="-Bls:OffsetZ" x="Bls:OffsetX34" y="-Bls:OffsetY34" />
+       <position z="-Bls:OffsetZ" x="Bls:OffsetX34"   y="-Bls:OffsetY34" />
        <rotation z="-90.0*degree" />
      </physvol>
      <physvol element="Bls" id="5" logvol="lvBls56" >
-       <position z="-Bls:OffsetZ" x="Bls:OffsetX56"  y="Bls:OffsetY56"/>
+       <position z="-Bls:OffsetZ" x="Bls:OffsetX56"   y="Bls:OffsetY56"/>
        <rotation z="-90*degree"/>
      </physvol>
      <physvol element="Bls" id="6" logvol="lvBls56">
@@ -123,7 +123,7 @@
        <position z="-Bls:OffsetZ" x="-Bls:OffsetX78"  y="Bls:OffsetY78"/>
      </physvol>
      <physvol element="Bls" id="8" logvol="lvBls78">
-       <position z="-Bls:OffsetZ" x="Bls:OffsetX78"  y="Bls:OffsetY78"/>
+       <position z="-Bls:OffsetZ" x="Bls:OffsetX78"   y="Bls:OffsetY78"/>
      </physvol>
 <!--
 -->
diff --git a/examples/DDUpgrade/compact/Bls/parameters.xml b/examples/DDUpgrade/compact/Bls/parameters.xml
index 9e19046d9..2d70f81b7 100644
--- a/examples/DDUpgrade/compact/Bls/parameters.xml
+++ b/examples/DDUpgrade/compact/Bls/parameters.xml
@@ -71,23 +71,23 @@
 
     <material name="Bls/Steel">
       <D type="density" value="7.87" unit="g/cm3"/>
-      <component name="Iron" fractionmass="0.97" />
-      <component name="Carbon" fractionmass="0.03" />
+      <component name="Iron"      fraction="0.97" />
+      <component name="Carbon"    fraction="0.03" />
     </material>
 
     <material name="Bls/Resin">
       <D type="density" value="0.04" unit="g/cm3"/>
-      <component name="Carbon" natoms="36" />
-      <component name="Hydrogen" natoms="4" />
-      <component name="Oxygen" natoms="10" />
-      <component name="Nitrogen" natoms="1" />
+      <component name="Carbon"    natoms="36" />
+      <component name="Hydrogen"  natoms="4" />
+      <component name="Oxygen"    natoms="10" />
+      <component name="Nitrogen"  natoms="1" />
     </material>
 
     <material name="Bls/Vinyp">
       <D type="density" value="1.38" unit="g/cm3"/>
-      <component name="Carbon" natoms="2" />
-      <component name="Hydrogen" natoms="3" />
-      <component name="Chlorine" natoms="1" />
+      <component name="Carbon"    natoms="2" />
+      <component name="Hydrogen"  natoms="3" />
+      <component name="Chlorine"  natoms="1" />
     </material>
 
     <material name="Bls/Alu">
diff --git a/examples/DDUpgrade/compact/test.xml b/examples/DDUpgrade/compact/test.xml
index 0f445d36c..960a3f2f5 100644
--- a/examples/DDUpgrade/compact/test.xml
+++ b/examples/DDUpgrade/compact/test.xml
@@ -62,10 +62,11 @@
     <constant name="Bls:ID"         value="30"/>
     <constant name="BcmUp:ID"       value="22"/>
     <constant name="BcmDown:ID"     value="23"/>
-
     <constant name="Bls:Parent"     value="/world/BeforeMagnetRegion/BeforeVelo" type="string"/>
     <constant name="BcmUp:Parent"   value="/world/BeforeMagnetRegion/BeforeVelo" type="string"/>
     <constant name="BcmDown:Parent" value="/world/MagnetRegion" type="string"/>
+<!--
+-->
 <!--
     <constant name="BcmUp:Parent"   value="/world" type="string"/>
     <constant name="BcmDown:Parent" value="/world" type="string"/>
@@ -84,12 +85,13 @@
   <include ref="Magnet/parameters.xml"/>
 
   <include ref="Regions/detector.xml"/>
-  <include ref="Bcm/detector.xml"/>
   <include ref="Bls/detector.xml"/>
-  <include ref="Magnet/detector.xml"/>
 <!--
+  <include ref="Bcm/detector.xml"/>
+  <include ref="Magnet/detector.xml"/>
   <include ref="GdmlImports.xml"/>
 -->
+  <include ref="Bcm/detector.xml"/>
 
   <detectors>
   </detectors>
-- 
GitLab