[Insight-developers] itkVTKImageIO testing question

[Gabe Hart]

Hmm... I'm probably missing something, but it seems to me that if the 
#ifdef is left in place, changing uint64_t to double won't fix the 
problem of Borland compilers being unable to process doubles.  Won't the 
case where size = 8 bytes (always the case for doubles) still be 
unavailable when compiled on Borland?

-Gabe

On 06/18/2010 05:11 PM, Bradley Lowekamp wrote:
> Gabe,
>
> The definition "ITK_HAS_INT_64" is defined when the system/compiler 
> has a functioning 64-bit integer type. Specifically the Borland 
> compiler does not have a useful 64-bit integer. It is related to 
> Common/itkIntTypes.h. It does not related to sizeof(int)==8 or 
> sizeof(void*)==0. Changing uint64_t to double should be ok.
>
> Brad
>
>
> On Jun 18, 2010, at 4:59 PM, Gabe Hart wrote:
>
>> That makes sense to me.  Like I said, this is one of my first 
>> projects, so I'm not yet intimately familiar with all of ITK's 
>> workings under the covers.  If this is indeed the case (that size is 
>> all that matters), would it make sense to just remove the #ifdef so 
>> that the case of 8 bytes is available to 32 bit machines as well?  I 
>> can imagine that this might break cause errors if a 32 bit machine 
>> tried to read a file created on a 64 bit machine with 8 byte 
>> integers, but it might be a better option that the current setup 
>> which just seems to reject doubles for 32 bit.
>>
>> As for the circular dependency issue, it sounds to me like they can't 
>> be automatically registered while they're still in review.  Does this 
>> sound right Luis?  If so, I think they may have to be manually 
>> registered until the new code is integrated into the main body of ITK.
>>
>> -Gabe
>>
>> On Fri, Jun 18, 2010 at 4:48 PM, Bradley Lowekamp 
>> <blowekamp at mail.nih.gov <mailto:blowekamp at mail.nih.gov>> wrote:
>>
>>     Hello Gabe,
>>
>>     This is the block of code you are referring too:
>>
>>
>>           int size = this->GetComponentSize();
>>           switch( size )
>>             {
>>             case 1:
>>               break;
>>             case 2:
>>             
>>      ByteSwapper<uint16_t>::SwapRangeFromSystemToBigEndian((uint16_t
>>     *)buffer, this->GetImageSizeInComponents() );
>>               break;
>>             case 4:
>>             
>>      ByteSwapper<uint32_t>::SwapRangeFromSystemToBigEndian((uint32_t
>>     *)buffer, this->GetImageSizeInComponents() );
>>               break;
>>     #ifdef ITK_HAS_INT_64
>>             case 8:
>>             
>>      ByteSwapper<uint64_t>::SwapRangeFromSystemToBigEndian((uint64_t
>>     *)buffer, this->GetImageSizeInComponents() );
>>               break;
>>     #endif
>>             default:
>>               itkExceptionMacro(<< "Unknown component size");
>>             }
>>
>>
>>     For all systems that I am aware which ITK supports, the byte
>>     order is dependent only on the size (number of bytes) not the
>>     specific type. I think that double is 8 bytes on all systems...
>>     so likely uint64_t could be changed to double. I just thought
>>     that fixed width integer conveyed the size switching better.
>>
>>
>>     I am trying to add ImageIOFactories for these new ImageIO classes
>>     and enable their automatic registration in ImageIOFactory. This
>>     would switch the current VTK tests to utilize the VTKImageIO2,
>>     and enable the reuse of the existing streaming IO related tests.
>>     However I have run into a road block. The library ITKIOReview
>>     depends on ITKIO, but adding the factories to ITKIO would add a
>>     circular dependency, and thankfully CMake was smart enough to
>>     know this was a problem before I did. So this is a significant
>>     problem to automatically using these ImageIOs, and it's unclear
>>     what the solution is.
>>
>>     Brad
>>
>>
>>     On Jun 18, 2010, at 4:30 PM, Gabe Hart wrote:
>>
>>>     Hi Brad,
>>>
>>>     As one of my first projects after joining Kitware, I've been
>>>     working on
>>>     importing the tests for itkMRCImageIO and itkVTKImageIO.  In
>>>     doing so,
>>>     I've run across an issue that Luis and I are confused on and
>>>     wanted your
>>>     input.  Following the format of your original test, my test for
>>>     itkVTKImageIO uses a templated class to execute write and read
>>>     tests for
>>>     all supported pixel types (and ensured that IO fails on some of the
>>>     unsupported ones).
>>>
>>>     The test (itkVTKImageIO2Test) executes nicely on my machine (Ubuntu
>>>     10.04 x64), but is failing in the continuous builds on BillsLaptop
>>>     (Win32 bcc5.5).  In particular, it fails to successfully read an
>>>     image
>>>     whose pixel type is itk::Vector<double,3> when reading in binary
>>>     mode.
>>>     I think it would also fail for a pixel type of double too.  The
>>>     failure
>>>     seems to be coming from line 452 where an exception is thrown
>>>     saying
>>>     "Unknown component size".  When adding a print statement to look
>>>     at the
>>>     size variable for the switch, it seems that indeed size = 8.
>>>      This would
>>>     explain the success on my machine (64-bit) but the failure on
>>>     BillsLaptop (32-bit).
>>>
>>>     The question this then raises is whether this is the correct
>>>     behavior
>>>     for this situation.  The switch statement seems to only consider
>>>     different sizes of ints and does not seem to handle the options for
>>>     floats or doubles.  In the present state, it seems to confuse a
>>>     double
>>>     with a 64 bit integer and thus is unable to read it on a 32 bit
>>>     machine.  Is this something that should be fixed, or is there
>>>     something
>>>     tricky going on behind the scenes?
>>>
>>>     Thanks for your help,
>>>     -Gabe
>>
>>     ========================================================
>>     Bradley Lowekamp
>>     Lockheed Martin Contractor for
>>     Office of High Performance Computing and Communications
>>     National Library of Medicine
>>     blowekamp at mail.nih.gov <mailto:blowekamp at mail.nih.gov>
>>
>>
>>
>
> ========================================================
>
> Bradley Lowekamp
>
> Lockheed Martin Contractor for
>
> Office of High Performance Computing and Communications
>
> National Library of Medicine
>
> blowekamp at mail.nih.gov <mailto:blowekamp at mail.nih.gov>
>
>
>

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