Skip to content

Commit 7afe436

Browse files
hjmjohnsonhjmjohnson
committed
ENH: Add GradientImageFilter::SetBoundaryCondition taking a unique_ptr
OverrideBoundaryCondition(T *) adopts its argument in GradientImageFilter but not in the five other classes that declare the same signature, so the name alone cannot tell a caller who frees the object. Add SetBoundaryCondition(std::unique_ptr<BoundaryConditionType>), whose parameter type states the transfer, plus GetBoundaryCondition and ResetBoundaryCondition. Deprecate the owning OverrideBoundaryCondition under ITK_FUTURE_LEGACY_REMOVE and document the remaining ones as non-owning. Supersedes #4533
1 parent d7608fd commit 7afe436

12 files changed

Lines changed: 203 additions & 30 deletions

File tree

‎Documentation/docs/migration_guides/itk_6_migration_guide.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,3 +1159,33 @@ approximation) are unchanged and intentionally do not return their own type.
11591159
`typeid` comparison, or a `static_cast` to a sibling type), update that code to
11601160
the new concrete type. Such a dependency is uncommon and was not found in a
11611161
survey of downstream ITK consumers.
1162+
1163+
## `GradientImageFilter::OverrideBoundaryCondition` deprecated in favor of `SetBoundaryCondition`
1164+
1165+
`GradientImageFilter` is the only ITK class whose `OverrideBoundaryCondition` *takes
1166+
ownership* of its argument; every other class of that name stores a non-owning
1167+
pointer. Because the two share a signature, passing a stack object to a
1168+
`GradientImageFilter` compiled silently and then double-freed.
1169+
1170+
The owning API now says so in its name and its type:
1171+
1172+
```cpp
1173+
// ITKv6
1174+
filter->SetBoundaryCondition(std::make_unique<itk::PeriodicBoundaryCondition<ImageType>>());
1175+
1176+
// ITKv5 (deprecated, removed when ITK_FUTURE_LEGACY_REMOVE is enabled)
1177+
filter->OverrideBoundaryCondition(new itk::PeriodicBoundaryCondition<ImageType>);
1178+
```
1179+
1180+
`GetBoundaryCondition()` and `ResetBoundaryCondition()` were added to complete the
1181+
interface, matching the rest of the family.
1182+
1183+
### What you need to do
1184+
1185+
Replace `OverrideBoundaryCondition(new X)` with `SetBoundaryCondition(std::make_unique<X>())`
1186+
on `GradientImageFilter`. In Python, pass the boundary condition to
1187+
`SetBoundaryCondition`; ownership moves to the filter, and re-using that object
1188+
afterwards raises `RuntimeError` rather than crashing.
1189+
1190+
Calls to `OverrideBoundaryCondition` on any *other* class are unaffected — those
1191+
never took ownership and keep their current behavior.

‎Modules/Core/Common/include/itkConstNeighborhoodIterator.h‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -483,11 +483,8 @@ class ITK_TEMPLATE_EXPORT ConstNeighborhoodIterator
483483
bool
484484
IndexInBounds(const NeighborIndexType n) const;
485485

486-
/** Allows a user to override the internal boundary condition. Care should
487-
* be taken to ensure that the overriding boundary condition is a persistent
488-
* object during the time it is referenced. The overriding condition
489-
* can be of a different type than the default type as long as it is
490-
* a subclass of ImageBoundaryCondition. */
486+
/** Overrides the internal boundary condition. Does not take ownership; the
487+
* caller must keep the object alive while it is referenced. */
491488
void
492489
OverrideBoundaryCondition(const ImageBoundaryConditionPointerType i)
493490
{

‎Modules/Filtering/BinaryMathematicalMorphology/include/itkObjectMorphologyImageFilter.h‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,8 @@ class ITK_TEMPLATE_EXPORT ObjectMorphologyImageFilter : public ImageToImageFilte
137137
void
138138
GenerateInputRequestedRegion() override;
139139

140-
/** Allows a user to override the internal boundary condition. Care should be
141-
* be taken to ensure that the overriding boundary condition is a persistent
142-
* object during the time it is referenced. The overriding condition
143-
* can be of a different type than the default type as long as it is
144-
* a subclass of ImageBoundaryCondition.
140+
/** Overrides the internal boundary condition. Does not take ownership; the
141+
* caller must keep the object alive while it is referenced.
145142
* NOTE: Don't forget to set UseBoundaryCondition to true! */
146143
void
147144
OverrideBoundaryCondition(const ImageBoundaryConditionPointerType i)

‎Modules/Filtering/ImageFilterBase/include/itkNeighborhoodOperatorImageFilter.h‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,8 @@ class ITK_TEMPLATE_EXPORT NeighborhoodOperatorImageFilter : public ImageToImageF
115115
return m_Operator;
116116
}
117117

118-
/** Allows a user to override the internal boundary condition. Care should be
119-
* be taken to ensure that the overriding boundary condition is a persistent
120-
* object during the time it is referenced. The overriding condition
121-
* can be of a different type than the default type as long as it is
122-
* a subclass of ImageBoundaryCondition. */
118+
/** Overrides the internal boundary condition. Does not take ownership; the
119+
* caller must keep the object alive while it is referenced. */
123120
void
124121
OverrideBoundaryCondition(const ImageBoundaryConditionPointerType i)
125122
{

‎Modules/Filtering/ImageFilterBase/include/itkVectorNeighborhoodOperatorImageFilter.h‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,8 @@ class ITK_TEMPLATE_EXPORT VectorNeighborhoodOperatorImageFilter : public ImageTo
105105
this->Modified();
106106
}
107107

108-
/** Allows a user to override the internal boundary condition. Care should be
109-
* be taken to ensure that the overriding boundary condition is a persistent
110-
* object during the time it is referenced. The overriding condition
111-
* can be of a different type than the default type as long as it is
112-
* a subclass of ImageBoundaryCondition. */
108+
/** Overrides the internal boundary condition. Does not take ownership; the
109+
* caller must keep the object alive while it is referenced. */
113110
void
114111
OverrideBoundaryCondition(const ImageBoundaryConditionPointerType i)
115112
{

‎Modules/Filtering/ImageGradient/include/itkGradientImageFilter.h‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,33 @@ class ITK_TEMPLATE_EXPORT GradientImageFilter : public ImageToImageFilter<TInput
139139
}
140140
#endif
141141

142-
/** Allows to change the default boundary condition */
142+
using BoundaryConditionType = ImageBoundaryCondition<TInputImage, TInputImage>;
143+
144+
/** Replaces the default boundary condition. The filter takes ownership. */
145+
void
146+
SetBoundaryCondition(std::unique_ptr<BoundaryConditionType> boundaryCondition);
147+
148+
/** Returns the boundary condition in use. The filter retains ownership. */
149+
[[nodiscard]] const BoundaryConditionType *
150+
GetBoundaryCondition() const
151+
{
152+
return m_BoundaryCondition.get();
153+
}
154+
155+
/** Restores the default ZeroFluxNeumann boundary condition. */
156+
void
157+
ResetBoundaryCondition()
158+
{
159+
m_BoundaryCondition = std::make_unique<ZeroFluxNeumannBoundaryCondition<TInputImage>>();
160+
}
161+
162+
#if !defined(ITK_FUTURE_LEGACY_REMOVE)
163+
/** Allows to change the default boundary condition. The filter takes ownership.
164+
\deprecated Use GradientImageFilter::SetBoundaryCondition instead, whose
165+
`unique_ptr` parameter states the ownership transfer. */
143166
void
144167
OverrideBoundaryCondition(ImageBoundaryCondition<TInputImage> * boundaryCondition);
168+
#endif
145169

146170
itkConceptMacro(InputConvertibleToOutputCheck, (Concept::Convertible<InputPixelType, OutputValueType>));
147171
itkConceptMacro(OutputHasNumericTraitsCheck, (Concept::HasNumericTraits<OutputValueType>));

‎Modules/Filtering/ImageGradient/include/itkGradientImageFilter.hxx‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,23 @@ GradientImageFilter<TInputImage, TOperatorValueType, TOutputValueType, TOutputIm
3737
this->ThreaderUpdateProgressOff();
3838
}
3939

40+
template <typename TInputImage, typename TOperatorValueType, typename TOutputValue, typename TOutputImage>
41+
void
42+
GradientImageFilter<TInputImage, TOperatorValueType, TOutputValue, TOutputImage>::SetBoundaryCondition(
43+
std::unique_ptr<BoundaryConditionType> boundaryCondition)
44+
{
45+
m_BoundaryCondition = std::move(boundaryCondition);
46+
}
47+
48+
#if !defined(ITK_FUTURE_LEGACY_REMOVE)
4049
template <typename TInputImage, typename TOperatorValueType, typename TOutputValue, typename TOutputImage>
4150
void
4251
GradientImageFilter<TInputImage, TOperatorValueType, TOutputValue, TOutputImage>::OverrideBoundaryCondition(
4352
ImageBoundaryCondition<TInputImage> * boundaryCondition)
4453
{
4554
m_BoundaryCondition.reset(boundaryCondition);
4655
}
56+
#endif
4757

4858
template <typename TInputImage, typename TOperatorValueType, typename TOutputValueType, typename TOutputImageType>
4959
void

‎Modules/Filtering/ImageGradient/test/itkGradientImageFilterGTest.cxx‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,16 @@
1919
// First include the header file to be tested:
2020
#include "itkGradientImageFilter.h"
2121

22+
#include "itkConstNeighborhoodIterator.h"
2223
#include "itkDeref.h"
2324
#include "itkImage.h"
2425
#include "itkImageBufferRange.h"
2526
#include "itkIndexRange.h"
27+
#include "itkNeighborhoodOperatorImageFilter.h"
28+
#include "itkZeroFluxNeumannBoundaryCondition.h"
2629

2730
#include <gtest/gtest.h>
31+
#include <memory>
2832

2933

3034
// Tests the output for a uniform input image.
@@ -115,3 +119,100 @@ TEST(GradientImageFilter, ConstantGradientInputImage)
115119
}
116120
}
117121
}
122+
123+
124+
namespace
125+
{
126+
// Counts its own destructions, so tests can observe who owns it.
127+
template <typename TImage>
128+
class DestructionCountingBoundaryCondition : public itk::ZeroFluxNeumannBoundaryCondition<TImage>
129+
{
130+
public:
131+
explicit DestructionCountingBoundaryCondition(unsigned int & counter)
132+
: m_Counter(&counter)
133+
{}
134+
135+
~DestructionCountingBoundaryCondition() override { ++(*m_Counter); }
136+
137+
private:
138+
unsigned int * m_Counter{};
139+
};
140+
} // namespace
141+
142+
143+
// The unique_ptr overload takes ownership: the filter destroys the boundary condition.
144+
TEST(GradientImageFilter, SetBoundaryConditionTakesOwnership)
145+
{
146+
using ImageType = itk::Image<int>;
147+
using BoundaryConditionType = DestructionCountingBoundaryCondition<ImageType>;
148+
149+
unsigned int destructionCount{ 0 };
150+
{
151+
const auto filter = itk::GradientImageFilter<ImageType>::New();
152+
filter->SetBoundaryCondition(std::make_unique<BoundaryConditionType>(destructionCount));
153+
EXPECT_EQ(destructionCount, 0u);
154+
}
155+
EXPECT_EQ(destructionCount, 1u);
156+
}
157+
158+
159+
// Get returns what was set; Reset restores the default and destroys the previous one.
160+
TEST(GradientImageFilter, GetAndResetBoundaryCondition)
161+
{
162+
using ImageType = itk::Image<int>;
163+
using BoundaryConditionType = DestructionCountingBoundaryCondition<ImageType>;
164+
165+
unsigned int destructionCount{ 0 };
166+
const auto filter = itk::GradientImageFilter<ImageType>::New();
167+
168+
auto boundaryCondition = std::make_unique<BoundaryConditionType>(destructionCount);
169+
const auto rawPointer = boundaryCondition.get();
170+
filter->SetBoundaryCondition(std::move(boundaryCondition));
171+
EXPECT_EQ(filter->GetBoundaryCondition(), rawPointer);
172+
173+
filter->ResetBoundaryCondition();
174+
EXPECT_EQ(destructionCount, 1u);
175+
EXPECT_NE(filter->GetBoundaryCondition(), nullptr);
176+
EXPECT_NE(filter->GetBoundaryCondition(), rawPointer);
177+
}
178+
179+
180+
// The rest of the OverrideBoundaryCondition family does not take ownership.
181+
TEST(GradientImageFilter, OverrideBoundaryConditionDoesNotTakeOwnership)
182+
{
183+
using ImageType = itk::Image<int>;
184+
using BoundaryConditionType = DestructionCountingBoundaryCondition<ImageType>;
185+
186+
unsigned int destructionCount{ 0 };
187+
{
188+
BoundaryConditionType boundaryCondition(destructionCount);
189+
190+
itk::ConstNeighborhoodIterator<ImageType> iterator;
191+
iterator.OverrideBoundaryCondition(&boundaryCondition);
192+
193+
const auto neighborhoodFilter = itk::NeighborhoodOperatorImageFilter<ImageType, ImageType>::New();
194+
neighborhoodFilter->OverrideBoundaryCondition(&boundaryCondition);
195+
196+
EXPECT_EQ(destructionCount, 0u);
197+
}
198+
// Destroyed exactly once, by leaving scope -- not by either consumer.
199+
EXPECT_EQ(destructionCount, 1u);
200+
}
201+
202+
203+
#if !defined(ITK_FUTURE_LEGACY_REMOVE)
204+
// The deprecated overload keeps its original ownership semantics.
205+
TEST(GradientImageFilter, DeprecatedOverrideBoundaryConditionStillTakesOwnership)
206+
{
207+
using ImageType = itk::Image<int>;
208+
using BoundaryConditionType = DestructionCountingBoundaryCondition<ImageType>;
209+
210+
unsigned int destructionCount{ 0 };
211+
{
212+
const auto filter = itk::GradientImageFilter<ImageType>::New();
213+
filter->OverrideBoundaryCondition(new BoundaryConditionType(destructionCount));
214+
EXPECT_EQ(destructionCount, 0u);
215+
}
216+
EXPECT_EQ(destructionCount, 1u);
217+
}
218+
#endif

‎Modules/Filtering/ImageGradient/test/itkGradientImageFilterTest.cxx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
#include "itkPeriodicBoundaryCondition.h"
2626

27+
#include <memory>
28+
2729
inline std::ostream &
2830
operator<<(std::ostream & o, const itk::CovariantVector<float, 3> & v)
2931
{
@@ -88,8 +90,8 @@ itkGradientImageFilterTest(int argc, char * argv[])
8890
auto filter2 = FilterType2::New();
8991

9092
using PeriodicBoundaryType = itk::PeriodicBoundaryCondition<InputImageType2>;
91-
// Test the OverrideBoundaryCondition setting;
92-
filter2->OverrideBoundaryCondition(new PeriodicBoundaryType);
93+
// Test the SetBoundaryCondition setting;
94+
filter2->SetBoundaryCondition(std::make_unique<PeriodicBoundaryType>());
9395

9496
ITK_EXERCISE_BASIC_OBJECT_METHODS(filter2, GradientImageFilter, ImageToImageFilter);
9597

‎Modules/Filtering/ImageGradient/wrapping/itkGradientImageFilter.wrap‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
1+
# Defines the %unique_ptr macro used per instantiation below.
2+
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "%include <std_unique_ptr.i>\n")
3+
14
itk_wrap_class("itk::GradientImageFilter" POINTER)
25
foreach(d ${ITK_WRAP_IMAGE_DIMS})
36

47
set(vector_dim ${d}) # Wrap only vector dimensions which are the same as image dimensions
58
foreach(t ${WRAP_ITK_SCALAR})
69

7-
# OverrideBoundaryCondition stores the argument in a unique_ptr member.
10+
# SetBoundaryCondition and OverrideBoundaryCondition both adopt their argument.
811
string(
912
APPEND
1013
ITK_WRAP_PYTHON_SWIG_EXT
14+
"%unique_ptr(itkImageBoundaryCondition${ITKM_I${t}${vector_dim}})\n"
1115
"%apply SWIGTYPE *DISOWN { itkImageBoundaryCondition${ITKM_I${t}${vector_dim}} * boundaryCondition };\n"
1216
)
1317

0 commit comments

Comments
 (0)