Weight Transfer review

[ideasman42, 2012-05-17 09:04:35]

http://codereview.appspot.com/6210064/diff/1/source/blender/editors/object/ob...
File source/blender/editors/object/object_vgroup.c (right):

http://codereview.appspot.com/6210064/diff/1/source/blender/editors/object/ob...
source/blender/editors/object/object_vgroup.c:380: int
ED_vgroup_copy_single(Object *ob_dst, const Object *ob_src)
the single vgroup to copy could be an argument - allows to be more flexible
later even if for now, the arg is "ob_src->actdef-1" for all callers.

http://codereview.appspot.com/6210064/diff/1/source/blender/editors/object/ob...
source/blender/editors/object/object_vgroup.c:390: dg_src=
BLI_findlink(&ob_src->defbase, (ob_src->actdef-1));
*knitpick* - please use our style guide - spaces around operators:
http://wiki.blender.org/index.php/Dev:Doc/CodeStyle

http://codereview.appspot.com/6210064/diff/1/source/blender/editors/object/ob...
source/blender/editors/object/object_vgroup.c:642: int
ED_vgroup_transfer_weight_by_nearest_vertex_in_face_single(Object *ob_dst,
Object *ob_src, short mode)
dont think it makes sense to split up single/all functions at this level. would
have one function. Then have an argument for which are copied.

Otherwise you end up with 2 almost identical functuions.

http://codereview.appspot.com/6210064/diff/1/source/blender/editors/object/ob...
source/blender/editors/object/object_vgroup.c:696: if(mode == 1){
Having the mode check here is not good. all interpolation should be done first -
then mode check ar line 727 or so.

around the comment /*copy weight*/

basically - you only have to do interpolation once.
then mode check once.

theres no need to expland these out.

http://codereview.appspot.com/6210064/diff/1/source/blender/editors/object/ob...
source/blender/editors/object/object_vgroup.c:696: if(mode == 1){
also mode == 1 isnt so readable, better define an enum.

http://codereview.appspot.com/6210064/diff/1/source/blender/editors/object/ob...
source/blender/editors/object/object_vgroup.c:707: dist_v1=
len_squared_v3v3(tmp_co, mv_src[mface_src[nearest.index].v1].co);
quite sure this isnt correct interpolation, should use areas, not distances,
check barycentric interpolation.

- so if it gives OK results leave for now but would want this checked on before
merging into trunk.

http://codereview.appspot.com/6210064/diff/1/source/blender/editors/object/ob...
source/blender/editors/object/object_vgroup.c:3400: void
OBJECT_OT_vertex_group_copy_to_selected_single(wmOperatorType *ot)
suggest to have one operator with single vgroup as an option, if this is a
hassle, it can be done later.