Simulation Nodes: bake simulation states to disk #106937

Merged
Jacques Lucke merged 116 commits from JacquesLucke/blender:sim-bake into geometry-nodes-simulation 2023-04-22 14:48:56 +02:00
Member

This adds baking support to simulation nodes. While there are still a couple things to do, I think this is in a state where it can be merged into geometry-nodes-simulation so that we can easier work on it together.

The following features are supported:

  • Bake simulation nodes of selected objects from the new "Baking" panel in the object properties.
  • Free baked/cached simulation data.
  • The bake is stored on disk in a folder next to the .blend file (so it's necessary to save before baking works).
  • Baked data is detected automatically when reloading the file.
  • The data stored on disk is partially deduplicated. Only duplicates that can be detected using implicit-sharing are taken into account.
  • The baked data can contain meshes, curves, pointclouds and instances.
  • The simulation state is written using a combination raw binary files for the data arrays and .json for meta data. Other formats besides .json could be used (most code is agnostic to that), but json is the easiest to use right now and seems to be good enough the common use cases (note that the size of the .json files do not depend on how large e.g. the baked mesh is).
  • During baking, there is a progress bar and it can be interrupted using escape.

Limitations:

  • Volumes are not written to disk yet.
  • Currently it always bakes the entire scene frame range.
  • Baking subframes is supported internally, but is not exposed in the UI.
  • Currently, all attributes are written, but that is likely not necessary in most cases (e.g. selection attributes are written as well).

The image below shows the file structure of the baked data. The file names are the (sub)frames. .bdata is an arbitrary name, we might want to use something else (currently it can stand for blender-data or binary-data). Also note now the first frame's file is larger. That's because the topology has only been stored in the first frame and was not modified afterwards.
image
The content of the .json files looks like so (without the indentation): https://hastebin.com/share/emaluparoh.json

image

This adds baking support to simulation nodes. While there are still a couple things to do, I think this is in a state where it can be merged into `geometry-nodes-simulation` so that we can easier work on it together. The following features are supported: * Bake simulation nodes of selected objects from the new "Baking" panel in the object properties. * Free baked/cached simulation data. * The bake is stored on disk in a folder next to the .blend file (so it's necessary to save before baking works). * Baked data is detected automatically when reloading the file. * The data stored on disk is partially deduplicated. Only duplicates that can be detected using implicit-sharing are taken into account. * The baked data can contain meshes, curves, pointclouds and instances. * The simulation state is written using a combination raw binary files for the data arrays and `.json` for meta data. Other formats besides `.json` could be used (most code is agnostic to that), but json is the easiest to use right now and seems to be good enough the common use cases (note that the size of the `.json` files do not depend on how large e.g. the baked mesh is). * During baking, there is a progress bar and it can be interrupted using escape. Limitations: * Volumes are not written to disk yet. * Currently it always bakes the entire scene frame range. * Baking subframes is supported internally, but is not exposed in the UI. * Currently, all attributes are written, but that is likely not necessary in most cases (e.g. selection attributes are written as well). The image below shows the file structure of the baked data. The file names are the (sub)frames. `.bdata` is an arbitrary name, we might want to use something else (currently it can stand for blender-data or binary-data). Also note now the first frame's file is larger. That's because the topology has only been stored in the first frame and was not modified afterwards. ![image](/attachments/9f1c7ad3-5214-4d84-93f1-d50e28c71893) The content of the `.json` files looks like so (without the indentation): https://hastebin.com/share/emaluparoh.json ![image](/attachments/b61963e9-7e26-4ee8-ba6c-bcee4e5fd81c)
Jacques Lucke added 35 commits 2023-04-14 10:55:53 +02:00
Jacques Lucke added 9 commits 2023-04-17 09:47:22 +02:00
Jacques Lucke added 3 commits 2023-04-17 10:15:42 +02:00
Jacques Lucke added 1 commit 2023-04-17 12:17:22 +02:00
Jacques Lucke added 2 commits 2023-04-17 13:11:28 +02:00
Jacques Lucke added 1 commit 2023-04-18 11:26:13 +02:00
Jacques Lucke added 10 commits 2023-04-18 14:03:18 +02:00
Jacques Lucke added 1 commit 2023-04-18 14:15:47 +02:00
Jacques Lucke added 4 commits 2023-04-18 14:41:03 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR106937) when ready.
Jacques Lucke added 6 commits 2023-04-19 10:54:19 +02:00
Jacques Lucke added 13 commits 2023-04-19 13:04:44 +02:00
Jacques Lucke added 5 commits 2023-04-19 14:41:00 +02:00
Jacques Lucke added 4 commits 2023-04-19 15:59:11 +02:00
Jacques Lucke changed title from WIP: Simulation Nodes: bake simulation state to disk to Simulation Nodes: bake simulation states to disk 2023-04-19 16:22:29 +02:00
Jacques Lucke requested review from Hans Goudey 2023-04-19 16:22:44 +02:00
Hans Goudey requested changes 2023-04-19 17:47:18 +02:00
Hans Goudey left a comment
Member

I only looked at the first half of the changed files so far. I still need to look at MOD_nodes.cc and the baking operator.

My only high-level comment so far is that it might be nice to separate the geometry set serialization from the idea of the simulation state a bit, because they're conceptually separate (right?) and we'll probably want to use it for caching.

UI-wise, I'd like to avoid the need for the "Simulation Nodes Cache:" label button, and the mixing of the "cache" and "bake" terms. It would be better to separate those more completely I think, and save "bake" for the file-writing and "cache" for the in-memory/automatic version. They can still both be in the "baking" panel, but separating the UI for baking and caching a bit more would be helpful I think (and also relate to the option for turning off the cache). Maybe separate subpanels?

The code that writes attributes is very nice, it's really satisfying to see our work on the attribute API and making that area more generic pay off here.

I only looked at the first half of the changed files so far. I still need to look at `MOD_nodes.cc` and the baking operator. My only high-level comment so far is that it might be nice to separate the geometry set serialization from the idea of the simulation state a bit, because they're conceptually separate (right?) and we'll probably want to use it for caching. UI-wise, I'd like to avoid the need for the "Simulation Nodes Cache:" label button, and the mixing of the "cache" and "bake" terms. It would be better to separate those more completely I think, and save "bake" for the file-writing and "cache" for the in-memory/automatic version. They can still both be in the "baking" panel, but separating the UI for baking and caching a bit more would be helpful I think (and also relate to the option for turning off the cache). Maybe separate subpanels? The code that writes attributes is very nice, it's really satisfying to see our work on the attribute API and making that area more generic pay off here.
@ -0,0 +22,4 @@
* Reference to a slice of memory typically stored on disk.
*/
struct BDataSlice {
std::string name;
Member

name -> file_name maybe? It could be anything right now

`name` -> `file_name` maybe? It could be anything right now
Author
Member

Could be file_name, but it's somewhat intentionally not, because it's not strictly necessary that this refers to a file (even though currently it always does).

Could be `file_name`, but it's somewhat intentionally not, because it's not strictly necessary that this refers to a file (even though currently it always does).
@ -0,0 +66,4 @@
*/
int64_t sharing_info_version;
/**
* Identifier of the stored data.
Member

A couple more words after "Identifier" could help the reader figure out the type of this value, something like "Identifier file name"

A couple more words after "Identifier" could help the reader figure out the type of this value, something like "Identifier file name"
@ -0,0 +72,4 @@
};
/**
* Map that is used to detect when a some data has been written previously. For that it keeps a
Member

Simpler wording suggestion:

Map used to detect when some data has already been written. It keeps a weak reference to #ImplicitSharingInfo, allowing it to check for equality of two arrays just by comparing the sharing info's pointer and version.

Simpler wording suggestion: >Map used to detect when some data has already been written. It keeps a weak reference to #ImplicitSharingInfo, allowing it to check for equality of two arrays just by comparing the sharing info's pointer and version.
HooglyBoogly marked this conversation as resolved
@ -0,0 +78,4 @@
*/
Map<const ImplicitSharingInfo *, StoredByRuntimeValue> stored_by_runtime_;
mutable std::mutex mutex_;
Member

A comment mentioning the need for a mutex might be nice here. Also fine to skip if you think it's obvious or not necessary for the user of the API

A comment mentioning the need for a mutex might be nice here. Also fine to skip if you think it's obvious or not necessary for the user of the API
@ -0,0 +80,4 @@
mutable std::mutex mutex_;
/**
* Map that is used to detect when some data has been previously loaded. This keeps strong
Member

Map that is used -> Map used

`Map that is used` -> `Map used`
HooglyBoogly marked this conversation as resolved
@ -0,0 +238,4 @@
{
const CPPType &type = data.type();
BLI_assert(type.is_trivial());
if (type.size() == 1 || type.is<ColorGeometry4b>()) {
Member

No need to do this now necessarily, but I wonder if a CPPType should know if its values need endian switches

No need to do this now necessarily, but I wonder if a `CPPType` should know if its values need endian switches
Author
Member

Hm it could. Only makes sense for trivial types of course.

Hm it could. Only makes sense for trivial types of course.
@ -0,0 +259,4 @@
}
if (type.is_any<float2, int2>()) {
return read_bdata_raw_data_with_endian(
bdata_reader, io_data, sizeof(float), r_data.size() * 2, r_data.data());
Member

Maybe replace float here with int32_t or 4?

Maybe replace `float` here with `int32_t` or `4`?

Voice against the magic four.

Voice against the magic four.
@ -0,0 +263,4 @@
}
if (type.is<float3>()) {
return read_bdata_raw_data_with_endian(
bdata_reader, io_data, sizeof(float), r_data.size() * 3, r_data.data());
Member

Better to add a size_in_bytes() method to GMutableSpan or GSpan IMO

Better to add a `size_in_bytes()` method to `GMutableSpan` or `GSpan` IMO
Member

Oops, never mind I misunderstood this.

Oops, never mind I misunderstood this.
HooglyBoogly marked this conversation as resolved
@ -0,0 +286,4 @@
sharing_info, [&]() { return write_bdata_simple_gspan(bdata_writer, data); });
}
[[nodiscard]] static bool read_bdata_shared_simple_gspan(
Member

When I was working on the stuff in the implicit_sharing namespace, I found void ** to be quite annoying, it's also not portable technically. Something like this avoids that issue, though you might not like it better, not sure:

[[nodiscard]] static const void *read_bdata_shared_simple_gspan(
    const DictionaryValue &io_data,
    const BDataReader &bdata_reader,
    const BDataSharing &bdata_sharing,
    const CPPType &cpp_type,
    const int size,
    const ImplicitSharingInfo **r_sharing_info)
{
  const std::optional<ImplicitSharingInfoAndData> sharing_info_and_data =
      bdata_sharing.read_shared(io_data, [&]() -> std::optional<ImplicitSharingInfoAndData> {
        void *data_mem = MEM_mallocN_aligned(
            size * cpp_type.size(), cpp_type.alignment(), __func__);
        if (!read_bdata_simple_gspan(bdata_reader, io_data, {cpp_type, data_mem, size})) {
          MEM_freeN(data_mem);
          return std::nullopt;
        }
        return ImplicitSharingInfoAndData{implicit_sharing::info_for_mem_free(data_mem), data_mem};
      });
  if (!sharing_info_and_data) {
    *r_sharing_info = nullptr;
    return nullptr;
  }
  *r_sharing_info = sharing_info_and_data->sharing_info;
  return sharing_info_and_data->data;
}

template<typename T>
[[nodiscard]] static bool read_bdata_shared_simple_span(const DictionaryValue &io_data,
                                                        const BDataReader &bdata_reader,
                                                        const BDataSharing &bdata_sharing,
                                                        const int size,
                                                        T **r_data,
                                                        ImplicitSharingInfo **r_sharing_info)
{
  *r_data = read_bdata_shared_simple_gspan(io_data,
                                           bdata_reader,
                                           bdata_sharing,
                                           CPPType::get<T>(),
                                           size,
                                           (const ImplicitSharingInfo **)r_sharing_info);
  return *r_data != nullptr;
}
When I was working on the stuff in the `implicit_sharing` namespace, I found `void **` to be quite annoying, it's also not portable technically. Something like this avoids that issue, though you might not like it better, not sure: ```cpp [[nodiscard]] static const void *read_bdata_shared_simple_gspan( const DictionaryValue &io_data, const BDataReader &bdata_reader, const BDataSharing &bdata_sharing, const CPPType &cpp_type, const int size, const ImplicitSharingInfo **r_sharing_info) { const std::optional<ImplicitSharingInfoAndData> sharing_info_and_data = bdata_sharing.read_shared(io_data, [&]() -> std::optional<ImplicitSharingInfoAndData> { void *data_mem = MEM_mallocN_aligned( size * cpp_type.size(), cpp_type.alignment(), __func__); if (!read_bdata_simple_gspan(bdata_reader, io_data, {cpp_type, data_mem, size})) { MEM_freeN(data_mem); return std::nullopt; } return ImplicitSharingInfoAndData{implicit_sharing::info_for_mem_free(data_mem), data_mem}; }); if (!sharing_info_and_data) { *r_sharing_info = nullptr; return nullptr; } *r_sharing_info = sharing_info_and_data->sharing_info; return sharing_info_and_data->data; } template<typename T> [[nodiscard]] static bool read_bdata_shared_simple_span(const DictionaryValue &io_data, const BDataReader &bdata_reader, const BDataSharing &bdata_sharing, const int size, T **r_data, ImplicitSharingInfo **r_sharing_info) { *r_data = read_bdata_shared_simple_gspan(io_data, bdata_reader, bdata_sharing, CPPType::get<T>(), size, (const ImplicitSharingInfo **)r_sharing_info); return *r_data != nullptr; } ```
@ -0,0 +428,4 @@
return nullptr;
}
Curves *curves_id = bke::curves_new_nomain(num_points, num_curves);
Member

It's probably worth creating empty curves first, then assigning the offsets, to avoid allocating them and then freeing them.

This could be a TODO comment too I guess

It's probably worth creating empty curves first, then assigning the offsets, to avoid allocating them and then freeing them. This could be a TODO comment too I guess
HooglyBoogly marked this conversation as resolved
@ -0,0 +536,4 @@
instances->resize(num_instances);
const auto cancel = [&]() {
delete instances;
Member

Using unique_ptr<bke::Instances> at least withing this function should remove the need for this

Using `unique_ptr<bke::Instances>` at least withing this function should remove the need for this
HooglyBoogly marked this conversation as resolved
@ -0,0 +576,4 @@
const BDataSharing &bdata_sharing)
{
GeometrySet geometry;
if (Mesh *mesh = try_load_mesh(io_geometry, bdata_reader, bdata_sharing)) {
Member

These functions handle null inputs already, this is a bit prettier :)

  GeometrySet geometry;
  geometry.replace_mesh(try_load_mesh(io_geometry, bdata_reader, bdata_sharing));
  geometry.replace_pointcloud(try_load_pointcloud(io_geometry, bdata_reader, bdata_sharing));
  geometry.replace_curves(try_load_curves(io_geometry, bdata_reader, bdata_sharing));
  geometry.replace_instances(try_load_instances(io_geometry, bdata_reader, bdata_sharing));
  return geometry;
These functions handle null inputs already, this is a bit prettier :) ```cpp GeometrySet geometry; geometry.replace_mesh(try_load_mesh(io_geometry, bdata_reader, bdata_sharing)); geometry.replace_pointcloud(try_load_pointcloud(io_geometry, bdata_reader, bdata_sharing)); geometry.replace_curves(try_load_curves(io_geometry, bdata_reader, bdata_sharing)); geometry.replace_instances(try_load_instances(io_geometry, bdata_reader, bdata_sharing)); return geometry; ```
HooglyBoogly marked this conversation as resolved
Hans Goudey added this to the Nodes & Physics project 2023-04-19 17:55:23 +02:00
Hans Goudey requested changes 2023-04-19 18:22:01 +02:00
@ -0,0 +280,4 @@
objects.append(object);
}
}
Member
  if (objects.is_empty()) {
    return OPERATOR_CANCELLED;
  }
```cpp if (objects.is_empty()) { return OPERATOR_CANCELLED; } ```
HooglyBoogly marked this conversation as resolved
@ -0,0 +286,4 @@
if (md->type == eModifierType_Nodes) {
NodesModifierData *nmd = reinterpret_cast<NodesModifierData *>(md);
const std::string bake_directory = bke::sim::get_bake_directory(*bmain, *object, *md);
BLI_delete(bake_directory.c_str(), true, true);
Member

An error message when this fails maybe?

        if (BLI_exists(bake_directory.c_str())) {
          if (!BLI_delete(bake_directory.c_str(), true, true)) {
            BKE_reportf(op->reports,
                        RPT_ERROR,
                        "Failed to remove bake directory %s",
                        bake_directory.c_str());
          }
        }
An error message when this fails maybe? ```cpp if (BLI_exists(bake_directory.c_str())) { if (!BLI_delete(bake_directory.c_str(), true, true)) { BKE_reportf(op->reports, RPT_ERROR, "Failed to remove bake directory %s", bake_directory.c_str()); } } ```
@ -1202,3 +1202,1 @@
if (DEG_is_active(ctx->depsgraph)) {
const Scene *scene = DEG_get_input_scene(ctx->depsgraph);
const int start_frame = scene->r.sfra;
const Main *bmain = DEG_get_bmain(ctx->depsgraph);
Member

Do you think you'd be able to split any of this new code in compute_geometry to a separate function(s)? I think that would go a long way toward making it more readable and less overwhelming

Do you think you'd be able to split any of this new code in `compute_geometry` to a separate function(s)? I think that would go a long way toward making it more readable and less overwhelming
@ -1219,0 +1232,4 @@
nmd_orig->simulation_cache->reset();
}
}
/* Decide of a new simulation state should be created in this evaluation. */
Member

Decide of -> Decide if

`Decide of` -> `Decide if`
HooglyBoogly marked this conversation as resolved
@ -208,0 +225,4 @@
const bke::sim::SimulationZoneState &next_state,
const float mix_factor) const
{
this->output_cached_state(params, prev_state);
Member

TODO comment

TODO comment
HooglyBoogly marked this conversation as resolved
Hans Goudey added 10 commits 2023-04-20 06:06:23 +02:00
Jacques Lucke added 1 commit 2023-04-20 10:17:17 +02:00
Author
Member

They can still both be in the "baking" panel, but separating the UI for baking and caching a bit more would be helpful I think (and also relate to the option for turning off the cache). Maybe separate subpanels?

That can work. I don't really have a good proposal for this part of the UI yet.

> They can still both be in the "baking" panel, but separating the UI for baking and caching a bit more would be helpful I think (and also relate to the option for turning off the cache). Maybe separate subpanels? That can work. I don't really have a good proposal for this part of the UI yet.
Jacques Lucke added 2 commits 2023-04-20 11:54:19 +02:00
de0d91030d skip drawing node editor while baking/rendering
This lead to a use-after-free bug when baking happens on
another thread at the same time.
Jacques Lucke requested review from Hans Goudey 2023-04-20 12:16:18 +02:00
Jacques Lucke added 4 commits 2023-04-20 12:16:24 +02:00
Jacques Lucke added 3 commits 2023-04-22 13:16:09 +02:00
Hans Goudey approved these changes 2023-04-22 13:31:11 +02:00
Hans Goudey left a comment
Member

There's definitely things that should go into main separately here, but this seems ready for the simulation branch.

There's definitely things that should go into main separately here, but this seems ready for the simulation branch.
@ -66,0 +92,4 @@
* Adding a weak owner prevents the #ImplicitSharingInfo from being freed but not the referenced
* data.
*
* \note Other than with std::shared_ptr a weak user cannot be turned into a strong user. This is
Member

Maybe "Unlike std::shared_ptr" instead of "Other than with std::shared_ptr"

Maybe "Unlike std::shared_ptr" instead of "Other than with std::shared_ptr"
@ -66,0 +109,4 @@
void tag_ensured_mutable() const
{
BLI_assert(this->is_mutable());
/* Does not need an atomic increment, because if the data is mutable, there is only a single
Member

"Doesn't need an atomic increment" but it seems to do one anyway, am I missing something?

"Doesn't need an atomic increment" but it seems to do one anyway, am I missing something?
Author
Member

Oops, yeah. I thought this wouldn't need one but then wasn't so sure anymore.

Oops, yeah. I thought this wouldn't need one but then wasn't so sure anymore.
@ -1162,1 +1163,4 @@
static void prepare_simulation_states_for_evaluation(
NodesModifierData *nmd,
NodesModifierData *nmd_orig,
Member

Pass by reference and maybe const reference?

Pass by reference and maybe const reference?
Jacques Lucke added 2 commits 2023-04-22 13:49:33 +02:00
Jacques Lucke merged commit 05ddbc20b8 into geometry-nodes-simulation 2023-04-22 14:48:56 +02:00
Jacques Lucke deleted branch sim-bake 2023-04-22 14:48:57 +02:00
Sign in to join this conversation.
No reviewers
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 Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#106937
No description provided.