Geometry Nodes: support packing bakes into .blend files #124230

Merged
Jacques Lucke merged 68 commits from JacquesLucke/blender:bake-packed into main 2024-09-20 16:18:31 +02:00
Member

Previously, it was only possible to bake to disk with geometry nodes. This patch adds support for storing the baked data directly in the .blend file.

By default, new bakes are stored in the .blend file now. Whether a new bake should be packed or stored on disk can be configured in two places: in the properties of the bake node and in the bake panel of the modifier. These settings don't affect existing bakes, only the next bake.

image

To unpack or pack an individual bake, there is a new operator button next to the bake button. The icon and the label below indicate where the bake is currently stored. The label now also contains the size of the bake.

image

To unpack or pack all bakes, the File > External Data > Pack Resources / Unpack Resources operators can be used. The unpack operator also has a new title that mentions the number if individual files separate from the number of bakes. This works better than just listing a number of files because a bake can consist of many files.

image

Previously, it was only possible to bake to disk with geometry nodes. This patch adds support for storing the baked data directly in the .blend file. By default, new bakes are stored in the .blend file now. Whether a new bake should be packed or stored on disk can be configured in two places: in the properties of the bake node and in the bake panel of the modifier. These settings don't affect existing bakes, only the next bake. ![image](/attachments/b766562b-7ed5-49c2-b49e-5c67ece72364) To unpack or pack an individual bake, there is a new operator button next to the bake button. The icon and the label below indicate where the bake is currently stored. The label now also contains the size of the bake. <img width="200" alt="image" src="attachments/42d22445-0ecf-4e9a-86d7-c9e160afe463"> To unpack or pack all bakes, the `File > External Data > Pack Resources / Unpack Resources` operators can be used. The unpack operator also has a new title that mentions the number if individual files separate from the number of bakes. This works better than just listing a number of files because a bake can consist of many files. ![image](/attachments/2dc7756c-73ca-49c1-b80e-1e53c217f169)
Jacques Lucke added 6 commits 2024-07-05 13:52:24 +02:00
Jacques Lucke added 3 commits 2024-07-05 16:08:06 +02:00
Jacques Lucke added 1 commit 2024-07-05 16:44:33 +02:00
Jacques Lucke added 2 commits 2024-07-18 15:49:33 +02:00
Jacques Lucke added 3 commits 2024-07-26 15:07:41 +02:00
Jacques Lucke added 1 commit 2024-07-29 20:47:26 +02:00
Jacques Lucke added 2 commits 2024-07-31 12:16:52 +02:00
Jacques Lucke added 2 commits 2024-07-31 13:29:16 +02:00
Jacques Lucke added 1 commit 2024-07-31 13:55:25 +02:00
Jacques Lucke added 5 commits 2024-07-31 17:03:38 +02:00
Jacques Lucke added 1 commit 2024-07-31 17:33:57 +02:00
Jacques Lucke added 2 commits 2024-07-31 18:15:29 +02:00
Jacques Lucke added 2 commits 2024-07-31 18:26:57 +02:00
Jacques Lucke added 2 commits 2024-08-07 13:46:54 +02:00
Jacques Lucke added 5 commits 2024-08-08 15:05:51 +02:00
Jacques Lucke added 2 commits 2024-08-08 15:30:06 +02:00
Jacques Lucke added 3 commits 2024-08-08 16:41:37 +02:00
improve versioning
Some checks failed
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
7d273a8660
Author
Member

@blender-bot build

@blender-bot build
Jacques Lucke changed title from WIP: Geometry Nodes: support packing bakes into .blend files to Geometry Nodes: support packing bakes into .blend files 2024-08-08 16:42:13 +02:00
Jacques Lucke requested review from Simon Thommes 2024-08-08 16:42:25 +02:00
Jacques Lucke requested review from Hans Goudey 2024-08-08 16:42:26 +02:00
Jacques Lucke added 1 commit 2024-08-08 16:45:11 +02:00
First-time contributor

Hey,
For the modifiers you call it "apply modifier", and here you call it "bake".
Is there any difference between the two? is there a reason you call it "bake" and not "apply"?
Does this mean that in the future you will change all modifiers to "bake modifier" instead of "apply modifier"?

Hey, For the modifiers you call it "apply modifier", and here you call it "bake". Is there any difference between the two? is there a reason you call it "bake" and not "apply"? Does this mean that in the future you will change all modifiers to "bake modifier" instead of "apply modifier"?
Author
Member

Applying a modifier and baking a node are two fairly different thing. Applying the modifier removes the modifier and changes the underlying object geometry (i.e. you can go into edit mode and change the applied geometry). Baking just (temporarily) stores an intermediate result so that it does not have to be recomputed all the time.

Does this mean that in the future you will change all modifiers to "bake modifier" instead of "apply modifier"?

There is no plan to do anything like that currently.

Applying a modifier and baking a node are two fairly different thing. Applying the modifier removes the modifier and changes the underlying object geometry (i.e. you can go into edit mode and change the applied geometry). Baking just (temporarily) stores an intermediate result so that it does not have to be recomputed all the time. > Does this mean that in the future you will change all modifiers to "bake modifier" instead of "apply modifier"? There is no plan to do anything like that currently.
Jacques Lucke added 2 commits 2024-08-12 12:28:22 +02:00
First-time contributor

Applying a modifier and baking a node are two fairly different thing. Applying the modifier removes the modifier and changes the underlying object geometry (i.e. you can go into edit mode and change the applied geometry). Baking just (temporarily) stores an intermediate result so that it does not have to be recomputed all the time.

Some questions out of curiosity (you don't have to answer).
Why do modifiers act like that?
Why does it remove the modifier completely when you apply it?
Why do you need to apply them first (what does "apply" even do in concept)?
Why can't you edit them at any time?
If you bake a the whole geo node can you edit it (meaning if you put a bake node at the end)? if not why not?

> Applying a modifier and baking a node are two fairly different thing. Applying the modifier removes the modifier and changes the underlying object geometry (i.e. you can go into edit mode and change the applied geometry). Baking just (temporarily) stores an intermediate result so that it does not have to be recomputed all the time. Some questions out of curiosity (you don't have to answer). Why do modifiers act like that? Why does it remove the modifier completely when you apply it? Why do you need to apply them first (what does "apply" even do in concept)? Why can't you edit them at any time? If you bake a the whole geo node can you edit it (meaning if you put a bake node at the end)? if not why not?
Jacques Lucke added 2 commits 2024-08-26 12:11:16 +02:00
Jacques Lucke added 2 commits 2024-08-29 15:58:03 +02:00
Simon Thommes requested changes 2024-08-29 19:40:17 +02:00
Dismissed
Simon Thommes left a comment
Member

This will be really useful to encapsulate simulated data as part of assets and seems to work great in terms of core functionality from my testing so far.
I do have some comments on UI/UX level. A lot of this is probably not really related to this PR specifically and pretty unsorted and incomplete for now, sorry about that.

Initial impressions/feedback:

  • With the inherit option in the node, it would still be useful to see the current result of that inheritance in this UI without hunting it down in the modifier interface.
  • With baked simulation contained in the .blend file now being possible it becomes even more obvious to me that we need to the ability to write the current cache as a bake. This is separate from this PR, just an observation.
  • This is besides this PR, but I'm noticing that it is probably a mistake to allow changing the bake settings without deleting the current bake. Right now you can easily land in the situation that you baked to a custom path, the settings are greyed out, you can still toggle the Custom Path or change the path itself and end up not reading the bake data back on next reload, while you don't see anything breaking immediately.
  • it should be harder to end up in paradoxical situations like this one:
    image
  • In the modifier UI, when the target is set to Packed the path should be greyed out and probably even uneditable. If there are any previous bakes that are still using that path it still shouldn't be edited anyways while the modifier target is set to Packed
    image
  • seeing the actual path that is derived from the modifier path is great, but it should be relative to the same base path as the modifier path as well. Right now it's always absolute, it seems.
  • Currently it's quite easy to lose the bake path, when. It would make sense to me if, in the case of existing bakes, the actual path is retained as a custom one whenever the modifier path (or bake target) is changed, just like when unpacking a bake.
This will be really useful to encapsulate simulated data as part of assets and seems to work great in terms of core functionality from my testing so far. I do have some comments on UI/UX level. A lot of this is probably not really related to this PR specifically and pretty unsorted and incomplete for now, sorry about that. Initial impressions/feedback: - With the inherit option in the node, it would still be useful to see the current result of that inheritance in this UI without hunting it down in the modifier interface. - With baked simulation contained in the .blend file now being possible it becomes even more obvious to me that we need to the ability to write the current cache as a bake. This is separate from this PR, just an observation. - This is besides this PR, but I'm noticing that it is probably a mistake to allow changing the bake settings without deleting the current bake. Right now you can easily land in the situation that you baked to a custom path, the settings are greyed out, you can still toggle the `Custom Path` or change the path itself and end up not reading the bake data back on next reload, while you don't see anything breaking immediately. - it should be harder to end up in paradoxical situations like this one: ![image](/attachments/695d3bf6-7e3d-47a3-b86d-9e04319c93d2) - In the modifier UI, when the target is set to `Packed` the path should be greyed out and probably even uneditable. If there are any previous bakes that are still using that path it still shouldn't be edited anyways while the modifier target is set to `Packed` ![image](/attachments/cc814dd1-9792-44a6-b8aa-cc959cb2dbcf) - seeing the actual path that is derived from the modifier path is great, but it should be relative to the same base path as the modifier path as well. Right now it's always absolute, it seems. - Currently it's quite easy to lose the bake path, when. It would make sense to me if, in the case of existing bakes, the actual path is retained as a custom one whenever the modifier path (or bake target) is changed, just like when unpacking a bake.
Author
Member

With the inherit option in the node, it would still be useful to see the current result of that inheritance in this UI without hunting it down in the modifier interface.

We talked about that before. This is already implemented but I understand that it's not very obvious. The pack/unpack icon next to the bake button changes based on the setting in the modifier.

With baked simulation contained in the .blend file now being possible it becomes even more obvious to me that we need to the ability to write the current cache as a bake. This is separate from this PR, just an observation.

Yeah, I can create a separate PR for that after this patch. With some luck, it's fairly straight forward.

This is besides this PR, but I'm noticing that it is probably a mistake to allow changing the bake settings without deleting the current bake. Right now you can easily land in the situation that you baked to a custom path, the settings are greyed out, you can still toggle the Custom Path or change the path itself and end up not reading the bake data back on next reload, while you don't see anything breaking immediately.

Well, it's tricky because there are also good reasons for being able to change the settings while there is still baked data:

  • When e.g. baking a single frame, unbake+bake would generally involve two node tree evaluations while just rebake is just one evaluation.
  • We had the issue in the past that when you copy an object, the new object still references the existing bake on disk. It should be possible to remove that reference without freeing the entire bake.

I'm not sure how doable that is yet, but it may be possible to show some kind of warning when the bake settings are modified while it's in a baked state.

In the modifier UI, when the target is set to Packed the path should be greyed out and probably even uneditable. If there are any previous bakes that are still using that path it still shouldn't be edited anyways while the modifier target is set to Packed

Also potentially tricky, because there may be individual bakes that do not inherit from the modifier.

seeing the actual path that is derived from the modifier path is great, but it should be relative to the same base path as the modifier path as well. Right now it's always absolute, it seems.

Sounds reasonable.

Currently it's quite easy to lose the bake path, when. It would make sense to me if, in the case of existing bakes, the actual path is retained as a custom one whenever the modifier path (or bake target) is changed, just like when unpacking a bake.

Hm, not totally sure what you mean. How do you loose the bake path?

> With the inherit option in the node, it would still be useful to see the current result of that inheritance in this UI without hunting it down in the modifier interface. We talked about that before. This is already implemented but I understand that it's not very obvious. The pack/unpack icon next to the bake button changes based on the setting in the modifier. > With baked simulation contained in the .blend file now being possible it becomes even more obvious to me that we need to the ability to write the current cache as a bake. This is separate from this PR, just an observation. Yeah, I can create a separate PR for that after this patch. With some luck, it's fairly straight forward. > This is besides this PR, but I'm noticing that it is probably a mistake to allow changing the bake settings without deleting the current bake. Right now you can easily land in the situation that you baked to a custom path, the settings are greyed out, you can still toggle the Custom Path or change the path itself and end up not reading the bake data back on next reload, while you don't see anything breaking immediately. Well, it's tricky because there are also good reasons for being able to change the settings while there is still baked data: * When e.g. baking a single frame, unbake+bake would generally involve two node tree evaluations while just rebake is just one evaluation. * We had the issue in the past that when you copy an object, the new object still references the existing bake on disk. It should be possible to remove that reference without freeing the entire bake. I'm not sure how doable that is yet, but it may be possible to show some kind of warning when the bake settings are modified while it's in a baked state. > In the modifier UI, when the target is set to Packed the path should be greyed out and probably even uneditable. If there are any previous bakes that are still using that path it still shouldn't be edited anyways while the modifier target is set to Packed Also potentially tricky, because there may be individual bakes that do not inherit from the modifier. > seeing the actual path that is derived from the modifier path is great, but it should be relative to the same base path as the modifier path as well. Right now it's always absolute, it seems. Sounds reasonable. > Currently it's quite easy to lose the bake path, when. It would make sense to me if, in the case of existing bakes, the actual path is retained as a custom one whenever the modifier path (or bake target) is changed, just like when unpacking a bake. Hm, not totally sure what you mean. How do you loose the bake path?
Jacques Lucke added 2 commits 2024-08-30 13:49:21 +02:00
Member

We talked about that before. This is already implemented but I understand that it's not very obvious. The pack/unpack icon next to the bake button changes based on the setting in the modifier.

I mean, I did notice that, but especially with the button being greyed out this is not enough imo. After baking you see the state explicitly as well, not only in the icon.

Also potentially tricky, because there may be individual bakes that do not inherit from the modifier.

Ah, I see... right...

Hm, not totally sure what you mean. How do you loose the bake path?

Oh sorry, looks like I didn't finish typing the sentence. I'm not sure anymore what exactly I did, but I believe it was about removing or changing the modifier bake path after baking to disk.
I'm starting to wonder if it would make sense to explicitly split the bake target path for writing and the actual bake path for reading per bake, so that the actual path cannot get lost, by being defined implicitly.

> We talked about that before. This is already implemented but I understand that it's not very obvious. The pack/unpack icon next to the bake button changes based on the setting in the modifier. I mean, I did notice that, but especially with the button being greyed out this is not enough imo. After baking you see the state explicitly as well, not only in the icon. > Also potentially tricky, because there may be individual bakes that do not inherit from the modifier. Ah, I see... right... > Hm, not totally sure what you mean. How do you loose the bake path? Oh sorry, looks like I didn't finish typing the sentence. I'm not sure anymore what exactly I did, but I believe it was about removing or changing the modifier bake path after baking to disk. I'm starting to wonder if it would make sense to explicitly split the bake target path for writing and the actual bake path for reading per bake, so that the actual path cannot get lost, by being defined implicitly.
Author
Member

I mean, I did notice that, but especially with the button being greyed out this is not enough imo. After baking you see the state explicitly as well, not only in the icon.

Do you have a suggestion for how that should be displayed?

> I mean, I did notice that, but especially with the button being greyed out this is not enough imo. After baking you see the state explicitly as well, not only in the icon. Do you have a suggestion for how that should be displayed?
Jacques Lucke added this to the Nodes & Physics project 2024-09-11 15:22:03 +02:00
Jacques Lucke added 1 commit 2024-09-11 15:27:57 +02:00
Simon Thommes requested changes 2024-09-12 16:11:32 +02:00
Dismissed
Simon Thommes left a comment
Member

When unpacking an existing packed bake to disk (bpy.ops.object.geometry_node_bake_unpack_single()), there should probably be the same dialog that you get from doing the same with image data-blocks.
image


Also, when doing that, I think it would make sense to change Inherit from Modifier to 'Disk' alongside writing to the custom path. Otherwise re-baking will change the target again and it seems more consistent to me.


Do you have a suggestion for how that should be displayed?

Maybe the button itself could just say Bake Packed or Bake to Disk to make it clear what will happen?


About this point:

  • it should be harder to end up in paradoxical situations like this one:
    image

Wouldn't it make sense to gray out the path and custom path toggle in this specific case?

When unpacking an existing packed bake to disk (`bpy.ops.object.geometry_node_bake_unpack_single()`), there should probably be the same dialog that you get from doing the same with image data-blocks. ![image](/attachments/e295bfb1-d9be-4efc-959a-b400a473e2f8) --- Also, when doing that, I think it would make sense to change `Inherit from Modifier` to 'Disk' alongside writing to the custom path. Otherwise re-baking will change the target again and it seems more consistent to me. --- > Do you have a suggestion for how that should be displayed? Maybe the button itself could just say `Bake Packed` or `Bake to Disk` to make it clear what will happen? --- About this point: > - it should be harder to end up in paradoxical situations like this one: > ![image](/attachments/695d3bf6-7e3d-47a3-b86d-9e04319c93d2) Wouldn't it make sense to gray out the path and custom path toggle in this specific case?
Jacques Lucke added 1 commit 2024-09-17 13:37:55 +02:00
Jacques Lucke added 5 commits 2024-09-18 18:12:09 +02:00
Jacques Lucke added 4 commits 2024-09-18 18:57:40 +02:00
Hans Goudey reviewed 2024-09-18 21:41:40 +02:00
Hans Goudey left a comment
Member

Reading through the whole PR, only saw two things that stuck out to me.

And in basic tests this definitely speeds up the process of opening Blender and using simulation nodes.

Reading through the whole PR, only saw two things that stuck out to me. And in basic tests this definitely speeds up the process of opening Blender and using simulation nodes.
@ -251,1 +251,4 @@
constexpr bool contains(const IndexRange range) const
{
if (range.is_empty()) {
Member

Not sure about this, it seems more obvious for the start_ should be checked regardless of the size.

Not sure about this, it seems more obvious for the `start_` should be checked regardless of the size.
Author
Member

Currently, I think that the start value shouldn't have any meaning. Any empty index range should be considered to be equal.

Currently, I think that the start value shouldn't have any meaning. Any empty index range should be considered to be equal.

range.intersect(other).is_empty()

`range.intersect(other).is_empty()`
JacquesLucke marked this conversation as resolved
@ -327,0 +393,4 @@
if (data_size == 0) {
continue;
}
void *data = MEM_mallocN(data_size, __func__);
Member

Seems like the packed file's implicit sharing should be used to avoid copying/freeing the data from the string. After this operation the implicit sharing info in the packed file should own the string.

Seems like the packed file's implicit sharing should be used to avoid copying/freeing the data from the string. After this operation the implicit sharing info in the packed file should own the string.
JacquesLucke marked this conversation as resolved
Simon Thommes requested changes 2024-09-19 18:17:09 +02:00
Dismissed
Simon Thommes left a comment
Member

When baking to disk, before the blend file is saved, it will bake as packed regardless.
I think that is generally fine, but there is not feedback at all right now.

When an absolute path is provided, I don't even think there is a need to pack it though, it could just work in that case.

Imo there should either be a popup informing the user that the file needs to be saved first to bake to a relative path, with the option to bake packed or at least there should be an info message after the fact.

When baking to disk, before the blend file is saved, it will bake as packed regardless. I think that is generally fine, but there is not feedback at all right now. When an absolute path is provided, I don't even think there is a need to pack it though, it could just work in that case. Imo there should either be a popup informing the user that the file needs to be saved first to bake to a relative path, with the option to bake packed or at least there should be an info message after the fact.
Jacques Lucke added 5 commits 2024-09-19 19:27:56 +02:00
Hans Goudey approved these changes 2024-09-19 20:46:45 +02:00
Simon Thommes approved these changes 2024-09-20 15:00:12 +02:00
Simon Thommes left a comment
Member

Looks good now!

Didn't find an issue with it anymore. The more features we add to baking though, the more it becomes obvious that we're missing the top level overview of bakes in the scene. Besides that, I think, now there is a pretty intuitive insight for the bake target.

I do feel like it might make sense in the future to differentiate bake target path and existing bake location before and after bake as 2 separate paths. But that topic is a bit separate and can still be done in the future.

As a side-note. The Custom Range settings should probably be grayed out in the bake node for Still mode, in a separate PR.

Looks good now! Didn't find an issue with it anymore. The more features we add to baking though, the more it becomes obvious that we're missing the top level overview of bakes in the scene. Besides that, I think, now there is a pretty intuitive insight for the bake target. I do feel like it might make sense in the future to differentiate bake target path and existing bake location before and after bake as 2 separate paths. But that topic is a bit separate and can still be done in the future. As a side-note. The Custom Range settings should probably be grayed out in the bake node for `Still` mode, in a separate PR.
Jacques Lucke merged commit 3ccfa65245 into main 2024-09-20 16:18:31 +02:00
Jacques Lucke deleted branch bake-packed 2024-09-20 16:18:35 +02:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
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
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
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
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
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
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
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
5 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#124230
No description provided.