[Insight-developers] Optimized ImageToImageMetric::SetFixedImageRegion signature

[Tom Vercauteren]

Thanks for the feedback Stephen. I have committed the patch:
http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkOptImageToImageMetric.h?root=Insight&r1=1.29&r2=1.30&sortby=date
http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkOptImageToImageMetric.txx?root=Insight&r1=1.35&r2=1.36&sortby=date

I have a similar but harder question on the same topic. The non
optimized implementation has the following code:

public:
  itkSetObjectMacro( MovingImageMask, MovingImageMaskType );
  itkSetObjectMacro( FixedImageMask, FixedImageMaskType );
protected:
  mutable FixedImageMaskPointer   m_FixedImageMask;
  mutable MovingImageMaskPointer  m_MovingImageMask;

whereas the optimized code has:

public:
  itkSetConstObjectMacro( MovingImageMask, MovingImageMaskType );
  itkSetConstObjectMacro( FixedImageMask, FixedImageMaskType );
protected:
  FixedImageMaskConstPointer  m_FixedImageMask;
  MovingImageMaskConstPointer m_MovingImageMask;


For the same reasons as previously, this discrepancy may be
problematic. In this case, the code from the optimized implementation
seems to be the correct one to me. I would therefore like to modify
the non-optimized version to match the optimized one.

However, I understand that this is not strictly backward compatible. I
have just tested the attached patch that deprecates the mutable masks
and it works like a charm on the ITK tree:
http://www.cdash.org/CDash/buildSummary.php?buildid=405762

Do you think such a patch is appropriate to move forward?

Tom


On Tue, Aug 18, 2009 at 19:50, Stephen
Aylward<stephen.aylward at kitware.com> wrote:
> Hi,
>
> Yup - no backward compatibility issues apply to code in Review.
>
> The change you've suggested seems appropriate to me.
>
> Good catch - thanks!
>
> Stephen
>
> On Tue, Aug 18, 2009 at 1:40 PM, Tom Vercauteren<tom.vercauteren at m4x.org> wrote:
>> Hi,
>>
>> In the non-optimized case, ImageToImageMetric::SetFixedImageRegion is
>> defined as follows (through a itkSetMacro):
>>
>>  void SetFixedImageRegion( const FixedImageRegionType reg );
>>
>> In the optimized case, it is manually defined as:
>>
>>  void SetFixedImageRegion( FixedImageRegionType reg );
>>
>>
>> This causes some warning in msvc when a class inherits from
>> ImageToImageMetric and needs to override SetFixedImageRegion:
>>
>>    virtual function overrides
>> 'itk::ImageToImageMetric<TFixedImage,TMovingImage>::SetFixedImageRegion',
>> previous versions of the compiler did not override when parameters
>> only differed by const/volatile qualifiers
>>
>>
>> Just to make sure: Since the optimized registration code is in review,
>> there is no backward compatibility problem if I change the signature
>> of SetFixedImageRegion to match the one from the non-optimized code,
>> right?
>>
>> Also, I realized that doxygen gets confused by the itkSetMacro.
>> Looking at ImageToImageMetric::SetFixedImageRegion it reports a
>> non-const argument:
>>
>>  http://www.itk.org/Doxygen/html/classitk_1_1ImageToImageMetric.html#7d192f58b6309c49ee01e47755faff03
>>
>> However the doxygen version is based on the non-optimized code:
>>
>>  http://www.itk.org/Doxygen/html/itkImageToImageMetric_8h-source.html
>>
>> and correctly shows the const in the itkSetMacro definition:
>>
>>  http://www.itk.org/Doxygen/html/itkMacro_8h.html#7ebdd33cc5e7d74720ced9099c034faa
>>
>> Regards,
>> Tom
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>>
>> Please keep messages on-topic and check the ITK FAQ at: http://www.itk.org/Wiki/ITK_FAQ
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.itk.org/mailman/listinfo/insight-developers
>>
>
>
>
> --
> Stephen R. Aylward, Ph.D.
> Director of Medical Imaging
> Kitware, Inc. - North Carolina Office
> http://www.kitware.com
> stephen.aylward (Skype)
> (919) 969-6990 x300
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: itk-imtoimconstmask-2009-08-19.patch
Type: text/x-diff
Size: 2671 bytes
Desc: not available
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090819/7814015b/attachment.patch>