@matt.mccormick In order to ensure that world space supports is added to GetCircles() the way you have in mind, can you please submit a patch for this issue?
I think it would be nice if a spatial object created by GetCircles() would store both the world coordinates and the original grid index coordinates of the center of the circle. Or if it would store the transformation that was applied from grid index to world space. Do you think that’s possible?
@matt.mccormick I guess you don’t have time anymore to create such a patch for ITK 5.0.0 alpha (estimating circles, based on world coordinates, instead of pixel coordinates), right…?
Do you think the new EllipseSpatialObject member functions ‘GetCenterPoint’ + ‘SetCenterPoint’ should still be renamed to ‘GetCenter’ + ‘SetCenter’, before ITK 5.0.0 alpha? The current ‘GetCenterPoint’ and ‘SetCenterPoint’ are fine to me, but I’d rather not have them deprecated immediately with the next release!
This is currently on by todo list, but I do not have time to work on it at the moment.
There is no rush to get these changes into the first ITK 5.0 alpha. There will be more ITK 5 alpha’s and API / breaking changes are expected between alphas – deprecation should not be a concern.
The current HoughTransform2DCirclesImageFilter template has TRadiusPixelType = TOutputPixelType, by default. This patch just removes the default argument, hoping to trigger ITK users to make the right choice themselves.
In general (also as ITK user), I do appreciate when a template has default arguments, if those correspond to the most common (or recommended) use case. However, with the HoughTransform2DCircles filter, I think it’s often preferable to use different types for TRadiusPixelType and TOutputPixelType.
So for TOutputPixelType, an unsigned integer is usually the best choice for this pixel type, whereas for TRadiusPixelType, a floating point type usually more appropriate.
Another proposed patch for HoughTransform2DCirclesImageFilter! The original code checked if the gradient (Vx, Vy) at input pixel location (x, y) is not flat by the following code:
// if the gradient is not flat
if ( ( std::fabs(Vx) > 1 ) || ( std::fabs(Vy) > 1 ) )
This test appears quite arbitrary, as Vx and Vy are floating point numbers, and for some images, (0.9, 0.9) might not be such a flat gradient. For other images, the gradient may still appear rather flat when Vx and Vy are significantly greater than one. Moreover, it often seems to make more sense to look at the norm of the gradient, instead of its individual (Vx, Vy) components. This is why I’m proposing to allow the user to specify their own (application specific) MinimumGradientNorm. Please review:
I already did some code improvement based on suggestions by @jhlegarreta
Note: The patch should be backward-compatible, because it still checks if (std::fabs(Vx) > 1) || (std::fabs(Vy) > 1) when the user does not set the value of MinimumGradientNorm.
Update: For the record, the MinimumGradientNorm proposal has been superseded by “ENH: Added GradientNormThreshold to HoughTransform2DCirclesImageFilter”: