Skip to content

Read correct size of TClonesArray collection value type #19140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tree/dataframe/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ endif()

ROOT_ADD_GTEST(dataframe_display dataframe_display.cxx LIBRARIES ROOTDataFrame)
ROOT_ADD_GTEST(dataframe_ranges dataframe_ranges.cxx LIBRARIES ROOTDataFrame)
ROOT_ADD_GTEST(dataframe_leaves dataframe_leaves.cxx LIBRARIES ROOTDataFrame)
ROOT_ADD_GTEST(dataframe_leaves dataframe_leaves.cxx LIBRARIES ROOTDataFrame Physics)
ROOT_ADD_GTEST(dataframe_resptr dataframe_resptr.cxx LIBRARIES ROOTDataFrame)
ROOT_ADD_GTEST(dataframe_take dataframe_take.cxx LIBRARIES ROOTDataFrame)
ROOT_ADD_GTEST(dataframe_entrylist dataframe_entrylist.cxx LIBRARIES ROOTDataFrame)
Expand Down
55 changes: 55 additions & 0 deletions tree/dataframe/test/dataframe_leaves.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,23 @@
#include "TTree.h"
#include "gtest/gtest.h"

#include <memory>
#include <TClonesArray.h>
#include <TLorentzVector.h>
#include <ROOT/RVec.hxx>
#include <ROOT/TestSupport.hxx>

using namespace ROOT::VecOps;

template <typename T0, typename T1>
void expect_vec_float_eq(const T0 &v1, const T1 &v2)
{
ASSERT_EQ(v1.size(), v2.size()) << "Vectors 'v1' and 'v2' are of unequal length";
for (std::size_t i = 0ull; i < v1.size(); ++i) {
EXPECT_FLOAT_EQ(v1[i], v2[i]) << "Vectors 'v1' and 'v2' differ at index " << i;
}
}

struct BrVal {
Float_t a;
Int_t i;
Expand Down Expand Up @@ -72,3 +86,44 @@ TEST(RDFLeaves, OneLeafWithDotNameClass)
EXPECT_EQ(*res, 40);
}

TEST(RDFLeaves, LeafFromTClonesArray)
{
// Regression test for https://github.com/root-project/root/issues/19104
ROOT::TestSupport::CheckDiagsRAII diagRAII;
diagRAII.requiredDiag(kWarning, "TTree::Bronch",
"Using split mode on a class: TLorentzVector with a custom Streamer");
#ifndef NDEBUG
diagRAII.requiredDiag(
kWarning, "RTreeColumnReader::Get",
"Branch ca.fE hangs from a non-split branch. A copy is being performed in order to properly read the content.");
#endif
constexpr static auto fName{"leaffromtclonesarray.root"};
struct Dataset {
Dataset()
{
auto f = std::make_unique<TFile>(fName, "recreate");
auto t = std::make_unique<TTree>("t", "t");
auto ca = std::make_unique<TClonesArray>("TLorentzVector");
auto branchData = ca.get();
auto &caRef = *ca;
t->Branch("ca", &branchData);
// Fill the array with two elements. The issue arose from the fact
// we were not computing the correct size of the correction value
// type, so the wrong offset was used when reading back next elements
// of the array
new (caRef[0]) TLorentzVector(0, 0, 0, 42.42);
new (caRef[1]) TLorentzVector(0, 0, 0, 84.84);
t->Fill();
f->Write();
}

~Dataset() { std::remove(fName); }
} _;

std::vector expected{42.42, 84.84};

ROOT::RDataFrame df("t", fName);
auto resultptr = df.Take<ROOT::RVecD>("ca.fE");
ASSERT_EQ(resultptr->size(), 1);
expect_vec_float_eq(expected, resultptr->at(0));
}
5 changes: 5 additions & 0 deletions tree/treeplayer/inc/TTreeReaderUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ namespace Internal {
TVirtualCollectionReader() : fReadStatus(TTreeReaderValueBase::kReadNothingYet) {}

virtual ~TVirtualCollectionReader();
TVirtualCollectionReader(const TVirtualCollectionReader &) = delete;
TVirtualCollectionReader &operator=(const TVirtualCollectionReader &) = delete;
TVirtualCollectionReader(TVirtualCollectionReader &&) = delete;
TVirtualCollectionReader &operator=(TVirtualCollectionReader &&) = delete;

virtual size_t GetSize(Detail::TBranchProxy*) = 0;
virtual void* At(Detail::TBranchProxy*, size_t /*idx*/) = 0;
virtual bool IsContiguous(Detail::TBranchProxy *) = 0;
Expand Down
1 change: 1 addition & 0 deletions tree/treeplayer/src/TBranchProxy.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ bool ROOT::Detail::TBranchProxy::Setup()
if (!fIsMember) fIsClone = true;
fIsaPointer = false;
fWhere = be->GetObject();
fValueSize = static_cast<TClonesArray *>(fWhere)->GetClass()->Size();

} else if (be->GetType()==4) {
// top level TClonesArray
Expand Down
24 changes: 21 additions & 3 deletions tree/treeplayer/src/TTreeReaderArray.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ using namespace ROOT::Internal;
// Reader interface for clones arrays
class TClonesReader : public TVirtualCollectionReader {
public:
~TClonesReader() override {}
TClonesReader() = default;
~TClonesReader() override = default;
TClonesReader(const TClonesReader &) = delete;
TClonesReader &operator=(const TClonesReader &) = delete;
TClonesReader(TClonesReader &&) = delete;
TClonesReader &operator=(TClonesReader &&) = delete;

TClonesArray *GetCA(ROOT::Detail::TBranchProxy *proxy)
{
if (!proxy->Read()) {
Expand Down Expand Up @@ -76,8 +82,14 @@ class TClonesReader : public TVirtualCollectionReader {

std::size_t GetValueSize(ROOT::Detail::TBranchProxy *proxy) override
{
auto *ca = GetCA(proxy);
return ca ? ca->GetClass()->Size() : 0;
if (!proxy->Read()) {
fReadStatus = TTreeReaderValueBase::kReadError;
if (!proxy->GetSuppressErrorsForMissingBranch())
Error("TClonesReader::GetValueSize()", "Read error in TBranchProxy.");
return 0;
}
fReadStatus = TTreeReaderValueBase::kReadSuccess;
return proxy->GetValueSize();
}
};

Expand Down Expand Up @@ -444,6 +456,12 @@ class TBasicTypeClonesReader final : public TClonesReader {
public:
TBasicTypeClonesReader(Int_t offsetArg) : fOffset(offsetArg) {}

~TBasicTypeClonesReader() final = default;
TBasicTypeClonesReader(const TBasicTypeClonesReader &) = delete;
TBasicTypeClonesReader &operator=(const TBasicTypeClonesReader &) = delete;
TBasicTypeClonesReader(TBasicTypeClonesReader &&) = delete;
TBasicTypeClonesReader &operator=(TBasicTypeClonesReader &&) = delete;

void *At(ROOT::Detail::TBranchProxy *proxy, size_t idx) override
{
TClonesArray *myClonesArray = GetCA(proxy);
Expand Down
Loading