Page MenuHome
Differential D481 Diff 1535 source/blender/bmesh/operators/bmo_primitive.c

Changeset View

Standalone View

View Options

source/blender/bmesh/operators/bmo_primitive.c

Context not available.
#include "MEM_guardedalloc.h" #include "MEM_guardedalloc.h"
#include "BLI_math.h" #include "BLI_math.h"
#include "BLI_alloca.h"
#include "BKE_editmesh.h"
#include "DNA_meshdata_types.h"
#include "bmesh.h" #include "bmesh.h"
#include "intern/bmesh_operators_private.h" #include "intern/bmesh_operators_private.h"
/* ************************ primitives ******************* */ /* ************************ primitives ******************* */
static const float icovert[12][3] = { static const float icovert[12][3] = {
Context not available.
} }
#undef XY #undef XY
BM_mesh_create_grid_uvs(bm, xtot, ytot);
}
void BM_mesh_create_grid_uvs(BMesh *bm, unsigned int x_segments, unsigned int y_segments)
{
BMFace *f;
BMLoop *l;
BMIter iter, liter;
MLoopUV *luv;
int cd_loop_uv_offset, loopindex;
float x, y, dx, dy;
campbellbartonUnsubmitted
Not Done
Inline Actions

Whats the purpose of disabling these flags?

campbellbarton: Whats the purpose of disabling these flags?
CommanderCorianderSalamanderUnsubmitted
Not Done
Inline Actions

If they stay enabled, those faces get unwrapped again next time BM_mesh_calc_uvs_grid is called on this mesh (ie. adding a second primitive, with unwrapping, to the same mesh as the first). Those flags were just enabled in this function directly above.

CommanderCorianderSalamander: If they stay enabled, those faces get unwrapped again next time BM_mesh_calc_uvs_grid is called…
campbellbartonUnsubmitted
Not Done
Inline Actions

These flags are reset for each bmesh-operator call, you shouldn't have to initialize them.

If you do - then it must be some bug in the operator flag stack.

campbellbarton: These flags are reset for each bmesh-operator call, you shouldn't have to initialize them. If…
cd_loop_uv_offset = CustomData_get_offset(&bm->ldata, CD_MLOOPUV);
if (cd_loop_uv_offset != -1) {
campbellbartonUnsubmitted
Not Done
Inline Actions

Better make caller responsible for ensuring BMesh has UV's, This goes for all other similar functions too.

campbellbarton: Better make caller responsible for ensuring BMesh has UV's, This goes for all other similar…
dx = 1.0f / (x_segments - 1);
dy = 1.0f / (y_segments - 1);
x = 0;
y = 0;
BM_ITER_MESH (f, &iter, bm, BM_FACES_OF_MESH) {
BM_ITER_ELEM_INDEX (l, &liter, f, BM_LOOPS_OF_FACE, loopindex) {
luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
switch (loopindex) {
case 0:
x += dx;
break;
case 1:
y += dy;
break;
case 2:
x -= dx;
break;
campbellbartonUnsubmitted
Not Done
Inline Actions

Why disable?

campbellbarton: Why disable?
case 3:
y -= dy;
break;
}
luv->uv[0] = x;
luv->uv[1] = y;
}
x += dx;
if (x >= 1.0f) {
x = 0;
y += dy;
}
}
}
} }
void bmo_create_uvsphere_exec(BMesh *bm, BMOperator *op) void bmo_create_uvsphere_exec(BMesh *bm, BMOperator *op)
Context not available.
BMO_op_callf(bm, op->flag, "remove_doubles verts=%fv dist=%f", VERT_MARK, min_ff(len, len2) / 3.0f); BMO_op_callf(bm, op->flag, "remove_doubles verts=%fv dist=%f", VERT_MARK, min_ff(len, len2) / 3.0f);
} }
BM_mesh_create_sphere_uvs(bm);
/* and now do imat */ /* and now do imat */
BM_ITER_MESH (eve, &iter, bm, BM_VERTS_OF_MESH) { BM_ITER_MESH (eve, &iter, bm, BM_VERTS_OF_MESH) {
if (BMO_elem_flag_test(bm, eve, VERT_MARK)) { if (BMO_elem_flag_test(bm, eve, VERT_MARK)) {
Context not available.
BMO_op_finish(bm, &bmop); BMO_op_finish(bm, &bmop);
} }
BM_mesh_create_sphere_uvs(bm);
/* must transform after because of sphere subdivision */ /* must transform after because of sphere subdivision */
BM_ITER_MESH (v, &viter, bm, BM_VERTS_OF_MESH) { BM_ITER_MESH (v, &viter, bm, BM_VERTS_OF_MESH) {
if (BMO_elem_flag_test(bm, v, VERT_MARK)) { if (BMO_elem_flag_test(bm, v, VERT_MARK)) {
Context not available.
BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "verts.out", BM_VERT, VERT_MARK); BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "verts.out", BM_VERT, VERT_MARK);
} }
void BM_mesh_create_sphere_uvs(BMesh *bm)
{
BMFace *f;
BMLoop *l;
BMIter iter, liter;
MLoopUV *luv;
int cd_loop_uv_offset;
float v[3] = { 0 };
float rot[4][4];
float axis[3] = { 0, 1, 1};
float angle = 30;
float size[3] = { 1, 1, 1};
float **uvs;
float dx;
int mi, i;
cd_loop_uv_offset = CustomData_get_offset(&bm->ldata, CD_MLOOPUV);
if (cd_loop_uv_offset != -1) {
loc_axisangle_size_to_mat4(rot, v, axis, angle, size);
BM_ITER_MESH (f, &iter, bm, BM_FACES_OF_MESH) {
uvs = BLI_array_alloca(uvs, f->len);
campbellbartonUnsubmitted
Not Done
Inline Actions

alloca wont be freed until the function exits. you have to be very careful calling alloca in a loop, and in this case it can easily run out of stack memory and crash.

In this case you could move the body of the for loop into a static function.

campbellbarton: `alloca` wont be freed until the function exits. you have to be very careful calling alloca in…
mont29AuthorUnsubmitted
Not Done
Inline Actions

shouldn’t this func be static?

If static, please also rename it to bm_mesh_calc_uvs_sphere_face

mont29: shouldn’t this func be static? If static, please also rename it to…
CommanderCorianderSalamanderUnsubmitted
Not Done
Inline Actions

It's declared static at the top of the file ;)

CommanderCorianderSalamander: It's declared static at the top of the file ;)
mont29AuthorUnsubmitted
Not Done
Inline Actions

With UVSphere or IcoSphere, len will never be > 4, no need to use alloca in this case imho, just simply use a float (*)[4] table (with a BLI_assert on f->len)?

Or do you want this func to be usable with any kind of sphere-like geometry?

mont29: With UVSphere or IcoSphere, len will never be > 4, no need to use alloca in this case imho…
BM_ITER_ELEM_INDEX (l, &liter, f, BM_LOOPS_OF_FACE, i) {
luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
copy_v3_v3(v, l->v->co);
mul_m4_v3(rot, v);
mont29AuthorUnsubmitted
Not Done
Inline Actions

better to use zero_v3(v) later. But do this vec need to be initialized at all?

mont29: better to use zero_v3(v) later. But do this vec need to be initialized at all?
map_to_sphere(&luv->uv[0], &luv->uv[1], v[0], v[1], v[2]);
uvs[i] = luv->uv;
}
mi = 0; /* fix awkwardly-wrapping UVs */
for (i = 1; i < f->len; ++i)
if (uvs[i][0] > uvs[mi][0])
mi = i;
for (i = 0; i < f->len; ++i) {
if (i != mi) {
dx = uvs[mi][0] - uvs[i][0];
if (dx > 0.5f) uvs[i][0] += 1.0f;
mont29AuthorUnsubmitted
Not Done
Inline Actions

Conventions are to use post-increment in our C code (same for other loops below).

mont29: Conventions are to use post-increment in our C code (same for other loops below).
}
}
}
}
}
void bmo_create_monkey_exec(BMesh *bm, BMOperator *op) void bmo_create_monkey_exec(BMesh *bm, BMOperator *op)
mont29AuthorUnsubmitted
Not Done
Inline Actions

eeeh, a one-line if() I missed!

mont29: eeeh, a one-line if() I missed!
{ {
BMVert *eve; BMVert *eve;
campbellbartonUnsubmitted
Not Done
Inline Actions

This is around 4440 degrees, This could be 2.1 since the value is in radians.

campbellbarton: This is around `4440` degrees, This could be `2.1` since the value is in radians.
mont29AuthorUnsubmitted
Not Done
Inline Actions

another one I missed, no spaces between brackets and numbers, and please use full float format (1.0f).

mont29: another one I missed, no spaces between brackets and numbers, and please use full float format…
campbellbartonUnsubmitted
Not Done
Inline Actions

This is applying to all faces, which will overwrite existing UV's if you add a new sphere while in edit-mode.

campbellbarton: This is applying to **all faces**, which will overwrite existing UV's if you add a new sphere…
CommanderCorianderSalamanderUnsubmitted
Not Done
Inline Actions

Yes - this is actually the problem I was trying to solve in the other primitives by enabling/disabling FACE_NEW. Forgot to deal with it here =(

CommanderCorianderSalamander: Yes - this is actually the problem I was trying to solve in the other primitives by…
Context not available.
f = BM_face_create_quad_tri(bm, cent1, v1, firstv1, NULL, NULL, false); f = BM_face_create_quad_tri(bm, cent1, v1, firstv1, NULL, NULL, false);
BMO_elem_flag_enable(bm, f, FACE_NEW); BMO_elem_flag_enable(bm, f, FACE_NEW);
BM_mesh_create_circle_uvs(bm);
} }
if (!cap_tris) { if (!cap_tris) {
Context not available.
BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "verts.out", BM_VERT, VERT_MARK); BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "verts.out", BM_VERT, VERT_MARK);
} }
void BM_mesh_create_circle_uvs(BMesh *bm) {
BMFace *f;
BMLoop *l;
BMIter iter, liter;
MLoopUV *luv;
int cd_loop_uv_offset;
float theta, dtheta;
int loopindex, segments;
cd_loop_uv_offset = CustomData_get_offset(&bm->ldata, CD_MLOOPUV);
segments = BM_iter_mesh_count(bm, BM_FACES_OF_MESH);
dtheta = 2 * M_PI / segments;
if (cd_loop_uv_offset != -1) {
theta = 0;
BM_ITER_MESH (f, &iter, bm, BM_FACES_OF_MESH) {
BM_ITER_ELEM_INDEX (l, &liter, f, BM_LOOPS_OF_FACE, loopindex) {
luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
switch (loopindex) {
case 0:
luv->uv[0] = .5;
kevindietrichUnsubmitted
Not Done
Inline Actions

Code style: use 0.5f, not .5

kevindietrich: Code style: use 0.5f, not .5
luv->uv[1] = .5;
break;
case 1:
luv->uv[0] = .5 + .5 * cosf(theta);
luv->uv[1] = .5 + .5 * sinf(theta);
break;
case 2:
theta += dtheta;
luv->uv[0] = .5 + .5 * cosf(theta);
luv->uv[1] = .5 + .5 * sinf(theta);
break;
campbellbartonUnsubmitted
Not Done
Inline Actions

style: http://wiki.blender.org/index.php/Dev:Doc/Code_Style#Braces

campbellbarton: style: http://wiki.blender.org/index.php/Dev:Doc/Code_Style#Braces
}
}
}
}
}
void bmo_create_cone_exec(BMesh *bm, BMOperator *op) void bmo_create_cone_exec(BMesh *bm, BMOperator *op)
{ {
BMVert *v1, *v2, *lastv1 = NULL, *lastv2 = NULL, *cent1, *cent2, *firstv1, *firstv2; BMVert *v1, *v2, *lastv1 = NULL, *lastv2 = NULL, *cent1, *cent2, *firstv1, *firstv2;
Context not available.
mul_m4_v3(mat, vec); mul_m4_v3(mat, vec);
cent1 = BM_vert_create(bm, vec, NULL, BM_CREATE_NOP); cent1 = BM_vert_create(bm, vec, NULL, BM_CREATE_NOP);
cent1->no[0] = cent1->no[1] = 0;
cent1->no[2] = 1;
mul_m4_v3(mat, cent1->no);
vec[0] = vec[1] = 0.0f; vec[0] = vec[1] = 0.0f;
vec[2] = depth; vec[2] = depth;
mul_m4_v3(mat, vec); mul_m4_v3(mat, vec);
cent2 = BM_vert_create(bm, vec, NULL, BM_CREATE_NOP); cent2 = BM_vert_create(bm, vec, NULL, BM_CREATE_NOP);
cent2->no[0] = cent2->no[1] = 0;
cent2->no[2] = -1;
mul_m4_v3(mat, cent2->no);
BMO_elem_flag_enable(bm, cent1, VERT_MARK); BMO_elem_flag_enable(bm, cent1, VERT_MARK);
BMO_elem_flag_enable(bm, cent2, VERT_MARK); BMO_elem_flag_enable(bm, cent2, VERT_MARK);
Context not available.
if (a) { if (a) {
if (cap_ends) { if (cap_ends) {
BMFace *f; BMFace *f;
f = BM_face_create_quad_tri(bm, cent1, lastv1, v1, NULL, NULL, false); f = BM_face_create_quad_tri(bm, cent1, lastv1, v1, NULL, NULL, false);
copy_v3_v3(f->no, cent1->no);
BMO_elem_flag_enable(bm, f, FACE_NEW); BMO_elem_flag_enable(bm, f, FACE_NEW);
f = BM_face_create_quad_tri(bm, cent2, v2, lastv2, NULL, NULL, false); f = BM_face_create_quad_tri(bm, cent2, v2, lastv2, NULL, NULL, false);
copy_v3_v3(f->no, cent2->no);
BMO_elem_flag_enable(bm, f, FACE_NEW); BMO_elem_flag_enable(bm, f, FACE_NEW);
} }
BM_face_create_quad_tri(bm, lastv1, lastv2, v2, v1, NULL, false); BM_face_create_quad_tri(bm, lastv1, lastv2, v2, v1, NULL, false);
Context not available.
if (!a) if (!a)
return; return;
BM_face_create_quad_tri(bm, v1, v2, firstv2, firstv1, NULL, false);
if (cap_ends) { if (cap_ends) {
BMFace *f; BMFace *f;
f = BM_face_create_quad_tri(bm, cent1, v1, firstv1, NULL, NULL, false); f = BM_face_create_quad_tri(bm, cent1, v1, firstv1, NULL, NULL, false);
copy_v3_v3(f->no, cent1->no);
campbellbartonUnsubmitted
Not Done
Inline Actions

With all this effort to calculate the normals, probably better just to call. BM_face_calc_normal

campbellbarton: With all this effort to calculate the normals, probably better just to call.
BMO_elem_flag_enable(bm, f, FACE_NEW); BMO_elem_flag_enable(bm, f, FACE_NEW);
f = BM_face_create_quad_tri(bm, cent2, firstv2, v2, NULL, NULL, false); f = BM_face_create_quad_tri(bm, cent2, firstv2, v2, NULL, NULL, false);
copy_v3_v3(f->no, cent2->no);
BMO_elem_flag_enable(bm, f, FACE_NEW); BMO_elem_flag_enable(bm, f, FACE_NEW);
}
BM_mesh_create_cone_uvs(bm, (dia1 > 0.001) ? cent1->no : NULL, (dia2 > .001) ? cent2->no : NULL);
} else {
BM_mesh_create_cone_uvs(bm, NULL, NULL);
}
if (!cap_tris) { if (!cap_tris) {
BMO_op_callf(bm, op->flag, "dissolve_faces faces=%ff", FACE_NEW); BMO_op_callf(bm, op->flag, "dissolve_faces faces=%ff", FACE_NEW);
} }
BM_face_create_quad_tri(bm, v1, v2, firstv2, firstv1, NULL, false);
mont29AuthorUnsubmitted
Not Done
Inline Actions

Why is this removed???

mont29: Why is this removed???
CommanderCorianderSalamanderUnsubmitted
Not Done
Inline Actions

Just moved up a few lines so all the faces were made before the dissolve_faces call - it made calc_uvs simpler if it could assume it was capped with triangles (I think! it's been a while)

CommanderCorianderSalamander: Just moved up a few lines so all the faces were made before the dissolve_faces call - it made…
BMO_op_callf(bm, op->flag, "remove_doubles verts=%fv dist=%f", VERT_MARK, 0.000001); BMO_op_callf(bm, op->flag, "remove_doubles verts=%fv dist=%f", VERT_MARK, 0.000001);
BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "verts.out", BM_VERT, VERT_MARK); BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "verts.out", BM_VERT, VERT_MARK);
} }
void BM_mesh_create_cone_uvs(BMesh *bm, float *normal_top, float *normal_bottom)
{
BMFace *f;
BMLoop *l;
BMIter iter, liter;
MLoopUV *luv;
int cd_loop_uv_offset;
float dx, dy, centerx, centery, dtheta, dtheta_top = 0, dtheta_bottom = 0;
int loopindex, sidecount;
cd_loop_uv_offset = CustomData_get_offset(&bm->ldata, CD_MLOOPUV);
if (cd_loop_uv_offset != -1) {
dx = 0;
dy = .5;
sidecount = BM_iter_mesh_count(bm, BM_FACES_OF_MESH);
if (normal_top || normal_bottom) {
sidecount /= 3;
}
centery = .25;
dtheta = 2 * M_PI / sidecount;
BM_ITER_MESH (f, &iter, bm, BM_FACES_OF_MESH) { /* three different kinds of faces to be unwrapped: top, bottom and side */
if (f->len > 3 && normal_top && normal_bottom) { /* side face */
BM_ITER_ELEM_INDEX (l, &liter, f, BM_LOOPS_OF_FACE, loopindex) {
luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
switch (loopindex) {
case 0:
dx += (1.0f / sidecount);
break;
case 1:
dy += 0.5;
break;
case 2:
dx -= (1.0f / sidecount);
break;
case 3:
dy -= 0.5;
break;
default:
break;
}
luv->uv[0] = dx;
luv->uv[1] = dy;
}
dx += (1.0f / sidecount);
} else if (f->len <= 3) { /* top or bottom face */
mont29AuthorUnsubmitted
Not Done
Inline Actions

This can rather simply be zero_v3(inv_mat[3])

mont29: This can rather simply be `zero_v3(inv_mat[3])`
if (normal_top && dot_v3v3(f->no, normal_top) > 0) { /* if it's a top face... */
centerx = .25;
BM_ITER_ELEM_INDEX (l, &liter, f, BM_LOOPS_OF_FACE, loopindex) {
luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
switch (loopindex) {
case 0:
luv->uv[0] = centerx;
luv->uv[1] = centery;
break;
case 1:
luv->uv[0] = centerx + .24 * cosf(dtheta_top);
luv->uv[1] = centery + .24 * sinf(dtheta_top);
break;
case 2:
dtheta_top += dtheta;
luv->uv[0] = centerx + .24 * cosf(dtheta_top);
luv->uv[1] = centery + .24 * sinf(dtheta_top);
break;
}
}
} else if (normal_bottom && dot_v3v3(f->no, normal_bottom) > 0) { /* if it's a bottom face... */
centerx = .75;
BM_ITER_ELEM_INDEX (l, &liter, f, BM_LOOPS_OF_FACE, loopindex) {
luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
switch (loopindex) {
case 0:
luv->uv[0] = centerx;
luv->uv[1] = centery;
break;
campbellbartonUnsubmitted
Not Done
Inline Actions

could just pass this in.

campbellbarton: could just pass this in.
campbellbartonUnsubmitted
Not Done
Inline Actions

Why disable?

campbellbarton: Why disable?
case 1:
luv->uv[0] = centerx + .24 * cosf(dtheta_bottom);
campbellbartonUnsubmitted
Not Done
Inline Actions

0.24f could be made a const float and assigned a value. Same for other values re-used many times in this patch.

campbellbarton: `0.24f` could be made a `const float` and assigned a value. Same for other values re-used many…
luv->uv[1] = centery + .24 * sinf(dtheta_bottom);
break;
case 2:
dtheta_bottom += dtheta;
luv->uv[0] = centerx + .24 * cosf(dtheta_bottom);
luv->uv[1] = centery + .24 * sinf(dtheta_bottom);
break;
}
}
}
} else { /* it's a side face, but we want to unwrap it in a circle because it's a true cone */
if (normal_top) {
centerx = .75;
} else {
centerx = .25;
}
BM_ITER_ELEM_INDEX (l, &liter, f, BM_LOOPS_OF_FACE, loopindex) {
luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
switch (loopindex) {
case 0:
luv->uv[0] = centerx + .24 * cosf(dtheta_bottom);
luv->uv[1] = centery + .24 * sinf(dtheta_bottom);
break;
case 1:
/* fall-through - at this stage of cone creation, side faces still have four verts even if two of them are at the point of the cone */
case 2:
luv->uv[0] = centerx;
luv->uv[1] = centery;
break;
case 3:
dtheta_bottom += dtheta;
luv->uv[0] = centerx + .24 * cosf(dtheta_bottom);
luv->uv[1] = centery + .24 * sinf(dtheta_bottom);;
break;
}
}
}
}
}
}
campbellbartonUnsubmitted
Not Done
Inline Actions

can use the dot product here.

campbellbarton: can use the dot product here.
void bmo_create_cube_exec(BMesh *bm, BMOperator *op) void bmo_create_cube_exec(BMesh *bm, BMOperator *op)
{ {
BMVert *v1, *v2, *v3, *v4, *v5, *v6, *v7, *v8; BMVert *v1, *v2, *v3, *v4, *v5, *v6, *v7, *v8;
Context not available.
BM_face_create_quad_tri(bm, v8, v7, v6, v5, NULL, false); BM_face_create_quad_tri(bm, v8, v7, v6, v5, NULL, false);
BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "verts.out", BM_VERT, VERT_MARK); BMO_slot_buffer_from_enabled_flag(bm, op, op->slots_out, "verts.out", BM_VERT, VERT_MARK);
BM_mesh_create_cube_uvs(bm);
}
void BM_mesh_create_cube_uvs(BMesh *bm)
{
int cd_loop_uv_offset, faceindex, loopindex;
float dx, dy;
BMFace *f;
BMLoop *l;
BMIter iter, liter;
MLoopUV *luv;
cd_loop_uv_offset = CustomData_get_offset(&bm->ldata, CD_MLOOPUV);
if (cd_loop_uv_offset != -1) {
dx = .375;
dy = 0;
BM_ITER_MESH_INDEX (f, &iter, bm, BM_FACES_OF_MESH, faceindex) {
BM_ITER_ELEM_INDEX (l, &liter, f, BM_LOOPS_OF_FACE, loopindex) {
luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
luv->uv[0] = dx;
mont29AuthorUnsubmitted
Not Done
Inline Actions

That flag is already disabled here. ;)

mont29: That flag is already disabled here. ;)
luv->uv[1] = dy;
switch (loopindex) {
case 0:
dx += .25;
break;
case 1:
dy += .25;
break;
case 2:
dx -= .25;
break;
case 3:
dy -= .25;
break;
default:
dx = dy = 0;
break;
}
}
switch (faceindex) {
case 3:
mont29AuthorUnsubmitted
Not Done
Inline Actions

No need to set dx/dy in the default case imho, this one will never be reached anyway.

mont29: No need to set dx/dy in the default case imho, this one will never be reached anyway.
dx = .125;
dy = .5;
break;
case 4:
dx = .625;
dy = .5;
break;
default:
dy += .25;
break;
}
}
}
} }
Context not available.