From e03bcd6a59d07a3b692c21fd1b422d3cb476c9d0 Mon Sep 17 00:00:00 2001
From: Amperiadou Dimitra <63058524+dimitra97@users.noreply.github.com>
Date: Wed, 10 Apr 2024 19:22:12 +0200
Subject: [PATCH] refactor: Change the MultiLayerSurfacesUpdater to use the
 local position and direction (#3041)

This PR introduces some changes in the `MultiLayerSurfacesUpdater` in order to consider the local position and direction from the navigation state and generates the path in the grid based on them. Also, since this Navigation Delegate is used in the `MultiWireStructureBuilder` I pass the transform of the multiLayer in the delegate, in order to be taken into account for the global->local transformations.

(I deleted a previous branch that I have created for this and created a new one, because I had messed up with some files from main)
---
 .../Navigation/MultiLayerSurfacesUpdater.hpp  | 26 +++++++++----------
 .../Detector/MultiWireStructureBuilder.cpp    |  4 ++-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/Core/include/Acts/Navigation/MultiLayerSurfacesUpdater.hpp b/Core/include/Acts/Navigation/MultiLayerSurfacesUpdater.hpp
index 787cad0f22d..ea9a059d44c 100644
--- a/Core/include/Acts/Navigation/MultiLayerSurfacesUpdater.hpp
+++ b/Core/include/Acts/Navigation/MultiLayerSurfacesUpdater.hpp
@@ -34,7 +34,7 @@ class MultiLayerSurfacesUpdaterImpl : public INavigationDelegate {
   /// These are the cast parameters - copied from constructor
   std::array<BinningValue, grid_type::DIM> casts{};
 
-  /// A transform to be applied to the position
+  /// An inverse transform to be applied to the position
   Transform3 transform = Transform3::Identity();
 
   /// @brief  Constructor for a grid based surface attacher
@@ -42,17 +42,20 @@ class MultiLayerSurfacesUpdaterImpl : public INavigationDelegate {
   /// @param icasts is the cast values array
   /// @param itr a transform applied to the global position
   MultiLayerSurfacesUpdaterImpl(
-      grid_type&& igrid, const std::array<BinningValue, grid_type::DIM>& icasts,
+      grid_type igrid, const std::array<BinningValue, grid_type::DIM>& icasts,
       const Transform3& itr = Transform3::Identity())
       : grid(std::move(igrid)), casts(icasts), transform(itr) {}
 
   MultiLayerSurfacesUpdaterImpl() = delete;
 
   void update(const GeometryContext& gctx, NavigationState& nState) const {
+    // get the local position and direction
+    auto lposition = transform * nState.position;
+    auto ldirection = transform.linear() * nState.direction;
+
     auto step = std::sqrt(std::pow(grid.binWidth()[0], 2) +
                           std::pow(grid.binWidth()[1], 2));
-    auto path = pgenerator(nState.position, nState.direction, step,
-                           grid.numLocalBins()[1]);
+    auto path = pgenerator(lposition, ldirection, step, grid.numLocalBins()[1]);
 
     std::vector<const Acts::Surface*> surfCandidates = {};
 
@@ -75,11 +78,8 @@ class MultiLayerSurfacesUpdaterImpl : public INavigationDelegate {
   /// @param position is the position of the update call
   std::array<ActsScalar, grid_type::DIM> castPosition(
       const Vector3& position) const {
-    // Transform into local 3D frame
-    Vector3 tposition = transform * position;
-
     std::array<ActsScalar, grid_type::DIM> casted{};
-    fillCasts(tposition, casted,
+    fillCasts(position, casted,
               std::make_integer_sequence<std::size_t, grid_type::DIM>{});
     return casted;
   }
@@ -125,14 +125,12 @@ struct PathGridSurfacesGenerator {
     std::vector<Vector3> pathCoordinates = {};
     pathCoordinates.reserve(numberOfSteps);
 
-    auto tposition = std::move(startPosition);
-    auto stepSizeY = stepSize * sin(Acts::VectorHelpers::phi(direction));
-    auto stepSizeX = stepSize * cos(Acts::VectorHelpers::phi(direction));
+    Vector3 position = std::move(startPosition);
+    Vector3 step = stepSize * direction;
 
     for (std::size_t i = 0; i < numberOfSteps; i++) {
-      pathCoordinates.push_back(tposition);
-      tposition.y() = tposition.y() + stepSizeY;
-      tposition.x() = tposition.x() + stepSizeX;
+      pathCoordinates.push_back(position);
+      position = position + step;
     }
 
     return pathCoordinates;
diff --git a/Core/src/Detector/MultiWireStructureBuilder.cpp b/Core/src/Detector/MultiWireStructureBuilder.cpp
index 86c8c4375fc..720b6af2c4b 100644
--- a/Core/src/Detector/MultiWireStructureBuilder.cpp
+++ b/Core/src/Detector/MultiWireStructureBuilder.cpp
@@ -72,7 +72,8 @@ class MultiWireInternalStructureBuilder
         isg{internalSurfaces,
             {},
             {m_cfg.binning[0u].binValue, m_cfg.binning[1u].binValue},
-            {m_cfg.binning[0u].expansion, m_cfg.binning[1u].expansion}};
+            {m_cfg.binning[0u].expansion, m_cfg.binning[1u].expansion},
+            m_cfg.transform};
     Acts::Experimental::detail::CenterReferenceGenerator rGenerator;
     Acts::GridAxisGenerators::EqBoundEqBound aGenerator{
         {m_cfg.binning[0u].edges.front(), m_cfg.binning[0u].edges.back()},
@@ -127,6 +128,7 @@ Acts::Experimental::MultiWireStructureBuilder::construct(
   MultiWireInternalStructureBuilder::Config iConfig;
   iConfig.iSurfaces = mCfg.mlSurfaces;
   iConfig.binning = mCfg.mlBinning;
+  iConfig.transform = mCfg.transform.inverse();
   iConfig.auxiliary = "Construct Internal Structure";
 
   Acts::Experimental::DetectorVolumeBuilder::Config dvConfig;