[Bf-codereview] Fix for BMesh Bevel Operator (issue 5967069)

ideasman42 at gmail.com ideasman42 at gmail.com
Thu Apr 5 11:58:18 CEST 2012


Added some comments & requests for clarification.


http://codereview.appspot.com/5967069/diff/1/source/blender/bmesh/operators/bmo_bevel.c
File source/blender/bmesh/operators/bmo_bevel.c (right):

http://codereview.appspot.com/5967069/diff/1/source/blender/bmesh/operators/bmo_bevel.c#newcode265
source/blender/bmesh/operators/bmo_bevel.c:265: #if 0
would be nice to give some brief description as to why this is disabled.
what issues did it cause? - or if its real useless, just remove.

http://codereview.appspot.com/5967069/diff/1/source/blender/bmesh/operators/bmo_bevel.c#newcode331
source/blender/bmesh/operators/bmo_bevel.c:331: mul_v3_fl(co, 1.0f /
sinf(angle));
can't shell_angle_to_dist be used here?

http://codereview.appspot.com/5967069/diff/1/source/blender/bmesh/operators/bmo_bevel.c#newcode367
source/blender/bmesh/operators/bmo_bevel.c:367: mul_v3_fl(co, 1.0f /
sinf(angle));
again? - shell_angle_to_dist

http://codereview.appspot.com/5967069/diff/1/source/blender/bmesh/operators/bmo_bevel.c#newcode402
source/blender/bmesh/operators/bmo_bevel.c:402: BM_ITER(l2, &liter2, bm,
BM_LOOPS_OF_VERT, l->v) {
I don't follow the logic here, might it be better to test
BM_EDGES_OF_VERT ?

http://codereview.appspot.com/5967069/diff/1/source/blender/bmesh/operators/bmo_bevel.c#newcode415
source/blender/bmesh/operators/bmo_bevel.c:415: angle =
angle_v3v3(co_other, co);
again - shell_angle_to_dist?

http://codereview.appspot.com/5967069/diff/1/source/blender/bmesh/operators/bmo_bevel.c#newcode420
source/blender/bmesh/operators/bmo_bevel.c:420: mul_v3_fl(co, 1.0f /
sinf(angle));
again - shell_angle_to_dist?

http://codereview.appspot.com/5967069/


More information about the Bf-codereview mailing list