Geometry Nodes: support baking individual simulations #112723

Merged
Jacques Lucke merged 15 commits from JacquesLucke/blender:main-bake-individual into main 2023-09-26 20:30:54 +02:00
Member

Previously, it was only possible to bake all simulations at once. This is great for simple use-cases that, but in more complex setups one can have independent simulations that should also be baked independently. This patch allows baking individual simulation zones.

Furthermore, each simulation zone can now also have its own bake path and simulation frame range. By default the simulation frame range is the scene frame range, but it can also be customized on the scene or simulation zone level. The bake path is generated based on the modifier bake path by default, but can be set to another absolute or relative (to the .blend file) path.

The timeline drawing has been modified as well to be able to show more information in the case when some simulations are baked and others are not. Instead of showing a line for every simulation, it shows a condensed view of the important information using at most two lines: Is something baked? Is something valid or invalid? Also see #112232.

Settings for each simulation zone:
image

Scene simulation settings:
image

Timeline drawing when at least one simulation is baked.
image

Previously, it was only possible to bake all simulations at once. This is great for simple use-cases that, but in more complex setups one can have independent simulations that should also be baked independently. This patch allows baking individual simulation zones. Furthermore, each simulation zone can now also have its own bake path and simulation frame range. By default the simulation frame range is the scene frame range, but it can also be customized on the scene or simulation zone level. The bake path is generated based on the modifier bake path by default, but can be set to another absolute or relative (to the .blend file) path. The timeline drawing has been modified as well to be able to show more information in the case when some simulations are baked and others are not. Instead of showing a line for every simulation, it shows a condensed view of the important information using at most two lines: Is something baked? Is something valid or invalid? Also see #112232. Settings for each simulation zone: ![image](/attachments/3b9027f9-0cd7-407b-9e00-a79554917408) Scene simulation settings: ![image](/attachments/96a8ea27-a8b4-4b97-9868-5eefad8e8f23) Timeline drawing when at least one simulation is baked. ![image](/attachments/61df456c-9677-45ea-97e2-1de1475680ee)
Jacques Lucke added 1 commit 2023-09-22 14:47:49 +02:00
Jacques Lucke requested review from Hans Goudey 2023-09-22 16:14:30 +02:00
Hans Goudey reviewed 2023-09-22 23:11:00 +02:00
Hans Goudey left a comment
Member

Overall this looks pretty good. The thing I find most controversial is including ED_node.hh in the simulation output node. It would be nice not to contribute to the confusion between modules whenever possible. Some of the "nested node ref" code is a bit confusing to read, but conceptually it makes sense.

Would the scene simulation frame range be used for baking once we have bake nodes? Or do we not know quite yet?

Overall this looks pretty good. The thing I find most controversial is including `ED_node.hh` in the simulation output node. It would be nice not to contribute to the confusion between modules whenever possible. Some of the "nested node ref" code is a bit confusing to read, but conceptually it makes sense. Would the scene simulation frame range be used for baking once we have bake nodes? Or do we not know quite yet?
@ -424,4 +422,0 @@
wmJob *wm_job = WM_jobs_get(wm,
CTX_wm_window(C),
CTX_data_scene(C),
"Bake Simulation Nodes",
Member

Changing Bake Simulation Nodes to Bake Nodes feels a bit premature IMO, not that it makes a big difference.

Changing `Bake Simulation Nodes` to `Bake Nodes` feels a bit premature IMO, not that it makes a big difference.
JacquesLucke marked this conversation as resolved
@ -428,2 +422,2 @@
WM_JOB_PROGRESS,
WM_JOB_TYPE_BAKE_SIMULATION_NODES);
Vector<ObjectBakeData> objects_to_bake;
for (Object *object : objects) {
Member

I think the code in the for (Object *object : objects) { loop would be clearer if it was in a function called something like collect_nodes_to_bake or something.

I think the code in the `for (Object *object : objects) {` loop would be clearer if it was in a function called something like `collect_nodes_to_bake` or something.
JacquesLucke marked this conversation as resolved
@ -436,0 +428,4 @@
ObjectBakeData bake_data;
bake_data.object = object;
LISTBASE_FOREACH (ModifierData *, md, &object->modifiers) {
if (md->type == eModifierType_Nodes) {
Member

Flip the check and continue

Flip the check and `continue`
JacquesLucke marked this conversation as resolved
@ -607,0 +662,4 @@
const char *meta_dir = bake_path->meta_dir.c_str();
if (BLI_exists(meta_dir)) {
if (BLI_delete(meta_dir, true, true)) {
BKE_reportf(reports, RPT_ERROR, "Failed to remove meta directory %s", meta_dir);
Member

meta -> metadata (since it's a UI message)

`meta` -> `metadata` (since it's a UI message)
JacquesLucke marked this conversation as resolved
@ -789,0 +785,4 @@
blender::Vector<int> status_change_frames;
status_change_frames.extend(status_change_frames_set.begin(), status_change_frames_set.end());
std::sort(status_change_frames.begin(), status_change_frames.end());
const blender::OffsetIndices<int> frame_ranges = status_change_frames.as_span();
Member

This is clever, I like it :)

This is clever, I like it :)
JacquesLucke marked this conversation as resolved
@ -8376,0 +8384,4 @@
prop = RNA_def_property(srna, "simulation_frame_start", PROP_INT, PROP_NONE);
RNA_def_property_ui_text(
prop, "Simulation Frame Start", "Frame at which simulations start running");
Member

"running" seems like an unnecessary word here and sounds a bit weird. Maybe just remove it?

"running" seems like an unnecessary word here and sounds a bit weird. Maybe just remove it?
JacquesLucke marked this conversation as resolved
@ -414,0 +453,4 @@
}
}
else if (old_bake_by_id.contains(ref.id)) {
/* Keep baked data in case linked data is missing so that it still exists when the linked
Member

I feel like I'm missing something here. Is this code only hit when a linked node group is "missing"? Does it not trigger when a simulation zone is deleted by the user, for example?

I feel like I'm missing something here. Is this code only hit when a linked node group is "missing"? Does it not trigger when a simulation zone is deleted by the user, for example?
Author
Member

Yes, because if a simulation zone is deleted, then it also won't be in nmd.node_group->nested_node_refs anymore. The case with missing node groups has a special case in update_nested_node_refs as well.

Yes, because if a simulation zone is deleted, then it also won't be in `nmd.node_group->nested_node_refs` anymore. The case with missing node groups has a special case in `update_nested_node_refs` as well.
HooglyBoogly marked this conversation as resolved
@ -414,0 +460,4 @@
}
NodesModifierBake *new_bake_data = static_cast<NodesModifierBake *>(
MEM_callocN(sizeof(NodesModifierBake) * new_bake_ids.size(), __func__));
Member

Might as well use MEM_cnew_array?

Might as well use `MEM_cnew_array`?
JacquesLucke marked this conversation as resolved
@ -414,0 +468,4 @@
if (old_bake) {
new_bake = *old_bake;
if (new_bake.directory) {
new_bake.directory = BLI_strdup(new_bake.directory);
Member

Seems nicer to just zero the old bake directory here, rather than duplicating the string

Seems nicer to just zero the old bake directory here, rather than duplicating the string
JacquesLucke marked this conversation as resolved
@ -722,3 +817,1 @@
if (node_cache->cache_status == bake::CacheStatus::Invalid) {
node_cache->reset();
}
for (auto item : modifier_cache_->cache_by_id.items()) {
Member

How about separating this loop into a function called reset_node_bakes or so?

How about separating this loop into a function called `reset_node_bakes` or so?
JacquesLucke marked this conversation as resolved
Jacques Lucke added 10 commits 2023-09-25 13:29:27 +02:00
Author
Member

The thing I find most controversial is including ED_node.hh in the simulation output node.

To me that's not too controversial. If we want node files to be as self-contained as possible, we need to allow importing such headers.

Would the scene simulation frame range be used for baking once we have bake nodes? Or do we not know quite yet?

We do not quite know yet. I figured that it would be easy enough to version whatever we decide.

> The thing I find most controversial is including `ED_node.hh` in the simulation output node. To me that's not too controversial. If we want node files to be as self-contained as possible, we need to allow importing such headers. > Would the scene simulation frame range be used for baking once we have bake nodes? Or do we not know quite yet? We do not quite know yet. I figured that it would be easy enough to version whatever we decide.
Jacques Lucke added 1 commit 2023-09-25 14:06:58 +02:00
Jacques Lucke added this to the Nodes & Physics project 2023-09-25 14:16:31 +02:00
Jacques Lucke requested review from Simon Thommes 2023-09-26 11:56:13 +02:00
Simon Thommes approved these changes 2023-09-26 13:12:18 +02:00
Simon Thommes left a comment
Member

Everything as we discussed. A bit unfortunate that things are still a bit scattered, but that will improve once we have the outliner view for baking and the modular paths with variables.

For now this gives all the flexibility and is non-intrusive by default.

See discussion here: #112179

Everything as we discussed. A bit unfortunate that things are still a bit scattered, but that will improve once we have the outliner view for baking and the modular paths with variables. For now this gives all the flexibility and is non-intrusive by default. See discussion here: #112179
Hans Goudey approved these changes 2023-09-26 19:46:41 +02:00
@ -728,0 +898,4 @@
"Modifier Name",
"Name of the modifier that contains the node to bake");
RNA_def_int(
ot->srna, "bake_id", 0, 0, INT32_MAX, "Bake ID", "ID of the node to bake", 0, INT32_MAX);
Member

ID of the node -> Nested node id, right?

`ID of the node` -> `Nested node id`, right?
JacquesLucke marked this conversation as resolved
@ -7070,0 +7097,4 @@
prop = RNA_def_property(srna, "use_custom_path", PROP_BOOLEAN, PROP_NONE);
RNA_def_property_boolean_sdna(prop, nullptr, "flag", NODES_MODIFIER_BAKE_CUSTOM_PATH);
RNA_def_property_ui_text(
prop, "Auto Path", "Specify a path where the baked data should be stored manually");
Member

Auto Path -> Custom Path

`Auto Path` -> `Custom Path`
JacquesLucke marked this conversation as resolved
Jacques Lucke added 3 commits 2023-09-26 20:29:07 +02:00
Jacques Lucke merged commit ad169ba67a into main 2023-09-26 20:30:54 +02:00
Jacques Lucke deleted branch main-bake-individual 2023-09-26 20:30:56 +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
3 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#112723
No description provided.