blender/blender
142
451
Fork 651
Code Issues 6.5k Pull Requests 823 Projects 18 Wiki Activity

Fix #109583: Avoid non-threadsafe writing to custom normals data #110478

Merged
Hans Goudey merged 7 commits from HooglyBoogly/blender:fix-no-custom-normals-write into main 2023-07-26 17:04:19 +02:00
Conversation 1 Commits 7 Files Changed 10 +31 -57
10 changed files with 31 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
source/blender/blenkernel/BKE_mesh.hh
View File

@ -156,9 +156,9 @@ void normals_calc_loop(Span<float3> vert_positions,
Span<float3> face_normals,
const bool *sharp_edges,
const bool *sharp_faces,
const short2 *clnors_data,
bool use_split_normals,
float split_angle,
short2 *clnors_data,
CornerNormalSpaceArray *r_lnors_spacearr,
MutableSpan<float3> r_loop_normals);

6
source/blender/blenkernel/intern/data_transfer.cc
View File

@ -371,8 +371,8 @@ static void data_transfer_dtdata_type_preprocess(const Mesh *me_src,
BLI_assert(CustomData_get_layer(&me_src->loop_data, CD_NORMAL) != nullptr);
(void)me_src;
blender::short2 *custom_nors_dst = static_cast<blender::short2 *>(
CustomData_get_layer_for_write(ldata_dst, CD_CUSTOMLOOPNORMAL, me_dst->totloop));
const blender::short2 *custom_nors_dst = static_cast<const blender::short2 *>(
CustomData_get_layer(ldata_dst, CD_CUSTOMLOOPNORMAL));
/* Cache loop nors into a temp CDLayer. */
blender::float3 *loop_nors_dst = static_cast<blender::float3 *>(
@ -398,9 +398,9 @@ static void data_transfer_dtdata_type_preprocess(const Mesh *me_src,
me_dst->face_normals(),
sharp_edges,
sharp_faces,
custom_nors_dst,
use_split_nors_dst,
split_angle_dst,
custom_nors_dst,
nullptr,
{loop_nors_dst, me_dst->totloop});
}

6
source/blender/blenkernel/intern/key.cc
View File

@ -2281,8 +2281,8 @@ void BKE_keyblock_mesh_calc_normals(const KeyBlock *kb,
{reinterpret_cast<blender::float3 *>(vert_normals), mesh->totvert});
}
if (loop_normals_needed) {
blender::short2 *clnors = static_cast<blender::short2 *>(CustomData_get_layer_for_write(
&mesh->loop_data, CD_CUSTOMLOOPNORMAL, corner_verts.size()));
const blender::short2 *clnors = static_cast<const blender::short2 *>(
CustomData_get_layer(&mesh->loop_data, CD_CUSTOMLOOPNORMAL));
const bool *sharp_edges = static_cast<const bool *>(
CustomData_get_layer_named(&mesh->edge_data, CD_PROP_BOOL, "sharp_edge"));
const bool *sharp_faces = static_cast<const bool *>(
@ -2298,9 +2298,9 @@ void BKE_keyblock_mesh_calc_normals(const KeyBlock *kb,
{reinterpret_cast<blender::float3 *>(face_normals), faces.size()},
sharp_edges,
sharp_faces,
clnors,
(mesh->flag & ME_AUTOSMOOTH) != 0,
mesh->smoothresh,
clnors,
nullptr,
{reinterpret_cast<blender::float3 *>(r_loop_normals), corner_verts.size()});
}

7
source/blender/blenkernel/intern/mesh.cc
View File

@ -1789,9 +1789,8 @@ void BKE_mesh_calc_normals_split_ex(Mesh *mesh,
((mesh->flag & ME_AUTOSMOOTH) != 0);
const float split_angle = (mesh->flag & ME_AUTOSMOOTH) != 0 ? mesh->smoothresh : float(M_PI);
/* may be nullptr */
blender::short2 *clnors = (blender::short2 *)CustomData_get_layer_for_write(
&mesh->loop_data, CD_CUSTOMLOOPNORMAL, mesh->totloop);
const blender::short2 *clnors = static_cast<const blender::short2 *>(
CustomData_get_layer(&mesh->loop_data, CD_CUSTOMLOOPNORMAL));
const bool *sharp_edges = static_cast<const bool *>(
CustomData_get_layer_named(&mesh->edge_data, CD_PROP_BOOL, "sharp_edge"));
const bool *sharp_faces = static_cast<const bool *>(
@ -1808,9 +1807,9 @@ void BKE_mesh_calc_normals_split_ex(Mesh *mesh,
mesh->face_normals(),
sharp_edges,
sharp_faces,
clnors,
use_split_normals,
split_angle,
clnors,
nullptr,
{reinterpret_cast<float3 *>(r_corner_normals), mesh->totloop});
}

2
source/blender/blenkernel/intern/mesh_mirror.cc
View File

@ -417,9 +417,9 @@ Mesh *BKE_mesh_mirror_apply_mirror_on_axis_for_modifier(MirrorModifierData *mmd,
result->face_normals(),
sharp_edges,
sharp_faces,
clnors,
true,
result->smoothresh,
clnors,
&lnors_spacearr,
loop_normals);

47
source/blender/blenkernel/intern/mesh_normals.cc
View File

@ -700,7 +700,6 @@ struct LoopSplitTaskDataCommon {
* different elements in the arrays. */
CornerNormalSpaceArray *lnors_spacearr;
MutableSpan<float3> loop_normals;
MutableSpan<short2> clnors_data;
/* Read-only. */
Span<float3> positions;
@ -712,6 +711,7 @@ struct LoopSplitTaskDataCommon {
Span<int> loop_to_face;
Span<float3> face_normals;
Span<float3> vert_normals;
Span<short2> clnors_data;
};
#define INDEX_UNSET INT_MIN
@ -916,7 +916,6 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
{
CornerNormalSpaceArray *lnors_spacearr = common_data->lnors_spacearr;
MutableSpan<float3> loop_normals = common_data->loop_normals;
MutableSpan<short2> clnors_data = common_data->clnors_data;
const Span<float3> positions = common_data->positions;
const Span<int2> edges = common_data->edges;
@ -926,6 +925,7 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
const Span<int2> edge_to_loops = common_data->edge_to_loops;
const Span<int> loop_to_face = common_data->loop_to_face;
const Span<float3> face_normals = common_data->face_normals;
const Span<short2> clnors_data = common_data->clnors_data;
const int face_index = loop_to_face[ml_curr_index];
const int ml_prev_index = face_corner_prev(faces[face_index], ml_curr_index);
@ -946,11 +946,7 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
float3 vec_org;
float3 lnor(0.0f);
/* We validate clnors data on the fly - cheapest way to do! */
int2 clnors_avg(0);
HooglyBoogly marked this conversation as resolved Outdated
Jacques Lucke commented 2023-07-26 15:42:08 +02:00
Outdated
Review
Copy Link

Probably need to update the comment.

Probably need to update the comment.
const short2 *clnor_ref = nullptr;
int clnors_count = 0;
bool clnors_invalid = false;
Vector<int, 8> processed_corners;
@ -995,19 +991,6 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
/* Calculate angle between the two face edges incident on this vertex. */
lnor += face_normals[loop_to_face[mlfan_curr_index]] * saacos(math::dot(vec_curr, vec_prev));
if (!clnors_data.is_empty()) {
/* Accumulate all clnors, if they are not all equal we have to fix that! */
const short2 &clnor = clnors_data[mlfan_vert_index];
if (clnors_count) {
clnors_invalid |= *clnor_ref != clnor;
}
else {
clnor_ref = &clnor;
}
clnors_avg += int2(clnor);
clnors_count++;
}
processed_corners.append(mlfan_vert_index);
if (lnors_spacearr) {
@ -1018,6 +1001,9 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
if (!lnors_spacearr->corners_by_space.is_empty()) {
lnors_spacearr->corners_by_space[space_index] = processed_corners.as_span();
}
if (!clnors_data.is_empty()) {
clnors_avg += int2(clnors_data[mlfan_vert_index]);
}
}
if (IS_EDGE_SHARP(edge_to_loops[corner_edges[mlfan_curr_index]]) || (edge == edge_orig)) {
@ -1058,18 +1044,8 @@ static void split_loop_nor_fan_do(LoopSplitTaskDataCommon *common_data,
edge_vectors->clear();
if (!clnors_data.is_empty()) {
if (clnors_invalid) {
clnors_avg /= 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");
}
clnors_data.fill_indices(processed_corners.as_span(), short2(clnors_avg));
}
/* Extra bonus: since small-stack is local to this function,
* no more need to empty it at all cost! */
lnor = lnor_space_custom_data_to_normal(lnor_space, *clnor_ref);
clnors_avg /= processed_corners.size();
lnor = lnor_space_custom_data_to_normal(lnor_space, short2(clnors_avg));
}
}
@ -1238,9 +1214,9 @@ void normals_calc_loop(const Span<float3> vert_positions,
const Span<float3> face_normals,
const bool *sharp_edges,
const bool *sharp_faces,
const short2 *clnors_data,
bool use_split_normals,
float split_angle,
short2 *clnors_data,
CornerNormalSpaceArray *r_lnors_spacearr,
MutableSpan<float3> r_loop_normals)
{
@ -1314,8 +1290,7 @@ void normals_calc_loop(const Span<float3> vert_positions,
LoopSplitTaskDataCommon common_data;
common_data.lnors_spacearr = r_lnors_spacearr;
common_data.loop_normals = r_loop_normals;
common_data.clnors_data = {reinterpret_cast<short2 *>(clnors_data),
clnors_data ? corner_verts.size() : 0};
common_data.clnors_data = {clnors_data, clnors_data ? corner_verts.size() : 0};
common_data.positions = vert_positions;
common_data.edges = edges;
common_data.faces = faces;
@ -1425,9 +1400,9 @@ static void mesh_normals_loop_custom_set(Span<float3> positions,
face_normals,
sharp_edges.data(),
sharp_faces,
r_clnors_data.data(),
use_split_normals,
split_angle,
r_clnors_data.data(),
&lnors_spacearr,
loop_normals);
@ -1547,9 +1522,9 @@ static void mesh_normals_loop_custom_set(Span<float3> positions,
face_normals,
sharp_edges.data(),
sharp_faces,
r_clnors_data.data(),
use_split_normals,
split_angle,
r_clnors_data.data(),
&lnors_spacearr,
loop_normals);
}

6
source/blender/blenkernel/intern/mesh_remap.cc
View File

@ -1343,8 +1343,8 @@ void BKE_mesh_remap_calc_loops_from_mesh(const int mode,
face_normals_dst = mesh_dst->face_normals();
}
if (need_lnors_dst) {
blender::short2 *custom_nors_dst = static_cast<blender::short2 *>(
CustomData_get_layer_for_write(ldata_dst, CD_CUSTOMLOOPNORMAL, numloops_dst));
const blender::short2 *custom_nors_dst = static_cast<const blender::short2 *>(
CustomData_get_layer(ldata_dst, CD_CUSTOMLOOPNORMAL));
/* Cache loop normals into a temporary custom data layer. */
loop_normals_dst = static_cast<blender::float3 *>(
@ -1372,9 +1372,9 @@ void BKE_mesh_remap_calc_loops_from_mesh(const int mode,
mesh_dst->face_normals(),
sharp_edges,
sharp_faces,
custom_nors_dst,
use_split_nors_dst,
split_angle_dst,
custom_nors_dst,
nullptr,
{loop_normals_dst, numloops_dst});
}

6
source/blender/draw/intern/draw_cache_extract_mesh_render_data.cc
View File

@ -337,8 +337,8 @@ void mesh_render_data_update_normals(MeshRenderData *mr, const eMRDataType data_
}
if (((data_flag & MR_DATA_LOOP_NOR) && is_auto_smooth) || (data_flag & MR_DATA_TAN_LOOP_NOR)) {
mr->loop_normals.reinitialize(mr->corner_verts.size());
blender::short2 *clnors = static_cast<blender::short2 *>(CustomData_get_layer_for_write(
&mr->me->loop_data, CD_CUSTOMLOOPNORMAL, mr->me->totloop));
const blender::short2 *clnors = static_cast<const blender::short2 *>(
CustomData_get_layer(&mr->me->loop_data, CD_CUSTOMLOOPNORMAL));
const bool *sharp_edges = static_cast<const bool *>(
CustomData_get_layer_named(&mr->me->edge_data, CD_PROP_BOOL, "sharp_edge"));
blender::bke::mesh::normals_calc_loop(mr->vert_positions,
@ -351,9 +351,9 @@ void mesh_render_data_update_normals(MeshRenderData *mr, const eMRDataType data_
mr->face_normals,
sharp_edges,
mr->sharp_faces,
clnors,
is_auto_smooth,
split_angle,
clnors,
nullptr,
mr->loop_normals);
}

2
source/blender/modifiers/intern/MOD_normal_edit.cc
View File

@ -553,9 +553,9 @@ static Mesh *normalEditModifier_do(NormalEditModifierData *enmd,
result->face_normals(),
sharp_edges.span.data(),
sharp_faces,
clnors,
true,
result->smoothresh,
clnors,
nullptr,
loop_normals);
}

4
source/blender/modifiers/intern/MOD_weighted_normal.cc
View File

@ -231,9 +231,9 @@ static void apply_weights_vertex_normal(WeightedNormalModifierData *wnmd,
wn_data->face_normals,
wn_data->sharp_edges.data(),
wn_data->sharp_faces,
has_clnors ? clnors.data() : nullptr,
true,
split_angle,
has_clnors ? clnors.data() : nullptr,
&lnors_spacearr,
loop_normals);
@ -362,9 +362,9 @@ static void apply_weights_vertex_normal(WeightedNormalModifierData *wnmd,
face_normals,
wn_data->sharp_edges.data(),
wn_data->sharp_faces,
has_clnors ? clnors.data() : nullptr,
true,
split_angle,
has_clnors ? clnors.data() : nullptr,
nullptr,
loop_normals);