[Bf-blender-cvs] [fd414b49068] master: Cleanup: Use const arguments for volume code
Hans Goudey
noreply at git.blender.org
Thu Apr 8 19:00:35 CEST 2021
Commit: fd414b49068c011a1fd63a304ea95bbe420d6570
Author: Hans Goudey
Date: Thu Apr 8 12:00:26 2021 -0500
Branches: master
https://developer.blender.org/rBfd414b49068c011a1fd63a304ea95bbe420d6570
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
===================================================================
M intern/cycles/blender/blender_volume.cpp
M source/blender/blenkernel/BKE_volume.h
M source/blender/blenkernel/BKE_volume_render.h
M source/blender/blenkernel/intern/volume.cc
M source/blender/blenkernel/intern/volume_render.cc
M source/blender/draw/engines/eevee/eevee_volumes.c
M source/blender/draw/engines/workbench/workbench_volume.c
M source/blender/draw/intern/draw_cache.h
M source/blender/draw/intern/draw_cache_impl_volume.c
M source/blender/makesrna/intern/rna_volume.c
M source/blender/modifiers/intern/MOD_volume_displace.cc
M source/blender/modifiers/intern/MOD_volume_to_mesh.cc
M source/blender/nodes/geometry/nodes/node_geo_transform.cc
M source/blender/nodes/geometry/nodes/node_geo_volume_to_mesh.cc
===================================================================
diff --git a/intern/cycles/blender/blender_volume.cpp b/intern/cycles/blender/blender_volume.cpp
index 0ff4de846e1..772ab9f5c8a 100644
--- a/intern/cycles/blender/blender_volume.cpp
+++ b/intern/cycles/blender/blender_volume.cpp
@@ -26,7 +26,7 @@
#ifdef WITH_OPENVDB
# include <openvdb/openvdb.h>
openvdb::GridBase::ConstPtr BKE_volume_grid_openvdb_for_read(const struct Volume *volume,
- struct VolumeGrid *grid);
+ const struct VolumeGrid *grid);
#endif
CCL_NAMESPACE_BEGIN
@@ -227,7 +227,7 @@ class BlenderVolumeLoader : public VDBImageLoader {
const bool unload = !b_volume_grid.is_loaded();
::Volume *volume = (::Volume *)b_volume.ptr.data;
- VolumeGrid *volume_grid = (VolumeGrid *)b_volume_grid.ptr.data;
+ const VolumeGrid *volume_grid = (VolumeGrid *)b_volume_grid.ptr.data;
grid = BKE_volume_grid_openvdb_for_read(volume, volume_grid);
if (unload) {
diff --git a/source/blender/blenkernel/BKE_volume.h b/source/blender/blenkernel/BKE_volume.h
index 53626dbeb1b..2b17cf26e0e 100644
--- a/source/blender/blenkernel/BKE_volume.h
+++ b/source/blender/blenkernel/BKE_volume.h
@@ -78,16 +78,17 @@ extern void (*BKE_volume_batch_cache_free_cb)(struct Volume *volume);
typedef struct VolumeGrid VolumeGrid;
-bool BKE_volume_load(struct Volume *volume, struct Main *bmain);
+bool BKE_volume_load(const struct Volume *volume, const struct Main *bmain);
void BKE_volume_unload(struct Volume *volume);
bool BKE_volume_is_loaded(const struct Volume *volume);
int BKE_volume_num_grids(const struct Volume *volume);
const char *BKE_volume_grids_error_msg(const struct Volume *volume);
const char *BKE_volume_grids_frame_filepath(const struct Volume *volume);
-VolumeGrid *BKE_volume_grid_get(const struct Volume *volume, int grid_index);
-VolumeGrid *BKE_volume_grid_active_get(const struct Volume *volume);
-VolumeGrid *BKE_volume_grid_find(const struct Volume *volume, const char *name);
+const VolumeGrid *BKE_volume_grid_get_for_read(const struct Volume *volume, int grid_index);
+VolumeGrid *BKE_volume_grid_get_for_write(struct Volume *volume, int grid_index);
+const VolumeGrid *BKE_volume_grid_active_get_for_read(const struct Volume *volume);
+const VolumeGrid *BKE_volume_grid_find_for_read(const struct Volume *volume, const char *name);
/* Grid
*
@@ -109,8 +110,8 @@ typedef enum VolumeGridType {
VOLUME_GRID_POINTS,
} VolumeGridType;
-bool BKE_volume_grid_load(const struct Volume *volume, struct VolumeGrid *grid);
-void BKE_volume_grid_unload(const struct Volume *volume, struct VolumeGrid *grid);
+bool BKE_volume_grid_load(const struct Volume *volume, const struct VolumeGrid *grid);
+void BKE_volume_grid_unload(const struct Volume *volume, const struct VolumeGrid *grid);
bool BKE_volume_grid_is_loaded(const struct VolumeGrid *grid);
/* Metadata */
@@ -145,8 +146,8 @@ int BKE_volume_simplify_level(const struct Depsgraph *depsgraph);
float BKE_volume_simplify_factor(const struct Depsgraph *depsgraph);
/* File Save */
-bool BKE_volume_save(struct Volume *volume,
- struct Main *bmain,
+bool BKE_volume_save(const struct Volume *volume,
+ const struct Main *bmain,
struct ReportList *reports,
const char *filepath);
@@ -165,7 +166,7 @@ bool BKE_volume_save(struct Volume *volume,
openvdb::GridBase::ConstPtr BKE_volume_grid_openvdb_for_metadata(const struct VolumeGrid *grid);
openvdb::GridBase::ConstPtr BKE_volume_grid_openvdb_for_read(const struct Volume *volume,
- struct VolumeGrid *grid);
+ const struct VolumeGrid *grid);
openvdb::GridBase::Ptr BKE_volume_grid_openvdb_for_write(const struct Volume *volume,
struct VolumeGrid *grid,
const bool clear);
diff --git a/source/blender/blenkernel/BKE_volume_render.h b/source/blender/blenkernel/BKE_volume_render.h
index d7553ccb10b..907e3cf0880 100644
--- a/source/blender/blenkernel/BKE_volume_render.h
+++ b/source/blender/blenkernel/BKE_volume_render.h
@@ -43,7 +43,7 @@ typedef struct DenseFloatVolumeGrid {
} DenseFloatVolumeGrid;
bool BKE_volume_grid_dense_floats(const struct Volume *volume,
- struct VolumeGrid *volume_grid,
+ const struct VolumeGrid *volume_grid,
DenseFloatVolumeGrid *r_dense_grid);
void BKE_volume_dense_float_grid_clear(DenseFloatVolumeGrid *dense_grid);
@@ -53,7 +53,7 @@ typedef void (*BKE_volume_wireframe_cb)(
void *userdata, float (*verts)[3], int (*edges)[2], int totvert, int totedge);
void BKE_volume_grid_wireframe(const struct Volume *volume,
- struct VolumeGrid *volume_grid,
+ const struct VolumeGrid *volume_grid,
BKE_volume_wireframe_cb cb,
void *cb_userdata);
@@ -63,7 +63,7 @@ typedef void (*BKE_volume_selection_surface_cb)(
void *userdata, float (*verts)[3], int (*tris)[3], int totvert, int tottris);
void BKE_volume_grid_selection_surface(const struct Volume *volume,
- struct VolumeGrid *volume_grid,
+ const struct VolumeGrid *volume_grid,
BKE_volume_selection_surface_cb cb,
void *cb_userdata);
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;
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list