Cleanup: Use simpler C++ types in mesh corner normals code

The various stacks are just filled and then emptied. We also expect
them to be fairly small. A vector can handle these cases fairly well.
Also store indices rather than pointers. I didn't notice any performance
changes from these changes.
This commit is contained in:
Hans Goudey
2023-04-26 23:56:45 -04:00
parent a6baf7beae
commit f8eebd3b25
3 changed files with 59 additions and 81 deletions

View File

@@ -449,6 +449,9 @@ void BKE_lnor_spacearr_tls_join(MLoopNorSpaceArray *lnors_spacearr,
MLoopNorSpaceArray *lnors_spacearr_tls);
MLoopNorSpace *BKE_lnor_space_create(MLoopNorSpaceArray *lnors_spacearr);
#ifdef __cplusplus
/**
* Should only be called once.
* Beware, this modifies ref_vec and other_vec in place!
@@ -459,7 +462,10 @@ void BKE_lnor_space_define(MLoopNorSpace *lnor_space,
const float lnor[3],
float vec_ref[3],
float vec_other[3],
struct BLI_Stack *edge_vectors);
blender::Span<blender::float3> edge_vectors);
#endif
/**
* Add a new given loop to given lnor_space.
* Depending on \a lnor_space->data_type, we expect \a bm_loop to be a pointer to BMLoop struct

View File

@@ -19,12 +19,10 @@
#include "BLI_array_utils.hh"
#include "BLI_bit_vector.hh"
#include "BLI_linklist.h"
#include "BLI_linklist_stack.h"
#include "BLI_math.h"
#include "BLI_math_vector.hh"
#include "BLI_memarena.h"
#include "BLI_span.hh"
#include "BLI_stack.h"
#include "BLI_task.hh"
#include "BLI_timeit.hh"
#include "BLI_utildefines.h"
@@ -473,7 +471,7 @@ void BKE_lnor_space_define(MLoopNorSpace *lnor_space,
const float lnor[3],
float vec_ref[3],
float vec_other[3],
BLI_Stack *edge_vectors)
const blender::Span<blender::float3> edge_vectors)
{
const float pi2 = float(M_PI) * 2.0f;
float tvec[3], dtp;
@@ -485,31 +483,24 @@ void BKE_lnor_space_define(MLoopNorSpace *lnor_space,
/* If vec_ref or vec_other are too much aligned with lnor, we can't build lnor space,
* tag it as invalid and abort. */
lnor_space->ref_alpha = lnor_space->ref_beta = 0.0f;
if (edge_vectors) {
BLI_stack_clear(edge_vectors);
}
return;
}
copy_v3_v3(lnor_space->vec_lnor, lnor);
/* Compute ref alpha, average angle of all available edge vectors to lnor. */
if (edge_vectors) {
if (!edge_vectors.is_empty()) {
float alpha = 0.0f;
int count = 0;
while (!BLI_stack_is_empty(edge_vectors)) {
const float *vec = (const float *)BLI_stack_peek(edge_vectors);
for (const blender::float3 &vec : edge_vectors) {
alpha += saacosf(dot_v3v3(vec, lnor));
BLI_stack_discard(edge_vectors);
count++;
}
/* This piece of code shall only be called for more than one loop. */
/* NOTE: In theory, this could be `count > 2`,
* but there is one case where we only have two edges for two loops:
* a smooth vertex with only two edges and two faces (our Monkey's nose has that, e.g.).
*/
BLI_assert(count >= 2); /* This piece of code shall only be called for more than one loop. */
lnor_space->ref_alpha = alpha / float(count);
BLI_assert(edge_vectors.size() >= 2);
lnor_space->ref_alpha = alpha / float(edge_vectors.size());
}
else {
lnor_space->ref_alpha = (saacosf(dot_v3v3(vec_ref, lnor)) +
@@ -869,7 +860,7 @@ static void lnor_space_for_single_fan(LoopSplitTaskDataCommon *common_data,
sub_v3_v3v3(vec_prev, positions[vert_3], positions[vert_pivot]);
normalize_v3(vec_prev);
BKE_lnor_space_define(lnor_space, loop_normals[ml_curr_index], vec_curr, vec_prev, nullptr);
BKE_lnor_space_define(lnor_space, loop_normals[ml_curr_index], vec_curr, vec_prev, {});
/* We know there is only one loop in this space, no need to create a link-list in this case. */
BKE_lnor_space_add_loop(lnors_spacearr, lnor_space, ml_curr_index, nullptr, true);
@@ -883,7 +874,7 @@ static void lnor_space_for_single_fan(LoopSplitTaskDataCommon *common_data,
static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
const int ml_curr_index,
MLoopNorSpace *lnor_space,
BLI_Stack *edge_vectors)
Vector<float3> *edge_vectors)
{
MLoopNorSpaceArray *lnors_spacearr = common_data->lnors_spacearr;
MutableSpan<float3> loop_normals = common_data->loop_normals;
@@ -913,7 +904,7 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
const int2 &edge_orig = edges[corner_edges[ml_curr_index]];
float vec_curr[3], vec_prev[3], vec_org[3];
float lnor[3] = {0.0f, 0.0f, 0.0f};
float3 lnor(0.0f);
/* We validate clnors data on the fly - cheapest way to do! */
int clnors_avg[2] = {0, 0};
@@ -921,10 +912,7 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
int clnors_count = 0;
bool clnors_invalid = false;
/* Temp loop normal stack. */
BLI_SMALLSTACK_DECLARE(normal, float *);
/* Temp clnors stack. */
BLI_SMALLSTACK_DECLARE(clnors, short *);
Vector<int, 8> processed_corners;
/* `mlfan_vert_index` the loop of our current edge might not be the loop of our current vertex!
*/
@@ -944,7 +932,7 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
copy_v3_v3(vec_prev, vec_org);
if (lnors_spacearr) {
BLI_stack_push(edge_vectors, vec_org);
edge_vectors->append(vec_org);
}
}
@@ -984,20 +972,17 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
clnors_avg[0] += (*clnor)[0];
clnors_avg[1] += (*clnor)[1];
clnors_count++;
/* We store here a pointer to all custom loop_normals processed. */
BLI_SMALLSTACK_PUSH(clnors, (short *)*clnor);
}
}
/* We store here a pointer to all loop-normals processed. */
BLI_SMALLSTACK_PUSH(normal, (float *)(loop_normals[mlfan_vert_index]));
processed_corners.append(mlfan_vert_index);
if (lnors_spacearr) {
/* Assign current lnor space to current 'vertex' loop. */
BKE_lnor_space_add_loop(lnors_spacearr, lnor_space, mlfan_vert_index, nullptr, false);
if (edge != edge_orig) {
/* We store here all edges-normalized vectors processed. */
BLI_stack_push(edge_vectors, vec_curr);
edge_vectors->append(vec_curr);
}
}
@@ -1034,23 +1019,19 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
lnor_len = 1.0f;
}
BKE_lnor_space_define(lnor_space, lnor, vec_org, vec_curr, edge_vectors);
BKE_lnor_space_define(lnor_space, lnor, vec_org, vec_curr, *edge_vectors);
edge_vectors->clear();
if (!clnors_data.is_empty()) {
if (clnors_invalid) {
short *clnor;
clnors_avg[0] /= clnors_count;
clnors_avg[1] /= clnors_count;
/* Fix/update all clnors of this fan with computed average value. */
if (G.debug & G_DEBUG) {
printf("Invalid clnors in this fan!\n");
}
while ((clnor = (short *)BLI_SMALLSTACK_POP(clnors))) {
// print_v2("org clnor", clnor);
clnor[0] = short(clnors_avg[0]);
clnor[1] = short(clnors_avg[1]);
}
clnors_data.fill_indices(processed_corners.as_span(),
short2(clnors_avg[0], clnors_avg[1]));
// print_v2("new clnors", clnors_avg);
}
/* Extra bonus: since small-stack is local to this function,
@@ -1063,14 +1044,8 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
/* In case we get a zero normal here, just use vertex normal already set! */
if (LIKELY(lnor_len != 0.0f)) {
/* Copy back the final computed normal into all related loop-normals. */
float *nor;
while ((nor = (float *)BLI_SMALLSTACK_POP(normal))) {
copy_v3_v3(nor, lnor);
}
loop_normals.fill_indices(processed_corners.as_span(), lnor);
}
/* Extra bonus: since small-stack is local to this function,
* no more need to empty it at all cost! */
}
}
@@ -1366,17 +1341,13 @@ void normals_calc_loop(const Span<float3> vert_positions,
});
threading::parallel_for(fan_corners.index_range(), 1024, [&](const IndexRange range) {
BLI_Stack *edge_vectors = r_lnors_spacearr ? BLI_stack_new(sizeof(float[3]), __func__) :
nullptr;
Vector<float3> edge_vectors;
for (const int i : range) {
const int corner = fan_corners[i];
split_loop_nor_fan_do(&common_data,
corner,
r_lnors_spacearr ? r_lnors_spacearr->lspacearr[corner] : nullptr,
edge_vectors);
}
if (edge_vectors) {
BLI_stack_free(edge_vectors);
&edge_vectors);
}
});
@@ -1428,7 +1399,7 @@ static void mesh_normals_loop_custom_set(Span<float3> positions,
const bool use_split_normals = true;
const float split_angle = float(M_PI);
BLI_SMALLSTACK_DECLARE(clnors_data, short *);
Vector<short *> clnors_data;
/* Compute current lnor spacearr. */
normals_calc_loop(positions,
@@ -1602,7 +1573,7 @@ static void mesh_normals_loop_custom_set(Span<float3> positions,
else {
int avg_nor_count = 0;
float avg_nor[3];
short clnor_data_tmp[2], *clnor_data;
short clnor_data_tmp[2];
zero_v3(avg_nor);
while (loop_link) {
@@ -1612,7 +1583,7 @@ static void mesh_normals_loop_custom_set(Span<float3> positions,
avg_nor_count++;
add_v3_v3(avg_nor, nor);
BLI_SMALLSTACK_PUSH(clnors_data, (short *)r_clnors_data[lidx]);
clnors_data.append(r_clnors_data[lidx]);
loop_link = loop_link->next;
done_loops[lidx].reset();
@@ -1621,7 +1592,8 @@ static void mesh_normals_loop_custom_set(Span<float3> positions,
mul_v3_fl(avg_nor, 1.0f / float(avg_nor_count));
BKE_lnor_space_custom_normal_to_data(lnors_spacearr.lspacearr[i], avg_nor, clnor_data_tmp);
while ((clnor_data = (short *)BLI_SMALLSTACK_POP(clnors_data))) {
while (!clnors_data.is_empty()) {
short *clnor_data = clnors_data.pop_last();
clnor_data[0] = clnor_data_tmp[0];
clnor_data[1] = clnor_data_tmp[1];
}

View File

@@ -15,9 +15,9 @@
#include "BLI_bitmap.h"
#include "BLI_linklist_stack.h"
#include "BLI_math.h"
#include "BLI_stack.h"
#include "BLI_task.h"
#include "BLI_utildefines.h"
#include "BLI_vector.hh"
#include "BKE_customdata.h"
#include "BKE_editmesh.h"
@@ -470,7 +470,7 @@ static int bm_mesh_loops_calc_normals_for_loop(BMesh *bm,
const int cd_loop_clnors_offset,
const bool has_clnors,
/* Cache. */
BLI_Stack *edge_vectors,
blender::Vector<blender::float3, 16> *edge_vectors,
/* Iterate. */
BMLoop *l_curr,
/* Result. */
@@ -534,7 +534,7 @@ static int bm_mesh_loops_calc_normals_for_loop(BMesh *bm,
normalize_v3(vec_prev);
}
BKE_lnor_space_define(lnor_space, r_lnos[l_curr_index], vec_curr, vec_prev, nullptr);
BKE_lnor_space_define(lnor_space, r_lnos[l_curr_index], vec_curr, vec_prev, {});
/* We know there is only one loop in this space,
* no need to create a linklist in this case... */
BKE_lnor_space_add_loop(r_lnors_spacearr, lnor_space, l_curr_index, l_curr, true);
@@ -605,7 +605,7 @@ static int bm_mesh_loops_calc_normals_for_loop(BMesh *bm,
copy_v3_v3(vec_curr, vec_org);
if (r_lnors_spacearr) {
BLI_stack_push(edge_vectors, vec_org);
edge_vectors->append(vec_org);
}
}
@@ -671,7 +671,7 @@ static int bm_mesh_loops_calc_normals_for_loop(BMesh *bm,
BKE_lnor_space_add_loop(r_lnors_spacearr, lnor_space, lfan_pivot_index, lfan_pivot, false);
if (e_next != e_org) {
/* We store here all edges-normalized vectors processed. */
BLI_stack_push(edge_vectors, vec_next);
edge_vectors->append(vec_next);
}
}
@@ -700,7 +700,7 @@ static int bm_mesh_loops_calc_normals_for_loop(BMesh *bm,
lnor_len = 1.0f;
}
BKE_lnor_space_define(lnor_space, lnor, vec_org, vec_next, edge_vectors);
BKE_lnor_space_define(lnor_space, lnor, vec_org, vec_next, *edge_vectors);
if (has_clnors) {
if (clnors_invalid) {
@@ -863,19 +863,20 @@ static void bm_edge_tag_from_smooth_and_set_sharp(const float (*fnos)[3],
* operating on vertices this is needed for multi-threading
* so there is a guarantee that each thread has isolated loops.
*/
static void bm_mesh_loops_calc_normals_for_vert_with_clnors(BMesh *bm,
const float (*vcos)[3],
const float (*fnos)[3],
float (*r_lnos)[3],
const short (*clnors_data)[2],
const int cd_loop_clnors_offset,
const bool do_rebuild,
const float split_angle_cos,
/* TLS */
MLoopNorSpaceArray *r_lnors_spacearr,
BLI_Stack *edge_vectors,
/* Iterate over. */
BMVert *v)
static void bm_mesh_loops_calc_normals_for_vert_with_clnors(
BMesh *bm,
const float (*vcos)[3],
const float (*fnos)[3],
float (*r_lnos)[3],
const short (*clnors_data)[2],
const int cd_loop_clnors_offset,
const bool do_rebuild,
const float split_angle_cos,
/* TLS */
MLoopNorSpaceArray *r_lnors_spacearr,
blender::Vector<blender::float3, 16> *edge_vectors,
/* Iterate over. */
BMVert *v)
{
/* Respecting face order is necessary so the initial starting loop is consistent
* with looping over loops of all faces.
@@ -992,7 +993,7 @@ static void bm_mesh_loops_calc_normals_for_vert_without_clnors(
const float split_angle_cos,
/* TLS */
MLoopNorSpaceArray *r_lnors_spacearr,
BLI_Stack *edge_vectors,
blender::Vector<blender::float3, 16> *edge_vectors,
/* Iterate over. */
BMVert *v)
{
@@ -1078,7 +1079,7 @@ static void bm_mesh_loops_calc_normals__single_threaded(BMesh *bm,
MLoopNorSpaceArray _lnors_spacearr = {nullptr};
BLI_Stack *edge_vectors = nullptr;
std::unique_ptr<blender::Vector<blender::float3, 16>> edge_vectors = nullptr;
{
char htype = 0;
@@ -1095,7 +1096,7 @@ static void bm_mesh_loops_calc_normals__single_threaded(BMesh *bm,
}
if (r_lnors_spacearr) {
BKE_lnor_spacearr_init(r_lnors_spacearr, bm->totloop, MLNOR_SPACEARR_BMLOOP_PTR);
edge_vectors = BLI_stack_new(sizeof(float[3]), __func__);
edge_vectors = std::make_unique<blender::Vector<blender::float3, 16>>();
}
/* Clear all loops' tags (means none are to be skipped for now). */
@@ -1138,7 +1139,7 @@ static void bm_mesh_loops_calc_normals__single_threaded(BMesh *bm,
clnors_data,
cd_loop_clnors_offset,
has_clnors,
edge_vectors,
edge_vectors.get(),
l_curr,
r_lnos,
r_lnors_spacearr);
@@ -1146,7 +1147,6 @@ static void bm_mesh_loops_calc_normals__single_threaded(BMesh *bm,
}
if (r_lnors_spacearr) {
BLI_stack_free(edge_vectors);
if (r_lnors_spacearr == &_lnors_spacearr) {
BKE_lnor_spacearr_free(r_lnors_spacearr);
}
@@ -1169,7 +1169,7 @@ typedef struct BMLoopsCalcNormalsWithCoordsData {
} BMLoopsCalcNormalsWithCoordsData;
typedef struct BMLoopsCalcNormalsWithCoords_TLS {
BLI_Stack *edge_vectors;
blender::Vector<blender::float3, 16> *edge_vectors;
/** Copied from #BMLoopsCalcNormalsWithCoordsData.r_lnors_spacearr when it's not nullptr. */
MLoopNorSpaceArray *lnors_spacearr;
@@ -1182,7 +1182,7 @@ static void bm_mesh_loops_calc_normals_for_vert_init_fn(const void *__restrict u
auto *data = static_cast<const BMLoopsCalcNormalsWithCoordsData *>(userdata);
auto *tls_data = static_cast<BMLoopsCalcNormalsWithCoords_TLS *>(chunk);
if (data->r_lnors_spacearr) {
tls_data->edge_vectors = BLI_stack_new(sizeof(float[3]), __func__);
tls_data->edge_vectors = MEM_new<blender::Vector<blender::float3, 16>>(__func__);
BKE_lnor_spacearr_tls_init(data->r_lnors_spacearr, &tls_data->lnors_spacearr_buf);
tls_data->lnors_spacearr = &tls_data->lnors_spacearr_buf;
}
@@ -1210,7 +1210,7 @@ static void bm_mesh_loops_calc_normals_for_vert_free_fn(const void *__restrict u
auto *tls_data = static_cast<BMLoopsCalcNormalsWithCoords_TLS *>(chunk);
if (data->r_lnors_spacearr) {
BLI_stack_free(tls_data->edge_vectors);
MEM_delete(tls_data->edge_vectors);
}
}