[Bf-codereview] WeightVG modifiers weight preview, and general weight preview enhancement (issue 5531047)

[ideasman42 at gmail.com]

result of some discussion on IRC + review.


http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/BKE_DerivedMesh.h
File source/blender/blenkernel/BKE_DerivedMesh.h (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/BKE_DerivedMesh.h#newcode72
source/blender/blenkernel/BKE_DerivedMesh.h:72: #define DM_MOD_DO_WMCOL
(1 << 0)
think this can be avoided, and mod stack detect this case without a flag
in the DM.

http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/intern/DerivedMesh.c
File source/blender/blenkernel/intern/DerivedMesh.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/intern/DerivedMesh.c#newcode681
source/blender/blenkernel/intern/DerivedMesh.c:681: r_col[3] =
FTOCHAR(colf[0]);
re FTOCHAR:
This isn't needed, we can assume values are already clamped.
existing code was fine.

http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/intern/DerivedMesh.c#newcode785
source/blender/blenkernel/intern/DerivedMesh.c:785: unsigned char
*wtcol_f = DM_get_face_data_layer(dm, CD_WEIGHT_MCOL);
prefer dm->getVertDataArray(), for now its unlikley to make a real
difference.

http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/intern/dynamicpaint.c
File source/blender/blenkernel/intern/dynamicpaint.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/intern/dynamicpaint.c#newcode1728
source/blender/blenkernel/intern/dynamicpaint.c:1728: int index =
*((&mface[i].v1)+j);
re: edits to the for loop and not doing quad/tri color edits.

this is an improvement, but would prefer these be committed to trunk
directly.

http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/intern/modifier.c
File source/blender/blenkernel/intern/modifier.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/blenkernel/intern/modifier.c#newcode146
source/blender/blenkernel/intern/modifier.c:146: int
modifier_usesWMColPreview(ModifierData *md)
with a generic flag on the modifier this function wont be needed. or at
least the type spesific parts wont.

http://codereview.appspot.com/5531047/diff/9001/source/blender/editors/space_view3d/view3d_header.c
File source/blender/editors/space_view3d/view3d_header.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/editors/space_view3d/view3d_header.c#newcode515
source/blender/editors/space_view3d/view3d_header.c:515: uiItemR(layout,
&toolsptr, "weights_preview_mode", UI_ITEM_R_ICON_ONLY|UI_ITEM_R_EXPAND,
better solve global data layer preview options in a different patch. uvs
can use too but rather exclude from wight preview patch.

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesdna/DNA_modifier_types.h
File source/blender/makesdna/DNA_modifier_types.h (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesdna/DNA_modifier_types.h#newcode881
source/blender/makesdna/DNA_modifier_types.h:881: int		common_flags;
think this should be removed and the modifiers flag used.

one of the flags like eModifierMode_OnCage

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesdna/DNA_modifier_types.h#newcode1025
source/blender/makesdna/DNA_modifier_types.h:1025: #define
MOD_WVG_CFLAG_WEIGHT_PREVIEW		(1 << 0)
dont think this flag is needed, the modifier stack function can see if
modifier preview is used on any mod and set a var on the stack based on
this.

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesdna/DNA_scene_types.h
File source/blender/makesdna/DNA_scene_types.h (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesdna/DNA_scene_types.h#newcode911
source/blender/makesdna/DNA_scene_types.h:911: short uv_selectmode;
please don't do tidy up edits in branch.

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesdna/DNA_scene_types.h#newcode1547
source/blender/makesdna/DNA_scene_types.h:1547: /*
toolsettings->weights_preview */
again, think this should be removed, and use operator option only.

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesrna/intern/rna_modifier.c
File source/blender/makesrna/intern/rna_modifier.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesrna/intern/rna_modifier.c#newcode2603
source/blender/makesrna/intern/rna_modifier.c:2603: prop=
RNA_def_property(srna, "use_weight_preview", PROP_BOOLEAN, PROP_NONE);
again, make generic option like cage.

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesrna/intern/rna_scene.c
File source/blender/makesrna/intern/rna_scene.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/makesrna/intern/rna_scene.c#newcode1447
source/blender/makesrna/intern/rna_scene.c:1447: prop=
RNA_def_property(srna, "weights_preview_mode", PROP_ENUM, PROP_NONE);
-1 for this option.

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvg_util.c
File source/blender/modifiers/intern/MOD_weightvg_util.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvg_util.c#newcode268
source/blender/modifiers/intern/MOD_weightvg_util.c:268: void
weightvg_set_weightcol(DerivedMesh *dm, int num, const int *indices,
float *weights)
Think this function can be merged with add_weight_mcol_dm()

probably rename and expose add_weight_mcol_dm() as an api funk of
BKE_derivedmesh.h

any important differences can be args.

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvg_util.c#newcode275
source/blender/modifiers/intern/MOD_weightvg_util.c:275: MCol *col =
CustomData_duplicate_referenced_layer(&dm->faceData, CD_WEIGHT_MCOL,
nfaces);
since this layer is temp view cache you can not bother checking if its
referenced.

you could ifdef this and note that editing original is ok.

or just leave it if your worried it causes problems later.

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvgedit.c
File source/blender/modifiers/intern/MOD_weightvgedit.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvgedit.c#newcode241
source/blender/modifiers/intern/MOD_weightvgedit.c:241: if(do_prev)
would prefer that the generic modifier flag be checked in the main
modifier loop and this function run there, not call from within each
vgroup modifier.

This way you have the advantage that if 2+ modifiers have this option
you can only calculate for the last one.

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvgmix.c
File source/blender/modifiers/intern/MOD_weightvgmix.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvgmix.c#newcode370
source/blender/modifiers/intern/MOD_weightvgmix.c:370: if(do_prev)
see prev comment, better in main mod calc.

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvgproximity.c
File source/blender/modifiers/intern/MOD_weightvgproximity.c (right):

http://codereview.appspot.com/5531047/diff/9001/source/blender/modifiers/intern/MOD_weightvgproximity.c#newcode509
source/blender/modifiers/intern/MOD_weightvgproximity.c:509: if(do_prev)
see prev comment, better in main mod calc.

http://codereview.appspot.com/5531047/