Geometry Nodes: refactor simulation storage and how simulation nodes access it #111623

Jacques Lucke · 2023-08-28T17:22:09+02:00

Jacques Lucke commented

2023-08-28 17:22:09 +02:00

Goals of the refactor:

Internal support for baking individual simulation zones (not exposed in the UI yet).
More well-defined access to simulation data in geometry nodes. Especially, it should be more obvious where data is modified. A similar approach should also work for the Bake node.

Previously, there were a bunch of simulation specific properties in GeoNodesModifierData and then the simulation input and output nodes would have to figure out what to do with that data. Now, there is a new GeoNodesSimulationParams which controls the behavior of simulation zones. Contrary to before, different simulation zones can now be handled independently, even if that is not really used yet. GeoNodesSimulationParams has to be subclassed by a user of the geometry nodes API. The subclass controls what each simulation input and output node does. This some of the logic that was part of the node before, into the modifier.

The way we store simulation data is "transposed". Previously, we stored zone data per frame, but now we store frame data per zone. This allows different zones to be more independent. Consequently, the way the simulation cache is accessed changed. I kept things simpler for now, avoiding many of the methods we had before, and directly accessing the data more often which is often simple enough. This change also makes it theoretically possible to store baked data for separate zones independently. A downside of this is, that existing baked data can't be read anymore. We don't really have compatibility guarantees for this format yet, so it's ok. Users will have to bake again. The bake folder for the modifier now contains an extra subfolder for every zone.

Drawing the cached/baked frames in the timeline is less straight forward now. Currently, it just draws the state of one of the zones, which usually is identical to that of all other zones. This will change in the future though, and then the timeline drawing also needs some new UI work.

There are a few things that could be worked on after this patch:

Move the writing and reading of baked storage on/from disk to a more centralized position.
Improve the namespaces for bake/simulation stuff in blenkernel.
Investigate the use of implicit sharing to better decouple different depsgraph (instead of the std::shared_ptr that is used currently).

Goals of the refactor: * Internal support for baking individual simulation zones (not exposed in the UI yet). * More well-defined access to simulation data in geometry nodes. Especially, it should be more obvious where data is modified. A similar approach should also work for the Bake node. Previously, there were a bunch of simulation specific properties in `GeoNodesModifierData` and then the simulation input and output nodes would have to figure out what to do with that data. Now, there is a new `GeoNodesSimulationParams` which controls the behavior of simulation zones. Contrary to before, different simulation zones can now be handled independently, even if that is not really used yet. `GeoNodesSimulationParams` has to be subclassed by a user of the geometry nodes API. The subclass controls what each simulation input and output node does. This some of the logic that was part of the node before, into the modifier. The way we store simulation data is "transposed". Previously, we stored zone data per frame, but now we store frame data per zone. This allows different zones to be more independent. Consequently, the way the simulation cache is accessed changed. I kept things simpler for now, avoiding many of the methods we had before, and directly accessing the data more often which is often simple enough. This change also makes it theoretically possible to store baked data for separate zones independently. A downside of this is, that existing baked data can't be read anymore. We don't really have compatibility guarantees for this format yet, so it's ok. Users will have to bake again. The bake folder for the modifier now contains an extra subfolder for every zone. Drawing the cached/baked frames in the timeline is less straight forward now. Currently, it just draws the state of one of the zones, which usually is identical to that of all other zones. This will change in the future though, and then the timeline drawing also needs some new UI work. There are a few things that could be worked on after this patch: * Move the writing and reading of baked storage on/from disk to a more centralized position. * Improve the namespaces for bake/simulation stuff in blenkernel. * Investigate the use of implicit sharing to better decouple different depsgraph (instead of the `std::shared_ptr` that is used currently).

Jacques Lucke added 16 commits 2023-08-28 17:22:17 +02:00

8318f4c444 initial API

ef959bc53b try same API design for baking

51a2e83102 progress

10d098617c Merge branch 'main' into better-simulation-state-api

97afce818c use new api in geometry nodes

7fbf9d51d6 progress

33d6b456e9 progress

8877fc20ad progress

6c2577cea8 progress

4c6efb8f63 simplify

a50163e7a3 progress

6083551e84 progress

46fe9430a0 Merge branch 'main' into better-simulation-state-api

540af068ae fixes

d38f227d2c Merge branch 'main' into better-simulation-state-api

f03ccfc4a4 refactor simulation storage to store data per zone

Jacques Lucke added 1 commit 2023-08-28 17:31:36 +02:00

61cfb8d453 fix cache invalidation

Jacques Lucke added 5 commits 2023-08-29 14:26:45 +02:00

69c4019361 Merge branch 'main' into better-simulation-state-api

53b2c1c76f Merge branch 'main' into better-simulation-state-api

3e49847053 bring back baking to disk

4e9abe51e5 fix loading baked data

5896802499 cleanup

Jacques Lucke added 3 commits 2023-08-29 14:56:47 +02:00

bad0320242 fix

0f9fa28e3a fix no-cache option

48db5801d0 add reset method

Jacques Lucke added 1 commit 2023-08-30 10:39:11 +02:00

39c7bf8a91 Merge branch 'main' into better-simulation-state-api

Jacques Lucke added 8 commits 2023-08-30 14:49:47 +02:00

Jacques Lucke added 1 commit 2023-08-30 14:59:22 +02:00

75006ff164 Merge branch 'main' into better-simulation-state-api

Jacques Lucke added 1 commit 2023-08-30 15:45:38 +02:00

buildbot/vexp-code-patch-coordinator Build done. Details

e9509eb760 cleanup magic number

Jacques Lucke commented

2023-08-30 15:53:31 +02:00

@blender-bot build

Jacques Lucke changed title from ~~WIP: Geometry Nodes: refactor simulation storage and how simulation nodes access it~~ to Geometry Nodes: refactor simulation storage and how simulation nodes access it

2023-08-30 15:53:55 +02:00

Jacques Lucke requested review from Hans Goudey 2023-08-30 15:54:04 +02:00

Hans Goudey requested changes 2023-08-30 17:22:31 +02:00

Hans Goudey left a comment

I guess mainly I'd like to see a few more comments. Even if they feel obvious, there's a lot going on here, and the functionality is spread out much more than usual. The only places that's a blocking comment for me I've noted in comments though.

What's the reasoning for splitting NodesModifierSimulationParams and GeoNodesSimulationParams? I guess you're thinking we might use simulation without the modifier in the future? That makes sense to me, just want to be sure.

I guess mainly I'd like to see a few more comments. Even if they feel obvious, there's a lot going on here, and the functionality is spread out much more than usual. The only places that's a blocking comment for me I've noted in comments though. What's the reasoning for splitting `NodesModifierSimulationParams` and `GeoNodesSimulationParams`? I guess you're thinking we might use simulation without the modifier in the future? That makes sense to me, just want to be sure.

source/blender/blenkernel/BKE_simulation_state.hh

						
				@ -88,1 +32,3 @@

				  const ModifierSimulationStateAtFrame *next = nullptr;

				struct SimulationZoneFrameCache {

				  SubFrame frame;

				  Map<int, std::unique_ptr<BakeItem>> items;

Hans Goudey commented

2023-08-30 16:38:22 +02:00

Here and below, add a comment like "map from X to Y". That's helpful because the int here isn't so helpful to the reader.

Hans Goudey commented

2023-08-31 14:53:33 +02:00

Hmm, I don't see a comment here :/ sorry if I'm missing something.

source/blender/editors/space_action/action_draw.cc Outdated

						
				@ -13,6 +13,7 @@

				#include <cstdlib>

				#include <cstring>

				#include "BLI_binary_search.hh"

Hans Goudey commented

2023-08-30 17:06:35 +02:00

Unused include?

JacquesLucke marked this conversation as resolved

source/blender/nodes/NOD_geometry_nodes_lazy_function.hh Outdated

						
				@ -43,0 +57,4 @@

				  Map<int, std::unique_ptr<bke::BakeItem>> prev_items;

				};

				using SimInputBehavior = std::variant<PassThrough, OutputCopy, OutputMove>;

Hans Goudey commented

2023-08-30 16:54:15 +02:00

I find these type names and namespaces redundant. What about:

namespace sim_output {
struct PassThrough {
};
...
using Behavior = std::variant<PassThrough, OutputCopy, OutputMove>; 
}

I think that gets the same thing across without the redundancy

I find these type names and namespaces redundant. What about: ``` namespace sim_output { struct PassThrough { }; ... using Behavior = std::variant<PassThrough, OutputCopy, OutputMove>; } ``` I think that gets the same thing across without the redundancy

JacquesLucke marked this conversation as resolved

source/blender/nodes/NOD_geometry_nodes_lazy_function.hh Outdated

						
				@ -43,0 +67,4 @@

				};

				struct StoreAndPassThrough {

				  std::function<void(Map<int, std::unique_ptr<bke::BakeItem>>)> store_fn;

Hans Goudey commented

2023-08-30 17:19:18 +02:00

Reading this code, it's a bit frustrating to encounter something like this without any comment. There's clearly a lot going on, and some reason for a function object, but to understand why I have to flip through the places it's assigned and used. That gives me the impression it's for code reuse between the "realtime cache" and "frame cache" versions? But it's hard to tell.

JacquesLucke marked this conversation as resolved

Hans Goudey added this to the Nodes & Physics project 2023-08-30 17:22:55 +02:00

Hans Goudey added the

 @ -40,6 +42,92 @@ namespace blender::nodes {
 using lf::LazyFunction;
 using mf::MultiFunction;
 /** The structs in here describe the different possible behaviors of a simulation input node. */

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Geometry Nodes: refactor simulation storage and how simulation nodes access it #111623