From afd92ac45770631fce350682bdba5d5b652c89f8 Mon Sep 17 00:00:00 2001 From: vhinger Date: Thu, 16 Jul 2026 11:04:43 +0200 Subject: [PATCH] Bugfix: Separate Rotation and Routing --- include/aare/RemapAlgorithm.hpp | 10 +++---- include/aare/RemapConfig.hpp | 21 +++++++++++--- include/aare/RemapDefs.hpp | 12 +++++++- include/aare/RemapGenerate.hpp | 17 +++++++---- src/RemapAlgorithm.cpp | 50 +++++++++++++++++++++++++++------ src/RemapFormat.cpp | 2 +- 6 files changed, 87 insertions(+), 25 deletions(-) diff --git a/include/aare/RemapAlgorithm.hpp b/include/aare/RemapAlgorithm.hpp index 797f0eb..e1f684c 100644 --- a/include/aare/RemapAlgorithm.hpp +++ b/include/aare/RemapAlgorithm.hpp @@ -6,13 +6,13 @@ namespace aare::remap::algo { constexpr defs::Rotation flip(defs::Rotation r) noexcept { switch (r) { - case defs::Rotation::Normal: - return defs::Rotation::Inverse; - case defs::Rotation::Inverse: - return defs::Rotation::Normal; + case defs::Rotation::Identity: + return defs::Rotation::Rotate180; + case defs::Rotation::Rotate180: + return defs::Rotation::Identity; } - return defs::Rotation::Normal; // or assert(false); + return defs::Rotation::Identity; // or assert(false); } // Is it better to pass defs::SensorGroupConfig const& and return a copy? diff --git a/include/aare/RemapConfig.hpp b/include/aare/RemapConfig.hpp index b826501..79fa92a 100644 --- a/include/aare/RemapConfig.hpp +++ b/include/aare/RemapConfig.hpp @@ -19,12 +19,14 @@ inline constexpr defs::SensorStrixelGeometry StrxP37{.multiplicity = 2, * Default sensor placements ************************************/ inline constexpr defs::SensorPlacement Chip1{ - .placement_on_module{256, 511, 0, 255}, .rotation = defs::Rotation::Normal}; + .placement_on_module{256, 511, 0, 255}, + .rotation = defs::Rotation::Identity}; inline constexpr defs::SensorPlacement Chip6{ .placement_on_module{512, 767, 256, 511}, - .rotation = defs::Rotation::Inverse}; + .rotation = defs::Rotation::Rotate180}; inline constexpr defs::SensorPlacement Quad{ - .placement_on_module{256, 767, 0, 511}, .rotation = defs::Rotation::Normal}; + .placement_on_module{256, 767, 0, 511}, + .rotation = defs::Rotation::Identity}; /************************************ * Single chip, multi-pitch, iLGAD @@ -120,7 +122,7 @@ inline constexpr defs::SensorGroupConfig SingleChipMP_TEW_P18{ inline constexpr defs::SensorPixelGeometry Quad_iLGAD_pix{ .num_pix_x = 512, .num_pix_y = 512, .guardring = {.x = 9, .y = 9}}; -inline constexpr defs::SensorGroupConfig Quad_iLGAD_half{ +inline constexpr defs::SensorGroupConfig Quad_iLGAD_bottomhalf{ .pixel = Quad_iLGAD_pix, .strixel = StrxP25, .placement_on_sensor = {Quad_iLGAD_pix.guardring.x + 2, // 11 @@ -128,6 +130,17 @@ inline constexpr defs::SensorGroupConfig Quad_iLGAD_half{ Quad_iLGAD_pix.guardring.x - 1, // 502 Quad_iLGAD_pix.guardring.y, // 9 (Quad_iLGAD_pix.num_pix_y / 2) - 2}}; // 254 + +inline constexpr defs::SensorGroupConfig Quad_iLGAD_tophalf{ + .pixel = Quad_iLGAD_pix, + .strixel = StrxP25, + .routing = {defs::ColumnModOrdering::Reverse}, + // Adapt placement to be correct! + .placement_on_sensor = { + Quad_iLGAD_pix.guardring.x + 2, // 11 + Quad_iLGAD_pix.num_pix_x - Quad_iLGAD_pix.guardring.x - 1, // 502 + Quad_iLGAD_pix.num_pix_y / 2 + 1, // 257 + Quad_iLGAD_pix.num_pix_y - Quad_iLGAD_pix.guardring.y - 1}}; // 502 } // namespace aare::remap::config::jungfrau /************************************** diff --git a/include/aare/RemapDefs.hpp b/include/aare/RemapDefs.hpp index 9c91cd5..4943e14 100644 --- a/include/aare/RemapDefs.hpp +++ b/include/aare/RemapDefs.hpp @@ -9,7 +9,11 @@ namespace aare::remap::defs { // Orientation of the sensor with respect to the ASICs / the HDI // Normal means lower part of sensor (as in GDS) aligns with lower part of HDI -enum class Rotation : int { Normal = 0, Inverse = 1 }; +enum class Rotation : int { Identity = 0, Rotate180 = 1 }; + +// enum class Transform : int { Identity, MirrorX, MirrorY, Rotate180 }; + +enum class ColumnModOrdering { Forward, Reverse }; struct Guardring { int x; @@ -34,10 +38,15 @@ struct SensorStrixelGeometry { double pitch_um; }; +struct SensorRouting { + ColumnModOrdering mod_order = ColumnModOrdering::Forward; +}; + // Fully characterize a strixel group (contiguous area that will be remapped) struct SensorGroupConfig { SensorPixelGeometry pixel; SensorStrixelGeometry strixel; + SensorRouting routing; InclusiveROI placement_on_sensor; // location of the group in the sensor // coordinate system (or reduced roi) }; @@ -52,6 +61,7 @@ struct SensorPlacement { InclusiveROI placement_on_module; // location of the sensor in full-module // coordinate system Rotation rotation; + // Transform transform; }; // Possibly, this may need to be moved to a dedicated "remapping" class diff --git a/include/aare/RemapGenerate.hpp b/include/aare/RemapGenerate.hpp index 97f9a08..fc5a9db 100644 --- a/include/aare/RemapGenerate.hpp +++ b/include/aare/RemapGenerate.hpp @@ -119,9 +119,16 @@ jungfrau_tew_singlechip_multipitch_strixel_maps(InclusiveROI rx_roi, * Quad, 25 um, iLGAD ************************************/ inline defs::StrixelGroupToPixelMap -jungfrau_ilgad_quadhalf_25um_strixel_map(InclusiveROI rx_roi, - defs::SensorPlacement placement) { - return algo::strixel_to_pixel_map(config::jungfrau::Quad_iLGAD_half, +jungfrau_ilgad_quadbottom_25um_strixel_map(InclusiveROI rx_roi, + defs::SensorPlacement placement) { + return algo::strixel_to_pixel_map(config::jungfrau::Quad_iLGAD_bottomhalf, + placement, rx_roi); +} + +inline defs::StrixelGroupToPixelMap +jungfrau_ilgad_quadtop_25um_strixel_map(InclusiveROI rx_roi, + defs::SensorPlacement placement) { + return algo::strixel_to_pixel_map(config::jungfrau::Quad_iLGAD_tophalf, placement, rx_roi); } @@ -129,8 +136,8 @@ inline std::vector jungfrau_ilgad_quad_25um_strixel_maps(InclusiveROI rx_roi, defs::SensorPlacement placement) { - defs::SensorConfig configs{ - {config::jungfrau::Quad_iLGAD_half, config::jungfrau::Quad_iLGAD_half}}; + defs::SensorConfig configs{{config::jungfrau::Quad_iLGAD_bottomhalf, + config::jungfrau::Quad_iLGAD_tophalf}}; std::vector placements(configs.group_configs.size(), placement); diff --git a/src/RemapAlgorithm.cpp b/src/RemapAlgorithm.cpp index 80c4119..382dfa9 100644 --- a/src/RemapAlgorithm.cpp +++ b/src/RemapAlgorithm.cpp @@ -5,6 +5,7 @@ namespace aare::remap::algo { // Is it better to pass defs::SensorGroupConfig const& and return a copy? +// Better not to, use shift_rotate_roi instead void apply_rotation_shift(defs::SensorGroupConfig &cfg, defs::BondShift bond_shift, defs::Rotation rot) { // Apply physical transforms @@ -12,11 +13,35 @@ void apply_rotation_shift(defs::SensorGroupConfig &cfg, cfg.placement_on_sensor = aare::inclusiveroi::geom::translate( cfg.placement_on_sensor, bond_shift.x, bond_shift.y); - if (rot == defs::Rotation::Inverse) + if (rot == defs::Rotation::Rotate180) cfg.placement_on_sensor = aare::inclusiveroi::geom::mirrorXY( cfg.placement_on_sensor, cfg.pixel.num_pix_x, cfg.pixel.num_pix_y); } +/** + * Apply physical transformations to a sensor-local ROI. + * + * IMPORTANT: + * Bond shifts are applied before rotation. + * The order is intentional because bond shifts are defined in the + * sensor's native coordinate system. + */ +inline InclusiveROI shift_rotate_roi(InclusiveROI roi, + defs::SensorPixelGeometry const &pixel, + defs::BondShift bond_shift, + defs::Rotation rot) { + // Apply physical transforms + if (bond_shift.x != 0 || bond_shift.y != 0) + roi = aare::inclusiveroi::geom::translate(roi, bond_shift.x, + bond_shift.y); + + if (rot == defs::Rotation::Rotate180) + roi = aare::inclusiveroi::geom::mirrorXY(roi, pixel.num_pix_x, + pixel.num_pix_y); + + return roi; +} + defs::StrixelGroupToPixelMap strixel_to_pixel_map(defs::SensorGroupConfig const &group_config, defs::SensorPlacement const &placement, @@ -24,7 +49,7 @@ strixel_to_pixel_map(defs::SensorGroupConfig const &group_config, int multiplicity = group_config.strixel.multiplicity; double pitch = group_config.strixel.pitch_um; - defs::Rotation rot = placement.rotation; + // defs::Rotation rot = placement.rotation; // Helper to make sure that we work with a correct number of strixel columns // (i.e. that we do not map pixel columns if the ncols in ASIC pixel @@ -37,9 +62,13 @@ strixel_to_pixel_map(defs::SensorGroupConfig const &group_config, // Define mod ordering (Normal or Inverse) std::vector mods(multiplicity); - for (int i = 0; i < multiplicity; i++) - mods[i] = i; - if (rot == defs::Rotation::Inverse) + std::iota(mods.begin(), mods.end(), 0); + // This is a problem! Rotation does not invert the mods! + // (only one-axis mirroring does) + // -> Top-half of quad is only mirrored in y! + // if (rot == defs::Rotation::Inverse) + // std::reverse(mods.begin(), mods.end()); + if (group_config.routing.mod_order == defs::ColumnModOrdering::Reverse) std::reverse(mods.begin(), mods.end()); // -- 1) Transform user roi (rx_roi) into sensor-local coordinates @@ -55,17 +84,20 @@ strixel_to_pixel_map(defs::SensorGroupConfig const &group_config, // -- 2a) bond_shift // -- 2b) rotation // -- IMPORTANT: bond_shift BEFORE rotation! - auto group_local = group_config; - apply_rotation_shift(group_local, bond_shift, placement.rotation); + // auto group_local = group_config; + // apply_rotation_shift(group_local, bond_shift, placement.rotation); // -- 2c) AFTER applying the transformations, we can grab the correct // strixel roi - auto roi_group = group_local.placement_on_sensor; + // auto roi_group = group_local.placement_on_sensor; + InclusiveROI roi_group = + shift_rotate_roi(group_config.placement_on_sensor, group_config.pixel, + bond_shift, placement.rotation); // DEBUG std::cout << "DEBUG: Group ROI after transformation (as in local transformation) " - << group_local.placement_on_sensor << '\n'; + << roi_group << '\n'; // -- 3) Compute effective ROI = intersection( roi_user, roi_group ) InclusiveROI eff = inclusiveroi::geom::intersect(roi_user_local, roi_group); diff --git a/src/RemapFormat.cpp b/src/RemapFormat.cpp index 06b68cd..16383b0 100644 --- a/src/RemapFormat.cpp +++ b/src/RemapFormat.cpp @@ -4,7 +4,7 @@ namespace aare::remap::format { static inline std::string toString(defs::Rotation r) { - return (r == defs::Rotation::Normal ? "Normal" : "Inverse"); + return (r == defs::Rotation::Identity ? "Identity" : "Rotate180"); } static inline std::string toString(defs::SensorPixelGeometry const &g) {