[Bf-codereview] Use DerivedMesh instead of Mesh in COLLADA GeometryExporter (issue4387052)

[danielmtavares at gmail.com]

Reviewers: bf-codereview_blender.org, jesterKing, dfelinto, brechtvl,  
official.address243,

Message:
On 2011/05/02 23:05:24, dfelinto wrote:
> Some comments on syntax:
> * Try to follow the file syntax (e.g. if { in oppose to if \n {).
> * Also as a general rule of thumb patch should have only the essential
code on
> it. e.g. changes of tabs stripping in the beginning of lines should be
avoided
> (they led to misleading traceback of commits when using svnblame/log.

> It's always recommended to use meld (linux) or svntortoise base (win)
to clean
> it first.

> Other than that patch LGTM. Although I'm not familiar with this area
of code.


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp
> File blender/source/blender/collada/GeometryExporter.cpp (left):


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#oldcode30
> blender/source/blender/collada/GeometryExporter.cpp:30:
> better not change lines if not needed. The cleaner the patch the
better (it
> helps svn praise/blame/log later)


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp
> File blender/source/blender/collada/GeometryExporter.cpp (right):


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode47
> blender/source/blender/collada/GeometryExporter.cpp:47:
> same comment as above.


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode127
> blender/source/blender/collada/GeometryExporter.cpp:127:
> avoid changes with no real change (e.g. stripping off tabs).


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode408
> blender/source/blender/collada/GeometryExporter.cpp:408:
if(!smoothnormal)	//
> flat
> use { in the same line as if (i.e. follow file syntax)


http://codereview.appspot.com/4387052/diff/1/blender/source/blender/collada/GeometryExporter.cpp#newcode413
> blender/source/blender/collada/GeometryExporter.cpp:413: {
> same as above

Hi Dalai,

Thanks for the feedback.

Do you mean using meld/tortoise to clean the trailing spaces/tabs as its
own patch? What's the general approach towards this? I'm assuming
there's nothing preventing people from checking-in code with trailing
tabs/spaces.

Description:
GeometryExporter refactor to allow use of DerivedMesh rather than Mesh.
I took the opportunity and cleaned up some of the class' interface, as
well as, tried to improve consistency.


Please review this at http://codereview.appspot.com/4387052/

Affected files:
   M blender/source/blender/blenkernel/BKE_DerivedMesh.h
   M blender/source/blender/collada/GeometryExporter.cpp
   M blender/source/blender/collada/GeometryExporter.h