[Insight-developers] RGB colormapping IJ contribution, Gaetan's comments

[Gaëtan Lehmann]

Le 6 janv. 09 à 18:25, Nicholas Tustison a écrit :

> Hi Gaetan,
>

Hi Nick,

> I hope you don't mind but the IJ commenting mechanism doesn't seem  
> to have the back-and-forth niceties of regular email and instead of  
> simply contacting you directly I thought the developer's list would  
> be more appropriate.  First off, thank you for the suggestions--they  
> were great.  In fact, I included you as a co-author on the revision  
> because of the resulting substantive changes.
>

Thank you, but I don't think I should be in the authors: I've only  
made a small reviewing work, nothing more - you made the hard job!

> As you can see below, I've made all the changes you suggested and am  
> currently revising the document accordingly.  However, I had a  
> couple of questions regarding a couple of your comments/suggestions.
>
> 1.  You ask why I didn't derive the filter from the  
> UnaryFunctorImageFilter class.  Actually, this was my original  
> implementation idea but it seems that swapping the colormap functors  
> is not possible within that framework since the  
> UnaryFunctorImageFilter class is templated over the specific  
> functor.  There is the SetFunctor() function but as its description  
> says "This allows the user to specify a functor that has ivars set  
> differently than the default functor."  Is there something that I'm  
> missing such that swapping of the different colormap functors would  
> be possible?

No, you're right. It would have be possible if the functor was managed  
with a pointer in UnaryFunctorImageFilter, but that's not the case.

>
> 2. I'm afraid I'm not quite understanding your concern about the rgb  
> channels and the rgb property not being respected by the custom  
> colormap functor.  Could you please elaborate a little more?
>

My mistake: I thought that it was a shared feature of all the  
colormaps, but I realize it's only in ScalarToRGBCustomColormapFunctor.
Please just forget about that.

> 3.  Unfortunately, I cannot access the attachment you give in your  
> review which is unfortunate because I would like to include your  
> colormap.  When I download it, all I get is a text file with a  
> single zero in it which doesn't seem to be a very interesting  
> colormap. :-)
>

It is attached to this mail.

Regards,

Gaëtan

> Thanks,
> Nick
>
>
>
>
>
>
> Reproducing the examples has required a bit more work, because the  
> test program simply doesn't write the output result to the disk. I  
> would suggest writing the output to the disk (see attached file) and  
> make the tests check the produced image. This can be acheived easily  
> with itkTestDriver with the following lines:
>
> FIND_PROGRAM(ITK_TEST_DRIVER itkTestDriver)
> SET(TEST_COMMAND ${ITK_TEST_DRIVER} --add-before-env PATH $ 
> {CMAKE_BINARY_DIR})
>
> ADD_TEST( RGBColormapTestSpring ${TEST_COMMAND} ${CurrentExe}
>           ${CMAKE_SOURCE_DIR}/images/grey.png spring.png spring
>           --compare spring.png ${CMAKE_SOURCE_DIR}/images/spring.png
> )
>
> ** done **
>
>
> IMO, color map functor should have the whole pixel type as template  
> parameter, to be able to use the RGBA pixels the same way
>
> ** done **
>
> are you sure that the Get/SetRed/Green/BlueChannel() method should  
> be there? I think they are quite strange, because itk::RGBPixel  
> clearly use the first channel as red, the second as green and the  
> last one as blue, so changing that in the functor means that this  
> property is not respected by the functor
>
> ** ?? **
>
> maybe m_MinimumRGBComponentValue and m_MaximumRGBComponentValue  
> default values should be min() and max() rather than Zero and One?
>
> ** done **
>
> is there a reason to not have implemented  
> ScalarToRGBColormapImageFilter as a subclass of  
> UnaryFunctorImageFilter? It is said in the paper that that filter  
> has large parts of code from UnaryFunctorImageFilter - maybe  
> subclassing it would be a better way to reuse the code without  
> duplication
>
> ** ?? **
>
> would you agree to add an option to compute the min and max values  
> to be used in the functor automatically? That way, the filter would  
> fit better in the pipeline architecture
>
> ** done **
>
> to make the usage of the predefined colormaps easier, I would  
> suggest the addition of a new method to  
> ScalarToRGBColormapImageFilter which would allow to use one of those  
> colormap in a single line. For example rgbfilter- 
> >SetColormap( RGBFilterType::HSV ) or rgbfilter->SetColormap( "hsv" )
>
> ** done **
>
> I would also suggest to drop the ScalarToRGB part in the colormap  
> names, and maybe also the Functor suffix -  
> ScalarToRGBJetColormapFunctor would become JetColormapFunctor, or  
> even JetColormap, which is a lot easier to read and write
>
> ** done **
>
> finally, I have contributed another colormap used quive often by  
> microscopists to see the saturation in the images - feel free to use  
> it :-)
>
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers

-------------- next part --------------
A non-text attachment was scrubbed...
Name: files.tgz
Type: application/octet-stream
Size: 2787 bytes
Desc: not available
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090106/e6b882dd/attachment.obj>
-------------- next part --------------

-- 
Gaëtan Lehmann
Biologie du Développement et de la Reproduction
INRA de Jouy-en-Josas (France)
tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
http://voxel.jouy.inra.fr  http://www.mandriva.org
http://www.itk.org  http://www.clavier-dvorak.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 186 bytes
Desc: Ceci est une signature ?lectronique PGP
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090106/e6b882dd/attachment.pgp>