[Insight-developers] Re: Adaptor update mechanism

[Karthik Krishnan]

Hi Jim,

Thanks again. The changes you just mentioned have been committed.

Regards
karthik

Miller, James V (Research) wrote:

>Karthik, 
>
>Then your modification to UpdateOutputData() is the appropriate fix.
>
>(It might be good to change the method ComputeOffsetTable(), ComputeOffset(), ComputeIndex()
> to something like below just to be safe. This avoids accessing m_BufferedRegion and always calls
>GetBufferedRegion(). )
>
>
>::ComputeOffsetTable()
>{
>  OffsetValueType num=1;
>  const SizeType& bufferSize = this->GetBufferedRegion().GetSize();
>  
>  m_OffsetTable[0] = num;
>  for (unsigned int i=0; i < VImageDimension; i++)
>    {
>    num *= bufferSize[i];
>    m_OffsetTable[i+1] = num;
>    }
>}
>
>OffsetValueType ComputeOffset(const IndexType &ind) const
>  {
>    // need to add bounds checking for the region/buffer?
>    OffsetValueType offset=0;
>    const IndexType &bufferedRegionIndex = this->GetBufferedRegion().GetIndex();
>  
>    // data is arranged as [][][][slice][row][col]
>    // with Index[0] = col, Index[1] = row, Index[2] = slice
>    for (int i=VImageDimension-1; i > 0; i--)
>      {
>      offset += (ind[i] - bufferedRegionIndex[i])*m_OffsetTable[i];
>      }
>    offset += (ind[0] - bufferedRegionIndex[0]);
>
>    return offset;
>  }
>
>IndexType ComputeIndex(OffsetValueType offset) const
>  {
>    IndexType index;
>    const IndexType &bufferedRegionIndex = this->GetBufferedRegion().GetIndex();
>    
>    for (int i=VImageDimension-1; i > 0; i--)
>      {
>      index[i] = static_cast<IndexValueType>(offset / m_OffsetTable[i]);
>      offset -= (index[i] * m_OffsetTable[i]);
>      index[i] += bufferedRegionIndex[i];
>      }
>    index[0] = bufferedRegionIndex[0] + static_cast<IndexValueType>(offset);
>
>    return index;
>  }
>
>
>-----Original Message-----
>From: Karthik Krishnan [mailto:Karthik.Krishnan at kitware.com]
>Sent: Wednesday, June 01, 2005 4:18 PM
>To: Miller, James V (Research)
>Cc: Insight-developers (E-mail); Luis Ibanez
>Subject: Re: Adaptor update mechanism
>
>
>Hi Jim,
>
>Thanks again for looking at this. As you just mentioned, that does solve 
>the the problem too.  The ComputeOffset() function in ImageBase.h would 
>then need an extra line
>    m_BufferedRegion = this->GetBufferedRegion();
>
>With this solution there is an implementation issue though. 
>ComputeOffset() is meant to be a const method.
>
>Thanks
>Regards
>karthik
>
>
>Miller, James V (Research) wrote:
>
>  
>
>>Karthik, 
>>
>>That is probabaly a reasonable fix. I am still not sure why it should
>>matter whether the ImageAdapter has a proper m_BufferedRegion.  If all
>>access to m_BufferedRegion is via GetBufferedRegion(), the adaptor 
>>should have delegated down to the internal image.  The only place
>>I saw that m_BufferedRegion was accessed directly was in ComputeOffsetTable(), 
>>ComputeOffset() and ComputeIndex() (all in ImageBase).
>>
>>Did you try changing these methods to call GetBufferedRegion()
>>instead of accessing m_BufferedRegion directly?
>>
>>That being said, it is probably best that the BufferedRegion ivar 
>>in the ImageAdaptor be kept consistent with the underlying image.
>>So calling this->SetBufferedRegion( m_Image->GetBufferedRegion() )
>>is a good idea.
>>
>>Jim
>>
>>
>>
>>-----Original Message-----
>>From: Karthik Krishnan [mailto:Karthik.Krishnan at kitware.com]
>>Sent: Wednesday, June 01, 2005 1:04 PM
>>To: Miller, James V (Research)
>>Cc: Insight-developers (E-mail); Luis Ibanez
>>Subject: Re: Adaptor update mechanism
>>
>>
>>Hi Jim,
>>
>>Thank you very much for looking at it.
>>
>>The changes I was thinking of in the last mail were incorrect. I think 
>>I've found it now though.
>>
>>First, I re-ran the example on Linux and on VS71 and ImageAdaptor2 
>>crashes if reader->Update() is not called prior to adaptor->SetImage(). 
>>
>>As you mentioned, the m_BufferedRegion was not being set correctly, 
>>because the ImageAdaptor calls SetBufferedRegion() only in the function 
>>SetImage(). So, if a pipeline is set-up prior to execution, I don't see 
>>how the buffered region could be propagated correctly.
>>
>>I did fix it though by adding SetBufferedRegion( 
>>m_Image->GetBufferedRegion() ) right after the m_Image updates its 
>>output data.
>>
>>So  ImageAdaptor::UpdateOutputData() would be :
>>
>>ImageAdaptor<TImage , TAccessor>::UpdateOutputData()
>>{
>> // call the superclass' method first, then delegate
>> Superclass::UpdateOutputData();
>>
>> // delegation to internal image
>> m_Image->UpdateOutputData();
>> SetBufferedRegion( m_Image->GetBufferedRegion() );
>>}
>>
>>That fixes it, but I am not sure if it is the correct thing to do.
>>
>>Thanks
>>regards
>>karthik
>>
>>PS: Here were the command line params I used to run ImageAdaptor2 (with 
>>nightly CVS source)
>>~/work/ITK/binaries/Insight/Nightly/bin/ImageAdaptor2 
>>~/work/ITK/src/Insight/Nightly/Examples/Data/VisibleWomanEyeSlice.png 
>>redEye.png
>>
>>
>>Miller, James V (Research) wrote:
>>
>> 
>>
>>    
>>
>>>Karthik, 
>>>
>>>I'd have to look at this in more detail.  Your suggested changes do not seem
>>>correct to me.  
>>>
>>>ImageAdaptor2 works fine for me if I remove the call to reader->Update().
>>>(Using .Net 2003).
>>>
>>>The LargestPossibleRegion and the RequestedRegion should have been propagated
>>>through the adaptered already.  If there is a problem, it might be somewhere 
>>>where m_BufferedRegion is accessed directly instead of via an access method.
>>>
>>>The only place I see this happening is in ComputeOffsetTable(). We could have
>>>that method call GetBufferedRegion().  However, I cannot duplicate the error 
>>>you are getting.
>>>
>>>
>>>Jim
>>>
>>>
>>>-----Original Message-----
>>>From: Karthik Krishnan [mailto:Karthik.Krishnan at kitware.com]
>>>Sent: Tuesday, May 31, 2005 5:22 PM
>>>To: Insight-developers (E-mail); Luis Ibanez; Miller, James V (Research)
>>>Subject: Adaptor update mechanism
>>>
>>>
>>>Hi,
>>>
>>>The adaptors don't seem to propagate information when plugged into the 
>>>pipeline. For example ImageAdaptor2 example segfaults if 
>>>reader->Update() is commented out.
>>>
>>>I believe that  the Update() call in ImageAdaptor.txx should be
>>>
>>>//----------------------------------------------------------------------------
>>>template <class TImage, class TAccessor >
>>>void
>>>ImageAdaptor<TImage , TAccessor>
>>>::Update()
>>>{
>>>Superclass::Update();
>>>
>>>Superclass::SetLargestPossibleRegion( 
>>>m_Image->GetLargestPossibleRegion() );
>>>Superclass::SetBufferedRegion( m_Image->GetBufferedRegion() );
>>>Superclass::SetRequestedRegion( m_Image->GetRequestedRegion() );
>>>m_Image->Update();
>>>}
>>>
>>>instead of what it is now. Please correct me if I am mistaken. I don't 
>>>want to commit incorrect code since I am not familiar enough with the 
>>>pipeline mechanism.
>>>
>>>Thanks
>>>Regards
>>>Karthik
>>>
>>>
>>>
>>>
>>>   
>>>
>>>      
>>>
>> 
>>
>>    
>>
>
>  
>