WIP: Prototype: Geometry Nodes: data block references list per node instance #117635

Jacques Lucke · 2024-01-29T16:07:09+01:00

Jacques Lucke commented

2024-01-29 16:07:09 +01:00

This tests a slightly different version of #117043 where each bake node instance has its own data block list. The following features are implemented:

View data blocks in node editor side bar.
Only writeback mapping to original data if bake node has actually baked anything.
Automatically clear data block references when freeing the bake.

The intention is to merge this into #117043 if it turns out to be the better solution.

This tests a slightly different version of #117043 where each bake node instance has its own data block list. The following features are implemented: * View data blocks in node editor side bar. * Only writeback mapping to original data if bake node has actually baked anything. * Automatically clear data block references when freeing the bake. ![image](/attachments/200a0039-7867-4157-badd-ca629fdf1e5c) The intention is to merge this into #117043 if it turns out to be the better solution.

image.png

29 KiB

Jacques Lucke added 63 commits 2024-01-29 16:07:22 +01:00

initial data block map b5e21c1e12

initial dna items 9406988542

Merge branch 'main' into bake-data-block-map 248f7ac56d

fix after merge c7e0700a7d

Merge branch 'main' into bake-data-block-map 76ad528fc9

Merge branch 'main' into bake-data-block-map 21773e8dda

add weak material references to mesh 24ebd01803

initial code to restore materials 521b88d92c

initial data block map implementation 2e3b46c8e7

progress 468b84e3f7

Merge branch 'main' into bake-data-block-map c4b54d4eb9

initial write-back to original modifier 5b3793d29e

serialize and load material names 2e7288a2f6

Merge branch 'main' into bake-data-block-map 4cf9725701

Merge branch 'main' into bake-data-block-map 4f8cb41558

use more general API for synchronous writebacks 6c2382f6b4

Merge branch 'main' into bake-data-block-map 20d01eed37

Merge branch 'main' into bake-data-block-map db9e2d314c

cleanup e8daa0d337

cleanup 9d089a007c

cleanup 78fd3eab91

Merge branch 'main' into bake-data-block-map d8c3d1de9c

Merge branch 'main' into bake-data-block-map 59c281c9c2

add missing data blocks items on original and evaluated modifier bb2c6c8c28

store id type as part of identifier 7e45613cc2

improve write-back 7d6a1994d7

add comment dad54bb749

Merge branch 'main' into bake-data-block-map 5114bfaab7

Merge branch 'main' into bake-data-block-map d27dba9b1f

add rna for data block mapping b041086e5c

initial data block list afdd1bf6b4

add ability to change data block 69c19d28c3

automatically evaluation again in case it's necessary f31af48eec

improve list ui e1e4789018

Merge branch 'main' into bake-data-block-map d2d992a4ef

Cleanup: Comment style b12c66eb84

Cleanup: Comment wording cd93334bda

Cleanup: Unnecessary namespace specification 846525224f

Cleanup: Unnecessary struct keyword f09fe2334d

Cleanup: Pass Span instead of const Vector & 089e59eda2

Cleanup: Make variable private 350c5fa00f

Merge branch 'main' into bake-data-block-map 3fcd489276

improve naming d7f064b35f

initial (non working remove operator) d12a16d5be

Merge branch 'main' into bake-data-block-map 4b7f1f2e5c

add remove operator 6a4be8db29

improve description

buildbot/vexp-code-patch-lint Build done.

Details

buildbot/vexp-code-patch-linux-x86_64 Build done.

Details

buildbot/vexp-code-patch-windows-amd64 Build done.

Details

buildbot/vexp-code-patch-darwin-arm64 Build done.

Details

buildbot/vexp-code-patch-darwin-x86_64 Build done.

Details

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

Details

0420547c21

Merge branch 'main' into bake-data-block-map c8b847737d

show data block selector directly in list 4e392c805f

Merge branch 'main' into bake-data-block-map 9922570227

fix after merge 7adc1ae0ec

run writeback in each pass e770ef1b5a

move sync_writeback api to depsgraph 5cbc39da31

store writeback tasks on depsgraph 696817fce8

Merge branch 'main' into bake-data-block-map 10bdf7b1f7

store mapping per node instance ead8ec84e4

add sync writeback as parameter to depsgraph evaluation functions 48b1eec072

Merge branch 'bake-data-block-map' into bake-data-block-map-instance fb499cdb5c

create data block map per instance a2f31f53e5

initial writeback to bake data 72a696cb04

show list in node sidebar 95b636f3d8

remove dead code 29e14c21fa

automatically clear data block list in bake node 096acff702

Jacques Lucke added 1 commit 2024-01-29 16:38:45 +01:00

support automatic data block list for simulations 3a099b3001

Simon Thommes requested changes 2024-01-29 17:48:59 +01:00

Simon Thommes left a comment

Thanks for addressing this so quickly, to me this feels a lot better now!

So far I haven't been able to find a situation where the automatic population and clearing of the list fails.

Obviously now, that the list is per bake and only visible in the node editor for the time being, it's a lot more hidden. Which now, that I'm quite confident that the automatic solution can just work, should be acceptable imo.

At least now the data is also clearly associated with the rest of the properties of the bake.

On the topic of having a separate list or one in the UI, I can see exposing a superset of all datablock references in the modifier for convenience, while still keeping the ground truth per bake, so that they can be edited on a lower level. E.g importing the same bake with different materials (or in the future, replacing the geometry that an instancer is referencing).

But that can come later imo, since now the reference data is accessible in the same way the bake properties are and for the vast majority of currently possible use-cases it should not be necessary to access. In the future this would change though and we'll need more UI.

I'd still like to hear from @brecht about the split list per bake, since the current approach is different from what we last discussed. To me the current approach is reasonable though.

The one thing that I'd like to have added in this patch a title Referenced Data, potentially by putting this UI in a subpanel.

Thanks for addressing this so quickly, to me this feels a lot better now! So far I haven't been able to find a situation where the automatic population and clearing of the list fails. Obviously now, that the list is per bake and only visible in the node editor for the time being, it's a lot more hidden. Which now, that I'm quite confident that the automatic solution can just work, should be acceptable imo. At least now the data is also clearly associated with the rest of the properties of the bake. On the topic of having a separate list or one in the UI, I can see exposing a superset of all datablock references in the modifier for convenience, while still keeping the ground truth per bake, so that they can be edited on a lower level. E.g importing the same bake with different materials (or in the future, replacing the geometry that an instancer is referencing). But that can come later imo, since now the reference data is accessible in the same way the bake properties are and for the vast majority of currently possible use-cases it should not be necessary to access. In the future this would change though and we'll need more UI. I'd still like to hear from @brecht about the split list per bake, since the current approach is different from what we last discussed. To me the current approach is reasonable though. The one thing that I'd like to have added in this patch a title `Referenced Data`, potentially by putting this UI in a subpanel.

Brecht Van Lommel commented

2024-01-29 18:15:41 +01:00

I'm fine with multiple lists. I can see some use cases when importing the same thing multiple times and you want different materials for each.

It's less convenient if you have multiple bake nodes just because you have some multi-step baking of the same geometry. But I guess in that case only the last bake node needs to be handled anyway.

I'm fine with multiple lists. I can see some use cases when importing the same thing multiple times and you want different materials for each. It's less convenient if you have multiple bake nodes just because you have some multi-step baking of the same geometry. But I guess in that case only the last bake node needs to be handled anyway.

👍 1

Donovane commented

2024-01-30 03:28:13 +01:00

First-time contributor

You must be explicit due to baking and his possibilty, he should be look more like :

Bake solution 1	Bake solution 2	Bake solution 3
Baked frame : 84	Baked frames : From 1 to 90 (Instead 1 - 90)	Baked multiples frames : 1 and 5 and 10
Only for one baked frames	Only for baked frames from X to X (Frames))	Only for multiples frames baked at different times

All bake solution (Enumered in table content (Is needed to merge 3 solutions in one and automatically detected by blender and update the text in the node and bake the good result choosen by the user in terms of the solution choosen by the user (Solution 1 or *Solution 2 or Solution 3

So in my proposal only the solution 3 are newed ! The rest are the same ! Expect updating the text in the node instead use char use text is more good for the user understand what he do ! use "From X To X" instead " - " for baking The solution 2 (Because actually we use the char " - " and is not good for user friendly.

You must be explicit due to baking and his possibilty, he should be look more like : ----- | Bake solution 1 | Bake solution 2 | Bake solution 3 | | :--------: | :--------: | :--------: | | Baked frame : **84** | Baked frames : **From 1 to 90** (Instead 1 - 90)| Baked multiples frames : **1 and 5 and 10** | | **Only for one baked frames**| **Only for baked frames from X to X (Frames)**)| **Only for multiples frames baked at different times** ----- |All bake solution (Enumered in table content (Is needed to merge 3 solutions in one and automatically detected by blender and update the text in the node and bake the good result choosen by the user in terms of the solution choosen by the user (**Solution 1** or **Solution 2* or **Solution 3**| |-----| > So in my proposal only the solution 3 are newed ! The rest are the same ! Expect updating the text in the node instead use char use text is more good for the user understand what he do ! use **"From X To X" instead " - "** for baking The solution 2 (Because actually we use the char " - " and is not good for user friendly.

Jacques Lucke commented

2024-01-30 15:50:10 +01:00

@dodo-2 this is unfortunately not related to what this patch is about. Currently, we (intentionally) only support baking still frames or frame ranges, but not multiple independent frames.

Will close this, because I'm merging the change into #117043. Thanks for the feedback on this approach.

@dodo-2 this is unfortunately not related to what this patch is about. Currently, we (intentionally) only support baking still frames or frame ranges, but not multiple independent frames. ----- Will close this, because I'm merging the change into #117043. Thanks for the feedback on this approach.

👍 1

Jacques Lucke closed this pull request

2024-01-30 15:50:12 +01:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

WIP: Prototype: Geometry Nodes: data block references list per node instance #117635

Pull request closed