From feaa7bbbcc05e726cda2009f35539cf047433363 Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Mon, 14 Oct 2024 22:46:30 +0200 Subject: [PATCH] Fix #129013: Certain brushes can cause artifacts Any brush that used the `restore_position_from_undo_step` codepath (i.e. brushes that use `OrigPositionData` to calculate their deformations) had the possibility to cause artifacts due to the nested tbb parallelization causing incorrect usage of TLS data. To fix this we add a call to `threading::isolate_task` to prevent thread stealing. Pull Request: https://projects.blender.org/blender/blender/pulls/129020 --- source/blender/editors/sculpt_paint/sculpt.cc | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/source/blender/editors/sculpt_paint/sculpt.cc b/source/blender/editors/sculpt_paint/sculpt.cc index 66fc62d02a2..377c1a5a89d 100644 --- a/source/blender/editors/sculpt_paint/sculpt.cc +++ b/source/blender/editors/sculpt_paint/sculpt.cc @@ -1038,40 +1038,43 @@ void restore_position_from_undo_step(const Depsgraph &depsgraph, Object &object) threading::EnumerableThreadSpecific all_tls; node_mask.foreach_index(GrainSize(1), [&](const int i) { - LocalData &tls = all_tls.local(); - const OrigPositionData orig_data = *orig_position_data_lookup_mesh(object, nodes[i]); + threading::isolate_task([&] { + LocalData &tls = all_tls.local(); + const OrigPositionData orig_data = *orig_position_data_lookup_mesh(object, nodes[i]); - const Span verts = nodes[i].verts(); - const Span undo_positions = orig_data.positions; - if (need_translations) { - tls.translations.resize(verts.size()); - translations_from_new_positions(undo_positions, verts, positions_eval, tls.translations); - } - - array_utils::scatter(undo_positions, verts, positions_eval); - - if (positions_eval.data() != positions_orig.data()) { - /* When the evaluated positions and original mesh positions don't point to the same - * array, they must both be updated. */ - if (ss.deform_imats.is_empty()) { - array_utils::scatter(undo_positions, verts, positions_orig); + const Span verts = nodes[i].verts(); + const Span undo_positions = orig_data.positions; + if (need_translations) { + tls.translations.resize(verts.size()); + translations_from_new_positions( + undo_positions, verts, positions_eval, tls.translations); } - else { - /* Because brush deformation is calculated for the evaluated deformed positions, - * the translations have to be transformed to the original space. */ - apply_crazyspace_to_translations(ss.deform_imats, verts, tls.translations); - if (ELEM(active_key, nullptr, mesh.key->refkey)) { - /* We only ever want to propagate changes back to the base mesh if we either have - * no shape key active, or we are working on the basis shape key. - * See #126199 for more information. */ - apply_translations(tls.translations, verts, positions_orig); + + array_utils::scatter(undo_positions, verts, positions_eval); + + if (positions_eval.data() != positions_orig.data()) { + /* When the evaluated positions and original mesh positions don't point to the same + * array, they must both be updated. */ + if (ss.deform_imats.is_empty()) { + array_utils::scatter(undo_positions, verts, positions_orig); + } + else { + /* Because brush deformation is calculated for the evaluated deformed positions, + * the translations have to be transformed to the original space. */ + apply_crazyspace_to_translations(ss.deform_imats, verts, tls.translations); + if (ELEM(active_key, nullptr, mesh.key->refkey)) { + /* We only ever want to propagate changes back to the base mesh if we either have + * no shape key active, or we are working on the basis shape key. + * See #126199 for more information. */ + apply_translations(tls.translations, verts, positions_orig); + } } } - } - if (active_key) { - update_shape_keys(object, mesh, *active_key, verts, tls.translations, positions_orig); - } + if (active_key) { + update_shape_keys(object, mesh, *active_key, verts, tls.translations, positions_orig); + } + }); }); pbvh.tag_positions_changed(node_mask); break;