Repository navigation
TMath::IsInside() returns wrong results when Int_t is used #23450
Description
Check duplicate issues.
- Checked for duplicates
Description
With Int_t function returns wrong result in about 50% of the cases.
One probably can change implementation of TMath::IsInside() - swap mult and div operators in expression.
Used only in TText::DistancetoPrimitive() where need to be fixed or replaced by Double_t signature.
Reproducer
void inside()
{
// these are corners of diamond
Int_t xx[4] = { 299, 41, 299, 558 };
Int_t yy[4] = { 531, 678, 825, 678 };
// testing point near center of the diamond 299, 678
auto res = TMath::IsInside(296, 683, 4, xx, yy);
if (res)
std::cout << "Correct\n";
else
std::cerr << "Point close to the center is not inside!\n";
}
ROOT version
master
Installation method
Build from source
Operating system
Linux
Additional context
No response
Metadata
Metadata
Assignees
Type
Projects
- Status
Activity
kr1xh commentedon Sep 22, 2026
I was able to reproduce this with the provided
Int_texample.The issue appears to come from integer division in the interpolation term of
TMath::IsInside(), where(yp-y[i])/(y[j]-y[i])can truncate to zero before the multiplication.I'd like to investigate a fix together with a regression test. In particular, I'll compare the suggested multiplication before division change with an explicit floating point computation, since multiplying the integer differences first could introduce an overflow risk for sufficiently large coordinates.
If there are no objections, I'd be happy to work on this.
linev commentedon Sep 22, 2026
You are welcome!
If possible - please also provide more advanced testing for this functionality.
At the moment it is rudimentary.
Integer arithmetic used mostly for graphics operations so values range between (-10000 .. 10000) is absolutely enough and one do not need have special workarounds for larger integers
kr1xh commentedon Sep 22, 2026
Thanks @linev for confirming the range that resolves my overflow concern, so I'll go with the multiply before divide fix rather than switching to floating point (keeps the integer semantics exact and avoids the extra cast)
Plan:
TMath::IsInside()so(xp[j]-xp[i]) * (y-yp[i])is computed before dividing by(yp[j]-yp[i]).Long64_tbefore the final division, just in case someone passes coordinates outside the documented ±10000 range.Once I'm assigned I'll open a PR with the fix + tests shortly, fr
kr1xh commentedon Sep 23, 2026
Hey @linev ,I solved this issue and opened a PR with a focused fix and regression test:
#23474
The fix changes the edge intersection calculation in
TMath::IsInside()so that multiplication occurs before integer division whenInt_tcoordinates are usedI reproduced the incorrect result with the original implementation and verified the corrected result with the regression case.
linev commentedon Oct 15, 2026
Fixed by #23474