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

4 files changed, 114 insertions, 3 deletions

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