Geometry Nodes: Revert current normal attribute implementation

After further thought, the implementation of the "normal" attribute
from D10541 is not the best approach to expose this data, mainly
because it blindly copied existing design rather than using the
best method in the context of the generalized attribute system.

In Blender, vertex normals are simply a cache of the average normals
from the surrounding / connected faces. Because we have automatic
interpolation between domains already, we don't need a special
`vertex_normal` attribute for this case, we can just let the
generalized interpolation do the hard work where necessary,
simplifying the set of built-in attributes to only include the
`normal` attribute from faces.

The fact that vertex normals are just a cache also raised another
issue, because the cache could be dirty, so mutex locks were
necessary to calculate normals. That isn't necessarily a problem,
but it's nice to avoid where possible.

Another downside of the current attribute naming is that after the
point distribute node there would be two normal attributes.

This commit reverts the `vertex_normal` attribute so that
it can be replaced by the implementation in D10677.

Differential Revision: https://developer.blender.org/D10676

4 files changed, 1 insertions, 65 deletions

diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc
index 01a1333c3ce..8482f096df1 100644
--- a/source/blender/blenkernel/intern/attribute_access.cc
+++ b/source/blender/blenkernel/intern/attribute_access.cc
@@ -31,7 +31,6 @@
 #include "BLI_color.hh"
 #include "BLI_float2.hh"
 #include "BLI_span.hh"
-#include "BLI_threads.h"
 
 #include "CLG_log.h"
 
@@ -375,10 +374,6 @@ ReadAttributePtr BuiltinCustomDataLayerProvider::try_get_for_read(
     return {};
   }
 
-  if (update_on_read_ != nullptr) {
-    update_on_read_(component);
-  }
-
   const int domain_size = component.attribute_domain_size(domain_);
   const void *data = CustomData_get_layer(custom_data, stored_type_);
   if (data == nullptr) {
diff --git a/source/blender/blenkernel/intern/attribute_access_intern.hh b/source/blender/blenkernel/intern/attribute_access_intern.hh
index 299da4c97fe..19349c69662 100644
--- a/source/blender/blenkernel/intern/attribute_access_intern.hh
+++ b/source/blender/blenkernel/intern/attribute_access_intern.hh
@@ -405,7 +405,6 @@ class BuiltinCustomDataLayerProvider final : public BuiltinAttributeProvider {
   const CustomDataAccessInfo custom_data_access_;
   const AsReadAttribute as_read_attribute_;
   const AsWriteAttribute as_write_attribute_;
-  const UpdateOnRead update_on_read_;
   const UpdateOnWrite update_on_write_;
 
  public:
@@ -419,7 +418,6 @@ class BuiltinCustomDataLayerProvider final : public BuiltinAttributeProvider {
                                  const CustomDataAccessInfo custom_data_access,
                                  const AsReadAttribute as_read_attribute,
                                  const AsWriteAttribute as_write_attribute,
-                                 const UpdateOnRead update_on_read,
                                  const UpdateOnWrite update_on_write)
       : BuiltinAttributeProvider(
             std::move(attribute_name), domain, attribute_type, creatable, writable, deletable),
@@ -427,7 +425,6 @@ class BuiltinCustomDataLayerProvider final : public BuiltinAttributeProvider {
         custom_data_access_(custom_data_access),
         as_read_attribute_(as_read_attribute),
         as_write_attribute_(as_write_attribute),
-        update_on_read_(update_on_read),
         update_on_write_(update_on_write)
   {
   }
diff --git a/source/blender/blenkernel/intern/geometry_component_mesh.cc b/source/blender/blenkernel/intern/geometry_component_mesh.cc
index 4019ceb123f..b25f5286cab 100644
--- a/source/blender/blenkernel/intern/geometry_component_mesh.cc
+++ b/source/blender/blenkernel/intern/geometry_component_mesh.cc
@@ -15,7 +15,6 @@
  */
 
 #include "BLI_listbase.h"
-#include "BLI_threads.h"
 
 #include "DNA_mesh_types.h"
 #include "DNA_meshdata_types.h"
@@ -535,43 +534,6 @@ static WriteAttributePtr make_material_index_write_attribute(void *data, const i
       ATTR_DOMAIN_POLYGON, MutableSpan<MPoly>((MPoly *)data, domain_size));
 }
 
-static float3 get_vertex_normal(const MVert &vert)
-{
-  float3 result;
-  normal_short_to_float_v3(result, vert.no);
-  return result;
-}
-
-static ReadAttributePtr make_vertex_normal_read_attribute(const void *data, const int domain_size)
-{
-  return std::make_unique<DerivedArrayReadAttribute<MVert, float3, get_vertex_normal>>(
-      ATTR_DOMAIN_POINT, Span<MVert>((const MVert *)data, domain_size));
-}
-
-static void update_vertex_normals_when_dirty(const GeometryComponent &component)
-{
-  const Mesh *mesh = get_mesh_from_component_for_read(component);
-  if (mesh == nullptr) {
-    return;
-  }
-
-  /* Since normals are derived data, const write access to them is okay. However, ensure that
-   * two threads don't use write normals to a mesh at the same time. Note that this relies on
-   * the idempotence of the operation; calculating the normals just fills the MVert struct
-   * rather than allocating new memory. */
-  if (mesh->runtime.cd_dirty_vert & CD_MASK_NORMAL) {
-    ThreadMutex *mesh_eval_mutex = (ThreadMutex *)mesh->runtime.eval_mutex;
-    BLI_mutex_lock(mesh_eval_mutex);
-
-    /* Check again to avoid a second thread needlessly recalculating the same normals. */
-    if (mesh->runtime.cd_dirty_vert & CD_MASK_NORMAL) {
-      BKE_mesh_calc_normals(const_cast<Mesh *>(mesh));
-    }
-
-    BLI_mutex_unlock(mesh_eval_mutex);
-  }
-}
-
 static bool get_shade_smooth(const MPoly &mpoly)
 {
   return mpoly.flag & ME_SMOOTH;
@@ -854,7 +816,6 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh()
                                                  point_access,
                                                  make_vertex_position_read_attribute,
                                                  make_vertex_position_write_attribute,
-                                                 nullptr,
                                                  tag_normals_dirty_when_writing_position);
 
   static BuiltinCustomDataLayerProvider material_index("material_index",
@@ -867,7 +828,6 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh()
                                                        polygon_access,
                                                        make_material_index_read_attribute,
                                                        make_material_index_write_attribute,
-                                                       nullptr,
                                                        nullptr);
 
   static BuiltinCustomDataLayerProvider shade_smooth("shade_smooth",
@@ -880,22 +840,8 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh()
                                                      polygon_access,
                                                      make_shade_smooth_read_attribute,
                                                      make_shade_smooth_write_attribute,
-                                                     nullptr,
                                                      nullptr);
 
-  static BuiltinCustomDataLayerProvider vertex_normal("vertex_normal",
-                                                      ATTR_DOMAIN_POINT,
-                                                      CD_PROP_FLOAT3,
-                                                      CD_MVERT,
-                                                      BuiltinAttributeProvider::NonCreatable,
-                                                      BuiltinAttributeProvider::Readonly,
-                                                      BuiltinAttributeProvider::NonDeletable,
-                                                      point_access,
-                                                      make_vertex_normal_read_attribute,
-                                                      nullptr,
-                                                      update_vertex_normals_when_dirty,
-                                                      nullptr);
-
   static NamedLegacyCustomDataProvider uvs(ATTR_DOMAIN_CORNER,
                                            CD_PROP_FLOAT2,
                                            CD_MLOOPUV,
@@ -916,7 +862,7 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh()
   static CustomDataAttributeProvider edge_custom_data(ATTR_DOMAIN_EDGE, edge_access);
   static CustomDataAttributeProvider polygon_custom_data(ATTR_DOMAIN_POLYGON, polygon_access);
 
-  return ComponentAttributeProviders({&position, &material_index, &vertex_normal, &shade_smooth},
+  return ComponentAttributeProviders({&position, &material_index, &shade_smooth},
                                      {&uvs,
                                       &vertex_colors,
                                       &corner_custom_data,
diff --git a/source/blender/blenkernel/intern/geometry_component_pointcloud.cc b/source/blender/blenkernel/intern/geometry_component_pointcloud.cc
index c428e93cfa9..073f457ae54 100644
--- a/source/blender/blenkernel/intern/geometry_component_pointcloud.cc
+++ b/source/blender/blenkernel/intern/geometry_component_pointcloud.cc
@@ -175,7 +175,6 @@ static ComponentAttributeProviders create_attribute_providers_for_point_cloud()
       point_access,
       make_array_read_attribute<float3, ATTR_DOMAIN_POINT>,
       make_array_write_attribute<float3, ATTR_DOMAIN_POINT>,
-      nullptr,
       nullptr);
   static BuiltinCustomDataLayerProvider radius(
       "radius",
@@ -188,7 +187,6 @@ static ComponentAttributeProviders create_attribute_providers_for_point_cloud()
       point_access,
       make_array_read_attribute<float, ATTR_DOMAIN_POINT>,
       make_array_write_attribute<float, ATTR_DOMAIN_POINT>,
-      nullptr,
       nullptr);
   static CustomDataAttributeProvider point_custom_data(ATTR_DOMAIN_POINT, point_access);
   return ComponentAttributeProviders({&position, &radius}, {&point_custom_data});