InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 660 forks source link

TubeSpatialObject bounding box includes rounded ends even after they are turned off #4657

Open auneri opened 1 month ago

auneri commented 1 month ago

Description

The bounding box for TubeSpatialObject always includes rounded ends.

Steps to Reproduce

tso = itk.TubeSpatialObject.x3.New()
tso.SetEndRounded(False)
for z in (-1, 1):
    tsop = itk.TubeSpatialObjectPoint.x3()
    tsop.SetPositionInObjectSpace((0, 0, z))
    tsop.SetRadiusInObjectSpace(1)
    tso.AddPoint(tsop)
tso.Update()
bbox = tso.GetMyBoundingBoxInWorldSpace()
print(bbox.GetMinimum(), bbox.GetMaximum())

Expected behavior

itkPointD3 ([-1, -1, -1]) itkPointD3 ([1, 1, 1])

Actual behavior

itkPointD3 ([-1, -1, -2]) itkPointD3 ([1, 1, 2])

The results do not change when EndRounded is toggled, though they should. Moreover, the bounding box extent in z increases when RadiusInObjectSpace is increased, which should only be expected when EndRounded is True.

Reproducibility

Happens every time.

Versions

Tested on v5.3 and v5.4rc4.

Environment

Additional Information

github-actions[bot] commented 1 month ago

Thank you for contributing an issue! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜 Also, please check existing open issues and consider discussion on the ITK Discourse. 📖

This is an automatic message. Allow for time for the ITK community to be able to read the issue and comment on it.

dzenanz commented 1 month ago

@aylward can you tackle this one?

aylward commented 1 month ago

It would be very hard (computationally expensive and quirky) to handle all points "near" an end-point as "special." Probably should just document that EndRounded() is always considered True when computing isInside(). More accurately, should remove the "EndRounded()" flag - it is an application/rendering specific option - but that would break backward compatibility.

auneri commented 1 month ago

Thank you for the prompt responses! I want to note that I am using TubeSpatialObject to describe a simple cylinder. I believe there used to be a CylinderSpatialObject, but was later removed. Considering the backward compatibility, I wonder if it would be easier to reintroduce one (without all the added options for "rounded ends" or "root")? One could even consider this as a base class for TubeSpatialObject.

aylward commented 1 month ago

Great idea! And much easier to compute flat ends for a cylinder instead of for a collection of points with arbitrary position and radius per point.

I can create that within a few days. Thanks for the clarifying info!

s

On Mon, May 13, 2024 at 1:49 PM Ali Uneri @.***> wrote:

Thank you for the prompt responses! I want to note that I am using TubeSpatialObject to describe a simple cylinder. I believe there used to be a CylinderSpatialObject, but was later removed. Considering the backward compatibility, I wonder if it would be easier to reintroduce one (without all the added options for "rounded ends" or "root")? One could even consider this as a base class for TubeSpatialObject.

— Reply to this email directly, view it on GitHub https://github.com/InsightSoftwareConsortium/ITK/issues/4657#issuecomment-2108422026, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACEJL6FPNVJ6Q3SMQFTUPTZCD4LPAVCNFSM6AAAAABHS6G4J6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBYGQZDEMBSGY . You are receiving this because you were mentioned.Message ID: @.***>

-- Stephen R. Aylward, Ph.D. Chair, MONAI Advisory Board Senior Director, Strategic Initiatives, Kitware