diff --git a/fairroot/base/sim/FairModule.cxx b/fairroot/base/sim/FairModule.cxx index 0d15912d5e..acf2856efd 100644 --- a/fairroot/base/sim/FairModule.cxx +++ b/fairroot/base/sim/FairModule.cxx @@ -26,7 +26,7 @@ #include "FairVolumeList.h" // for FairVolumeList #include // for TBuffer, operator<<, etc -#include // for TIter +#include // for TIter, TRangeDynCast #include // for TDirectory::TContext #include // for TFile #include // for TGeoManager, gGeoManager @@ -36,7 +36,9 @@ #include // for TKey #include // for TObjArray #include // for TObject +#include // for TSeqCollection #include // for TSystem, gSystem +#include #ifdef ROOT_HAS_GDML #include @@ -46,6 +48,7 @@ #include // for strcmp, strlen #include #include +#include // for std::ignore thread_local std::vector FairModule::fAllSensitiveVolumes; @@ -197,7 +200,15 @@ void FairModule::SetGeometryFileName(TString fname, TString) fgeoName = ""; } -void FairModule::ProcessNodes(TList* aList) +void FairModule::RegisterSensitiveVolume(FairVolume& vol) +{ + vol.setModId(fModId); + vol.SetModule(this); + fAllSensitiveVolumes.push_back(&vol); + ++fNbOfSensitiveVol; +} + +void FairModule::ProcessNodes(TList* nodes) { if (FairMCApplicationState::kConstructGeometry != FairMCApplication::Instance()->GetState()) { LOG(fatal) << "Detected call to FairModule::ProcessNodes() \ @@ -210,55 +221,53 @@ void FairModule::ProcessNodes(TList* aList) vList = new FairVolumeList(); } - TListIter iter(aList); - FairGeoNode* node = nullptr; - FairGeoNode* MotherNode = nullptr; - FairVolume* volume = nullptr; - FairRuntimeDb* rtdb = FairRun::Instance()->GetRuntimeDb(); - FairGeoParSet* par = static_cast(rtdb->getContainer("FairGeoParSet")); - TObjArray* fNodes = par->GetGeoNodes(); - while ((node = static_cast(iter.Next()))) { - - node->calcLabTransform(); - MotherNode = node->getMotherNode(); - volume = new FairVolume(node->getTruncName(), fNbOfVolumes++); + auto rtdb = FairRun::Instance()->GetRuntimeDb(); + auto par = static_cast(rtdb->getContainer("FairGeoParSet")); + TSeqCollection* parNodes = par->GetGeoNodes(); + for (auto node : TRangeDynCast(nodes)) { + if (!node) { + continue; + } + std::ignore = node->calcLabTransform(); + + auto nodeTruncName = node->getTruncName(); + auto volume = std::make_unique(nodeTruncName, fNbOfVolumes); volume->setRealName(node->GetName()); - vList->addVolume(volume); - volume->setGeoNode(node); - volume->setCopyNo(node->getCopyNo()); - if (MotherNode != 0) { - volume->setMotherId(node->getMCid()); - volume->setMotherCopyNo(MotherNode->getCopyNo()); + auto addedVol = vList->addVolume(std::move(volume)); + if (!addedVol) { + LOG(warn) << "Skipping further processing of FairGeoNode " << nodeTruncName + << ". It is already present in the global volume list."; + continue; + } + ++fNbOfVolumes; + addedVol->setGeoNode(node); + addedVol->setCopyNo(node->getCopyNo()); + + auto motherNode = node->getMotherNode(); + if (motherNode) { + addedVol->setMotherId(node->getMCid()); + addedVol->setMotherCopyNo(motherNode->getCopyNo()); } - FairGeoVolume* aVol = nullptr; if (node->isSensitive() && fActive) { - volume->setModId(fModId); - volume->SetModule(this); - fAllSensitiveVolumes.push_back(volume); - aVol = dynamic_cast(node); - fNodes->AddLast(aVol); - fNbOfSensitiveVol++; + RegisterSensitiveVolume(*addedVol); + parNodes->AddLast(node); } } } -void FairModule::AddSensitiveVolume(TGeoVolume* v) +void FairModule::AddSensitiveVolume(TGeoVolume* vol) { - LOG(debug2) << "AddSensitiveVolume " << v->GetName(); - - // Only register volumes which are not already registered - // Otherwise the stepping will be slowed down - if (!vList->findObject(v->GetName())) { - FairVolume* volume = nullptr; - volume = new FairVolume(v->GetName(), fNbOfVolumes++); - vList->addVolume(volume); - volume->setModId(fModId); - volume->SetModule(this); - fAllSensitiveVolumes.push_back(volume); - fNbOfSensitiveVol++; + auto volName = vol->GetName(); + LOG(debug2) << "AddSensitiveVolume " << volName; + + auto addedVol = vList->addVolume(std::make_unique(volName, fNbOfVolumes)); + if (!addedVol) { + return; } + ++fNbOfVolumes; + RegisterSensitiveVolume(*addedVol); } FairVolume* FairModule::getFairVolume(FairGeoNode* fN) diff --git a/fairroot/base/sim/FairModule.h b/fairroot/base/sim/FairModule.h index 7ca7b11ea4..8281892f83 100644 --- a/fairroot/base/sim/FairModule.h +++ b/fairroot/base/sim/FairModule.h @@ -158,6 +158,8 @@ class FairModule : public TNamed void ReAssignMediaId(); void swap(FairModule& other) throw(); + void RegisterSensitiveVolume(FairVolume&); + protected: FairModule(const FairModule&); FairModule& operator=(const FairModule&); diff --git a/fairroot/base/sim/FairVolumeList.cxx b/fairroot/base/sim/FairVolumeList.cxx index a2158cfcab..519cd9b39f 100644 --- a/fairroot/base/sim/FairVolumeList.cxx +++ b/fairroot/base/sim/FairVolumeList.cxx @@ -12,68 +12,35 @@ #include "FairVolumeList.h" -#include "FairLogger.h" // for logging -#include "FairVolume.h" // for FairVolume - -FairVolumeList::FairVolumeList() - : TObject() - , fData(new TObjArray()) -{} - -FairVolumeList::~FairVolumeList() -{ - if (fData) { - fData->Delete(); - delete fData; - } -} - -FairVolume* FairVolumeList::getVolume(TString* name) +FairVolume* FairVolumeList::getVolume(const TString& name) { + auto obj = findObject(name); - TObject* obj = findObject(*name); if (obj) { - LOG(info) << "FairVolume getVolume " << name->Data() << "found"; + LOG(info) << "FairVolume getVolume " << name << " found"; } return static_cast(obj); } -Int_t FairVolumeList::getVolumeId(TString* name) +Int_t FairVolumeList::getVolumeId(const TString& name) { - FairVolume* vol = getVolume(name); + auto vol = getVolume(name); - if (vol) { - return vol->getVolumeId(); - } else { - return -111; - } + return vol ? vol->getVolumeId() : fgkNotFound; } -FairVolume* FairVolumeList::findObject(TString name) +FairVolume* FairVolumeList::addVolume(std::unique_ptr vol) { - FairVolume* obj = nullptr; + auto vol_found = findObject(vol->GetName()); - for (int i = 0; i < fData->GetEntriesFast(); i++) { - obj = static_cast(fData->At(i)); - if (obj) { - if (obj->GetName() == name) { - return obj; - } - } + if (vol_found) { + LOG(error) << "FairVolumeList element: " << vol->GetName() << " VolId : " << vol->getVolumeId() + << " already defined " << vol_found->getVolumeId(); + return nullptr; } - return nullptr; -} - -void FairVolumeList::addVolume(FairVolume* elem) -{ - FairVolume* v = findObject(elem->GetName()); - - if (v) { - LOG(error) << "FairVolumeList element: " << elem->GetName() << " VolId : " << elem->getVolumeId() - << " already defined " << v->getVolumeId(); - } else { - fData->Add(elem); - } + auto vol_added = vol.get(); + fData.Add(vol.release()); + return vol_added; } diff --git a/fairroot/base/sim/FairVolumeList.h b/fairroot/base/sim/FairVolumeList.h index ebca2ea32c..76a3e23af5 100644 --- a/fairroot/base/sim/FairVolumeList.h +++ b/fairroot/base/sim/FairVolumeList.h @@ -1,5 +1,5 @@ /******************************************************************************** - * Copyright (C) 2014-2023 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH * + * Copyright (C) 2014-2024 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH * * * * This software is distributed under the terms of the * * GNU Lesser General Public Licence (LGPL) version 3, * @@ -14,6 +14,8 @@ #include // for TObjArray #include // for TObject #include // for TString +#include // for assert +#include // for std::unique_ptr /** * This Object is only used for internal book keeping! @@ -25,24 +27,39 @@ class FairVolumeList : public TObject { private: - TObjArray* fData; - FairVolumeList(const FairVolumeList&); - FairVolumeList& operator=(const FairVolumeList&); + TObjArray fData; public: - FairVolumeList(); - ~FairVolumeList() override; + FairVolumeList() { fData.SetOwner(kTRUE); } + FairVolumeList(const FairVolumeList&) = delete; + FairVolumeList& operator=(const FairVolumeList&) = delete; + ~FairVolumeList() override = default; - FairVolume* getVolume(TString* name); - Int_t getVolumeId(TString* name); + static constexpr Int_t fgkNotFound = -111; - FairVolume* findObject(TString name); - void addVolume(FairVolume* elem); + FairVolume* getVolume(const TString& name); + [[deprecated]] FairVolume* getVolume(TString* name) + { + assert(name); + return getVolume(*name); + } - Int_t getEntries() { return fData->GetEntries(); } - FairVolume* At(Int_t pos) { return (dynamic_cast(fData->At(pos))); } + Int_t getVolumeId(const TString& name); + [[deprecated]] Int_t getVolumeId(TString* name) + { + assert(name); + return getVolumeId(*name); + } - ClassDefOverride(FairVolumeList, 1); + FairVolume* findObject(const TString& name) { return static_cast(fData.FindObject(name.Data())); } + + [[nodiscard]] FairVolume* addVolume(std::unique_ptr vol); + [[deprecated]] FairVolume* addVolume(FairVolume* elem) { return addVolume(std::unique_ptr(elem)); } + + Int_t getEntries() { return fData.GetEntries(); } + FairVolume* At(Int_t pos) { return (dynamic_cast(fData.At(pos))); } + + ClassDefOverride(FairVolumeList, 0); }; #endif // FAIR_VOLUMELIST_H diff --git a/tests/base/CMakeLists.txt b/tests/base/CMakeLists.txt index ce505ec031..7ea4526995 100644 --- a/tests/base/CMakeLists.txt +++ b/tests/base/CMakeLists.txt @@ -1,5 +1,5 @@ ################################################################################ -# Copyright (C) 2023 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH # +# Copyright (C) 2023-2024 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH # # # # This software is distributed under the terms of the # # GNU Lesser General Public Licence (LGPL) version 3, # @@ -8,10 +8,7 @@ fairroot_add_catch2_test_suite(Base SOURCES - dummy_example.cxx # remove me once first real tests are added - # add more test files if appropriate + sim/test_FairVolumeList.cxx - # DEPENDENCIES - # FairRoot::Base - # list all the targets to build the test suite here + DEPENDENCIES FairRoot::Base ) diff --git a/tests/base/dummy_example.cxx b/tests/base/dummy_example.cxx deleted file mode 100644 index da797787a9..0000000000 --- a/tests/base/dummy_example.cxx +++ /dev/null @@ -1,49 +0,0 @@ -/******************************************************************************** - * Copyright (C) 2023 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH * - * * - * This software is distributed under the terms of the * - * GNU Lesser General Public Licence (LGPL) version 3, * - * copied verbatim in the file "LICENSE" * - ********************************************************************************/ - -#include -#include - -// shamelessly stolen from https://github.com/catchorg/Catch2/blob/devel/docs/tutorial.md#test-cases-and-sections -TEST_CASE("vectors can be sized and resized") -{ - - std::vector v(5); - - REQUIRE(v.size() == 5); - REQUIRE(v.capacity() >= 5); - - SECTION("resizing bigger changes size and capacity") - { - v.resize(10); - - REQUIRE(v.size() == 10); - REQUIRE(v.capacity() >= 10); - } - SECTION("resizing smaller changes size but not capacity") - { - v.resize(0); - - REQUIRE(v.size() == 0); - REQUIRE(v.capacity() >= 5); - } - SECTION("reserving bigger changes capacity but not size") - { - v.reserve(10); - - REQUIRE(v.size() == 5); - REQUIRE(v.capacity() >= 10); - } - SECTION("reserving smaller does not change size or capacity") - { - v.reserve(0); - - REQUIRE(v.size() == 5); - REQUIRE(v.capacity() >= 5); - } -} diff --git a/tests/base/sim/test_FairVolumeList.cxx b/tests/base/sim/test_FairVolumeList.cxx new file mode 100644 index 0000000000..7b7badcf72 --- /dev/null +++ b/tests/base/sim/test_FairVolumeList.cxx @@ -0,0 +1,52 @@ +/******************************************************************************** + * Copyright (C) 2024 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH * + * * + * This software is distributed under the terms of the * + * GNU Lesser General Public Licence (LGPL) version 3, * + * copied verbatim in the file "LICENSE" * + ********************************************************************************/ + +#include +#include +#include +#include +#include +#include + +static auto MakeVolume(std::string_view name) +{ + return std::make_unique(TString{name}); +} + +TEST_CASE("FairVolumeList") +{ + FairVolumeList volumes; + + SECTION("is a set") + { + auto a = MakeVolume("a"); + auto* a_ptr = a.get(); + + REQUIRE(volumes.addVolume(std::move(a))); + REQUIRE(volumes.addVolume(MakeVolume("b"))); + REQUIRE(volumes.addVolume(MakeVolume("a")) == nullptr); + + REQUIRE(volumes.getEntries() == 2); + REQUIRE(volumes.getVolume("a") == a_ptr); + } + + SECTION("handles reading a non-existent volume") + { + REQUIRE(volumes.getVolume("z") == nullptr); + REQUIRE(volumes.getVolumeId("z") == FairVolumeList::fgkNotFound); + } + + SECTION("adding volumes returns added vol or nullptr") + { + auto a = MakeVolume("a"); + auto* a_ptr = a.get(); + + REQUIRE(volumes.addVolume(std::move(a)) == a_ptr); + REQUIRE(volumes.addVolume(MakeVolume("a")) == nullptr); + } +}