From be307864cd59c6cee5d1dff755ec2873a6f3662e Mon Sep 17 00:00:00 2001 From: "Alexander J. Pfleger" <70842573+AJPfleger@users.noreply.github.com> Date: Sat, 23 Nov 2024 11:48:41 +0100 Subject: [PATCH] refactor!: explicit `isPresent()` in cylinder volume builder (#3796) Breaking: The operator `bool()` is removed from the struct `VolumeConfig`. --- .../Acts/Geometry/CylinderVolumeBuilder.hpp | 66 +++++++++---------- Core/src/Geometry/CylinderVolumeBuilder.cpp | 16 ++--- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/Core/include/Acts/Geometry/CylinderVolumeBuilder.hpp b/Core/include/Acts/Geometry/CylinderVolumeBuilder.hpp index 1e09b95e114..4356e26f313 100644 --- a/Core/include/Acts/Geometry/CylinderVolumeBuilder.hpp +++ b/Core/include/Acts/Geometry/CylinderVolumeBuilder.hpp @@ -65,9 +65,9 @@ struct VolumeConfig { /// Adapt to the dimensions of another config in Z /// it will take the maximum/minimum values and just overwrite them /// - /// @param [in] lConfig is the config to which it should be adapded + /// @param [in] lConfig is the config to which it should be adapted void adaptZ(const VolumeConfig& lConfig) { - if (lConfig) { + if (lConfig.present) { zMin = std::min(zMin, lConfig.zMin); zMax = std::max(zMax, lConfig.zMax); } @@ -76,9 +76,9 @@ struct VolumeConfig { /// Adapt to the dimensions of another config in R /// it will take the maximum/minimum values and just overwrite them /// - /// @param [in] lConfig is the config to which it should be adapded + /// @param [in] lConfig is the config to which it should be adapted void adaptR(const VolumeConfig& lConfig) { - if (lConfig) { + if (lConfig.present) { rMin = std::min(rMin, lConfig.rMin); rMax = std::max(rMax, lConfig.rMax); } @@ -87,7 +87,7 @@ struct VolumeConfig { /// Adapt to the dimensions of another config /// it will take the maximum/minimum values and just overwrite them /// - /// @param [in] lConfig is the config to which it should be adapded + /// @param [in] lConfig is the config to which it should be adapted void adapt(const VolumeConfig& lConfig) { adaptZ(lConfig); adaptR(lConfig); @@ -184,9 +184,6 @@ struct VolumeConfig { sl << rMin << ", " << rMax << " / " << zMin << ", " << zMax; return sl.str(); } - - /// Conversion operator to bool - operator bool() const { return present; } }; /// @brief The WrappingSetup that is happening here @@ -222,39 +219,40 @@ struct WrappingConfig { containerVolumeConfig.present = true; std::string wConditionAddon = ""; // if we have more than one config present - if ((nVolumeConfig && cVolumeConfig) || (cVolumeConfig && pVolumeConfig) || - (nVolumeConfig && pVolumeConfig)) { + if ((nVolumeConfig.present && cVolumeConfig.present) || + (cVolumeConfig.present && pVolumeConfig.present) || + (nVolumeConfig.present && pVolumeConfig.present)) { wCondition = Wrapping; wConditionScreen = "grouped to "; } // adapt the new volume config to the existing configs - if (nVolumeConfig) { + if (nVolumeConfig.present) { containerVolumeConfig.adapt(nVolumeConfig); wConditionScreen += "[n]"; } - if (cVolumeConfig) { + if (cVolumeConfig.present) { containerVolumeConfig.adapt(cVolumeConfig); wConditionScreen += "[c]"; } - if (pVolumeConfig) { + if (pVolumeConfig.present) { containerVolumeConfig.adapt(pVolumeConfig); wConditionScreen += "[p]"; } // adapt the external one - if (externalVolumeConfig) { + if (externalVolumeConfig.present) { containerVolumeConfig.adapt(externalVolumeConfig); } // attach the volume configs - if (nVolumeConfig && cVolumeConfig) { + if (nVolumeConfig.present && cVolumeConfig.present) { nVolumeConfig.midPointAttachZ(cVolumeConfig); } - if (cVolumeConfig && pVolumeConfig) { + if (cVolumeConfig.present && pVolumeConfig.present) { cVolumeConfig.midPointAttachZ(pVolumeConfig); } // adapt r afterwards // - easy if no existing volume // - possible if no central volume - if (!existingVolumeConfig || !cVolumeConfig) { + if (!existingVolumeConfig.present || !cVolumeConfig.present) { nVolumeConfig.adaptR(containerVolumeConfig); cVolumeConfig.adaptR(containerVolumeConfig); pVolumeConfig.adaptR(containerVolumeConfig); @@ -265,17 +263,19 @@ struct WrappingConfig { void wrapInsertAttach() { // action is only needed if an existing volume // is present - if (existingVolumeConfig) { + if (existingVolumeConfig.present) { // 0 - simple attachment case - if (!cVolumeConfig) { + if (!cVolumeConfig.present) { // check if it can be easily attached - if (nVolumeConfig && nVolumeConfig.zMax < existingVolumeConfig.zMin) { + if (nVolumeConfig.present && + nVolumeConfig.zMax < existingVolumeConfig.zMin) { nVolumeConfig.attachZ(existingVolumeConfig); // will attach the new volume(s) wCondition = Attaching; wConditionScreen = "[n attached]"; } - if (pVolumeConfig && pVolumeConfig.zMin > existingVolumeConfig.zMax) { + if (pVolumeConfig.present && + pVolumeConfig.zMin > existingVolumeConfig.zMax) { pVolumeConfig.attachZ(existingVolumeConfig); // will attach the new volume(s) wCondition = Attaching; @@ -385,9 +385,9 @@ struct WrappingConfig { fGapVolumeConfig.zMax = existingVolumeConfig.zMin; } else { // adapt lower z boundary - if (nVolumeConfig) { + if (nVolumeConfig.present) { nVolumeConfig.zMin = existingVolumeConfig.zMin; - } else if (cVolumeConfig) { + } else if (cVolumeConfig.present) { cVolumeConfig.zMin = existingVolumeConfig.zMin; } } @@ -399,9 +399,9 @@ struct WrappingConfig { sGapVolumeConfig.zMax = referenceVolume.zMax; } else { // adapt higher z boundary - if (pVolumeConfig) { + if (pVolumeConfig.present) { pVolumeConfig.zMax = existingVolumeConfig.zMax; - } else if (cVolumeConfig) { + } else if (cVolumeConfig.present) { cVolumeConfig.zMax = existingVolumeConfig.zMax; } } @@ -414,31 +414,31 @@ struct WrappingConfig { std::string toString() const { // for screen output std::stringstream sl; - if (containerVolumeConfig) { + if (containerVolumeConfig.present) { sl << "New container built with configuration: " << containerVolumeConfig.toString() << '\n'; } // go through the new ones first - if (nVolumeConfig) { + if (nVolumeConfig.present) { sl << " - n: Negative Endcap, current configuration: " << nVolumeConfig.toString() << '\n'; } - if (cVolumeConfig) { + if (cVolumeConfig.present) { sl << " - c: Barrel, current configuration: " << cVolumeConfig.toString() << '\n'; } - if (pVolumeConfig) { + if (pVolumeConfig.present) { sl << " - p: Negative Endcap, current configuration: " << pVolumeConfig.toString() << '\n'; } - if (existingVolumeConfig) { + if (existingVolumeConfig.present) { sl << "Existing volume with configuration: " << existingVolumeConfig.toString() << '\n'; - if (fGapVolumeConfig) { + if (fGapVolumeConfig.present) { sl << " - g1: First gap volume, configuration : " << fGapVolumeConfig.toString() << '\n'; } - if (sGapVolumeConfig) { + if (sGapVolumeConfig.present) { sl << " - g2: Second gap volume, configuration : " << sGapVolumeConfig.toString() << '\n'; } @@ -452,7 +452,7 @@ struct WrappingConfig { /// @class CylinderVolumeBuilder /// -/// A volume builder to be used for building a concentrical cylindrical volumes +/// A volume builder to be used for building concentric cylinder volumes /// - a) configured volume /// - b) wrapping around a cylindrical/disk layer config /// diff --git a/Core/src/Geometry/CylinderVolumeBuilder.cpp b/Core/src/Geometry/CylinderVolumeBuilder.cpp index 47de379d9be..4f99a77eabd 100644 --- a/Core/src/Geometry/CylinderVolumeBuilder.cpp +++ b/Core/src/Geometry/CylinderVolumeBuilder.cpp @@ -171,7 +171,7 @@ Acts::CylinderVolumeBuilder::trackingVolume( } std::string layerConfiguration = "|"; - if (wConfig.nVolumeConfig) { + if (wConfig.nVolumeConfig.present) { // negative layers are present ACTS_VERBOSE("Negative layers are present: rmin, rmax | zmin, zmax = " << wConfig.nVolumeConfig.toString()); @@ -185,7 +185,7 @@ Acts::CylinderVolumeBuilder::trackingVolume( // add to the string output layerConfiguration += " Negative Endcap |"; } - if (wConfig.cVolumeConfig) { + if (wConfig.cVolumeConfig.present) { // central layers are present ACTS_VERBOSE("Central layers are present: rmin, rmax | zmin, zmax = " << wConfig.cVolumeConfig.toString()); @@ -199,7 +199,7 @@ Acts::CylinderVolumeBuilder::trackingVolume( // add to the string output layerConfiguration += " Barrel |"; } - if (wConfig.pVolumeConfig) { + if (wConfig.pVolumeConfig.present) { // positive layers are present ACTS_VERBOSE("Positive layers are present: rmin, rmax | zmin, zmax = " << wConfig.pVolumeConfig.toString()); @@ -226,7 +226,7 @@ Acts::CylinderVolumeBuilder::trackingVolume( << '\n' << wConfig.toString()); // now let's understand the wrapping if needed - if (wConfig.existingVolumeConfig) { + if (wConfig.existingVolumeConfig.present) { wConfig.wrapInsertAttach(); ACTS_VERBOSE("Configuration after wrapping, insertion, attachment " << '\n' @@ -242,7 +242,7 @@ Acts::CylinderVolumeBuilder::trackingVolume( auto tvHelper = m_cfg.trackingVolumeHelper; // the barrel is always created auto barrel = - wConfig.cVolumeConfig + wConfig.cVolumeConfig.present ? tvHelper->createTrackingVolume( gctx, wConfig.cVolumeConfig.layers, wConfig.cVolumeConfig.volumes, m_cfg.volumeMaterial, @@ -258,7 +258,7 @@ Acts::CylinderVolumeBuilder::trackingVolume( [&](VolumeConfig& centralConfig, VolumeConfig& endcapConfig, const std::string& endcapName) -> MutableTrackingVolumePtr { // No config - no volume - if (!endcapConfig) { + if (!endcapConfig.present) { return nullptr; } // Check for ring layout @@ -478,7 +478,7 @@ Acts::CylinderVolumeBuilder::trackingVolume( if (existingVolumeCp) { // Check if gaps are needed std::vector existingContainer; - if (wConfig.fGapVolumeConfig) { + if (wConfig.fGapVolumeConfig.present) { // create the gap volume auto fGap = tvHelper->createGapTrackingVolume( gctx, wConfig.cVolumeConfig.volumes, m_cfg.volumeMaterial, @@ -489,7 +489,7 @@ Acts::CylinderVolumeBuilder::trackingVolume( existingContainer.push_back(fGap); } existingContainer.push_back(existingVolumeCp); - if (wConfig.sGapVolumeConfig) { + if (wConfig.sGapVolumeConfig.present) { // create the gap volume auto sGap = tvHelper->createGapTrackingVolume( gctx, wConfig.cVolumeConfig.volumes, m_cfg.volumeMaterial,