[Dcmlib] GDCM commit

[Mathieu Malaterre]

En fait ton lien s'applique aux fonctions non-membre. Les fonctions dont 
je parlais etait membre de classes gdcm.

Mais sinon pour les fonctions non membres, je suis d'accord il faut bien 
sur mettre 'inline'

Mathieu

Jean-Pierre Roux wrote:
> Mathieu Malaterre wrote:
> 
>>Salut,
>>
>>   J'ai fais des commit, rien de tres spectaculaire, juste j'ai beaucoup de mal a lire les sources de gdcm. Est-ce qu'on peut se mettre d'accord sur certaines convention d'ecriture:
>>
> 
> Pas d'objection à tes suggestions, SAUF pour les inline.
> 
> Le code complet de la fonction doit effectivement être dand le .h (on 
> est bien d'accord : le compilo ne pourrait pas intégrer le code 
> directement dans l'application appelante s'il ne le connait pas  --> 
> unresolved external lors du link)
> 
> Mais si elle n'est pas déclarée inline, elle sera traitée comme une 
> fonction normale (pas inline)
> 
> le lien
> http://www.parashift.com/c++-faq-lite/inline-functions.html#faq-9.8
> donne une description parfaitement confusionniste à ce sujet.
> 
> --------------------------------------------------------------------------------------------------
> 
> 
>       [9.6] How do you tell the compiler to make a non-member function
>       inline?
> 
> When you declare an inline function, it looks just like a normal function:
> 
>  void f(int i, char c);
> 
> ===> Puisque la définition se fait en même temps que la déclaration, à 
> quoi ça sert de présenter les deux, de manière contradictoire ?!?
> 
> But when you define an inline function, you prepend the function's 
> definition with the keyword inline, and you put the definition into a 
> header file:
> 
>  inline
>  void f(int i, char c)
>  {
>    /.../
>  }
> 
> Note: It's imperative that the function's definition (the part between 
> the {...}) be placed in a header file, unless the function is used only 
> in a single .cpp file. In particular, if you put the inline function's 
> definition into a .cpp file and you call it from some other .cpp file, 
> you'll get an "unresolved external" error from the linker.
> 
> --------------------------------------------------------------------------------------------
> 
>>
>>
>>- pas de printf
>>- pas de parentheses aux return:
>>return true;
>>au lieu de
>>return(true);
>>
>>- pas besoin de void quand la fonction ne prend pas de parametre:
>>
>>foo()
>>au lieu de
>>foo(void)
>>
> 
>>
>>- le parenthesage:
>>
> Ca a l'air d'être un problème religieux !
> Il faut qu'on réunisse un web-concile à ce sujet, car il est clair que 
> s'il y a deux chapelles dans le Labo, dont les affidés font des modifs 
> en sens inverse chaque fois qu'ils ouvrent un .cxx, on n'est pas prêt de 
> s'en sortir !
> 
>>
>>
>>void foo() {
>>...
>>}
>>
>>au lieu
>>
>>void foo()
>>{
>>...
>>}
>>
>>- Est-ce que "virtual" est repete meme dans les classes heritees (convention d'ecriture car non necessaire).
>>
>>- Il n'y a besoin de specifier explicitement 'inline' lorsqu'une methode est implementee dans la classe (fichier header).
>>http://www.parashift.com/c++-faq-lite/inline-functions.html#faq-9.8
>>
> Voir ma remarque + haut.
> 
>>
>>
>>- Tolere pour l'instant: sprintf, FILE*
>>Mais j'aimerais passer a une approche plus C++ avec les iostream dans l'avenir
>>
> 
> A part le fait de compliquer l'écriture, ça a un réel intérêt les iostream ?
> 
>>
>>
>>Je ne reproche rien a personne, je me porte meme volontaire pour les faire. Donc est-ce que ca vous va ou vous voulez des choses differentes?
>>
>>
>>---
>>
>>Sinon plus grave je suis tombe sur ca dans gdcmFile:
>>
>>1.
>>
> Bien d'accord.
> Il y a des modifs 'profondes' à faire la-dedans (qui ne changeront pas 
> l'API)
> 
>>
>>
>>void * gdcmFile::GetImageDataRaw () {
>>  if (Header->HasLUT())
>>     /// \todo Let gdcmHeadar user a chance to get the right value
>>       /// Create a member lgrTotaleRaw ???
>>     lgrTotale /= 3;
>>  PixelData = new unsigned char[lgrTotale];
>>...
>>
>>Ca veut dire que plus j'appelle cette methode plus la taille de l'image est petite.
>>Plus serieusement, on peut pas simplifier les var membres de gdcmFile.
>>Ex1:
>>  gdcmHeader *Header;
>>  bool SelfHeader;
>>
>>Y'a t'il vraiment besoin du booleen, est-ce qu'on ne peut pas directement regarder si Header est different de NULL ?
>>
>>Ex2:
>>void* PixelData;
>>Est-ce que ce membre va disparaitre vu que l'image est dans le Header (cf dernier mail de JP).
>>
>>Ex3:
>>  size_t lgrTotaleRaw;
>>  size_t lgrTotale;
>>  int PixelRead;
>>  int Parsed;
>>
>>Je suppose que c'est liee a la question 2, mais qd meme on ne peut pas avoir une seule lgtTotale. Pour moi l'operation de tra
>>nformation via lookup table est commande par l'utilisateur. Ca veut dire qu'un volume va etre cree a la demande de l'utilisateur qd l'image a une lookup table
>>et qu'il veut appliquee celle la.
>>
> En fait, c'est lorsqu'on a une image gray level + LUT qu'on 'fabrique' 
> une zone mémoire contenant une image RGB, par GetPixelData, ou une image 
> Gray Level, par GetPixelDataRaw.
> 
>>
>>Autre ex, si on lit + ecrit l'image est-ce que l'on veut vraiment appliquee la lookup table a l'image ? 
>>
> Lorsqu'on réécrit l'image, c'est forcement l'image avec des pixels RGB :
> - si elle était stockée sous forme Plan R+ Plan G + Plan B, ou sous 
> forme de pixels YBR, ce qui n'est pas génant dans ce cas là,
> - si elle était stockée sous forme de GrayLevel + LUT, et là, ça se discute
> 
>>En plus vu qu'il n'y a qu'un seul void* PixelData si l'utilisateur demande d'abord le volume raw puis le volume filtree puis encore le volume raw on ne peut pas mettre en cache les volumes precalcules.
>>
>>2.
>>
>>Le choix unsigned char vs char semble complement arbitraire. Par defaut sur toutes les plateformes (sauf SUN), char est signed donc il y a bien une difference. Est-ce qu'on peut donc tout passer en unsigned char ?
>>
>>3.
>>
>>
>>C'est quoi: bool ParsePixelData(); Normalement VC++ ne sait pas gerer les fonctions non implementees...
>>
> C'est une fonction qui inspecte l'organisation du Dicom Element des Pixels
> En cas d'embedded JPEG, ou embedded RLE, ca peut relever d'un foutoir 
> sans nom - soit des trucs qui ressemblent à des SQItems dans  etre dans 
> un SeqEntry et qui contiennent les 'JPEG fragments', soit une table de 
> longueur variable contennant l'offset du début de chaque fragment, etc.
> Elle n'est pas appellée dans la dcmLib elle même, seulement dans un 
> exécutable. Elle est précieuse pour savoir si une image Jpeg qui pête a 
> une description kasher pour son Pixel Element.
> La méthode  se trouve dans :
> gdcmParsePixels.cxx
> comme methode de la classe gdcmFile:  
> bool gdcmFile::ParsePixelData(void) {
> ...
> }
> Je ne l'ai pas mise dans gdcmParsePixels.cxx pour ne pas allonger le 
> temps de compile.
> 
> C'est sa présence dans un autre .cxx qui pose un pb ?
> 
>>
>>
>>
>>----
>>
>>De maniere plus general, j'aurais aime un retour d'experience de cmake.
>>J'ai compris que pour l'installation python ce n'etait pas au point.
>>Mais en ce qui concerne :
>>- la compilation
>>- interface pour regler les options de compil
>>- les tests
>>- l'install des lib c++
>>
>>Meme si ce n'est que des questions merci de me les envoyer
>>
>>Voila voila merci d'avoir lu jusque la,
>>Mathieu
>>
>>Au fait j'ai envoyer sur fr.comp.lang.c++ le probleme des const char* + string ... j'espere que j'aurais une reponse.
>>
>>
>