From 9b867f2e90bd5ba9c2e37ed39cf8b1ad28e39b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Mon, 27 Jul 2020 08:53:32 +0200 Subject: Fix T79121: Dependency cycle when driver points to prop with 'scale' in name This makes `RNANodeQuery::construct_node_identifier()` more strict in its matching of certain property names. The downside of this approach is that it's not possible any more to use `"rotation"` and expect a match for `"rotation_euler"` and friends, so the list of strings to test against is now 3x as long. Reviewed By: sergey Maniphest Tasks: T79121 Differential Revision: https://developer.blender.org/D8375 --- source/blender/depsgraph/CMakeLists.txt | 11 ++++ .../depsgraph/intern/builder/deg_builder_rna.cc | 33 ++++++++++-- .../depsgraph/intern/builder/deg_builder_rna.h | 13 +++++ .../intern/builder/deg_builder_rna_test.cc | 60 ++++++++++++++++++++++ 4 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 source/blender/depsgraph/intern/builder/deg_builder_rna_test.cc (limited to 'source/blender/depsgraph') diff --git a/source/blender/depsgraph/CMakeLists.txt b/source/blender/depsgraph/CMakeLists.txt index 839a3712129..51fce738700 100644 --- a/source/blender/depsgraph/CMakeLists.txt +++ b/source/blender/depsgraph/CMakeLists.txt @@ -147,3 +147,14 @@ set(LIB ) blender_add_lib(bf_depsgraph "${SRC}" "${INC}" "${INC_SYS}" "${LIB}") + +if(WITH_GTESTS) + set(TEST_SRC + intern/builder/deg_builder_rna_test.cc + ) + set(TEST_LIB + bf_depsgraph + ) + include(GTestTesting) + blender_add_test_lib(bf_depsgraph_tests "${TEST_SRC}" "${INC};${TEST_INC}" "${INC_SYS}" "${LIB}") +endif() diff --git a/source/blender/depsgraph/intern/builder/deg_builder_rna.cc b/source/blender/depsgraph/intern/builder/deg_builder_rna.cc index 98ae653d294..e3b667427a2 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_rna.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_rna.cc @@ -149,6 +149,25 @@ Node *RNANodeQuery::find_node(const PointerRNA *ptr, node_identifier.operation_name_tag); } +bool RNANodeQuery::contains(const char *prop_identifier, const char *rna_path_component) +{ + const char *substr = strstr(prop_identifier, rna_path_component); + if (substr == nullptr) { + return false; + } + + // If substr != prop_identifier, it means that the substring is found further in prop_identifier, + // and that thus index -1 is a valid memory location. + const bool start_ok = substr == prop_identifier || substr[-1] == '.'; + if (!start_ok) { + return false; + } + + const size_t component_len = strlen(rna_path_component); + const bool end_ok = ELEM(substr[component_len], '\0', '.', '['); + return end_ok; +} + RNANodeIdentifier RNANodeQuery::construct_node_identifier(const PointerRNA *ptr, const PropertyRNA *prop, RNAPointerSource source) @@ -283,12 +302,20 @@ RNANodeIdentifier RNANodeQuery::construct_node_identifier(const PointerRNA *ptr, if (prop != nullptr) { const char *prop_identifier = RNA_property_identifier((PropertyRNA *)prop); /* TODO(sergey): How to optimize this? */ - if (strstr(prop_identifier, "location") || strstr(prop_identifier, "rotation") || - strstr(prop_identifier, "scale") || strstr(prop_identifier, "matrix_")) { + if (contains(prop_identifier, "location") || contains(prop_identifier, "matrix_basis") || + contains(prop_identifier, "matrix_channel") || + contains(prop_identifier, "matrix_inverse") || + contains(prop_identifier, "matrix_local") || + contains(prop_identifier, "matrix_parent_inverse") || + contains(prop_identifier, "matrix_world") || + contains(prop_identifier, "rotation_axis_angle") || + contains(prop_identifier, "rotation_euler") || + contains(prop_identifier, "rotation_mode") || + contains(prop_identifier, "rotation_quaternion") || contains(prop_identifier, "scale")) { node_identifier.type = NodeType::TRANSFORM; return node_identifier; } - else if (strstr(prop_identifier, "data")) { + else if (contains(prop_identifier, "data")) { /* We access object.data, most likely a geometry. * Might be a bone tho. */ node_identifier.type = NodeType::GEOMETRY; diff --git a/source/blender/depsgraph/intern/builder/deg_builder_rna.h b/source/blender/depsgraph/intern/builder/deg_builder_rna.h index 52d0e5f6b12..c48c6489c47 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_rna.h +++ b/source/blender/depsgraph/intern/builder/deg_builder_rna.h @@ -93,6 +93,19 @@ class RNANodeQuery { /* Make sure ID data exists for the given ID, and returns it. */ RNANodeQueryIDData *ensure_id_data(const ID *id); + + /* Check whether prop_identifier contains rna_path_component. + * + * This checks more than a substring: + * + * prop_identifier contains(prop_identifier, "location") + * ------------------------ ------------------------------------- + * location true + * ["test_location"] false + * pose["bone"].location true + * pose["bone"].location.x true + */ + static bool contains(const char *prop_identifier, const char *rna_path_component); }; } // namespace deg diff --git a/source/blender/depsgraph/intern/builder/deg_builder_rna_test.cc b/source/blender/depsgraph/intern/builder/deg_builder_rna_test.cc new file mode 100644 index 00000000000..c91dda87190 --- /dev/null +++ b/source/blender/depsgraph/intern/builder/deg_builder_rna_test.cc @@ -0,0 +1,60 @@ +/* + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * The Original Code is Copyright (C) 2020 Blender Foundation. + * All rights reserved. + */ + +/** \file + * \ingroup depsgraph + */ + +#include "intern/builder/deg_builder_rna.h" + +#include "testing/testing.h" + +namespace blender { +namespace deg { +namespace tests { + +class TestableRNANodeQuery : public RNANodeQuery { + public: + static bool contains(const char *prop_identifier, const char *rna_path_component) + { + return RNANodeQuery::contains(prop_identifier, rna_path_component); + } +}; + +TEST(deg_builder_rna, contains) +{ + EXPECT_TRUE(TestableRNANodeQuery::contains("location", "location")); + EXPECT_TRUE(TestableRNANodeQuery::contains("location.x", "location")); + EXPECT_TRUE(TestableRNANodeQuery::contains("pose.bone[\"blork\"].location", "location")); + EXPECT_TRUE(TestableRNANodeQuery::contains("pose.bone[\"blork\"].location.x", "location")); + EXPECT_TRUE(TestableRNANodeQuery::contains("pose.bone[\"blork\"].location[0]", "location")); + + EXPECT_FALSE(TestableRNANodeQuery::contains("", "location")); + EXPECT_FALSE(TestableRNANodeQuery::contains("locatio", "location")); + EXPECT_FALSE(TestableRNANodeQuery::contains("locationnn", "location")); + EXPECT_FALSE(TestableRNANodeQuery::contains("test_location", "location")); + EXPECT_FALSE(TestableRNANodeQuery::contains("location_test", "location")); + EXPECT_FALSE(TestableRNANodeQuery::contains("test_location_test", "location")); + EXPECT_FALSE(TestableRNANodeQuery::contains("pose.bone[\"location\"].scale", "location")); + EXPECT_FALSE(TestableRNANodeQuery::contains("pose.bone[\"location\"].scale[0]", "location")); +} + +} // namespace tests +} // namespace deg +} // namespace blender -- cgit v1.2.3