blender/blender-asset-tracer
2
5
Fork 5
Code Issues 4 Pull Requests 1 Projects Releases Wiki Activity

Add support for Open VDB #92884

Manually merged
Sybren A. Stüvel merged 4 commits from oliverpool/blender-asset-tracer:external_vdb into main 2024-01-02 16:36:53 +01:00
Conversation 13 Commits 4 Files Changed 2 +11
Olivier C commented 2023-10-18 15:10:50 +02:00
Contributor
Copy Link

Closes #92881 (cc @razed @lichtwerk)

This PR adds support for packing OpenVDB data blocks. I have a couple of question (@dr.sybren you might have some answers ?)

  • I had to comment the @skip_packed decorator, since I have an example scene, where the data block has the packedfile property, but the vdb is missing in the output. So I guess the packedfile property refers to something else. Can you confirm?

  • in the issue #92881, it was wished for a "sort of gate the user has to accept/agree to". My code currently always packs OpenVDBs. A simple solution would be to comment out the @dna_code("VO") decorator and force the caller to register this handler manually by calling bat_trace_blocks2assets.dna_code("VO")(bat_trace_blocks2assets.open_vdb). Or do you think that there might be an even better approach?

contribution on behalf of RANCH Computing

PS: to ease support of other data blocks in the future, I added the filepath to the debug message on unsupported block types (if present).

_Closes #92881 (cc @razed @lichtwerk)_ This PR adds support for packing OpenVDB data blocks. I have a couple of question (@dr.sybren you might have some answers ?) - I had to comment the `@skip_packed` decorator, since I have an example scene, where the data block has the `packedfile` property, but the vdb is missing in the output. So I guess the `packedfile` property refers to something else. Can you confirm? - in the issue #92881, it was wished for a "sort of gate the user has to accept/agree to". My code currently always packs OpenVDBs. A simple solution would be to comment out the `@dna_code("VO")` decorator and force the caller to register this handler manually by calling `bat_trace_blocks2assets.dna_code("VO")(bat_trace_blocks2assets.open_vdb)`. Or do you think that there might be an even better approach? _contribution on behalf of [RANCH Computing](https://www.ranchcomputing.com/)_ --- PS: to ease support of other data blocks in the future, I added the filepath to the debug message on unsupported block types (if present).
Olivier C added 2 commits 2023-10-18 15:10:52 +02:00
Olivier C commented 2023-10-18 15:12:48 +02:00
Author
Contributor
Copy Link

Scene example with vdb (newly supported with this PR) and fluid cache (already supported):

Scene example with vdb (newly supported with this PR) and fluid cache (already supported):
scene_with_vdb_and_fluid_cache.zip
6.3 MiB
Sybren A. Stüvel commented 2023-10-31 14:49:23 +01:00
Member
Copy Link

Thanks for the PR!

This PR adds support for packing OpenVDB data blocks. I have a couple of question (@dr.sybren you might have some answers ?)

  • I had to comment the @skip_packed decorator, since I have an example scene, where the data block has the packedfile property, but the vdb is missing in the output. So I guess the packedfile property refers to something else. Can you confirm?

From what I can tell, it's the normal file packing approach. If you have some example files for me I might be able to find out more.

  • in the issue #92881, it was wished for a "sort of gate the user has to accept/agree to". My code currently always packs OpenVDBs. A simple solution would be to comment out the @dna_code("VO") decorator and force the caller to register this handler manually by calling bat_trace_blocks2assets.dna_code("VO")(bat_trace_blocks2assets.open_vdb). Or do you think that there might be an even better approach?

BAT already has a few options for this:

  • --relative-only will skip any asset that's referenced by absolute path. Flamenco uses this, as it's assumed that these absolute paths will remain valid on all worker nodes.
  • --exclude "*.vdb" can also be used to specifically skip VDB files.

PS: to ease support of other data blocks in the future, I added the filepath to the debug message on unsupported block types (if present).

That's better to put into a separate PR. That way it can be handled as a separate contribution, which means a separate commit, which means that these independent concerns are also tracked independently. For example, when bisecting the Git history it helps when changes are small and atomic. Also when there is some hypothetical issue that means a commit has to be rolled back, we don't loose any other changes.

The change itself seems good though, so that'll certainly be accepted. It's just easier to accept if it's not intermixed with any VDB-related work.

Thanks for the PR! > This PR adds support for packing OpenVDB data blocks. I have a couple of question (@dr.sybren you might have some answers ?) > - I had to comment the `@skip_packed` decorator, since I have an example scene, where the data block has the `packedfile` property, but the vdb is missing in the output. So I guess the `packedfile` property refers to something else. Can you confirm? From what I can tell, it's the normal file packing approach. If you have some example files for me I might be able to find out more. > - in the issue #92881, it was wished for a "sort of gate the user has to accept/agree to". My code currently always packs OpenVDBs. A simple solution would be to comment out the `@dna_code("VO")` decorator and force the caller to register this handler manually by calling `bat_trace_blocks2assets.dna_code("VO")(bat_trace_blocks2assets.open_vdb)`. Or do you think that there might be an even better approach? BAT already has a few options for this: - `--relative-only` will skip any asset that's referenced by absolute path. Flamenco uses this, as it's assumed that these absolute paths will remain valid on all worker nodes. - `--exclude "*.vdb"` can also be used to specifically skip VDB files. > PS: to ease support of other data blocks in the future, I added the filepath to the debug message on unsupported block types (if present). That's better to put into a separate PR. That way it can be handled as a separate contribution, which means a separate commit, which means that these independent concerns are also tracked independently. For example, when bisecting the Git history it helps when changes are small and atomic. Also when there is some hypothetical issue that means a commit has to be rolled back, we don't loose any other changes. The change itself seems good though, so that'll certainly be accepted. It's just easier to accept if it's not intermixed with any VDB-related work.
Philipp Oeser commented 2023-11-01 15:26:19 +01:00
Member
Copy Link

CC @TinyNick (this is the one we were talking about at the conference)

CC @TinyNick (this is the one we were talking about at the conference)
❤️ 1
Olivier C commented 2023-11-08 08:51:59 +01:00
Author
Contributor
Copy Link

Thanks for the feedback!

I moved the debug logic to a dedicated PR #92885 and rebased this one on top of main.

I had to comment the @skip_packed decorator, since I have an example scene, where the data block has the packedfile property, but the vdb is missing in the output. So I guess the packedfile property refers to something else. Can you confirm?

From what I can tell, it's the normal file packing approach. If you have some example files for me I might be able to find out more.

You can take the scene that I shared scene_with_vdb_and_fluid_cache.zip and run "External Data > Pack resources" before saving the scene.
When running BAT the cache_vdb/CCup_ws_01/fluid_data_0001.vdb files will be marked as "packedfile" and will not be exported (if @skip_packed is uncommented). However the vdb will be missing from the final scene. You can check by rendering the frame 1:

expected ( with # @skip_packed as a comment):
bat_with_vdb_0001.png

actual (with @skip_packed):
bat_skip_packed_0001.png

Thanks for the feedback! I moved the debug logic to a dedicated PR #92885 and rebased this one on top of main. >> I had to comment the @skip_packed decorator, since I have an example scene, where the data block has the packedfile property, but the vdb is missing in the output. So I guess the packedfile property refers to something else. Can you confirm? > > From what I can tell, it's the normal file packing approach. If you have some example files for me I might be able to find out more. You can take the scene that I shared [scene_with_vdb_and_fluid_cache.zip](https://projects.blender.org/attachments/a1bdd002-d684-4709-bb0a-744590606413) and run "External Data > Pack resources" before saving the scene. When running BAT the `cache_vdb/CCup_ws_01/fluid_data_0001.vdb` files will be marked as "packedfile" and will not be exported (if `@skip_packed` is uncommented). However the vdb will be missing from the final scene. You can check by rendering the frame 1: expected ( with `# @skip_packed` as a comment): ![bat_with_vdb_0001.png](/attachments/6b98900c-fb5d-4b84-b88a-09269549b7e5) actual (with `@skip_packed`): ![bat_skip_packed_0001.png](/attachments/b251c5ff-e9a0-4bef-a112-95a827c3ec80)
bat_skip_packed_0001.png
131 KiB
bat_with_vdb_0001.png
132 KiB
Olivier C force-pushed external_vdb from 1f5658f8d2 to 0962efcfc0 2023-11-08 08:52:41 +01:00 Compare
Olivier C commented 2023-11-24 09:24:56 +01:00
Author
Contributor
Copy Link

@dr.sybren friendly ping regarding this PR and #92885

@dr.sybren friendly ping regarding this PR and #92885
Sybren A. Stüvel requested review from Sybren A. Stüvel 2023-12-14 10:45:04 +01:00
Sybren A. Stüvel commented 2023-12-14 10:59:07 +01:00
Member
Copy Link

When running BAT the cache_vdb/CCup_ws_01/fluid_data_0001.vdb files will be marked as "packedfile" and will not be exported (if @skip_packed is uncommented). However the vdb will be missing from the final scene.

If the VDB file is packed inside the .blend file, I wouldn't expect it to be copied along with the BAT pack. After all, if it's already packed inside the .blend, why would BAT copy it again as an external file?

You can check by rendering the frame 1:

expected ( with # @skip_packed as a comment):
bat_with_vdb_0001.png

actual (with @skip_packed):
bat_skip_packed_0001.png

So this seems to be an issue with Blender, then? From https://docs.blender.org/manual/en/latest/files/blend/packed_data.html it seems that the fluid cache files cannot be packed in the .blend file; after all, there is no 'little “gift box” icon to the left of its file-path UI widget'.

> When running BAT the `cache_vdb/CCup_ws_01/fluid_data_0001.vdb` files will be marked as "packedfile" and will not be exported (if `@skip_packed` is uncommented). However the vdb will be missing from the final scene. If the VDB file is packed inside the .blend file, I wouldn't expect it to be copied along with the BAT pack. After all, if it's already packed inside the .blend, why would BAT copy it again as an external file? > You can check by rendering the frame 1: > > expected ( with `# @skip_packed` as a comment): > ![bat_with_vdb_0001.png](/attachments/6b98900c-fb5d-4b84-b88a-09269549b7e5) > > actual (with `@skip_packed`): > ![bat_skip_packed_0001.png](/attachments/b251c5ff-e9a0-4bef-a112-95a827c3ec80) So this seems to be an issue with Blender, then? From https://docs.blender.org/manual/en/latest/files/blend/packed_data.html it seems that the fluid cache files cannot be packed in the .blend file; after all, there is no 'little “gift box” icon to the left of its file-path UI widget'.
Olivier C commented 2023-12-15 10:17:24 +01:00
Author
Contributor
Copy Link

So this seems to be an issue with Blender, then?

Probably (I have to confess that my experience with blender is very limited, I am a software developer and not a 3D artist)

How should we move forward? Should a bug report be opened on blender? Can you open it or should maybe @ariane-genty open it?

> So this seems to be an issue with Blender, then? Probably (I have to confess that my experience with blender is very limited, I am a software developer and not a 3D artist) How should we move forward? Should a bug report be opened on blender? Can you open it or should maybe @ariane-genty open it?
Sybren A. Stüvel commented 2023-12-15 11:58:37 +01:00
Member
Copy Link

I think the confusion in this PR is about the word "packing". A "BAT pack" is something completely different than "packed files inside a blend file". The former is a feature of BAT, whereas the latter is a feature of Blender, and both work in very different ways. If you want to add support to BAT for BAT-packing OpenVDB files, that's great, but please separate that from "Blender cannot blendfile-pack VDB files".

I think the confusion in this PR is about the word "packing". A "BAT pack" is something completely different than "packed files inside a blend file". The former is a feature of BAT, whereas the latter is a feature of Blender, and both work in very different ways. If you want to add support to BAT for BAT-packing OpenVDB files, that's great, but please separate that from "Blender cannot blendfile-pack VDB files".
ariane-genty commented 2023-12-15 13:59:17 +01:00
First-time contributor
Copy Link

Hello,
It seems to be related to this issue : #100592 ?
I thought only image textures or audio files could be packed (Blender packed) (see here) , but not volume sequences or movies ("heavy files").
I wonder if the first vdb of the sequence is marked as "Blender packed", but it doesn't work as expected and induce a confusion in the bat-packer ?

Hello, It seems to be related to this issue : [#100592](https://projects.blender.org/blender/blender/issues/100592) ? I thought only image textures or audio files could be packed (Blender packed) ([see here](https://blender.stackexchange.com/questions/279481/send-blend-file-with-vdb)) , but not volume sequences or movies ("heavy files"). I wonder if the first vdb of the sequence is marked as "Blender packed", but it doesn't work as expected and induce a confusion in the bat-packer ?
Olivier C force-pushed external_vdb from 0962efcfc0 to 2024845082 2023-12-15 14:16:11 +01:00 Compare
Olivier C commented 2023-12-15 14:16:48 +01:00
Author
Contributor
Copy Link

A "BAT pack" is something completely different than "packed files inside a blend file".

On which of this feature does the @skip_packed decorator act?

I thought that @skip_packed allowed to skip any BAT-processing if a given block was already "Blender packed" (since I thought that being "Blender packed" meant that the block was already inside the .blend file and hence no external inclusion was needed by BAT).

To move forward, I propose to merge this with an expanded comment regarding the @skip_packed.

> A "BAT pack" is something completely different than "packed files inside a blend file". On which of this feature does the `@skip_packed` decorator act? I thought that `@skip_packed` allowed to skip any BAT-processing if a given block was already "Blender packed" (since I thought that being "Blender packed" meant that the block was already inside the .blend file and hence no external inclusion was needed by BAT). To move forward, I propose to merge this with an expanded comment regarding the `@skip_packed`.
Sybren A. Stüvel commented 2023-12-15 16:40:55 +01:00
Member
Copy Link

A "BAT pack" is something completely different than "packed files inside a blend file".

On which of this feature does the @skip_packed decorator act?

It makes BAT skip resources that are packed inside the blend file.

I thought that @skip_packed allowed to skip any BAT-processing if a given block was already "Blender packed" (since I thought that being "Blender packed" meant that the block was already inside the .blend file and hence no external inclusion was needed by BAT).

That's correct.

To move forward, I propose to merge this with an expanded comment regarding the @skip_packed.

I wouldn't accept that, as it makes BAT work around an issue in Blender. If Blender cannot deal properly with packed VDB files, it's something to fix in Blender. Or simply don't pack your VDB files when submitting to Flamenco.

> > A "BAT pack" is something completely different than "packed files inside a blend file". > > On which of this feature does the `@skip_packed` decorator act? It makes BAT skip resources that are packed inside the blend file. > I thought that `@skip_packed` allowed to skip any BAT-processing if a given block was already "Blender packed" (since I thought that being "Blender packed" meant that the block was already inside the .blend file and hence no external inclusion was needed by BAT). That's correct. > To move forward, I propose to merge this with an expanded comment regarding the `@skip_packed`. I wouldn't accept that, as it makes BAT work around an issue in Blender. If Blender cannot deal properly with packed VDB files, it's something to fix in Blender. Or simply don't pack your VDB files when submitting to Flamenco.
Olivier C commented 2023-12-15 17:33:00 +01:00
Author
Contributor
Copy Link

Fair enough. Do you think that the issue blender/blender#100592 pointed out by @ariane-genty is the right one for this bug? Or should another issue be opened? (The issue only mentions Eevee, but I believe that Cycles is affected as well)

Thank you for your time to help us debug the root cause of this issue.

Fair enough. Do you think that the issue https://projects.blender.org/blender/blender/issues/100592 pointed out by @ariane-genty is the right one for this bug? Or should another issue be opened? (The issue only mentions Eevee, but I believe that Cycles is affected as well) Thank you for your time to help us debug the root cause of this issue.
Sybren A. Stüvel added 3 commits 2024-01-02 16:28:28 +01:00
Sybren A. Stüvel commented 2024-01-02 16:28:55 +01:00
Member
Copy Link

Yeah, I think that report should be enough.

BAT should properly handle non-Blender-packed VDB files just fine though, and if it doesn't that requires fixing. I'll push some changes to this PR to reinstate the commented-out @skip_packed, as then I can just land it ;)

Yeah, I think that report should be enough. BAT should properly handle non-Blender-packed VDB files just fine though, and if it doesn't that requires fixing. I'll push some changes to this PR to reinstate the commented-out `@skip_packed`, as then I can just land it ;)
Sybren A. Stüvel approved these changes 2024-01-02 16:29:42 +01:00
Sybren A. Stüvel manually merged commit ced4af98a3 into main 2024-01-02 16:36:53 +01:00
Olivier C deleted branch external_vdb 2024-01-10 16:32:26 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
Sybren A. Stüvel
Labels
Clear labels   
legacy module
Platforms, Builds, Tests & Devices

  
legacy project
Blender Asset Tracer

  
legacy project
Platform: Windows

  
Priority::Normal

   
Status::Archived

   
Status::Confirmed

   
Status::Needs Triage

   
Status::Resolved

   
Type::Design

   
Type::Report

   
Type::To Do
No Label
legacy module
Platforms, Builds, Tests & Devices
legacy project
Blender Asset Tracer
legacy project
Platform: Windows
Priority::Normal
Status::Archived
Status::Confirmed
Status::Needs Triage
Status::Resolved
Type::Design
Type::Report
Type::To Do
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
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-asset-tracer#92884
No description provided.
Delete Branch "oliverpool/blender-asset-tracer:external_vdb"

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?