Decrease property registration time 36/269636/3
authorDavid Steele <david.steele@samsung.com>
Wed, 19 Jan 2022 10:17:37 +0000 (10:17 +0000)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Thu, 20 Jan 2022 15:39:55 +0000 (15:39 +0000)
For animated custom properties attached to objects, such as uniforms
attached to visual renderers, the registration time is too long.

One of the culprits is the Object::RegisterProperty() method iterating
over all the preceding property names to ensure the uniqueness of the
property name.

In the visual case, we can remove this check, as we can ensure that
each call to RegisterProperty is only done once per uniform.

Added a method RegisterUniqueProperty (which should really be called
RegisterPropertyWithoutUniquenessCheck, but hey!) to perform the same
registration without the name lookup checks.

Change-Id: Id3b78e342415aef65f0ff121ae2c8ec4a9ec80a6
Signed-off-by: David Steele <david.steele@samsung.com>
automated-tests/src/dali/utc-Dali-Handle.cpp
dali/internal/event/common/object-impl.cpp
dali/internal/event/common/object-impl.h
dali/public-api/object/handle.cpp
dali/public-api/object/handle.h

index a950104..e66312f 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -772,6 +772,165 @@ int UtcDaliHandleRegisterProperty02(void)
   END_TEST;
 }
 
+int UtcDaliHandleRegisterProperty03(void)
+{
+  tet_infoline("Test Dali::Handle::RegisterUniqueProperty() int key against default property");
+  TestApplication application;
+
+  Integration::Scene stage = application.GetScene();
+
+  Actor actor = Actor::New();
+  stage.Add(actor);
+
+  const unsigned int defaultPropertyCount = actor.GetPropertyCount();
+
+  application.SendNotification();
+  application.Render();
+
+  const Vector4 testColor(0.5f, 0.2f, 0.9f, 1.0f);
+  actor.SetProperty(Actor::Property::COLOR, testColor);
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS(actor.GetPropertyCount(), defaultPropertyCount, TEST_LOCATION);
+
+  // No uniqueness tests are done on the properties. Check that the indices are different
+
+  Property::Index key       = CORE_PROPERTY_MAX_INDEX + 1;
+  Property::Index testIndex = actor.RegisterUniqueProperty(key, "color", Color::BLACK);
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS(testIndex != Actor::Property::COLOR, true, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetPropertyCount(), defaultPropertyCount + 1, TEST_LOCATION); // Property count should be different
+
+  DALI_TEST_EQUALS(actor.GetProperty<Vector4>(Actor::Property::COLOR), testColor, TEST_LOCATION); // Value should not have changed
+  DALI_TEST_EQUALS(actor.GetProperty<Vector4>(testIndex), Color::BLACK, TEST_LOCATION);           // Value should not have changed
+
+  // Check that name lookup returns the default property
+  DALI_TEST_EQUALS((int)actor.GetPropertyIndex(Property::Key("color")), (int)Actor::Property::COLOR, TEST_LOCATION);
+
+  // Check that they have different scene graph properties
+  DALI_TEST_EQUALS(actor.GetCurrentProperty(Actor::Property::COLOR), Property::Value(testColor), 0.0001f, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetCurrentProperty(testIndex), Property::Value(Color::BLACK), 0.0001f, TEST_LOCATION);
+
+  END_TEST;
+}
+
+int UtcDaliHandleRegisterProperty04(void)
+{
+  tet_infoline("Negative Test Dali::Handle::RegisterUniqueProperty() same int key against custom property");
+  TestApplication application;
+
+  Integration::Scene stage = application.GetScene();
+
+  Actor actor = Actor::New();
+  stage.Add(actor);
+
+  const unsigned int defaultPropertyCount = actor.GetPropertyCount();
+
+  application.SendNotification();
+  application.Render();
+
+  Property::Index key1 = CORE_PROPERTY_MAX_INDEX + 1;
+  Property::Index key2 = CORE_PROPERTY_MAX_INDEX + 2;
+
+  const Vector4 testColor(0.5f, 0.2f, 0.9f, 1.0f);
+  const float   withFlake(99.f);
+
+  Property::Index index1 = actor.RegisterProperty("MyPropertyOne", Vector3::ONE);
+  Property::Index index2 = actor.RegisterUniqueProperty(key1, "sideColor", testColor);
+  Property::Index index3 = actor.RegisterUniqueProperty(key2, "iceCream", withFlake);
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS(actor.GetPropertyCount(), defaultPropertyCount + 3, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetProperty<Vector3>(index1), Vector3::ONE, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetProperty<Vector4>(index2), testColor, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetProperty<float>(index3), withFlake, TEST_LOCATION);
+
+  // If property key matches, ignore new name.
+  Property::Index testIndex2 = actor.RegisterUniqueProperty(key1, "sideColor", Color::BLACK);
+  Property::Index testIndex3 = actor.RegisterUniqueProperty(key2, "iceCream", 2200.f);
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS(index2 == testIndex2, true, TEST_LOCATION);
+  DALI_TEST_EQUALS(index3 == testIndex3, true, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetPropertyCount(), defaultPropertyCount + 3, TEST_LOCATION); // Property count should be same
+  // Test that the value has actually been updated
+  DALI_TEST_EQUALS(actor.GetProperty<Vector4>(index2), Color::BLACK, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetProperty<float>(index3), 2200.f, TEST_LOCATION);
+
+  // Check that name lookup returns the first registered property
+  DALI_TEST_EQUALS(actor.GetPropertyIndex(Property::Key("sideColor")), index2, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetPropertyIndex(Property::Key("iceCream")), index3, TEST_LOCATION);
+
+  END_TEST;
+}
+
+int UtcDaliHandleRegisterProperty05(void)
+{
+  tet_infoline("Positive Test Dali::Handle::RegisterUniqueProperty() different int key against custom property");
+  TestApplication application;
+
+  Integration::Scene stage = application.GetScene();
+
+  Actor actor = Actor::New();
+  stage.Add(actor);
+
+  const unsigned int defaultPropertyCount = actor.GetPropertyCount();
+
+  application.SendNotification();
+  application.Render();
+
+  Property::Index key1 = CORE_PROPERTY_MAX_INDEX + 1;
+  Property::Index key2 = CORE_PROPERTY_MAX_INDEX + 2;
+
+  const Vector4 testColor(0.5f, 0.2f, 0.9f, 1.0f);
+  const float   withFlake(99.f);
+
+  Property::Index index1 = actor.RegisterProperty("MyPropertyOne", Vector3::ONE);
+  Property::Index index2 = actor.RegisterUniqueProperty(key1, "sideColor", testColor);
+  Property::Index index3 = actor.RegisterUniqueProperty(key2, "iceCream", withFlake);
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS(actor.GetPropertyCount(), defaultPropertyCount + 3, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetProperty<Vector3>(index1), Vector3::ONE, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetProperty<Vector4>(index2), testColor, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetProperty<float>(index3), withFlake, TEST_LOCATION);
+
+  // No uniqueness tests are done on the names. Check that the indices are different if the keys are different
+  Property::Index newKey1 = key2 + 1;
+  Property::Index newKey2 = key2 + 2;
+
+  Property::Index testIndex2 = actor.RegisterUniqueProperty(newKey1, "sideColor", Color::BLACK);
+  Property::Index testIndex3 = actor.RegisterUniqueProperty(newKey2, "iceCream", 2200.f);
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS(index2 == testIndex2, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(index3 == testIndex3, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetPropertyCount(), defaultPropertyCount + 5, TEST_LOCATION); // Property count should be different
+
+  DALI_TEST_EQUALS(actor.GetProperty<Vector4>(index2), testColor, TEST_LOCATION); // Value should not have changed
+  DALI_TEST_EQUALS(actor.GetProperty<float>(index3), withFlake, TEST_LOCATION);
+
+  DALI_TEST_EQUALS(actor.GetProperty<Vector4>(testIndex2), Color::BLACK, TEST_LOCATION); // Value should not have changed
+  DALI_TEST_EQUALS(actor.GetProperty<float>(testIndex3), 2200.f, TEST_LOCATION);
+
+  // Check that name lookup returns the first registered property
+  DALI_TEST_EQUALS(actor.GetPropertyIndex(Property::Key("sideColor")), index2, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor.GetPropertyIndex(Property::Key("iceCream")), index3, TEST_LOCATION);
+
+  END_TEST;
+}
+
 int UtcDaliHandleGetProperty(void)
 {
   tet_infoline("Positive Test Dali::Handle::GetProperty()");
index 10982d3..dec0470 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -535,16 +535,6 @@ void Object::GetPropertyIndices(Property::IndexContainer& indices) const
   }
 }
 
-Property::Index Object::RegisterProperty(std::string_view name, Property::Value propertyValue)
-{
-  return RegisterProperty(name, Property::INVALID_KEY, std::move(propertyValue), Property::ANIMATABLE);
-}
-
-Property::Index Object::RegisterProperty(std::string_view name, Property::Index key, Property::Value propertyValue)
-{
-  return RegisterProperty(name, key, std::move(propertyValue), Property::ANIMATABLE);
-}
-
 void Object::SetProperties(const Property::Map& properties)
 {
   const auto count = properties.Count();
@@ -577,26 +567,39 @@ void Object::GetProperties(Property::Map& properties)
   }
 }
 
+Property::Index Object::RegisterProperty(std::string_view name, Property::Value propertyValue)
+{
+  return RegisterProperty(name, Property::INVALID_KEY, std::move(propertyValue), Property::ANIMATABLE, true);
+}
+
+Property::Index Object::RegisterProperty(std::string_view name, Property::Index key, Property::Value propertyValue, bool checkForUniqueName)
+{
+  return RegisterProperty(name, key, std::move(propertyValue), Property::ANIMATABLE, checkForUniqueName);
+}
+
 Property::Index Object::RegisterProperty(std::string_view     name,
                                          Property::Value      propertyValue,
                                          Property::AccessMode accessMode)
 {
-  return RegisterProperty(name, Property::INVALID_KEY, std::move(propertyValue), accessMode);
+  return RegisterProperty(name, Property::INVALID_KEY, std::move(propertyValue), accessMode, true);
 }
 
 Property::Index Object::RegisterProperty(std::string_view     name,
                                          Property::Index      key,
                                          Property::Value      propertyValue,
-                                         Property::AccessMode accessMode)
+                                         Property::AccessMode accessMode,
+                                         bool                 checkForUniqueName)
 {
   auto constString = ConstString(name);
+
   // If property with the required key already exists, then just set it.
+  // Don't search names if checkForUniqueName is false.
   Property::Index index = Property::INVALID_INDEX;
   if(key != Property::INVALID_KEY) // Try integer key first if it's valid
   {
     index = GetPropertyIndex(key);
   }
-  if(index == Property::INVALID_INDEX) // If it wasn't valid, or doesn't exist, try name
+  if(index == Property::INVALID_INDEX && checkForUniqueName) // try name search if requested
   {
     index = GetPropertyIndex(constString);
   }
index 2fe6eba..702c12b 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_INTERNAL_OBJECT_H
 
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -222,24 +222,24 @@ public:
   void GetPropertyIndices(Property::IndexContainer& indices) const;
 
   /**
-   * @copydoc Dali::Handle::RegisterProperty()
+   * @copydoc Dali::DevelHandle::SetProperties()
    */
-  Property::Index RegisterProperty(std::string_view name, Property::Value propertyValue);
+  void SetProperties(const Property::Map& properties);
 
   /**
-   * @copydoc Dali::Handle::RegisterProperty()
+   * @copydoc Dali::DevelHandle::GetProperties()
    */
-  Property::Index RegisterProperty(std::string_view name, Property::Index key, Property::Value propertyValue);
+  void GetProperties(Property::Map& properties);
 
   /**
-   * @copydoc Dali::DevelHandle::SetProperties()
+   * @copydoc Dali::Handle::RegisterProperty()
    */
-  void SetProperties(const Property::Map& properties);
+  Property::Index RegisterProperty(std::string_view name, Property::Value propertyValue);
 
   /**
-   * @copydoc Dali::DevelHandle::GetProperties()
+   * @copydoc Dali::Handle::RegisterProperty()
    */
-  void GetProperties(Property::Map& properties);
+  Property::Index RegisterProperty(std::string_view name, Property::Index key, Property::Value propertyValue, bool checkForUniqueName);
 
   /**
    * @copydoc Dali::Handle::RegisterProperty(std::string name, Property::Value propertyValue, Property::AccessMode accessMode)
@@ -252,7 +252,8 @@ public:
   Property::Index RegisterProperty(std::string_view     name,
                                    Property::Index      key,
                                    Property::Value      propertyValue,
-                                   Property::AccessMode accessMode);
+                                   Property::AccessMode accessMode,
+                                   bool                 checkForUniqueName);
 
   /**
    * @brief returns true if the custom property exists on this object.
index 08b331b..ecac036 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -107,7 +107,7 @@ Property::Index Handle::RegisterProperty(std::string_view name, Property::Value
 
 Property::Index Handle::RegisterProperty(Property::Index key, std::string_view name, Property::Value propertyValue)
 {
-  return GetImplementation(*this).RegisterProperty(name, key, std::move(propertyValue));
+  return GetImplementation(*this).RegisterProperty(name, key, std::move(propertyValue), true);
 }
 
 Property::Index Handle::RegisterProperty(std::string_view name, Property::Value propertyValue, Property::AccessMode accessMode)
@@ -115,6 +115,16 @@ Property::Index Handle::RegisterProperty(std::string_view name, Property::Value
   return GetImplementation(*this).RegisterProperty(name, std::move(propertyValue), accessMode);
 }
 
+Property::Index Handle::RegisterUniqueProperty(std::string_view name, Property::Value propertyValue)
+{
+  return GetImplementation(*this).RegisterProperty(name, Property::INVALID_KEY, std::move(propertyValue), false);
+}
+
+Property::Index Handle::RegisterUniqueProperty(Property::Index key, std::string_view name, Property::Value propertyValue)
+{
+  return GetImplementation(*this).RegisterProperty(name, key, std::move(propertyValue), false);
+}
+
 Property::Value Handle::GetProperty(Property::Index index) const
 {
   return GetImplementation(*this).GetProperty(index);
index 879786f..65604f1 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_HANDLE_H
 
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -292,6 +292,36 @@ public:
   Property::Index RegisterProperty(std::string_view name, Property::Value propertyValue);
 
   /**
+   * @brief Registers a new animatable property.
+   *
+   * @SINCE_2_1.6
+   * @param[in] name The name of the property
+   * @param[in] propertyValue The new value of the property
+   * @return The index of the property or Property::INVALID_INDEX if registration failed
+   * @pre The object supports dynamic properties i.e. Supports(Handle::DYNAMIC_PROPERTIES) returns true.
+   * Property names are expected to be unique, but this is DEFINITELY not enforced. It is up to the
+   * caller to enforce uniqueness.
+   *
+   * Property indices are unique to each registered custom property in a given object.
+   * returns Property::INVALID_INDEX if registration failed. This can happen if you try to register
+   * animatable property on an object that does not have scene graph object.
+   * @note Only the following types can be animated:
+   *       - Property::BOOLEAN
+   *       - Property::FLOAT
+   *       - Property::INTEGER
+   *       - Property::VECTOR2
+   *       - Property::VECTOR3
+   *       - Property::VECTOR4
+   *       - Property::MATRIX3
+   *       - Property::MATRIX
+   *       - Property::ROTATION
+   * @note If a property with the desired name already exists, then this creates a secondary
+   * entry to a different scene graph property; Access by index works as expected, but uniform
+   * values will use the last registered version, not the existing version, so things may break.
+   */
+  Property::Index RegisterUniqueProperty(std::string_view name, Property::Value propertyValue);
+
+  /**
    * @brief Register a new animatable property with an integer key.
    *
    * @SINCE_1_9.27
@@ -305,6 +335,7 @@ public:
    * i.e. Supports(Handle::DYNAMIC_PROPERTIES) returns true.  Property names and keys
    * are expected to be unique, but this is not enforced.  Property indices are unique
    * to each registered custom property in a given object.
+   * @todo CHECK THIS!
    *
    * @note Returns Property::INVALID_INDEX if registration failed. This can happen if
    * you try to register animatable property on an object that does not have scene graph
@@ -334,6 +365,55 @@ public:
                                    Property::Value  propertyValue);
 
   /**
+   * @brief Register a new animatable property with an integer key.
+   *
+   * @SINCE_2_1.6
+   * @param[in] key  The integer key of the property.
+   * @param[in] name The text key of the property.
+   * @param[in] propertyValue The new value of the property.
+   *
+   * @return The index of the property or Property::INVALID_INDEX if registration failed
+   * It is up to the caller to guarantee that the property is unique. This allows many
+   * checks to be skipped.
+   *
+   * @pre The object supports dynamic properties
+   * i.e. Supports(Handle::DYNAMIC_PROPERTIES) returns true.  Property names and keys
+   * are expected to be unique, and are therefore just added without any checks.
+   * Property indices are unique to each registered custom property in a given object.
+   *
+   * @note Returns Property::INVALID_INDEX if registration failed. This can happen if
+   * you try to register animatable property on an object that does not have scene graph
+   * object.
+   *
+   * @note The returned property index is not the same as the integer key (though it
+   * shares a type)
+   *
+   * This version of RegisterProperty associates both an integer key and the text key
+   * with the property, allowing for lookup of the property index by either key or name
+   * ( which is useful when other classes know the key but not the name )
+   *
+   * @note Only the following types can be animated:
+   *       - Property::BOOLEAN
+   *       - Property::FLOAT
+   *       - Property::INTEGER
+   *       - Property::VECTOR2
+   *       - Property::VECTOR3
+   *       - Property::VECTOR4
+   *       - Property::MATRIX3
+   *       - Property::MATRIX
+   *       - Property::ROTATION
+   *
+   * @note If a property with the desired name already exists, then this will create a second entry with
+   * the same name, and may cause problems. It is up to the caller to prevent this happening. Possible side
+   * effects are: lookup by name always finds the first such property, not the second; whereas, writing
+   * uniform value to shader will use the second, not the first;
+   * Using the returned Property::Index for future reference will always access the correct property.
+   */
+  Property::Index RegisterUniqueProperty(Property::Index  key,
+                                         std::string_view name,
+                                         Property::Value  propertyValue);
+
+  /**
    * @brief Registers a new property.
    *
    * Properties can be set as non animatable using property attributes.