Cleanup: Use const arguments for volume code

The problem was that you could getting write access to a grid from a
`const Volume *` without breaking const correctness. I encountered this
when working on support for volumes in the bounding box node. For
geometry nodes there is an important distinction between getting data
"for read" and "for write", with the former returning a `const` version
of the data.

Also, for volumes it was necessary to cast away const, since all of
the relevant functions in `volume.cc` didn't have const versions. This
patch adds `const` in these places, distinguising between "for read"
and "for write" versions of functions where necessary.

The downside is that loading and unloading in the global volume cache
needs const write-access to some member variables. I see that as an
inherent problem that comes up with caching that never has a beautiful
solution anyway.

Some of the const-ness could probably be propogated futher in EEVEE
code, but I'll leave that out, since there is another level of caching.

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

1 files changed, 55 insertions, 27 deletions

diff --git a/source/blender/blenkernel/intern/volume.cc b/source/blender/blenkernel/intern/volume.cc
index 6c0d361a64c..a51828453ca 100644
--- a/source/blender/blenkernel/intern/volume.cc
+++ b/source/blender/blenkernel/intern/volume.cc
@@ -144,14 +144,14 @@ static struct VolumeFileCache {
     blender::Map<int, openvdb::GridBase::Ptr> simplified_grids;
 
     /* Has the grid tree been loaded? */
-    bool is_loaded;
+    mutable bool is_loaded;
     /* Error message if an error occurred while loading. */
     std::string error_msg;
     /* User counting. */
     int num_metadata_users;
     int num_tree_users;
     /* Mutex for on-demand reading of tree. */
-    std::mutex mutex;
+    mutable std::mutex mutex;
   };
 
   struct EntryHasher {
@@ -289,7 +289,7 @@ struct VolumeGrid {
     }
   }
 
-  void load(const char *volume_name, const char *filepath)
+  void load(const char *volume_name, const char *filepath) const
   {
     /* If already loaded or not file-backed, nothing to do. */
     if (is_loaded || entry == nullptr) {
@@ -331,7 +331,7 @@ struct VolumeGrid {
     is_loaded = true;
   }
 
-  void unload(const char *volume_name)
+  void unload(const char *volume_name) const
   {
     /* Not loaded or not file-backed, nothing to do. */
     if (!is_loaded || entry == nullptr) {
@@ -432,10 +432,14 @@ struct VolumeGrid {
   int simplify_level = 0;
   /* OpenVDB grid if it's not shared through the file cache. */
   openvdb::GridBase::Ptr local_grid;
-  /* Indicates if the tree has been loaded for this grid. Note that vdb.tree()
+  /**
+   * Indicates if the tree has been loaded for this grid. Note that vdb.tree()
    * may actually be loaded by another user while this is false. But only after
-   * calling load() and is_loaded changes to true is it safe to access. */
-  bool is_loaded;
+   * calling load() and is_loaded changes to true is it safe to access.
+   *
+   * Const write access to this must be protected by `entry->mutex`.
+   */
+  mutable bool is_loaded;
 };
 
 /* Volume Grid Vector
@@ -468,14 +472,15 @@ struct VolumeGridVector : public std::list<VolumeGrid> {
     metadata.reset();
   }
 
+  /* Mutex for file loading of grids list. Const write access to the fields after this must be
+   * protected by locking with this mutex. */
+  mutable std::mutex mutex;
   /* Absolute file path that grids have been loaded from. */
   char filepath[FILE_MAX];
   /* File loading error message. */
   std::string error_msg;
   /* File Metadata. */
   openvdb::MetaMap::Ptr metadata;
-  /* Mutex for file loading of grids list. */
-  std::mutex mutex;
 };
 #endif
 
@@ -762,10 +767,10 @@ bool BKE_volume_is_loaded(const Volume *volume)
 #endif
 }
 
-bool BKE_volume_load(Volume *volume, Main *bmain)
+bool BKE_volume_load(const Volume *volume, const Main *bmain)
 {
 #ifdef WITH_OPENVDB
-  VolumeGridVector &grids = *volume->runtime.grids;
+  const VolumeGridVector &const_grids = *volume->runtime.grids;
 
   if (volume->runtime.frame == VOLUME_FRAME_NONE) {
     /* Skip loading this frame, outside of sequence range. */
@@ -773,15 +778,19 @@ bool BKE_volume_load(Volume *volume, Main *bmain)
   }
 
   if (BKE_volume_is_loaded(volume)) {
-    return grids.error_msg.empty();
+    return const_grids.error_msg.empty();
   }
 
   /* Double-checked lock. */
-  std::lock_guard<std::mutex> lock(grids.mutex);
+  std::lock_guard<std::mutex> lock(const_grids.mutex);
   if (BKE_volume_is_loaded(volume)) {
-    return grids.error_msg.empty();
+    return const_grids.error_msg.empty();
   }
 
+  /* Guarded by the lock, we can continue to access the grid vector,
+   * adding error messages or a new grid, etc. */
+  VolumeGridVector &grids = const_cast<VolumeGridVector &>(const_grids);
+
   /* Get absolute file path at current frame. */
   const char *volume_name = volume->id.name + 2;
   char filepath[FILE_MAX];
@@ -846,7 +855,10 @@ void BKE_volume_unload(Volume *volume)
 
 /* File Save */
 
-bool BKE_volume_save(Volume *volume, Main *bmain, ReportList *reports, const char *filepath)
+bool BKE_volume_save(const Volume *volume,
+                     const Main *bmain,
+                     ReportList *reports,
+                     const char *filepath)
 {
 #ifdef WITH_OPENVDB
   if (!BKE_volume_load(volume, bmain)) {
@@ -887,7 +899,7 @@ BoundBox *BKE_volume_boundbox_get(Object *ob)
   }
 
   if (ob->runtime.bb == nullptr) {
-    Volume *volume = (Volume *)ob->data;
+    const Volume *volume = (const Volume *)ob->data;
 
     ob->runtime.bb = (BoundBox *)MEM_callocN(sizeof(BoundBox), "volume boundbox");
 
@@ -902,7 +914,7 @@ BoundBox *BKE_volume_boundbox_get(Object *ob)
       const int num_grids = BKE_volume_num_grids(volume);
 
       for (int i = 0; i < num_grids; i++) {
-        VolumeGrid *grid = BKE_volume_grid_get(volume, i);
+        const VolumeGrid *grid = BKE_volume_grid_get_for_read(volume, i);
         float grid_min[3], grid_max[3];
 
         BKE_volume_grid_load(volume, grid);
@@ -953,7 +965,7 @@ bool BKE_volume_is_points_only(const Volume *volume)
   }
 
   for (int i = 0; i < num_grids; i++) {
-    VolumeGrid *grid = BKE_volume_grid_get(volume, i);
+    const VolumeGrid *grid = BKE_volume_grid_get_for_read(volume, i);
     if (BKE_volume_grid_type(grid) != VOLUME_GRID_POINTS) {
       return false;
     }
@@ -1140,7 +1152,23 @@ const char *BKE_volume_grids_frame_filepath(const Volume *volume)
 #endif
 }
 
-VolumeGrid *BKE_volume_grid_get(const Volume *volume, int grid_index)
+const VolumeGrid *BKE_volume_grid_get_for_read(const Volume *volume, int grid_index)
+{
+#ifdef WITH_OPENVDB
+  const VolumeGridVector &grids = *volume->runtime.grids;
+  for (const VolumeGrid &grid : grids) {
+    if (grid_index-- == 0) {
+      return &grid;
+    }
+  }
+  return nullptr;
+#else
+  UNUSED_VARS(volume, grid_index);
+  return nullptr;
+#endif
+}
+
+VolumeGrid *BKE_volume_grid_get_for_write(Volume *volume, int grid_index)
 {
 #ifdef WITH_OPENVDB
   VolumeGridVector &grids = *volume->runtime.grids;
@@ -1156,7 +1184,7 @@ VolumeGrid *BKE_volume_grid_get(const Volume *volume, int grid_index)
 #endif
 }
 
-VolumeGrid *BKE_volume_grid_active_get(const Volume *volume)
+const VolumeGrid *BKE_volume_grid_active_get_for_read(const Volume *volume)
 {
   const int num_grids = BKE_volume_num_grids(volume);
   if (num_grids == 0) {
@@ -1164,15 +1192,15 @@ VolumeGrid *BKE_volume_grid_active_get(const Volume *volume)
   }
 
   const int index = clamp_i(volume->active_grid, 0, num_grids - 1);
-  return BKE_volume_grid_get(volume, index);
+  return BKE_volume_grid_get_for_read(volume, index);
 }
 
 /* Tries to find a grid with the given name. Make sure that that the volume has been loaded. */
-VolumeGrid *BKE_volume_grid_find(const Volume *volume, const char *name)
+const VolumeGrid *BKE_volume_grid_find_for_read(const Volume *volume, const char *name)
 {
   int num_grids = BKE_volume_num_grids(volume);
   for (int i = 0; i < num_grids; i++) {
-    VolumeGrid *grid = BKE_volume_grid_get(volume, i);
+    const VolumeGrid *grid = BKE_volume_grid_get_for_read(volume, i);
     if (STREQ(BKE_volume_grid_name(grid), name)) {
       return grid;
     }
@@ -1183,7 +1211,7 @@ VolumeGrid *BKE_volume_grid_find(const Volume *volume, const char *name)
 
 /* Grid Loading */
 
-bool BKE_volume_grid_load(const Volume *volume, VolumeGrid *grid)
+bool BKE_volume_grid_load(const Volume *volume, const VolumeGrid *grid)
 {
 #ifdef WITH_OPENVDB
   VolumeGridVector &grids = *volume->runtime.grids;
@@ -1201,7 +1229,7 @@ bool BKE_volume_grid_load(const Volume *volume, VolumeGrid *grid)
 #endif
 }
 
-void BKE_volume_grid_unload(const Volume *volume, VolumeGrid *grid)
+void BKE_volume_grid_unload(const Volume *volume, const VolumeGrid *grid)
 {
 #ifdef WITH_OPENVDB
   const char *volume_name = volume->id.name + 2;
@@ -1406,7 +1434,7 @@ VolumeGrid *BKE_volume_grid_add(Volume *volume, const char *name, VolumeGridType
 {
 #ifdef WITH_OPENVDB
   VolumeGridVector &grids = *volume->runtime.grids;
-  BLI_assert(BKE_volume_grid_find(volume, name) == nullptr);
+  BLI_assert(BKE_volume_grid_find_for_read(volume, name) == nullptr);
   BLI_assert(type != VOLUME_GRID_UNKNOWN);
 
   openvdb::GridBase::Ptr vdb_grid = BKE_volume_grid_type_operation(type, CreateGridOp{});
@@ -1474,7 +1502,7 @@ openvdb::GridBase::ConstPtr BKE_volume_grid_openvdb_for_metadata(const VolumeGri
 }
 
 openvdb::GridBase::ConstPtr BKE_volume_grid_openvdb_for_read(const Volume *volume,
-                                                             VolumeGrid *grid)
+                                                             const VolumeGrid *grid)
 {
   BKE_volume_grid_load(volume, grid);
   return grid->grid();