Geometry Nodes: initial Volume Grid socket support #115270
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Blocks
#116021 WIP: Volume Grid Nodes
blender/blender
Reference: blender/blender#115270
Loading…
Reference in New Issue
No description provided.
Delete Branch "LukasTonne/blender:volume-grid-sockets"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is the initial implementation for the volume grid socket that has been discussed during the November 2023 geometry nodes workshop.
It adds initial support for passing volume grids around in sockets. Furthermore, it adds two new nodes. Both are initially hidden under the "New Volume Nodes" experimental option until we have a few mode nodes.
SocketValueVariant
is extended to support grids besides single values and fields.Next steps:
7009f8cb21
tof34032ca03
WIP: Volume Grid Socketsto Volume Grid SocketsI didn't look at the node implementations yet, or compile and test this. But from what I read, the code looks great. I got the feeling the conversion code could be a bit simpler, but I'd probably have to spend more time looking at that too. For now I just left a few small comments.
@ -0,0 +59,4 @@
};
template<typename FieldValueType, typename GridValueType, typename LeafBufferValueType>
struct GridConverter_CopyConstruct {
This seems to be unused. Anyway, I'm a bit unsure about the goals here or if this is used in this PR. I'd hope this could be structured a bit more similarly to our
attribute_convert.hh
header, which is a relatively common pattern when converting to formats from other libraries.FieldValue
(assuming it's referring tofn::Field
) seems like a bit of a misnomer, there is no type system specific to fields.I've cleaned this up a bit now (these converters have gone through multiple iterations in different branches).
Converters are in
blender::bke::grids
namespace now to avoid possible conflicts with the existingAttributeConverter
. I have two separate converter classes for the Blender->OpenVDB (grids::AttributeConverter
) and the OpenVDB->Blender direction (grids::GridConverter
). I'm using "AttributeType" here to refer to Blender types (CPPType, customdata, sockets) even though grids aren't actually attributes. Have to call it something ...@ -0,0 +275,4 @@
namespace grid_utils {
template<typename T> bool get_background_value(const VolumeGridPtr<T> &grid, T &r_value)
std::optional<T>
?@ -0,0 +8,4 @@
* \ingroup bli
*/
enum VolumeGridType : int8_t {
Maybe
BKE_volume_enums.hh
makes more sense. "Types" seems a bit redundant withBKE_volume_grid.hh
@ -412,6 +415,7 @@ DefNode(GeometryNode, GEO_NODE_RESAMPLE_CURVE, 0, "RESAMPLE_CURVE", ResampleCurv
DefNode(GeometryNode, GEO_NODE_REVERSE_CURVE, 0, "REVERSE_CURVE", ReverseCurve, "Reverse Curve", "Change the direction of curves by swapping their start and end data")
DefNode(GeometryNode, GEO_NODE_ROTATE_INSTANCES, 0, "ROTATE_INSTANCES", RotateInstances, "Rotate Instances", "Rotate geometry instances in local or global space")
DefNode(GeometryNode, GEO_NODE_SAMPLE_CURVE, def_geo_curve_sample, "SAMPLE_CURVE", SampleCurve, "Sample Curve", "Retrieve data from a point on a curve at a certain distance from its start")
DefNode(GeometryNode, GEO_NODE_SAMPLE_GRID, 0, "SAMPLE_GRID", SampleGrid, "Sample Grid", "Sample a volume grid value")
Sample values from a volume grid at positions
is a bit more specific I supposeDilate Grid
node here results in a crash intry_dilate_grid
atBLI_assert(grid);
.@ -51,2 +70,4 @@
return fn::evaluate_constant_field(this->field);
}
if (this->grid) {
/* Returns the grid background value. */
I think we can just always return the default value (
0
) here when there is a grid. This function mainly exists because sometimes single values are stored as constant fields, but we shouldn't do that with grids.@ -73,2 +80,2 @@
VolumeGrid *BKE_volume_grid_get_for_write(Volume *volume, int grid_index);
const VolumeGrid *BKE_volume_grid_active_get_for_read(const Volume *volume);
GVolumeGridPtr BKE_volume_grid_get_for_read(const Volume *volume, int grid_index);
GVolumeGridPtr BKE_volume_grid_get_for_write(Volume *volume, int grid_index);
I think this will probably have to keep returning
VolumeGrid *
orVolumeGrid &
. ReturningGVolumeGridPtr
is problematic, because that increases the user count (unless I'm missing something), which would make the grid immutable.Note how e.g.
GeometrySet::get_component
returns a raw pointer instead of aGeometryComponentPtr
.API is back to returning
VolumeGrid *
.@ -0,0 +45,4 @@
* Procedurally generated grids are not.
* \{ */
struct GVolumeGridPtr : public VolumeGridPtrCommon {
I'm not entirely convinced by the need for
GVolumeGridPtr
andVolumeGridPtr
yet, instead of just usingImplicitSharingPtr<VolumeGrid>
.We could have a single (untemplated)
VolumeGridPtr
and then only access the internal OpenVDB grid as typed:get_grid() -> shared_ptr<openvdb::GridBase>
get_grid<T>() -> shared_ptr<VolumeGridType<T>>
However, having a typed wrapper is quite useful since we are storing it inside
ValueOrField<T>
with a known type. Thefield
is also a typed wrapper around a genericshared_ptr<FieldNode>
, with the genericGField
variant.I think the thin typed wrapper we have no is ok.
@ -0,0 +72,4 @@
}
}
params.set_default_remaining_outputs();
The behavior isn't quite right here. The output geometry should always be the same as the input geometry, except that it might have one grid less.
@ -0,0 +27,4 @@
b.add_input<decl::Geometry>("Volume");
b.add_input<decl::String>("Name");
grids::declare_grid_type_input(b, eCustomDataType(node->custom1), "Grid");
Use
hide_value
for the grid input. This input only works when you actually connect a grid.@ -0,0 +68,4 @@
if (volume) {
bke::GVolumeGridPtr grid = grids::extract_grid_input(params, "Grid", data_type);
if (!grid) {
return;
This forgets to set the output.
@ -0,0 +82,4 @@
return;
}
params.set_default_remaining_outputs();
Note that here we also want that the output is generally the same as the input, but potentially with one extra/replaced grid.
@ -0,0 +9,4 @@
*/
#include <mutex>
#include <unordered_set>
Seems unused.
@ -0,0 +39,4 @@
#ifdef WITH_OPENVDB
namespace grids {
template<typename T> struct AttributeConverter {
Maybe call this
TypeConverter
with two functions:to_blender
andto_openvdb
. That might make the purpose more clear.I was confused at first that there is
AttributeConverter
andGridConverter
. It wasn't obvious that those are just opposites of each other.Yeah that's what i had before
@ -0,0 +147,4 @@
VolumeGrid(const char *template_file_path,
const GridBasePtr &template_grid,
int simplify_level = 0);
VolumeGrid(const VolumeGrid &other);
This copy constructor seems confusing, since it's not actually making a copy of the grid.
Leftover from old code ...
VolumeGrid
is actually non-copyable now, so trying to use this constructor would cause an error anyway. Removed.@ -0,0 +150,4 @@
VolumeGrid(const VolumeGrid &other);
~VolumeGrid();
const char *name() const;
Seems ok for now, but eventually we shouldn't rely on this anymore because it hinders reusability of grids.
Old code, i suggest we clean this up when we know how to handle the cache and simplified grids etc.
Can you explain this some more? The code comment says it's just a wrapper around grid metadata to avoid a string copy. It's only used for logging and when loading a vdb file. That won't work if the grid name does not exist in the file it's associated with, so renaming a grid after loading can "break" in that sense. But i don't see how re-using the grid itself would be an issue.
IIRC we wanted to (eventually) store a name/pointer map in Volume and only use the grid name for debugging purposes.
That's exactly the point I wanted to make. No need to change anything here now.
@ -0,0 +152,4 @@
const char *name() const;
const char *error_message() const;
This could use some comment.
@ -0,0 +156,4 @@
bool grid_is_loaded() const;
void load(const char *volume_name, const char *filepath) const;
Feels a bit weird that this takes the filepath but is still
const
. Not sure how this is used currently.This is basically an internal function, it only gets called by other API functions
BKE_volume_grid_load
andBKE_volume_grid_openvdb_for_write
(basically a "make local" that loads the grid and then makes a deep copy induplicate_reference
). Thefilepath
argument is always thegrids.filepath
passed down from the container.The function can be const because it only affects the cache entry and the mutable
is_loaded_
flag. Should probably make this protected with a friend class or something, and/or maybe move the actual loading to the cache entry.Just mentioning that in a comment for now is fine.
@ -0,0 +161,4 @@
void set_simplify_level(int simplify_level);
void clear_reference(const char *volume_name);
This stuff shouldn't really be necessary with implicit-sharing anymore.
Agreed.
Currently:
VolumeGridVector
we use implicit sharing withVolumeGrid
. Eachopenvdb::GridBase::Ptr
(std::shared_ptr
) should always have exactly 1 user. Except ...VolumeGrid
also potentially has a cache entry, which uses the sameopenvdb::GridBase::Ptr
. So volumes in bmain can have multiple users of theshared_ptr
, while transient GN output grids never have a file cache and never more than 1 user.What could work:
VolumeFileCacheEntry
points toVolumeGrid
(implicit sharing data) instead of theopenvdb::GridBase::Ptr
.VolumeFileCacheEntry
only adds a weak user to theVolumeGrid
. This ensures that cache entries can be freed when nothing uses the grid any more.The explicit user counting in
VolumeFileCacheEntry
could then be removed, relying on the implicit sharing user count instead. However, the cache entry distinguishes between "metadata users" and "tree users". I'm not sure how important this distinction is and if it ever becomes relevant in practice.It looks like tree and metadata users are mutually exclusive, so this can probably be simplified (confirmed with Brecht)
VolumeGrid
constructors) starts out with 1 metadata user.VolumeGrid
removes a tree user if the grid is loaded and a metadata user otherwise.copy_user
is unused (cache entries are never actually copied).If a cached grid is fully unloaded (zero tree users) but still has un-loaded grids referencing it (1+ metadata users), the openvdb grid is replaced with an empty grid.
It gets a bit weird now because the entry is referenced by the
VolumeGrid
inVolume
but also references theVolumeGrid
in turn, rather than both sharing the lower-level openvdb grid pointer.I think the
VolumeGrid::entry_
pointer isn't really necessary, it's just set in the constructor and never changed. We could just do a lookup whenever the cache entry associated with aVolumeGrid
is required, without theVolumeGrid
itself having a dependency. The functions accessing the cache entry are only called infrequently.Currently, it feels like the global cache for meta data and actual voxel data could be separate. I'm not even sure if the meta data really needs a cache at all (I'd expect it to be very cheap to load compared to other stuff that is loaded).. Or it could be cached independently.
In theory, that's a nice idea. Unfortunately, weak users don't quite work the same way with implicit sharing as with
std::shared_ptr
as is mentioned in theImplicitSharingInfo::add_weak_user()
comment. A weak user can not be turned into a strong user and it also does not protect against changes to the underlying data.Currently, it feels like that additional logic would be necessary in
remove_user_and_delete_if_last
and the*_for_write
methods would be necessary to get the same behavior from before.To me personally, it still seems like it would be easier to design the caching mechanism from scratch given the new environment and to start out with a clean state.
Ok, to summarize the discussion:
VolumeGrid
is the shared data, previouslyVolumeFileCacheEntry
had this job.VolumeGridInstance
struct, but want to avoid this complication.VolumeFileCache
also storesVolumeGrid
but uses the implicit sharing refcount instead of thetree_users
andmetadata_users
.VolumeGrid
, but selecting the level happens on-the-fly using scene setting. This is a drawing/rendering feature so should not be relevant to generated grids.@ -0,0 +23,4 @@
/* -------------------------------------------------------------------- */
/** \name Common Grid Wrapper
*
* Base class for both generic and typed grid wrapper classes.
Base class for #GVolumeGridPtr and #VolumeGridPtr.
@ -0,0 +28,4 @@
struct VolumeGridPtrCommon : ImplicitSharingPtr<VolumeGrid> {
#ifdef WITH_OPENVDB
using GridPtr = std::shared_ptr<openvdb::GridBase>;
Both are unused I think.
@ -0,0 +37,4 @@
/* Enable implicit conversion from nullptr. */
VolumeGridPtrCommon(std::nullptr_t) : ImplicitSharingPtr<VolumeGrid>(nullptr) {}
using ImplicitSharingPtr<VolumeGrid>::ImplicitSharingPtr;
virtual ~VolumeGridPtrCommon();
This class doesn't need a vtable afaik.
@ -0,0 +45,4 @@
/* -------------------------------------------------------------------- */
/** \name Generic Volume Grid
*
* Wrapper around a generic OpenVDB grid.
A slightly better one-sentence-description might be:
Owning pointer to a #VolumeGrid.
@ -0,0 +46,4 @@
/** \name Generic Volume Grid
*
* Wrapper around a generic OpenVDB grid.
* Grids loaded from OpenVDB files are always stored in the global cache.
This information does not seem to belong here.
@ -0,0 +142,4 @@
};
/* Stub class for string attributes, not supported. */
template<> struct VolumeGridTraits<std::string> {
using TreeType = openvdb::MaskTree;
Should probably use
void
instead ofopenvdb::MaskTree
. Other code has to handle this case in some way anyway.@ -0,0 +159,4 @@
using GridPtr = std::shared_ptr<GridType>;
using GridConstPtr = std::shared_ptr<const GridType>;
VolumeGridPtr(const GVolumeGridPtr &other) : VolumeGridPtrCommon(other.get()) {}
Is this constructor necessary?
It also seems to have reference count issue: it creates a new owning pointer without increasing the user count.
This allows casting a type
VolumeGridPtr<T>
toGVolumeGridPtr
. VArray/GVArray works the same way, while Field/GField uses inheritance (Field is a GField). Probably both could work here, although i like to not use inheritance when the conversion is the only thing needed here. refcount already fixed.Ah ok, that's fine then. I still don't see how you fixed the refcount though.
I see, I think that's the wrong solution. This constructor should add the user instead, otherwise the automatic reference counting is kinda broken.
This should not result in a memory bug, because one is just dealing with managed pointers. Haven't tried, but right now this probably results in a use-after-free on destruction.
Not sure what you mean, the snippet above is in the constructor.
Not sure if I'm missing something. It looks like it is in the
typed()
method and not in the constructor, but I think it should be in the constructor. Maybe the constructor should also be private so that one is forced to calltyped()
?Oh sorry, i was getting confused by the cast direction. Moved this to the constructor.
Yes the constructor should be private, mostly because there is no good way to handle errors in case of type mismatch. The
typed<T>()
function can return null at least.@ -0,0 +170,4 @@
GridPtr grid_for_write() const
{
const VolumeGrid *data = this->get();
if (data->is_mutable()) {
Call
tag_ensured_mutable
. In theory that could be part ofis_mutable
, but it's not because I don't want it to be called by e.g. asserts that check mutability.@ -0,0 +173,4 @@
if (data->is_mutable()) {
return openvdb::GridBase::grid<GridType>(const_cast<VolumeGrid *>(data)->grid_for_write());
}
return openvdb::GridBase::grid<GridType>(data->grid()->deepCopyGrid());
I think it would be expected that this creates a new
VolumeGrid
and changesthis
to point to the new grid. This is how the*_for_write
methods work elsewhere.This also applies to the untyped version of this function.
That would be the case for getting a
VolumeGrid
fromVolume
, but not for accessing theopenvdb::GridBase
fromVolumeGrid
. I don't know of other cases of implicit sharing data changing their data in this way. This function would be comparable to e.g.CurveComponent::get_for_write
, where it asserts that the ImplicitSharingData itself is mutable and then returns a mutable pointer to the internals.56be404b0f/source/blender/blenkernel/intern/geometry_component_curves.cc (L84-L97)
VolumeGrid::grid_for_write()
is more likeCurves *CurveComponent::get_for_write()
:GridPtr VolumeGridPtr::grid_for_write() const
is more likeCurves *GeometrySet::get_curves_for_write()
orMesh::vert_positions_for_write()
:*_for_write
method on something that owns implicitly shared data might update the internal data pointer if a copy has to be made.Never really thought about this in this explicit way, but I hope this comparison helps.
@ -0,0 +203,4 @@
template<typename T> std::optional<T> get_background_value(const VolumeGridPtr<T> &grid)
{
#ifdef WITH_OPENVDB
if constexpr (std::is_same_v<T, std::string>) {
Would be nice if we could check for the string type at a higher level, so that we don't have to have special cases for it in volume handling.
Removed, it's not needed at this point.
This distinction becomes necessary when using a static dispatch "apply" function where the set of possible types includes strings. For now the set of supported attribute types includes only
float
andfloat3
. Handling all socket types and possibly type-lessMaskGrid
is a future problem.@ -1241,2 +860,3 @@
for (GVolumeGridPtr &grid : grids) {
if (grid_index-- == 0) {
return &grid;
return const_cast<VolumeGrid *>(grid.get());
This isn't actually ensuring that the grid is mutable. Should probably use
GVolumeGridPtr.grid_for_write()
.Should be fixed now. The
BKE_volume_grid_get_for_write
function makes sure the data is mutable and otherwise creates a copy. Accessing OpenVDB grids asserts that the data is mutable before returning the pointer, since we don't know if data mutable just from the type (depends on runtime user count).Will need a cleanup pass to ensure nodes and utility functions aren't making unnecessary copies now.
@ -133,2 +134,4 @@
};
template<typename T, typename... Args>
inline ImplicitSharingPtr<T> make_implicit_shared(Args &&...args)
Looks like this is currently unused.
@ -0,0 +69,4 @@
params.set_output("Volume", geometry_set);
if (remove_grid) {
BKE_volume_grid_remove(volume, grid);
Remove the grid before calling
set_output
.@ -0,0 +75,4 @@
}
}
grids::set_output_grid(params, "Grid", data_type, nullptr);
Currently crashes if the grid name is not found (e.g. when just inserting a new
Get Named Grid
node after aMesh to Volume
node).@ -158,6 +158,10 @@ static void transform_volume(GeoNodeExecParams ¶ms,
const int grids_num = BKE_volume_num_grids(&volume);
for (const int i : IndexRange(grids_num)) {
VolumeGrid *volume_grid = BKE_volume_grid_get_for_write(&volume, i);
if (!volume_grid->is_mutable()) {
A grid returned from
BKE_volume_grid_get_for_write
should always be mutable (or null).@blender-bot build
@ -0,0 +66,4 @@
}
#ifdef WITH_OPENVDB
GridConstPtr grid() const;
Note to self: remove these, they don't copy on write and are shortcuts to the same functions in VolumeGrid.
It's a bit confusing how there is a
BKE_volume_load
(loads metadata for all grids) and aBKE_volume_grid_load
(loads actual tree data) function. I'd like to make naming a bit more explicit:BKE_volume_load
->BKE_volume_ensure_loaded
This function is called in many places before accessing the volume, similar to how we use
ensure_xxx
functions for other lazy-loaded/generated data.The current implementation will only load the metadata. Either the name should indicate that (
BKE_volume_ensure_metadata_loaded
) or there should be an option to also load tree data (default off) so it's clear what this does.BKE_volume_grid_load
->BKE_volume_grid_ensure_tree_loaded
There may be nicer options for naming ...
It feels like we don't really need
BKE_volume_grid_ensure_tree_loaded
if we do the load lazily when accessingVolumeGrid::grid()
.Technically, I'd expect
BKE_volume_load
to automatically run for all volume data blocks on file load. Maybe we can just hide this loading behind an access API as well? So when some code first tries to access the grid meta data, it's loaded.That could work, it's what the C api does now too:
089383a53a/source/blender/blenkernel/intern/volume.cc (L1632)
I'm a bit surprised this doesn't seem to happen currently (have to confirm on main). But i guess if any code accessing grid info calls
BKE_volume_load
anyway before using the API there isn't any need to load metadata in advance.Possibly, but i think it's still good to have an explicit function to load file metadata upfront, rather than deferring file I/O to some unknown point inside another function. This would apply to any function accessing grid info:
It mainly feels weird to always have to call
BKE_volume_load
before doing anything with a volume. It may be more obvious now, because we don't support storing grids directly in .blend files yet. It feels like it will become a common issue to forget callingBKE_volume_load
. If you want to ensure some data is loaded at a specific point in time, then you can also just "touch" the data (e.g. callingBKE_volume_num_grids
without actually needing it right now). That feels like the more scalable approach to me.I'm fine with some more internal
BKE_volume_ensure_loaded
function.@ -0,0 +142,4 @@
/* Source of the tree data stored in the grid. */
VolumeTreeSource tree_source() const;
/* Make sure the tree data has been loaded if the grid has a file source. */
bool ensure_tree_loaded(
It still seems somewhat weird that the filepath is passed into a
const
method. I'd expect all the data required for lazy-loading the grid to be passed into the constructor ofVolumeGrid
.@ -0,0 +145,4 @@
bool ensure_tree_loaded(
StringRef filepath, FunctionRef<void(StringRef)> error_fn = [](StringRef) {}) const;
/* Unload the tree data but keep metadata. */
void unload_tree() const;
I don't think it's possible for this to be
const
. We can't unload the tree when some other user ofVolumeGrid
is using the underlying grid currently.@ -0,0 +148,4 @@
void unload_tree() const;
protected:
GridBasePtr main_grid() const;
Can be removed now.
@ -0,0 +20,4 @@
bool VolumeFileCache::has_grid(const VolumeFileCacheKey &key) const
{
return file_grids_.contains(key);
Every access to
file_grids_
needs to be protected by the mutex.@ -0,0 +48,4 @@
}
/* Returns the original grid or a simplified version depending on the given #simplify_level. */
GVolumeGridPtr VolumeFileCache::get_simplified_grid(const VolumeFileCacheKey &key,
I find this signature misleading because
VolumeFileCacheKey
already contains thesimplify_level
. Based on the signature I'd expect that the simplify level is applied on top of the simplify level stored in thekey
.I think the lookup of simplified grids could just use the normal
lookup
function as well.volume_update_simplify_level
can set thesimplify_level
in the key.Regarding the
GVolumeGridPtr
/VolumeGridPtr<T>
:IIRC we briefly touched on removing the typed pointer variant and just use a generic
ImplicitSharingPtr
. Such pointers will always return a un-typedconst VolumeGrid *
. The caller then has to:const_cast
orgrid_ptr->copy();
to get a mutableVolumeGrid *
VolumeGrid::get
orVolumeGrid::get_for_write
functions to get aopenvdb::GridBase::Ptr
OR use type-casting functions
VolumeGrid::get<T>
orVolumeGrid::get_for_write<T>
to get aVolumeGridType<T>
(anopenvdb::Grid
type based on the Blender typeT
)I think this would be more in line with the intended use of
ImplicitSharingData
. I don't expect use cases to get more complex by this change. TheBKE_volume_grid_ptr.hh
header would become unnecessary.@blender-bot build
Volume Grid Socketsto Geometry Nodes: initial Volume Grid socket support@blender-bot build
@ -332,6 +332,7 @@ DefNode(GeometryNode, GEO_NODE_FILL_CURVE, 0, "FILL_CURVE", FillCurve, "Fill Cur
DefNode(GeometryNode, GEO_NODE_FILLET_CURVE, 0, "FILLET_CURVE", FilletCurve, "Fillet Curve", "Round corners by generating circular arcs on each control point")
DefNode(GeometryNode, GEO_NODE_FLIP_FACES, 0, "FLIP_FACES", FlipFaces, "Flip Faces", "Reverse the order of the vertices and edges of selected faces, flipping their normal direction")
DefNode(GeometryNode, GEO_NODE_GEOMETRY_TO_INSTANCE, 0, "GEOMETRY_TO_INSTANCE", GeometryToInstance, "Geometry to Instance", "Convert each input geometry into an instance, which can be much faster than the Join Geometry node when the inputs are large")
DefNode(GeometryNode, GEO_NODE_GET_NAMED_GRID, 0, "GET_NAMED_GRID", GetNamedGrid, "Get Named Grid", "Get grid data from a volume geometry with the specified name")
Get grid data
->Get volume grid data
Same below
@ -0,0 +55,4 @@
static void try_store_grid(GeoNodeExecParams params, Volume *volume)
{
BLI_assert(volume);
Pass a reference instead of asserting the pointer is non-null