[Dcmlib] GDCM commit

[Jean-Pierre Roux]

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 tranformation 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.
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.creatis.insa-lyon.fr/pipermail/dcmlib/attachments/20040623/5aad9741/attachment.html>