Please have a look at my pull request, which aims to improve our SobelOperator, and add ND support (including 4D):
I think it is ready, but there are a few things to consider:
The proposed new implementation of SobelOperator::GenerateCoefficients() is based on a function make_nd_sobel_kernels generated by AI, and presented by Hans (@hjmjohnson) at A ND Sobel implementation is needed. · Issue #5702 · InsightSoftwareConsortium/ITK · GitHub . I don’t fully understand this function, but it appears to produce the same numbers as the original implementation of itk::SobelOperator, for 2D. So I think it’s OK, but an extra review might be helpful.
The AI generated make_nd_sobel_kernels produces different coordinates for 3D than the original implementation of itk::SobelOperator (from ITK <= 5.4). The pull request proposes to add an option UseLegacyCoefficients, to get the coordinates of the original ITK implementation. I guess, legacy code should use the original coordinates for 3D, and new code should use the coordinates from the AI generated function, right?
The coordinates for 3D from the original implementation of itk::SobelOperator (from ITK <= 5.4) were based on “Irwin Sobel. An Isotropic 3x3x3 Volume Gradient Operator. Technical report, Hewlett-Packard Laboratories, April 1995.” I propose to remove the reference to this publication, because I cannot find it. But is this report still available somewhere?
Unfortunately, the technical report of Irwin Sobel, “An Isotropic 3x3x3 Volume Gradient Operator” (which is specifically about extending the operator to 3D) is not there!!!
I. Sobel. An Isotropic 3x3x3 Volume Gradient Operator. Hewlett-Packard’s Voxelator V-3 CD-ROM, Aug. 1996. Online Information at http://www.hp.com/info/voxelator (Paper is only on CD, not online)
Looking at archive.org, it appears that HP’s Voxelator CD, containing the Sobel paper was distributed at the SIGGRAPH 1996 conference! Unfortunately I wasn’t there
This looks like the right move. the ND Sobel cleanup finally makes the 3D case behave like 2D instead of carrying historical quirks forward.
UseLegacyCoefficients is a good safety valve — there will be code relying on the old 3D behavior, even if it was never mathematically clean. defaulting to the new ND Sobel and keeping legacy behind a flag is exactly how ITK should handle this.
on the Irwin Sobel report: if it’s effectively lost to a SIGGRAPH ’96 CD-ROM, it’s not a usable reference anymore. the implementation + documented construction logic matters more than a citation nobody can verify.
net: solid PR, sensible compatibility story, and the Sobel operator is better off without mystery math baked in.