[Insight-developers] My first bug fix (10770)

Bradley Lowekamp blowekamp at mail.nih.gov
Wed Jun 9 14:57:58 EDT 2010


Hello,

I committed a patch to get things building again.

http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/BasicFilters/itkTriangleMeshToBinaryImageFilter.txx?root=Insight&r1=1.13&r2=1.14

Brad

On Jun 9, 2010, at 9:02 AM, Karthik Krishnan wrote:

> Dzenan:
> 
> Thank you very much for taking the effort to adopt the bug. 
> 
> You do not want to have the "<double>" there
> 
>   image->TransformPhysicalPointToContinuousIndex< double > (point, ind);
> 
> "TransformPhysicalPointToContinuousIndex" is a templated member function and the type should be resolved implicitly based on the precision type of the arguments.
> 
> -----
> 
> The reason for the compile error on removing the "<double>" is that you have different precisions for the "point" and for the "ContinuousIndex". One is double and the other is float. They are expected to be the same, as you can see from the signature of the function :
> 
>   template<class TCoordRep>
>   bool TransformPhysicalPointToContinuousIndex(
>               const Point<TCoordRep, VImageDimension>& point,
>               ContinuousIndex<TCoordRep, VImageDimension>& index   ) const
> 
> 
> ----
> 
> Hope this helps.
> 
> Thanks
> --
> karthik
> 
> 2010/6/9 Dženan Zukić <dzenanz at gmail.com>
> Hm, after committing the code it fails to compile on GCC (http://www.cdash.org/CDash/viewBuildError.php?buildid=630135),
> 
> itkTriangleMeshToBinaryImageFilter.txx:441: error: expected primary-expression before 'double'
> 
> the line is as follows:
> 
> OutputImage->TransformPhysicalPointToContinuousIndex<double>(point, ind);
> 
> However, if I remove <double>, I get following error on VS2008SP1 (x64 build target):
> 
> itkTriangleMeshToBinaryImageFilter.txx(441) : error C2782: 'bool itk::ImageBase<VImageDimension>::TransformPhysicalPointToContinuousIndex(const itk::Point<TCoordRep,3> &,itk::ContinuousIndex<TCoordRep,3> &) const' : template parameter 'TCoordRep' is ambiguous.
> 
> What is the common way of resolving this type of errors? Macros (ifdef win32 ...), or attempt to comprehend all relevant template parameters of all classes involved and try use another syntactic construct instead of explicit template instantiation?
> 
> Regards,
> Dženan
> 
> 2010/6/9 Dženan Zukić <dzenanz at gmail.com>
> Hi,
> 
> There is a bug in the io common test (patch attached). I will do experimental build on cleanly checked-out code, and if those 2 test fail (as I expect), I will proceed with committing the fixes to meshToImage filter and also to the itkiocommon test.
> 
> Regards,
> Dženan
> 
> 
> On Tue, Jun 8, 2010 at 19:15, Casey Goodlett <casey.goodlett at kitware.com> wrote:
> Hi Brad,
> 
> I've noticed itkIOCommonTest to fail on win64 recently as well. This happens when ITK_LEGACY_REMOVE is configured to true.   You can observe the problem under linux if you run it under valgrind as well.  I think it relates to allocation of some zero length c strings in the test.  This is occurring in a clean checkout.
> 
> Casey
> 
> 2010/6/8 Bradley Lowekamp <blowekamp at mail.nih.gov>
> Hello Dženan,
> 
> Congratulations on your first bug fix! It will get easier in the future when you know the system and tools a little better. I do hope that you are not including the testing as part of the "administration" costs. Testing and validations are very important things, as much of the time if it is not tested it will not be working ( or at the least it will break and no one would know ).
> 
> Were the two tests failing before you made the changes? 
> 
> The itkImageFillBufferTest4.1 in known to fail on windows 64, but itkIOCommonTest I don't know about.
> 
> Thanks,
> Brad
> 
> 
> On Jun 8, 2010, at 7:21 AM, Dženan Zukić wrote:
> 
>> Hi guys,
>> 
>> I fixed a bug http://public.kitware.com/Bug/view.php?id=10770 (I made changes to local source code files). I made an experimental build http://www.cdash.org/CDash/buildSummary.php?buildid=629158 (release 3.18.0 with my changes), and the are 2 tests failing (they don't seem to be related to my changes though). Is that a problem?
>> 
>> Side notes:
>> I downloaded and installed KWStyle version 1.0 (the only binary pre-built version from the website), but cmake whines about it being too old. I was too lazy to configure and build it from source, so I used the online tool for checking the 2 changed files.
>> It took me maybe 10 minutes to correct the code, but nearly a day for all the administrative work around it :D
>> 
>> Regards,
>> Dženan
>> <itkTriangleMeshToBinaryImageFilter.patch><ATT00001..txt>
> 
> 
> _______________________________________________
> Powered by www.kitware.com
> 
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
> 
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.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
> 
> 
> 
> 
> 
> 
> 
> _______________________________________________
> Powered by www.kitware.com
> 
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
> 
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.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
> 
> 
> <ATT00001..txt>

========================================================
Bradley Lowekamp  
Lockheed Martin Contractor for
Office of High Performance Computing and Communications
National Library of Medicine 
blowekamp at mail.nih.gov


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100609/6d44a817/attachment.htm>


More information about the Insight-developers mailing list