[Bf-committers] Modal bevel operator (issue 6208066)

[ideasman42 at gmail.com]

one main request - regarding storing copies of operator settings, other
changes are knitpicks.


http://codereview.appspot.com/6208066/diff/8001/source/blender/editors/mesh/editmesh_tools.c
File source/blender/editors/mesh/editmesh_tools.c (right):

http://codereview.appspot.com/6208066/diff/8001/source/blender/editors/mesh/editmesh_tools.c#newcode4317
source/blender/editors/mesh/editmesh_tools.c:4317: int modal;
*knitpick*, prefer 'is_modal' since modal is a callback struct member
elsewhere.

http://codereview.appspot.com/6208066/diff/8001/source/blender/editors/mesh/editmesh_tools.c#newcode4503
source/blender/editors/mesh/editmesh_tools.c:4503:
if(!edbm_bevel_init(C, op, 1))
*knitpick*, and own personal pref - use TRUE/FALSE makes bool args more
readable.

http://codereview.appspot.com/6208066/diff/8001/source/blender/editors/mesh/editmesh_tools.c#newcode4634
source/blender/editors/mesh/editmesh_tools.c:4634: int use_boundary;
storing settings for modal ops like mouse coords for comparison makes
sense but think this is overkill storing many settings twice. mot
related to storage - but general maintanance - we have settings in
wmOperator and BMesh-operators double up already. couldnt the modal
operator allocate (or duplicate) the op->ptr? - of you need original
settings it could make 2 duplicates and then use RNA access.

Internally if a var has to be accessed in a loop or so ofcourse it
should be made a local var to avoid many string lookups.

http://codereview.appspot.com/6208066/diff/8001/source/blender/editors/mesh/editmesh_utils.c
File source/blender/editors/mesh/editmesh_utils.c (right):

http://codereview.appspot.com/6208066/diff/8001/source/blender/editors/mesh/editmesh_utils.c#newcode64
source/blender/editors/mesh/editmesh_utils.c:64: BMBackup
EDBM_save_mesh_backup(BMEditMesh *em)
*knitpick*, backup is a bitof an odd name here...
perhaps
EDBM_redo_state_store()
EDBM_redo_state_restore()
EDBM_redo_state_free()
  ?

http://codereview.appspot.com/6208066/diff/8001/source/blender/editors/transform/transform.c
File source/blender/editors/transform/transform.c (right):

http://codereview.appspot.com/6208066/diff/8001/source/blender/editors/transform/transform.c#newcode1137
source/blender/editors/transform/transform.c:1137: int
calculateTransformCenter(bContext *C, int centerMode, float *vec, int
*ivec)
*knitpiick* named here are not too helpful - suggest
float tx_cent_3d[3], float tx_cent_region[2] ?
or just cent3d[3], cent2d as struct members are.

http://codereview.appspot.com/6208066/