Attributes: Improve custom data initialization options

When allocating new `CustomData` layers, often we do redundant
initialization of arrays. For example, it's common that values are
allocated, set to their default value, and then set to some other
value. This is wasteful, and it negates the benefits of optimizations
to the allocator like D15082. There are two reasons for this. The
first is array-of-structs storage that makes it annoying to initialize
values manually, and the second is confusing options in the Custom Data
API. This patch addresses the latter.

The `CustomData` "alloc type" options are rearranged. Now, besides
the options that use existing layers, there are two remaining:
* `CD_SET_DEFAULT` sets the default value.
  * Usually zeroes, but for colors this is white (how it was before).
  * Should be used when you add the layer but don't set all values.
* `CD_CONSTRUCT` refers to the "default construct" C++ term.
  * Only necessary or defined for non-trivial types like vertex groups.
  * Doesn't do anything for trivial types like `int` or `float3`.
  * Should be used every other time, when all values will be set.

The attribute API's `AttributeInit` types are updated as well.
To update code, replace `CD_CALLOC` with `CD_SET_DEFAULT` and
`CD_DEFAULT` with `CD_CONSTRUCT`. This doesn't cause any functional
changes yet. Follow-up commits will change to avoid initializing
new layers where the correctness is clear.

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

1 files changed, 106 insertions, 65 deletions

diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc
index 1f70ab587bf..447921b6d84 100644
--- a/source/blender/blenkernel/intern/customdata.cc
+++ b/source/blender/blenkernel/intern/customdata.cc
@@ -154,9 +154,15 @@ struct LayerTypeInfo {
   void (*swap)(void *data, const int *corner_indices);
 
   /**
-   * a function to set a layer's data to default values. if null, the
-   * default is assumed to be all zeros */
-  void (*set_default)(void *data, int count);
+   * Set values to the type's default. If undefined, the default is assumed to be zeroes.
+   * Memory pointed to by #data is expected to be uninitialized.
+   */
+  void (*set_default_value)(void *data, int count);
+  /**
+   * Construct and fill a valid value for the type. Necessary for non-trivial types.
+   * Memory pointed to by #data is expected to be uninitialized.
+   */
+  void (*construct)(void *data, int count);
 
   /** A function used by mesh validating code, must ensures passed item has valid data. */
   cd_validate validate;
@@ -312,6 +318,11 @@ static void layerInterp_mdeformvert(const void **sources,
   }
 }
 
+static void layerConstruct_mdeformvert(void *data, const int count)
+{
+  memset(data, 0, sizeof(MDeformVert) * count);
+}
+
 /** \} */
 
 /* -------------------------------------------------------------------- */
@@ -497,11 +508,6 @@ static bool layerValidate_propFloat(void *data, const uint totitems, const bool
 /** \name Callbacks for (#MIntProperty, #CD_PROP_INT32)
  * \{ */
 
-static void layerCopy_propInt(const void *source, void *dest, const int count)
-{
-  memcpy(dest, source, sizeof(MIntProperty) * count);
-}
-
 static void layerInterp_propInt(const void **sources,
                                 const float *weights,
                                 const float *UNUSED(sub_weights),
@@ -671,6 +677,11 @@ static void layerFree_mdisps(void *data, const int count, const int UNUSED(size)
   }
 }
 
+static void layerConstruct_mdisps(void *data, const int count)
+{
+  memset(data, 0, sizeof(MDisps) * count);
+}
+
 static bool layerRead_mdisps(CDataFile *cdf, void *data, const int count)
 {
   MDisps *d = static_cast<MDisps *>(data);
@@ -803,6 +814,11 @@ static void layerFree_grid_paint_mask(void *data, const int count, const int UNU
   }
 }
 
+static void layerConstruct_grid_paint_mask(void *data, const int count)
+{
+  memset(data, 0, sizeof(GridPaintMask) * count);
+}
+
 /** \} */
 
 /* -------------------------------------------------------------------- */
@@ -1645,30 +1661,23 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      layerFree_mdeformvert,
      layerInterp_mdeformvert,
      nullptr,
+     layerConstruct_mdeformvert,
      nullptr},
     /* 3: CD_MEDGE */
     {sizeof(MEdge), "MEdge", 1, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr},
     /* 4: CD_MFACE */
     {sizeof(MFace), "MFace", 1, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr},
     /* 5: CD_MTFACE */
-    {sizeof(MTFace),    "MTFace",         1,
-     N_("UVMap"),       layerCopy_tface,  nullptr,
-     layerInterp_tface, layerSwap_tface,  layerDefault_tface,
-     nullptr,           nullptr,          nullptr,
-     nullptr,           nullptr,          nullptr,
-     nullptr,           nullptr,          nullptr,
-     nullptr,           layerMaxNum_tface},
-    /* 6: CD_MCOL */
-    /* 4 MCol structs per face */
-    {sizeof(MCol[4]),
-     "MCol",
-     4,
-     N_("Col"),
+    {sizeof(MTFace),
+     "MTFace",
+     1,
+     N_("UVMap"),
+     layerCopy_tface,
      nullptr,
+     layerInterp_tface,
+     layerSwap_tface,
      nullptr,
-     layerInterp_mcol,
-     layerSwap_mcol,
-     layerDefault_mcol,
+     layerDefault_tface,
      nullptr,
      nullptr,
      nullptr,
@@ -1679,7 +1688,16 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      nullptr,
      nullptr,
      nullptr,
-     layerMaxNum_mloopcol},
+     layerMaxNum_tface},
+    /* 6: CD_MCOL */
+    /* 4 MCol structs per face */
+    {sizeof(MCol[4]),  "MCol",         4,
+     N_("Col"),        nullptr,        nullptr,
+     layerInterp_mcol, layerSwap_mcol, layerDefault_mcol,
+     nullptr,          nullptr,        nullptr,
+     nullptr,          nullptr,        nullptr,
+     nullptr,          nullptr,        nullptr,
+     nullptr,          nullptr,        layerMaxNum_mloopcol},
     /* 7: CD_ORIGINDEX */
     {sizeof(int), "", 0, nullptr, nullptr, nullptr, nullptr, nullptr, layerDefault_origindex},
     /* 8: CD_NORMAL */
@@ -1699,6 +1717,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      nullptr,
      nullptr,
      nullptr,
+     nullptr,
      layerCopyValue_normal},
     /* 9: CD_FACEMAP */
     {sizeof(int), "", 0, nullptr, nullptr, nullptr, nullptr, nullptr, layerDefault_fmap, nullptr},
@@ -1712,13 +1731,14 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      layerInterp_propFloat,
      nullptr,
      nullptr,
+     nullptr,
      layerValidate_propFloat},
     /* 11: CD_PROP_INT32 */
     {sizeof(MIntProperty),
      "MIntProperty",
      1,
      N_("Int"),
-     layerCopy_propInt,
+     nullptr,
      nullptr,
      layerInterp_propInt,
      nullptr},
@@ -1757,6 +1777,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      layerInterp_mloopuv,
      nullptr,
      nullptr,
+     nullptr,
      layerValidate_mloopuv,
      layerEqual_mloopuv,
      layerMultiply_mloopuv,
@@ -1779,6 +1800,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      nullptr,
      layerDefault_mloopcol,
      nullptr,
+     nullptr,
      layerEqual_mloopcol,
      layerMultiply_mloopcol,
      layerInitMinMax_mloopcol,
@@ -1801,6 +1823,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      nullptr,
      layerSwap_mdisps,
      nullptr,
+     layerConstruct_mdisps,
      nullptr,
      nullptr,
      nullptr,
@@ -1870,6 +1893,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      nullptr,
      nullptr,
      nullptr,
+     nullptr,
      layerEqual_mloop_origspace,
      layerMultiply_mloop_origspace,
      layerInitMinMax_mloop_origspace,
@@ -1887,6 +1911,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      nullptr,
      layerDefault_mloopcol,
      nullptr,
+     nullptr,
      layerEqual_mloopcol,
      layerMultiply_mloopcol,
      layerInitMinMax_mloopcol,
@@ -1914,7 +1939,8 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      layerFree_grid_paint_mask,
      nullptr,
      nullptr,
-     nullptr},
+     nullptr,
+     layerConstruct_grid_paint_mask},
     /* 36: CD_MVERT_SKIN */
     {sizeof(MVertSkin),
      "MVertSkin",
@@ -1972,6 +1998,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      nullptr,
      layerDefault_propcol,
      nullptr,
+     nullptr,
      layerEqual_propcol,
      layerMultiply_propcol,
      layerInitMinMax_propcol,
@@ -1992,6 +2019,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      layerInterp_propfloat3,
      nullptr,
      nullptr,
+     nullptr,
      layerValidate_propfloat3,
      nullptr,
      layerMultiply_propfloat3,
@@ -2007,6 +2035,7 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = {
      layerInterp_propfloat2,
      nullptr,
      nullptr,
+     nullptr,
      layerValidate_propfloat2,
      nullptr,
      layerMultiply_propfloat2,
@@ -2765,48 +2794,60 @@ static CustomDataLayer *customData_add_layer__internal(CustomData *data,
   const LayerTypeInfo *typeInfo = layerType_getInfo(type);
   int flag = 0;
 
-  /* Passing a layer-data to copy from with an alloctype that won't copy is
-   * most likely a bug */
-  BLI_assert(!layerdata || ELEM(alloctype, CD_ASSIGN, CD_DUPLICATE, CD_REFERENCE));
-
   if (!typeInfo->defaultname && CustomData_has_layer(data, type)) {
     return &data->layers[CustomData_get_layer_index(data, type)];
   }
 
   void *newlayerdata = nullptr;
-  if (ELEM(alloctype, CD_ASSIGN, CD_REFERENCE)) {
-    newlayerdata = layerdata;
-  }
-  else if (totelem > 0 && typeInfo->size > 0) {
-    if (alloctype == CD_DUPLICATE && layerdata) {
-      newlayerdata = MEM_malloc_arrayN((size_t)totelem, typeInfo->size, layerType_getName(type));
-    }
-    else {
-      newlayerdata = MEM_calloc_arrayN((size_t)totelem, typeInfo->size, layerType_getName(type));
-    }
-
-    if (!newlayerdata) {
-      return nullptr;
-    }
-  }
-
-  if (alloctype == CD_DUPLICATE && layerdata) {
-    if (totelem > 0) {
-      if (typeInfo->copy) {
-        typeInfo->copy(layerdata, newlayerdata, totelem);
+  switch (alloctype) {
+    case CD_SET_DEFAULT:
+      if (totelem > 0) {
+        if (typeInfo->set_default_value) {
+          newlayerdata = MEM_malloc_arrayN(totelem, typeInfo->size, layerType_getName(type));
+          typeInfo->set_default_value(newlayerdata, totelem);
+        }
+        else {
+          newlayerdata = MEM_calloc_arrayN(totelem, typeInfo->size, layerType_getName(type));
+        }
+      }
+      break;
+    case CD_CONSTRUCT:
+      if (totelem > 0) {
+        newlayerdata = MEM_malloc_arrayN(totelem, typeInfo->size, layerType_getName(type));
+        if (typeInfo->construct) {
+          typeInfo->construct(newlayerdata, totelem);
+        }
+      }
+      break;
+    case CD_ASSIGN:
+      if (totelem > 0) {
+        BLI_assert(layerdata != nullptr);
+        newlayerdata = layerdata;
       }
       else {
-        memcpy(newlayerdata, layerdata, (size_t)totelem * typeInfo->size);
+        MEM_SAFE_FREE(layerdata);
       }
-    }
-  }
-  else if (alloctype == CD_DEFAULT) {
-    if (typeInfo->set_default) {
-      typeInfo->set_default(newlayerdata, totelem);
-    }
-  }
-  else if (alloctype == CD_REFERENCE) {
-    flag |= CD_FLAG_NOFREE;
+      break;
+    case CD_REFERENCE:
+      if (totelem > 0) {
+        BLI_assert(layerdata != nullptr);
+        newlayerdata = layerdata;
+        flag |= CD_FLAG_NOFREE;
+      }
+      break;
+    case CD_DUPLICATE:
+      if (totelem > 0) {
+        newlayerdata = MEM_malloc_arrayN(totelem, typeInfo->size, layerType_getName(type));
+        if (typeInfo->copy) {
+          typeInfo->copy(layerdata, newlayerdata, totelem);
+        }
+        else {
+          BLI_assert(layerdata != nullptr);
+          BLI_assert(newlayerdata != nullptr);
+          memcpy(newlayerdata, layerdata, totelem * typeInfo->size);
+        }
+      }
+      break;
   }
 
   int index = data->totlayer;
@@ -3850,8 +3891,8 @@ static void CustomData_bmesh_set_default_n(CustomData *data, void **block, const
   int offset = data->layers[n].offset;
   const LayerTypeInfo *typeInfo = layerType_getInfo(data->layers[n].type);
 
-  if (typeInfo->set_default) {
-    typeInfo->set_default(POINTER_OFFSET(*block, offset), 1);
+  if (typeInfo->set_default_value) {
+    typeInfo->set_default_value(POINTER_OFFSET(*block, offset), 1);
   }
   else {
     memset(POINTER_OFFSET(*block, offset), 0, typeInfo->size);
@@ -4558,8 +4599,8 @@ static bool CustomData_layer_ensure_data_exists(CustomDataLayer *layer, size_t c
     case CD_MLOOPUV:   /* See T90620. */
       layer->data = MEM_calloc_arrayN(count, typeInfo->size, layerType_getName(layer->type));
       BLI_assert(layer->data);
-      if (typeInfo->set_default) {
-        typeInfo->set_default(layer->data, count);
+      if (typeInfo->set_default_value) {
+        typeInfo->set_default_value(layer->data, count);
       }
       return true;
       break;