From 41d8066b1ed3198a6c481b109a9cd1f456cd8b40 Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Mon, 23 Jun 2025 19:21:53 +0200 Subject: [PATCH 1/2] Fix: Potential NaN when calculating average values It is possible for a vertex to be processed by some function that attempts to calculate the average of an attribute of all neighboring elements when all elements are hidden. This results in NaN when using `math::rcp` for the size of the related indices. This commit switches the `rcp` call for the safe variant and removes an invalid assert that previously existed. Further cleanup to consolidate the various different average methods will occur in the main branch (e.g. the `check_loose` and `interior` functions) Pull Request: https://projects.blender.org/blender/blender/pulls/140569 --- .../blender/editors/sculpt_paint/sculpt_smooth.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/source/blender/editors/sculpt_paint/sculpt_smooth.cc b/source/blender/editors/sculpt_paint/sculpt_smooth.cc index f20f347eca0..38ec44b50a9 100644 --- a/source/blender/editors/sculpt_paint/sculpt_smooth.cc +++ b/source/blender/editors/sculpt_paint/sculpt_smooth.cc @@ -31,12 +31,12 @@ namespace blender::ed::sculpt_paint::smooth { -template T calc_average(const Span positions, const Span indices) +template T calc_average(const Span values, const Span indices) { - const float factor = math::rcp(float(indices.size())); + const float factor = math::safe_rcp(float(indices.size())); T result{}; for (const int i : indices) { - result += positions[i] * factor; + result += values[i] * factor; } return result; } @@ -78,7 +78,6 @@ void neighbor_data_average_mesh(const Span src, BLI_assert(vert_neighbors.size() == dst.size()); for (const int i : vert_neighbors.index_range()) { - BLI_assert(!vert_neighbors[i].is_empty()); dst[i] = calc_average(src, vert_neighbors[i]); } } @@ -233,7 +232,7 @@ void average_data_grids(const SubdivCCG &subdiv_ccg, CCG_grid_xy_to_index(key.grid_size, neighbor.x, neighbor.y); sum += src[index]; } - dst[node_vert_index] = sum / neighbors.coords.size(); + dst[node_vert_index] = math::safe_divide(sum, float(neighbors.coords.size())); } } } @@ -251,7 +250,7 @@ void average_data_bmesh(const Span src, const Set &verts, const for (const BMVert *neighbor : neighbors) { sum += src[BM_elem_index_get(neighbor)]; } - dst[i] = sum / neighbors.size(); + dst[i] = math::safe_divide(sum, float(neighbors.size())); i++; } } @@ -273,7 +272,7 @@ template void average_data_bmesh(Span src, static float3 average_positions(const Span verts) { - const float factor = math::rcp(float(verts.size())); + const float factor = math::safe_rcp(float(verts.size())); float3 result(0); for (const BMVert *vert : verts) { result += float3(vert->co) * factor; From 7274fdb377ba87d1b71787485c5645a341e77c4e Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Mon, 23 Jun 2025 19:22:33 +0200 Subject: [PATCH 2/2] Fix #140556: Mask filter operations behave incorrectly on dense meshes Mistake in 57c4e9dd2c93a34dee0e26ccbe8d80456698487a Pull Request: https://projects.blender.org/blender/blender/pulls/140570 --- .../sculpt_paint/sculpt_filter_mask.cc | 2 +- .../sculpting/partially_masked_sphere.blend | 3 + tests/python/CMakeLists.txt | 7 ++ tests/python/sculpt_paint/mask_filter_test.py | 85 +++++++++++++++++++ 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 tests/files/sculpting/partially_masked_sphere.blend create mode 100644 tests/python/sculpt_paint/mask_filter_test.py diff --git a/source/blender/editors/sculpt_paint/sculpt_filter_mask.cc b/source/blender/editors/sculpt_paint/sculpt_filter_mask.cc index c4f7a88eb98..38d00eb2226 100644 --- a/source/blender/editors/sculpt_paint/sculpt_filter_mask.cc +++ b/source/blender/editors/sculpt_paint/sculpt_filter_mask.cc @@ -140,7 +140,7 @@ static void apply_new_mask_mesh(const Depsgraph &depsgraph, node_mask.foreach_index(GrainSize(1), [&](const int i, const int pos) { const Span verts = nodes[i].verts(); const Span new_node_mask = new_mask.slice(node_verts[pos]); - if (array_utils::indexed_data_equal(mask, verts, new_mask)) { + if (array_utils::indexed_data_equal(mask, verts, new_node_mask)) { return; } undo::push_node(depsgraph, object, &nodes[i], undo::Type::Mask); diff --git a/tests/files/sculpting/partially_masked_sphere.blend b/tests/files/sculpting/partially_masked_sphere.blend new file mode 100644 index 00000000000..d65110e0b08 --- /dev/null +++ b/tests/files/sculpting/partially_masked_sphere.blend @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:699d53c04cd80fddf1677d0cee252620467f287d232f7223b302736c44f38ad0 +size 7881250 diff --git a/tests/python/CMakeLists.txt b/tests/python/CMakeLists.txt index 78cd38d7a20..c4d4b18ea3b 100644 --- a/tests/python/CMakeLists.txt +++ b/tests/python/CMakeLists.txt @@ -1263,6 +1263,13 @@ if(TEST_SRC_DIR_EXISTS) -- --testdir "${TEST_SRC_DIR}/sculpting" ) + + add_blender_test( + bl_sculpt_mask_filter + --python ${CMAKE_CURRENT_LIST_DIR}/sculpt_paint/mask_filter_test.py + -- + --testdir "${TEST_SRC_DIR}/sculpting" + ) endif() if(WITH_GPU_MESH_PAINT_TESTS AND TEST_SRC_DIR_EXISTS) diff --git a/tests/python/sculpt_paint/mask_filter_test.py b/tests/python/sculpt_paint/mask_filter_test.py new file mode 100644 index 00000000000..6c83ffa5bed --- /dev/null +++ b/tests/python/sculpt_paint/mask_filter_test.py @@ -0,0 +1,85 @@ +# SPDX-FileCopyrightText: 2025 Blender Authors +# +# SPDX-License-Identifier: GPL-2.0-or-later */ + +__all__ = ( + "main", +) + +import math +import unittest +import sys +import pathlib +import numpy as np + +import bpy + +""" +blender -b --factory-startup --python tests/python/sculpt_paint/mask_filter_test.py -- --testdir tests/files/sculpting/ +""" + +args = None + + +class GrowMaskTest(unittest.TestCase): + def setUp(self): + bpy.ops.wm.open_mainfile(filepath=str(args.testdir / "partially_masked_sphere.blend"), load_ui=False) + bpy.ops.ed.undo_push() + + def test_grow_increases_number_of_masked_vertices(self): + mesh = bpy.context.object.data + mask_attr = mesh.attributes['.sculpt_mask'] + + num_vertices = mesh.attributes.domain_size('POINT') + + old_mask_data = np.zeros(num_vertices, dtype=np.float32) + mask_attr.data.foreach_get('value', old_mask_data) + + bpy.ops.sculpt.mask_filter(filter_type='GROW') + + new_mask_data = np.zeros(num_vertices, dtype=np.float32) + mask_attr.data.foreach_get('value', new_mask_data) + + self.assertGreater(np.count_nonzero(new_mask_data), np.count_nonzero(old_mask_data)) + + +class ShrinkMaskTest(unittest.TestCase): + def setUp(self): + bpy.ops.wm.open_mainfile(filepath=str(args.testdir / "partially_masked_sphere.blend"), load_ui=False) + bpy.ops.ed.undo_push() + + def test_shrink_decreases_number_of_masked_vertices(self): + mesh = bpy.context.object.data + mask_attr = mesh.attributes['.sculpt_mask'] + + num_vertices = mesh.attributes.domain_size('POINT') + + old_mask_data = np.zeros(num_vertices, dtype=np.float32) + mask_attr.data.foreach_get('value', old_mask_data) + + bpy.ops.sculpt.mask_filter(filter_type='SHRINK') + + new_mask_data = np.zeros(num_vertices, dtype=np.float32) + mask_attr.data.foreach_get('value', new_mask_data) + + self.assertLess(np.count_nonzero(new_mask_data), np.count_nonzero(old_mask_data)) + + +def main(): + global args + import argparse + + argv = [sys.argv[0]] + if '--' in sys.argv: + argv += sys.argv[sys.argv.index('--') + 1:] + + parser = argparse.ArgumentParser() + parser.add_argument('--testdir', required=True, type=pathlib.Path) + + args, remaining = parser.parse_known_args(argv) + + unittest.main(argv=remaining) + + +if __name__ == "__main__": + main()