Skip to content

Commit bafe84e

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 bafe84e

12 files changed

Lines changed: 245 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: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,36 @@ 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+
* Throws when the argument is empty, as the boundary condition is dereferenced
146+
* unconditionally while processing boundary faces. */
147+
void
148+
SetBoundaryCondition(std::unique_ptr<BoundaryConditionType> boundaryCondition);
149+
150+
/** Returns the boundary condition in use. The filter retains ownership. */
151+
[[nodiscard]] const BoundaryConditionType *
152+
GetBoundaryCondition() const
153+
{
154+
return m_BoundaryCondition.get();
155+
}
156+
157+
/** Restores the default ZeroFluxNeumann boundary condition. */
158+
void
159+
ResetBoundaryCondition()
160+
{
161+
m_BoundaryCondition = std::make_unique<ZeroFluxNeumannBoundaryCondition<TInputImage>>();
162+
this->Modified();
163+
}
164+
165+
#if !defined(ITK_FUTURE_LEGACY_REMOVE)
166+
/** Allows to change the default boundary condition. The filter takes ownership.
167+
\deprecated Use GradientImageFilter::SetBoundaryCondition instead, whose
168+
`unique_ptr` parameter states the ownership transfer. */
143169
void
144170
OverrideBoundaryCondition(ImageBoundaryCondition<TInputImage> * boundaryCondition);
171+
#endif
145172

146173
itkConceptMacro(InputConvertibleToOutputCheck, (Concept::Convertible<InputPixelType, OutputValueType>));
147174
itkConceptMacro(OutputHasNumericTraitsCheck, (Concept::HasNumericTraits<OutputValueType>));

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,29 @@ 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+
if (boundaryCondition == nullptr)
46+
{
47+
itkExceptionMacro("The boundary condition should not be null!");
48+
}
49+
m_BoundaryCondition = std::move(boundaryCondition);
50+
this->Modified();
51+
}
52+
53+
#if !defined(ITK_FUTURE_LEGACY_REMOVE)
4054
template <typename TInputImage, typename TOperatorValueType, typename TOutputValue, typename TOutputImage>
4155
void
4256
GradientImageFilter<TInputImage, TOperatorValueType, TOutputValue, TOutputImage>::OverrideBoundaryCondition(
4357
ImageBoundaryCondition<TInputImage> * boundaryCondition)
4458
{
4559
m_BoundaryCondition.reset(boundaryCondition);
60+
this->Modified();
4661
}
62+
#endif
4763

4864
template <typename TInputImage, typename TOperatorValueType, typename TOutputValueType, typename TOutputImageType>
4965
void

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

Lines changed: 134 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,133 @@ 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+
// Changing the boundary condition must invalidate an already computed output.
181+
TEST(GradientImageFilter, SetBoundaryConditionModifiesFilter)
182+
{
183+
using ImageType = itk::Image<int>;
184+
185+
const auto inputImage = ImageType::New();
186+
inputImage->SetRegions(itk::Size<2>::Filled(4));
187+
inputImage->Allocate(true);
188+
189+
const auto filter = itk::GradientImageFilter<ImageType>::New();
190+
filter->SetInput(inputImage);
191+
filter->Update();
192+
const auto modifiedTimeAfterUpdate = filter->GetMTime();
193+
194+
filter->SetBoundaryCondition(std::make_unique<itk::ZeroFluxNeumannBoundaryCondition<ImageType>>());
195+
EXPECT_GT(filter->GetMTime(), modifiedTimeAfterUpdate);
196+
197+
filter->ResetBoundaryCondition();
198+
EXPECT_GT(filter->GetMTime(), modifiedTimeAfterUpdate);
199+
}
200+
201+
202+
// A null boundary condition would be dereferenced while processing boundary faces.
203+
TEST(GradientImageFilter, SetBoundaryConditionRejectsNull)
204+
{
205+
using ImageType = itk::Image<int>;
206+
207+
const auto filter = itk::GradientImageFilter<ImageType>::New();
208+
EXPECT_THROW(filter->SetBoundaryCondition(nullptr), itk::ExceptionObject);
209+
EXPECT_NE(filter->GetBoundaryCondition(), nullptr);
210+
}
211+
212+
213+
// The rest of the OverrideBoundaryCondition family does not take ownership.
214+
TEST(GradientImageFilter, OverrideBoundaryConditionDoesNotTakeOwnership)
215+
{
216+
using ImageType = itk::Image<int>;
217+
using BoundaryConditionType = DestructionCountingBoundaryCondition<ImageType>;
218+
219+
unsigned int destructionCount{ 0 };
220+
{
221+
BoundaryConditionType boundaryCondition(destructionCount);
222+
223+
itk::ConstNeighborhoodIterator<ImageType> iterator;
224+
iterator.OverrideBoundaryCondition(&boundaryCondition);
225+
226+
const auto neighborhoodFilter = itk::NeighborhoodOperatorImageFilter<ImageType, ImageType>::New();
227+
neighborhoodFilter->OverrideBoundaryCondition(&boundaryCondition);
228+
229+
EXPECT_EQ(destructionCount, 0u);
230+
}
231+
// Destroyed exactly once, by leaving scope -- not by either consumer.
232+
EXPECT_EQ(destructionCount, 1u);
233+
}
234+
235+
236+
#if !defined(ITK_FUTURE_LEGACY_REMOVE)
237+
// The deprecated overload keeps its original ownership semantics.
238+
TEST(GradientImageFilter, DeprecatedOverrideBoundaryConditionStillTakesOwnership)
239+
{
240+
using ImageType = itk::Image<int>;
241+
using BoundaryConditionType = DestructionCountingBoundaryCondition<ImageType>;
242+
243+
unsigned int destructionCount{ 0 };
244+
{
245+
const auto filter = itk::GradientImageFilter<ImageType>::New();
246+
filter->OverrideBoundaryCondition(new BoundaryConditionType(destructionCount));
247+
EXPECT_EQ(destructionCount, 0u);
248+
}
249+
EXPECT_EQ(destructionCount, 1u);
250+
}
251+
#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)