From ef3ba6dfa2cb812ab2d0bde00287fbf9b2ec64f4 Mon Sep 17 00:00:00 2001 From: laurynas Date: Sat, 13 Jan 2024 18:53:54 +0200 Subject: [PATCH 1/3] Fix: Curves extrude with all points selected CurvesGeometry has no ".selected" attribute when all control points are selected. The earlier code assumed that ".selected" always exists. Also Python test is added for "extrude" operator. --- .../editors/curves/intern/curves_extrude.cc | 9 +- tests/python/CMakeLists.txt | 8 + tests/python/curves_extrude.py | 185 ++++++++++++++++++ 3 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 tests/python/curves_extrude.py diff --git a/source/blender/editors/curves/intern/curves_extrude.cc b/source/blender/editors/curves/intern/curves_extrude.cc index b36a8b05300..eea52a4d1df 100644 --- a/source/blender/editors/curves/intern/curves_extrude.cc +++ b/source/blender/editors/curves/intern/curves_extrude.cc @@ -17,7 +17,7 @@ namespace blender::ed::curves { /** * Merges copy intervals at curve endings to minimize number of copy operations. - * For example above intervals [0, 3, 4, 4, 4] became [0, 4, 4]. + * For example given in function 'extrude_curves' intervals [0, 3, 4, 4, 4] became [0, 4, 4]. * Leading to only two copy operations. */ static Span compress_intervals(const Span curve_interval_ranges, @@ -263,7 +263,8 @@ static void extrude_curves(Curves &curves_id) Array curve_interval_ranges(curves_num); /* Per curve boolean indicating if first interval in a curve is selected. - * Other can be calculated as in a curve two adjacent intervals can have same selection state. */ + * Other can be calculated as in a curve two adjacent intervals can not have same selection + * state. */ Array is_first_selected(curves_num); calc_curves_extrusion(extruded_points, @@ -276,7 +277,9 @@ static void extrude_curves(Curves &curves_id) new_curves.resize(new_offsets.last(), new_curves.curves_num()); const bke::AttributeAccessor src_attributes = curves.attributes(); - const GVArraySpan src_selection = *src_attributes.lookup(".selection", bke::AttrDomain::Point); + const bool true_ = true; + const GVArraySpan src_selection = *src_attributes.lookup_or_default( + ".selection", bke::AttrDomain::Point, CD_PROP_BOOL, &true_); const CPPType &src_selection_type = src_selection.type(); bke::GSpanAttributeWriter dst_selection = ensure_selection_attribute( new_curves, diff --git a/tests/python/CMakeLists.txt b/tests/python/CMakeLists.txt index 6df6b903a0c..52b104f9382 100644 --- a/tests/python/CMakeLists.txt +++ b/tests/python/CMakeLists.txt @@ -265,6 +265,14 @@ add_blender_test( --run-all-tests ) +add_blender_test( + curves_extrude + ${TEST_SRC_DIR}/modeling/curves_extrude.blend + --python ${TEST_PYTHON_DIR}/curves_extrude.py + -- + --run-all-tests +) + # ------------------------------------------------------------------------------ # MODIFIERS TESTS add_blender_test( diff --git a/tests/python/curves_extrude.py b/tests/python/curves_extrude.py new file mode 100644 index 00000000000..6a96f5c5f6f --- /dev/null +++ b/tests/python/curves_extrude.py @@ -0,0 +1,185 @@ +# SPDX-FileCopyrightText: 2020-2023 Blender Authors +# +# SPDX-License-Identifier: GPL-2.0-or-later + +from abc import ABC, abstractmethod +import bpy +import os +import sys + +sys.path.append(os.path.dirname(os.path.realpath(__file__))) +from modules.mesh_test import RunTest + +class CurvesTest(ABC): + + def __init__(self, test_object_name, exp_object_name, test_name = None): + self.test_object_name = test_object_name + self.exp_object_name = exp_object_name + self.test_object = bpy.data.objects[self.test_object_name] + self.expected_object = bpy.data.objects[self.exp_object_name] + self.verbose = os.getenv("BLENDER_VERBOSE") is not None + + if test_name: + self.test_name = test_name + else: + filepath = bpy.data.filepath + self.test_name = bpy.path.display_name_from_filepath(filepath) + self._failed_tests_list = [] + + def create_evaluated_object(self): + """ + Creates an evaluated object. + """ + bpy.context.view_layer.objects.active = self.test_object + + # Duplicate test object. + bpy.ops.object.mode_set(mode="OBJECT") + bpy.ops.object.select_all(action="DESELECT") + bpy.context.view_layer.objects.active = self.test_object + + self.test_object.select_set(True) + bpy.ops.object.duplicate() + self.evaluated_object = bpy.context.active_object + self.evaluated_object.name = "evaluated_object" + + @staticmethod + def _print_result(result): + """ + Prints the comparison, selection and validation result. + """ + print("Results:") + for key in result: + print("{} : {}".format(key, result[key][1])) + print() + + def run_test(self): + """ + Runs a single test, runs it again if test file is updated. + """ + print("\nSTART {} test.".format(self.test_name)) + + self.create_evaluated_object() + self.apply_operations() + + result = self.compare_objects(self.evaluated_object, self.expected_object) + + # Initializing with True to get correct resultant of result_code booleans. + success = True + inside_loop_flag = False + for key in result: + inside_loop_flag = True + success = success and result[key][0] + + # Check "success" is actually evaluated and is not the default True value. + if not inside_loop_flag: + success = False + + if success: + self.print_passed_test_result(result) + # Clean up. + if self.verbose: + print("Cleaning up...") + # Delete evaluated_test_object. + bpy.ops.object.delete() + return True + + else: + self.print_failed_test_result(result) + return False + + @abstractmethod + def apply_operations(self, evaluated_test_object_name): + pass + + @staticmethod + def compare_curves(evaluated_curves, expected_curves): + if len(evaluated_curves.attributes.items()) != len(expected_curves.attributes.items()): + print("Attribute count doesn't match") + + for a_idx, attribute in evaluated_curves.attributes.items(): + expected_attribute = expected_curves.attributes[a_idx] + + if len(attribute.data.items()) != len(expected_attribute.data.items()): + print("Attribute data length doesn't match") + + value_attr_name = ('vector' if attribute.data_type == 'FLOAT_VECTOR' + or attribute.data_type == 'FLOAT2' else + 'color' if attribute.data_type == 'FLOAT_COLOR' else 'value') + + for v_idx, attribute_value in attribute.data.items(): + if getattr(attribute_value, value_attr_name) != getattr(expected_attribute.data[v_idx], value_attr_name): + print("Attribute '{}' values do not match".format(attribute.name)) + return False + + return True + + def compare_objects(self, evaluated_object, expected_object): + result_codes = {} + + equal = self.compare_curves( evaluated_object.data, expected_object.data) + + result_codes['Curves Comparison'] = (equal, evaluated_object.data) + return result_codes + + def print_failed_test_result(self, result): + """ + Print results for failed test. + """ + print("FAILED {} test with the following: ".format(self.test_name)) + + def print_passed_test_result(self, result): + """ + Print results for passing test. + """ + print("PASSED {} test successfully.".format(self.test_name)) + +class CurvesOpTest(CurvesTest): + + def __init__(self, test_name, test_object_name, exp_object_name, operators_stack): + super().__init__(test_object_name, exp_object_name, test_name) + self.operators_stack = operators_stack + + def apply_operations(self): + for operator_name in self.operators_stack: + bpy.ops.object.mode_set(mode='EDIT') + curves_operator = getattr(bpy.ops.curves, operator_name) + + try: + retval = curves_operator() + except AttributeError: + raise AttributeError("bpy.ops.curves has no attribute {}".format(operator_name)) + except TypeError as ex: + raise TypeError("Incorrect operator parameters {!r} raised {!r}".format([], ex)) + + if retval != {'FINISHED'}: + raise RuntimeError("Unexpected operator return value: {}".format(operator_name)) + bpy.ops.object.mode_set(mode='OBJECT') + +def main(): + tests = [ + CurvesOpTest("Extrude 1 Point Curve", "a_test1PointCurve", "a_test1PointCurveExpected", ['extrude']), + CurvesOpTest("Extrude Middle Points", "b_testMiddlePoints", "b_testMiddlePointsExpected", ['extrude']), + CurvesOpTest("Extrude End Points", "c_testEndPoints", "c_testEndPointsExpected", ['extrude']), + CurvesOpTest("Extrude Neighbors In Separate Curves", "d_testNeighborsInCurves", "d_testNeighborsInCurvesExpected", ['extrude']), + CurvesOpTest("Extrude Edge Curves", "e_testEdgeCurves", "e_testEdgeCurvesExpected", ['extrude']), + CurvesOpTest("Extrude Middle Curve", "f_testMiddleCurve", "f_testMiddleCurveExpected", ['extrude']), + CurvesOpTest("Extrude All Points", "g_testAllPoints", "g_testAllPointsExpected", ['extrude']) + ] + + curves_extrude_test = RunTest(tests) + + command = list(sys.argv) + for i, cmd in enumerate(command): + if cmd == "--run-all-tests": + curves_extrude_test.do_compare = True + curves_extrude_test.run_all_tests() + break + elif cmd == "--run-test": + curves_extrude_test.do_compare = False + name = command[i + 1] + curves_extrude_test.run_test(name) + break + + +if __name__ == "__main__": + main() \ No newline at end of file -- 2.30.2 From 6e1218b3112027b266b8128ea020eb714db28c6b Mon Sep 17 00:00:00 2001 From: laurynas Date: Tue, 16 Jan 2024 21:16:26 +0200 Subject: [PATCH 2/3] lookup_or_default to ForSingle --- source/blender/editors/curves/intern/curves_extrude.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source/blender/editors/curves/intern/curves_extrude.cc b/source/blender/editors/curves/intern/curves_extrude.cc index eea52a4d1df..769c9753c3f 100644 --- a/source/blender/editors/curves/intern/curves_extrude.cc +++ b/source/blender/editors/curves/intern/curves_extrude.cc @@ -277,9 +277,11 @@ static void extrude_curves(Curves &curves_id) new_curves.resize(new_offsets.last(), new_curves.curves_num()); const bke::AttributeAccessor src_attributes = curves.attributes(); - const bool true_ = true; - const GVArraySpan src_selection = *src_attributes.lookup_or_default( - ".selection", bke::AttrDomain::Point, CD_PROP_BOOL, &true_); + GVArray src_selection_array = *src_attributes.lookup(".selection", bke::AttrDomain::Point); + if (!src_selection_array) { + src_selection_array = VArray::ForSingle(true, curves.points_num()); + } + const GVArraySpan src_selection = src_selection_array; const CPPType &src_selection_type = src_selection.type(); bke::GSpanAttributeWriter dst_selection = ensure_selection_attribute( new_curves, -- 2.30.2 From 1683e35d3adfab61d443d5f9383ca752c56997e2 Mon Sep 17 00:00:00 2001 From: laurynas Date: Tue, 16 Jan 2024 21:16:45 +0200 Subject: [PATCH 3/3] make format --- tests/python/curves_extrude.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/tests/python/curves_extrude.py b/tests/python/curves_extrude.py index 6a96f5c5f6f..34fcb740a56 100644 --- a/tests/python/curves_extrude.py +++ b/tests/python/curves_extrude.py @@ -10,9 +10,10 @@ import sys sys.path.append(os.path.dirname(os.path.realpath(__file__))) from modules.mesh_test import RunTest + class CurvesTest(ABC): - def __init__(self, test_object_name, exp_object_name, test_name = None): + def __init__(self, test_object_name, exp_object_name, test_name=None): self.test_object_name = test_object_name self.exp_object_name = exp_object_name self.test_object = bpy.data.objects[self.test_object_name] @@ -87,18 +88,18 @@ class CurvesTest(ABC): self.print_failed_test_result(result) return False - @abstractmethod + @abstractmethod def apply_operations(self, evaluated_test_object_name): pass @staticmethod def compare_curves(evaluated_curves, expected_curves): if len(evaluated_curves.attributes.items()) != len(expected_curves.attributes.items()): - print("Attribute count doesn't match") + print("Attribute count doesn't match") for a_idx, attribute in evaluated_curves.attributes.items(): expected_attribute = expected_curves.attributes[a_idx] - + if len(attribute.data.items()) != len(expected_attribute.data.items()): print("Attribute data length doesn't match") @@ -107,19 +108,23 @@ class CurvesTest(ABC): 'color' if attribute.data_type == 'FLOAT_COLOR' else 'value') for v_idx, attribute_value in attribute.data.items(): - if getattr(attribute_value, value_attr_name) != getattr(expected_attribute.data[v_idx], value_attr_name): + if getattr( + attribute_value, + value_attr_name) != getattr( + expected_attribute.data[v_idx], + value_attr_name): print("Attribute '{}' values do not match".format(attribute.name)) return False return True def compare_objects(self, evaluated_object, expected_object): - result_codes = {} + result_codes = {} - equal = self.compare_curves( evaluated_object.data, expected_object.data) + equal = self.compare_curves(evaluated_object.data, expected_object.data) - result_codes['Curves Comparison'] = (equal, evaluated_object.data) - return result_codes + result_codes['Curves Comparison'] = (equal, evaluated_object.data) + return result_codes def print_failed_test_result(self, result): """ @@ -133,6 +138,7 @@ class CurvesTest(ABC): """ print("PASSED {} test successfully.".format(self.test_name)) + class CurvesOpTest(CurvesTest): def __init__(self, test_name, test_object_name, exp_object_name, operators_stack): @@ -150,11 +156,12 @@ class CurvesOpTest(CurvesTest): raise AttributeError("bpy.ops.curves has no attribute {}".format(operator_name)) except TypeError as ex: raise TypeError("Incorrect operator parameters {!r} raised {!r}".format([], ex)) - + if retval != {'FINISHED'}: raise RuntimeError("Unexpected operator return value: {}".format(operator_name)) bpy.ops.object.mode_set(mode='OBJECT') + def main(): tests = [ CurvesOpTest("Extrude 1 Point Curve", "a_test1PointCurve", "a_test1PointCurveExpected", ['extrude']), @@ -165,7 +172,7 @@ def main(): CurvesOpTest("Extrude Middle Curve", "f_testMiddleCurve", "f_testMiddleCurveExpected", ['extrude']), CurvesOpTest("Extrude All Points", "g_testAllPoints", "g_testAllPointsExpected", ['extrude']) ] - + curves_extrude_test = RunTest(tests) command = list(sys.argv) @@ -182,4 +189,4 @@ def main(): if __name__ == "__main__": - main() \ No newline at end of file + main() -- 2.30.2