[Bf-codereview] Pepper to trunk merge (issue 4934047)

brechtvanlommel at gmail.com brechtvanlommel at gmail.com
Thu Aug 25 16:43:58 CEST 2011


Did a superficial review of collada code, found some small issues, will
leave more detailed review to Nathan.


http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationExporter.cpp
File source/blender/collada/AnimationExporter.cpp (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationExporter.cpp#newcode36
source/blender/collada/AnimationExporter.cpp:36:
This function doesn't take into account the export_selected setting, is
that intentional? Also e.g. forEachMeshObjectInScene checks the scene
layers, but e.g. for lights it's not done, it's not clear to me that
these functions are consistent.

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationExporter.cpp#newcode44
source/blender/collada/AnimationExporter.cpp:44: {
There's some strange indenting going on here, everything below this line
seems indented for no particular reason.

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationExporter.cpp#newcode71
source/blender/collada/AnimationExporter.cpp:71: if (!ob->data) return;
This test is unnecessary.

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationExporter.cpp#newcode170
source/blender/collada/AnimationExporter.cpp:170: float *quat =
(float*)MEM_callocN(sizeof(float) * fcu->totvert * 4, "quat output
source values");
Memory leak: quat is never freed.

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationExporter.cpp#newcode286
source/blender/collada/AnimationExporter.cpp:286: float * eul_axis =
(float*)MEM_callocN(sizeof(float) * fcu->totvert, "quat output source
values");
Memory leak: eul and eul_axis are never freed.

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationImporter.cpp
File source/blender/collada/AnimationImporter.cpp (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationImporter.cpp#newcode182
source/blender/collada/AnimationImporter.cpp:182: cu->bezt[i].vec[1][0];
This statement does nothing, remove.

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationImporter.cpp#newcode192
source/blender/collada/AnimationImporter.cpp:192: else act =
ob->adt->action;
These two lines and others further in this file can be simplified to
just "act = verify_adt_action((ID*)&ob->id, 1);"

http://codereview.appspot.com/4934047/diff/6043/source/blender/collada/AnimationImporter.cpp#newcode442
source/blender/collada/AnimationImporter.cpp:442: fcu->rna_path =
BLI_strdupn(rna_path, strlen(rna_path));
Can just use BLI_strdup(rna_path).

http://codereview.appspot.com/4934047/


More information about the Bf-codereview mailing list