griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #129013: Certain brushes can cause artifacts

Browse Source 
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
This commit is contained in:
Sean Kim
2024-10-14 22:46:30 +02:00
committed by Sean Kim
parent 3d40d6e188
commit feaa7bbbcc
1 changed files with 32 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
source/blender/editors/sculpt_paint/sculpt.cc
View File

@@ -1038,40 +1038,43 @@ void restore_position_from_undo_step(const Depsgraph &depsgraph, Object &object)
threading::EnumerableThreadSpecific<LocalData> 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<int> verts = nodes[i].verts();
const Span<float3> 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<int> verts = nodes[i].verts();
const Span<float3> 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;