User:Mathieu/Slicer

From NAMIC Wiki
Jump to: navigation, search

Current status of Work on slicer

MRML Styles

Use of 'this->' over the m_ notation for the following reason. There are still cases where the compiler cannot decide to use member variable from dependant class: For more info see: GCC Changes

In a template definition, unqualified names will no longer find members of a dependent base (as specified by [temp.dep]/3 in the C++ standard). For example, 


	template <typename T> struct B {
	  int m;
	  int n;
	  int f ();
	  int g ();
	};
	int n;
	int g ();
	template <typename T> struct C : B<T> {
	  void h ()
	  {
	    m = 0; // error
	    f ();  // error
	    n = 0; // ::n is modified
	    g ();  // ::g is called
	  }
	};


Issues

Not using m_ or an vtk style notation one cannot anymore write: 


vtkFoo *Foo;

or 


namespace itk {
  Foo *m_Foo;
}

since it becomes: 


namespace {
  Foo *Foo;
}

One solution is simply to give more descriptive name to your member variable (extreme programming). 


namespace {
  Foo *MoreDescriptiveName;
}

Note: vxl append a trailing _ to there members variables.

MRML Copy Constructor-alike

Currently the mechanism is the following:

(Assume Node is a sublass of a mrml::Object) 


namespace mrml
{
virtual void VolumeNode::Copy(Node *anode)
{
  Node::Copy(anode);
  VolumeNode *node = (vtkMRMLVolumeNode *) anode;

  // Copy private data of VolumeNode
}
}

Using this scheme, the following becomes perfectly legal. Unfortunately there is not relationship in between the object. So the current behavior is not user friendly.

File:9859c9634094438c4efcd660bde8c509.png

Since one can easily write (and compile): 


A *a;
B *b;
a->Copy(b);


A more commonly used pattern is instead the following (aka ShallowCopy in VTK): 


namespace mrml
{
void VolumeNode::Copy(VolumeNode *node)
{
  Superclass::Copy(node); // Superclass is a typedef to the actual superclass

  // Copy private data of VolumeNode *node
}
}

Therefore the following code will not compile: 


A *a;
B *b;
a->Copy(b);

PIMPL

Issues #1

Implementation in Slicer/MRML. Since MRML is using SmartPointer, in order to respect PIMPL, we cannot include the declaration of a particular class. What this means is that we cannot return a SmartPointer of a particular member variable.

--- A.h 

 #include "Object.h"
 class A : public Object
 {
   B* GetB();
   B *b;
 }

--- B.h 

 #include "Object.h"
 class B : public Object
 {
   typdef SmartPointer<B> Pointer;
 }

Thus user can be tempted to do: 


A::Pointer a = A::New();
B *local = a->GetB();

'local' is not a smartpointer anymore and thus the reference count is not incremented...

In comparison ITK always return a SmartPointer and thus this user mistake is avoided. On the other hand some other project uses PIMPL. And indeed return a class pointer. But uses MACROs that call GetPointer on the SmartPointer, thus the declaration file of the object used within the SmartPointer need to be included (no improvement toward compilation speed).

Issues #2

Now one has to actually creates the structure 'Internal' using a new and then calling delete. Nothing garantee than people won't forgot. 


class AItnternals; //Forward declaration
class A : public Object
{
public:
  Object *GetP();
private:
  AInternals *Internal; // Internal implementation, to hide smart pointer
}



class AInternals
{
  AInternals()
    {
    this->P = Object::New(); // SmartPointer
    }
  typedef Object::Pointer ObjectPointerType
  ObjectPointerType P;
};

A::A()
{
  this->Internal = new AInternals;
}
A::~A()
{
  delete this->Internal; // Leak is people forgot... which defeat the whole purpose of SmartPointer ??
}

vtkXMLParser

Done! Removed dependencie to the vtkXMLParser by providing a much lightwieth mrml::XmlParser that is encapsulate into the existing mrml::Parser class. mrml::XMLParser is a general XMLParser and all the MRML tags (StartElement, EndElement) are implemented by overloading virtual call in mrml::Parser.

As a side effect, mrml now require an expat library. TODO: Need to do the mangling properly. Also need to check for newer version of expat. How about DTD ?

Doxygen

Currently MRML follow VTK path. And thus we need the full perl hand written parser to generate the documentation. This makes is very difficult to extend, maintain. Suggestion: Do not use VTK style comment, and use plain doxygen style from begining (ITK style).

mrml::Scene

mrml::Scene is a multiple object: it used to do input stream AND output stream. This is highly unusual (looks like a FILE* implementation). This should be redone to split implementation of input vs output.

On top of that it act like a Collection of object with meta request. Note: And also a bug for the uniq call (uniq is uniq to the previous one that's all POSIX require). Need to split Scene into:

  1. SceneReader
  2. SceneWriter
  3. SceneObjectFactory
  4. SceneCollection (or pure mrml::Collection might be better)

instead of a giant army-knife class.


mrml::Collection

Introducing a mrml::Collection implementation. Basically just a wrapper around std::list. 


class MRML_EXPORT Collection : public Object
{
public:
  ...
  typedef Object* Item;
  void AddItem(Item elem);
  void RemoveItem (unsigned long i);
  void RemoveAllItems();

  unsigned long GetNumberOfItems();
  bool IsEmpty();
  Object* GetItemAsObject (unsigned long i);
}

Note: Should't it be a std::set instead of a std::list ?

mrml::Transform

Just duplicate exactly itk::Transform, assuming this will be generic enough and well suited for us.

mrml::Image

Define the common set of operation on an image:

  • Spacing
  • Direction
  • Origin
  • Direction

TODO: What is the dimensionality of the Origin for a 2D Image ? 2 or 3 ?? DataDimension vs WorldRepresentationDimension. eg. DICOM 2D slice are still expressed in 3D space.

Wrapping

No problem so far. The build system might need to be enhanced to be more flexible, right now the class name are hardcoded in the cxx file, instead of a generated file. 03/22: Done: the class are not hardcoded in the cxx file. This is generated via a list in the CMakeLists.txt.


TODO

  1. Check for the vjObject wrapper around any VTK object. What needs to be done to get VTK wrapping and mrml wrapping to coexist and see each other ?
in particular a vtkKWApplication.
  1. Do we need Tcl, Python, Java, perl...

Swig Wrapping

Swig wrapping is slightly different from the VTK style wrapping and commands are like that: 


package require mrml

set o [mrmlObject_New]
puts [$o GetNameOfClass]

instead of the VTK-style 


package require mrml

mrmlObject o
puts [o GetNameOfClass]

Copyright header

The current header is wider than the 80 lines, will make kwStyle unhappy. Also it containes CVS keyword which are not replaced anymore by svn. 

 /*=auto========================================================================
 
 Portions (c) Copyright 2005 Brigham and Women's Hospital (BWH) All Rights Reserved.
 
 See Doc/copyright/copyright.txt
 or http://www.slicer.org/copyright/copyright.txt for details.
 
 Program:   3D Slicer
 Module:    $RCSfile: VolumeNode.cxx,v $
 Date:      $Date: 2006/03/03 22:26:41 $
 Version:   $Revision: 1.12 $
 
 ========================================================================auto=*/

Proposed one: 

 /*=auto=========================================================================
 
   Portions (c) Copyright 2005 Brigham and Women's Hospital (BWH)
   All Rights Reserved.
 
   See Doc/copyright/copyright.txt
   or http://www.slicer.org/copyright/copyright.txt for details.
 
   Program:   MRML
   Module:    $URL$
   Date:      $Date$
   Version:   $Rev$
 
 =========================================================================auto=*/

One also needs to explicitely set the proset:keyword : 

 svn propset svn:keywords "Date URL Rev" mrmlNode.h

Ref: http://svnbook.red-bean.com/nightly/en/svn.advanced.props.html#svn.advanced.props.special.keywords


Sub namespaces

Which solution to adopt. Assuming we have: 


namespace mrml
{
  class Node
  {
  };
} // end namespace mrml

Which one is the most intuitive:

  • Solution #1


namespace mrml
{
  namespace vtk
  {
    class Node : public ::mrml::Node
    {
    };
  } // end namespace vtk
} // end namespace mrml
  • Solution #2


namespace mrml
{
  class vtkNode : public Node
  {
  };
} // end namespace mrml
  • Solution #3


namespace vtkmrml
{
  class Node : public ::mrml::Node
  {
  };
} // end namespace vtkmrml
Retrieved from "https://www.na-mic.org/w/index.php?title=User:Mathieu/Slicer&oldid=4181"