Geometry Node: Make Capture Attributes more lazy #105264

Iliya Katushenock · 2023-02-27T18:59:44+01:00

Iliya Katushenock commented

2023-02-27 18:59:44 +01:00

The more complex node trees become, the more often they are decomposed.
With abundant use of both downloaded assets and simply nodes made long
ago, some data may be unused. Therefore, there are extra calculations.
But laziness is not yet widespread enough everywhere to solve the problems
of complex projects. This commit is solving the task: #102511.

The changes can be described as follows. Group all setpoints of value-used
facts into a single milestone. It is important.

Reason: Related values such as geometry and field are always returned
together. That is, in order to be correct, we must strive to ensure that
the information about whether values are needed will also always be together.

We can't query the geometry. After that say - we do not need a field.
This means that when the geometry is created, the field will be in the status: maybe.
And because of that, it will still be computed.

This solution only gives hope for a successful outcome, in cases where
many nodes want a result, situations can be more complicated and less
pleasant. But I don't see an easy solution, because basically we would
like to return the geometry without the attribute (source node still
work and contain pointer to reterned geometry). Let her check this and
say if the attribute is needed (and source node will compute and add
this attribute to geometry). Sounds weird?

So the change just groups the geometry and field request into one milestone.

A more general task may be presented later (with list of all nodes that can by more lazier).

Example:

	Main	PR
No evaluation required when switching
Evaluation required when switching
No evaluation needed
Evaluation needed

Original Patch D17143

The more complex node trees become, the more often they are decomposed. With abundant use of both downloaded assets and simply nodes made long ago, some data may be unused. Therefore, there are extra calculations. But laziness is not yet widespread enough everywhere to solve the problems of complex projects. This commit is solving the task: https://projects.blender.org/blender/blender/issues/102511. The changes can be described as follows. Group all setpoints of value-used facts into a single milestone. It is important. Reason: Related values such as geometry and field are always returned together. That is, in order to be correct, we must strive to ensure that the information about whether values are needed will also always be together. We can't query the geometry. After that say - we do not need a field. This means that when the geometry is created, the field will be in the status: maybe. And because of that, it will still be computed. This solution only gives hope for a successful outcome, in cases where many nodes want a result, situations can be more complicated and less pleasant. But I don't see an easy solution, because basically we would like to return the geometry without the attribute (source node still work and contain pointer to reterned geometry). Let her check this and say if the attribute is needed (and source node will compute and add this attribute to geometry). Sounds weird? So the change just groups the geometry and field request into one milestone. A more general task may be presented later (with list of all nodes that can by more lazier). Example: | | Main | PR | | -------- | -------- | -------- | | No evaluation required when switching | ![image](/attachments/58ebcf08-04fe-41df-8c9d-9fd27c12eb05) | ![image](/attachments/8a67be8e-c840-4cf1-b68c-354aa42b2a33) | | Evaluation required when switching | ![image](/attachments/d4444c12-263b-4161-bd53-d9b1a2d5e96b) | ![image](/attachments/bdbae815-e44f-45b0-a99f-bf7c1e1695e0) | | No evaluation needed | ![image](/attachments/4c6e6e78-d085-4678-88cf-2692e9c0e785) | ![image](/attachments/23e1bf09-79ef-4b65-a973-158de57046ff) | | Evaluation needed | ![image](/attachments/b7d1677c-3d79-4a8b-b389-556c0998b367) | ![image](/attachments/c07a37c1-c964-48c2-8c36-945eed58540a) | Original Patch [D17143](https://archive.blender.org/developer/D17143)

image.png

106 KiB

Why is important for assets.blend

900 KiB

image.png

90 KiB

image.png

104 KiB

image.png

93 KiB

image.png

88 KiB

image.png

106 KiB

image.png

92 KiB

image.png

106 KiB

Iliya Katushenock added 1 commit 2023-02-27 18:59:55 +01:00

e045eb40aa init commit

Iliya Katushenock added this to the Nodes & Physics project 2023-02-27 19:00:58 +01:00

Iliya Katushenock requested review from Hans Goudey 2023-02-27 19:01:02 +01:00

Iliya Katushenock requested review from Jacques Lucke 2023-02-27 19:01:07 +01:00

Iliya Katushenock added 1 commit 2023-02-27 19:21:24 +01:00

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

73514e5327 Merge branch 'main' into more_lazynes_capture_attribute_node

Hans Goudey changed title from ~~Geometry Node: Make Capture Attributes more laziness~~ to Geometry Node: Make Capture Attributes more lazy

2023-02-28 04:38:34 +01:00

Hans Goudey approved these changes 2023-02-28 05:01:04 +01:00

Hans Goudey left a comment

If I understand correctly, the important part of the change is waiting to request the input field until after checking whether the output anonymous attribute is used. It would be good for the PR description to contain a high-level description of what's different like that.

Other than that, the code change looks correct to me.

If I understand correctly, the important part of the change is waiting to request the input field until after checking whether the output anonymous attribute is used. It would be good for the PR description to contain a high-level description of what's different like that. Other than that, the code change looks correct to me.

source/blender/nodes/geometry/nodes/node_geo_attribute_capture.cc

						
				@ -152,3 +155,3 @@

				  }

				  const std::string input_identifier = "Value" + identifier_suffix(data_type);

				  if (params.lazy_require_input(input_identifier) | params.lazy_require_input("Geometry")) {

Hans Goudey commented

2023-02-28 04:52:45 +01:00

Use || instead of |

Use `||` instead of `|`

Iliya Katushenock commented

2023-02-28 06:22:50 +01:00

A frequently used pattern to get both functions fired in one execution pass, and not just the first one with a true return

Hans Goudey commented

2023-02-28 14:22:22 +01:00

Ah, not sure I've noticed that before. Seems a bit hidden at first, but it makes sense. We'll see what Jacques thinks.

Iliya Katushenock commented

2023-02-28 14:31:30 +01:00

This seems to be used everywhere, for asking for multiple lazy inputs (in switch node for example).

Hans Goudey commented

2023-02-28 05:01:27 +01:00

@blender-bot build

Iliya Katushenock commented

2023-02-28 06:23:37 +01:00

Yes, a little later I will add a more material description of the changes

Iliya Katushenock commented

2023-02-28 17:28:35 +01:00

I hope the description does not suffer from the excessive complexity of the situation described there. It took me a long time to figure it out myself.

Also, the build bot seems to suffer from unrelated things.

I hope the description does not suffer from the excessive complexity of the situation described there. It took me a long time to figure it out myself. Also, the build bot seems to suffer from unrelated things.

Iliya Katushenock added 1 commit 2023-02-28 17:31:48 +01:00

06e456c64f Merge branch 'main' into more_lazynes_capture_attribute_node

Iliya Katushenock added 1 commit 2023-03-01 13:32:46 +01:00

6c2f42946e Merge branch 'main' into more_lazynes_capture_attribute_node

Iliya Katushenock referenced this pull request

2023-03-12 19:17:33 +01:00

Geometry Nodes: use lazy-function directly for switch node #105696

Iliya Katushenock closed this pull request

2023-04-04 21:11:52 +02:00