Geometry Nodes: Update node tool menus dynamically #112166

Hans Goudey · 2023-09-09T00:27:37+02:00

Hans Goudey commented

2023-09-09 00:27:37 +02:00

Remove the requirement of restarting Blender to refresh the
extended 3D view menus for node group changes. Also avoid
rebuilding the tree of relevant assets and catalogs on every
redraw, since parsing asset libraries, etc. could become more
expensive than we want. Those two goals combined mean we
have to be more rigorous in how we invalidate the cached
catalog tree.

The first main change required is to clear the tree as asset libraries
are being read, similar to other dynamic asset menus. This is done
with a 3D view header listener rather than a menu listener in this case.

However, that isn't enough, because there is an issue with the asset
system where the "all" library isn't updated when the current file library
changes. The solution here needs iteration, but two options are calling
AS_asset_library_load, or eagerly rebuilding the all library after asset
catalog changes.

The other necessity for dynamic updates is clearing the catalog tree
to be rebuilt when the node group "asset traits" are changed. This is
done with a new notifier type (with the goal of being a bit selective
about when we re-read assets). This also requires running the
"presave" callback that builds asset metadata when updating the
property. Otherwise saving the file and sending the notifier is
necessary, which is too confusing.

I would like feedback on the two solutions to the issue with the "all"
asset library and on the method of updating asset metadata on
property changes.

See #101778 Remove the requirement of restarting Blender to refresh the extended 3D view menus for node group changes. Also avoid rebuilding the tree of relevant assets and catalogs on every redraw, since parsing asset libraries, etc. could become more expensive than we want. Those two goals combined mean we have to be more rigorous in how we invalidate the cached catalog tree. The first main change required is to clear the tree as asset libraries are being read, similar to other dynamic asset menus. This is done with a 3D view header listener rather than a menu listener in this case. However, that isn't enough, because there is an issue with the asset system where the "all" library isn't updated when the current file library changes. The solution here needs iteration, but two options are calling `AS_asset_library_load`, or eagerly rebuilding the all library after asset catalog changes. The other necessity for dynamic updates is clearing the catalog tree to be rebuilt when the node group "asset traits" are changed. This is done with a new notifier type (with the goal of being a bit selective about when we re-read assets). This _also_ requires running the "presave" callback that builds asset metadata when updating the property. Otherwise saving the file and sending the notifier is necessary, which is too confusing. --- I would like feedback on the two solutions to the issue with the "all" asset library and on the method of updating asset metadata on property changes.

Screencast from 2023-09-08 18-28-15.webm

330 KiB

❤️ 1

Hans Goudey added 4 commits 2023-09-09 00:27:46 +02:00

81758a9ebc Geometry Nodes: Update node tool menus dynamically

TODO: There are issues with `AssetCatalogService`

7b20b1c32e Merge branch 'main' into node-operator-catalog-update

8d24b0731c Merge branch 'main' into node-operator-catalog-update

9834223093 Fixes

Hans Goudey requested review from Julian Eisel 2023-09-09 00:29:23 +02:00

Hans Goudey added this to the 4.0 milestone 2023-09-09 00:29:27 +02:00

~~Hans Goudey referenced this pull request 2023-09-09 00:30:04 +02:00~~

Node Tools Overview #101778

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

Julian Eisel commented

2023-09-12 14:24:50 +02:00

The first main change required is to clear the tree as asset libraries are being read, similar to other dynamic asset menus. This is done with a 3D view header listener rather than a menu listener in this case.

Seems reasonable. The UI should listen to updates and redraw when needed.

However, that isn't enough, because there is an issue with the asset system where the "all" library isn't updated when the current file library changes. The solution here needs iteration, but two options are calling AS_asset_library_load, or eagerly rebuilding the all library after asset catalog changes.

Think this should be handled by the asset system internally. So the all library should be updated to reflect the changes. We only need to rebuild the catalog tree, right? Should be fine.

The other necessity for dynamic updates is clearing the catalog tree to be rebuilt when the node group "asset traits" are changed. This is done with a new notifier type (with the goal of being a bit selective about when we re-read assets).

Seems fine as well.

This also requires running the "presave" callback that builds asset metadata when updating the property. Otherwise saving the file and sending the notifier is necessary, which is too confusing.

Instead of calling the presave callback, it's nicer to just have a regular function doing this that the presave callback can call.

> The first main change required is to clear the tree as asset libraries are being read, similar to other dynamic asset menus. This is done with a 3D view header listener rather than a menu listener in this case. Seems reasonable. The UI should listen to updates and redraw when needed. > However, that isn't enough, because there is an issue with the asset system where the "all" library isn't updated when the current file library changes. The solution here needs iteration, but two options are calling AS_asset_library_load, or eagerly rebuilding the all library after asset catalog changes. Think this should be handled by the asset system internally. So the all library should be updated to reflect the changes. We only need to rebuild the catalog tree, right? Should be fine. > The other necessity for dynamic updates is clearing the catalog tree to be rebuilt when the node group "asset traits" are changed. This is done with a new notifier type (with the goal of being a bit selective about when we re-read assets). Seems fine as well. > This also requires running the "presave" callback that builds asset metadata when updating the property. Otherwise saving the file and sending the notifier is necessary, which is too confusing. Instead of calling the presave callback, it's nicer to just have a regular function doing this that the presave callback can call.

👍 1

Hans Goudey added 3 commits 2023-09-12 23:16:54 +02:00

d1bc5fc3c3 Merge branch 'main' into node-operator-catalog-update

bc9e3ce2bf Cleanup: Remove unused code and switch to eager update solution

56bcb55924 Move asset metadata update to separate function

Julian Eisel commented

2023-09-14 14:14:39 +02:00

The AS_asset_library_rebuild_all() calls is not exactly what I meant. I think when calling functions affecting the catalog tree the asset system (as in, code under blender/asset_system) should make sure this is respected somehow. Not sure if it would immediately rebuild the tree, or set some flag to do the update later (some lazy update?).
I much prefer this to be handled internally as this is a data-update (not UI). I rather avoid explicit update function calls, they are brittle.

The `AS_asset_library_rebuild_all()` calls is not exactly what I meant. I think when calling functions affecting the catalog tree the asset system (as in, code under `blender/asset_system`) should make sure this is respected somehow. Not sure if it would immediately rebuild the tree, or set some flag to do the update later (some lazy update?). I much prefer this to be handled internally as this is a data-update (not UI). I rather avoid explicit update function calls, they are brittle.

Hans Goudey added 2 commits 2023-09-14 14:47:24 +02:00

ff8c78686c Merge branch 'main' into node-operator-catalog-update

3cfccf77ac Move rebuilding all library to internal asset system, cleanup

Julian Eisel reviewed 2023-09-14 15:32:52 +02:00

source/blender/asset_system/intern/asset_library_service.cc Outdated

						
				@ -198,0 +198,4 @@

				void AssetLibraryService::rebuild_all_library()

				{

				  if (all_library_) {

				    rebuild_all_library_ex(*all_library_, true);

Julian Eisel commented

2023-09-14 15:32:38 +02:00

I don't think catalogs should be reloaded after these changes? Keeping the catalog data updated is already done, I think we just care about updating the tree.

👍 1

HooglyBoogly marked this conversation as resolved

Hans Goudey added 1 commit 2023-09-14 15:56:16 +02:00

4a2487cffe Avoid unnecessarily reloading catalogs

Julian Eisel approved these changes 2023-09-14 17:33:22 +02:00

Hans Goudey added 1 commit 2023-09-14 17:42:15 +02:00

6a9e7d00fd Merge branch 'main' into node-operator-catalog-update

Hans Goudey merged commit cc83951951 into main

2023-09-14 17:43:40 +02:00

Hans Goudey referenced this issue from a commit

2023-09-14 17:43:40 +02:00

Geometry Nodes: Update node tool menus dynamically