Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(213)

Issue 5967069: Fix for BMesh Bevel Operator

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 1 month ago by TrumanBlending
Modified:
12 years, 1 month ago
Reviewers:
ideasman42, bf-codereview
Base URL:
https://svn.blender.org/svnroot/bf-blender/trunk/blender/
Visibility:
Public.

Description

This patch fixes issues with the current bevel operator. 1. Correct absolute distance bevelling. The user can now specify an absolute distance which all edges will then be offset. This doesn't work in the trunk bevel op. 2. Better defaults. Now by default it is set up to do absolute distance bevel. 3. Consistency. Options are now named in the same fashion as the inset operator for consistency between tools. The options also have the same effects as other tools.

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+135 lines, -111 lines) Patch
source/blender/bmesh/intern/bmesh_opdefines.c View 1 chunk +3 lines, -3 lines 0 comments Download
source/blender/bmesh/operators/bmo_bevel.c View 9 chunks +116 lines, -98 lines 6 comments Download
source/blender/editors/mesh/editmesh_tools.c View 4 chunks +16 lines, -10 lines 0 comments Download

Messages

Total messages: 2
TrumanBlending
12 years, 1 month ago (2012-04-03 22:11:57 UTC) #1
ideasman42
12 years, 1 month ago (2012-04-05 09:58:18 UTC) #2
Added some comments & requests for clarification.

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

http://codereview.appspot.com/5967069/diff/1/source/blender/bmesh/operators/b...
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/b...
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/b...
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/b...
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/b...
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/b...
source/blender/bmesh/operators/bmo_bevel.c:420: mul_v3_fl(co, 1.0f /
sinf(angle));
again - shell_angle_to_dist?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b