Geometry Nodes: new repeat zone #109164

Merged
Jacques Lucke merged 98 commits from JacquesLucke/blender:serial-loop into main 2023-07-11 22:36:17 +02:00
Member

This adds support for running a set of nodes repeatedly. The number of iterations can be controlled dynamically as an input of the repeat zone.

image

The main use case is to replace long repetitive node chains with a more flexible alternative. Technically, repeat zones can also be used for many other use cases. However, due to their serial nature, performance is very sub-optimal when they are used to solve problems that could be processed in parallel. Better solutions for such use cases will be worked on separately.

Repeat zones are similar to simulation zones. The major difference is that they have no concept of time and are always evaluated entirely in the current frame, while in simulations only a single iteration is evaluated per frame.

Stopping the repetition early using a dynamic condition is not yet supported. "Break" functionality can be implemented manually using Switch nodes in the loop for now. It's likely that this functionality will be built into the repeat zone in the future. For now, things are kept more simple.

Remaining Todos after this first version:

  • Improve socket inspection and viewer node support. Currently, only the first iteration is taken into account for socket inspection and the viewer.
  • Make loop evaluation more lazy. Currently, the evaluation is eager, meaning that it evaluates some nodes even though their output may not be required.

image

This adds support for running a set of nodes repeatedly. The number of iterations can be controlled dynamically as an input of the repeat zone. ![image](/attachments/714057f0-02fa-4965-b266-2fee2d42d157) The main use case is to replace long repetitive node chains with a more flexible alternative. Technically, repeat zones can also be used for many other use cases. However, due to their serial nature, performance is very sub-optimal when they are used to solve problems that could be processed in parallel. Better solutions for such use cases will be worked on separately. Repeat zones are similar to simulation zones. The major difference is that they have no concept of time and are always evaluated entirely in the current frame, while in simulations only a single iteration is evaluated per frame. Stopping the repetition early using a dynamic condition is not yet supported. "Break" functionality can be implemented manually using Switch nodes in the loop for now. It's likely that this functionality will be built into the repeat zone in the future. For now, things are kept more simple. Remaining Todos after this first version: * Improve socket inspection and viewer node support. Currently, only the first iteration is taken into account for socket inspection and the viewer. * Make loop evaluation more lazy. Currently, the evaluation is eager, meaning that it evaluates some nodes even though their output may not be required. ![image](/attachments/b98b40e6-7875-40bf-bb9e-a455131e8bdf)
Jacques Lucke added 22 commits 2023-06-20 17:39:16 +02:00
Jacques Lucke added 2 commits 2023-06-21 10:39:59 +02:00
Jacques Lucke added 3 commits 2023-06-21 14:53:12 +02:00
Jacques Lucke added 5 commits 2023-06-22 09:38:45 +02:00
Jacques Lucke added 5 commits 2023-06-22 11:08:56 +02:00
Jacques Lucke added 1 commit 2023-06-22 11:21:12 +02:00
Jacques Lucke added 2 commits 2023-06-22 11:31:00 +02:00
Jacques Lucke added 4 commits 2023-06-22 12:03:37 +02:00
Jacques Lucke added 1 commit 2023-06-22 12:16:20 +02:00
Jacques Lucke added 19 commits 2023-06-23 12:59:42 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR109164) when ready.
Jacques Lucke added 4 commits 2023-06-26 11:23:26 +02:00
Jacques Lucke added 3 commits 2023-06-26 14:36:52 +02:00
Jacques Lucke added 4 commits 2023-06-27 12:08:00 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR109164) when ready.
Jacques Lucke changed title from WIP: Geometry Nodes: add support for serial loops to Geometry Nodes: add support for serial loops 2023-06-27 12:52:11 +02:00
Jacques Lucke requested review from Dalai Felinto 2023-06-27 12:52:46 +02:00
Jacques Lucke requested review from Lukas Tönne 2023-06-27 12:52:46 +02:00
Jacques Lucke requested review from Hans Goudey 2023-06-27 12:52:46 +02:00

At the moment if I mouse over the Menu entry for Serial Loop Zone it says "undocumented operator".

Also I'm not sure which category this should go to. In the future once we get Parallel Loop as well, would it also go to Layout?

At the moment if I mouse over the Menu entry for Serial Loop Zone it says "undocumented operator". Also I'm not sure which category this should go to. In the future once we get Parallel Loop as well, would it also go to Layout?

Why not replace the Simulation category with an Scopes category? (Simulation scopes, Serial Loops scopes, Parallel Loop scopes, ...).

Why not replace the Simulation category with an Scopes category? (Simulation scopes, Serial Loops scopes, Parallel Loop scopes, ...).
Jacques Lucke added 1 commit 2023-06-27 13:24:40 +02:00
Author
Member

Also I'm not sure which category this should go to. In the future once we get Parallel Loop as well, would it also go to Layout?

Yes, that's a good question. I don't really have a great answer yet. Right now I'd say yes.

Why not replace the Simulation category with an Scopes category? (Simulation scopes, Serial Loops scopes, Parallel Loop scopes, ...).

We call them zones, not scopes. Having a zones category is a possibility, but currently I'm not convinced that this is the way to go as they will all be kind of independent.

> Also I'm not sure which category this should go to. In the future once we get Parallel Loop as well, would it also go to Layout? Yes, that's a good question. I don't really have a great answer yet. Right now I'd say yes. > Why not replace the Simulation category with an Scopes category? (Simulation scopes, Serial Loops scopes, Parallel Loop scopes, ...). We call them zones, not scopes. Having a zones category is a possibility, but currently I'm not convinced that this is the way to go as they will all be kind of independent.

Having Simulation on its own category also makes it easier for assets to get added there. So not sure I would combine them both. I would combine the loops, not sure if on its own category or what.

Having Simulation on its own category also makes it easier for assets to get added there. So not sure I would combine them both. I would combine the loops, not sure if on its own category or what.
Hans Goudey reviewed 2023-06-28 04:28:08 +02:00
Hans Goudey left a comment
Member

I ran out of time before I got to geometry_nodes_lazy_function.cc, so I didn't read the interesting part yet. Just a couple small comments so far.

I ran out of time before I got to `geometry_nodes_lazy_function.cc`, so I didn't read the interesting part yet. Just a couple small comments so far.
@ -348,0 +384,4 @@
node = self.get_output_node(context)
loop_items = node.loop_items
# Remember index to move the item.
Member

This should copy the active type when possible. See 0f36f44db8

This should copy the active type when possible. See 0f36f44db85635bb8616e07b0ba6b6bcce4024af
JacquesLucke marked this conversation as resolved
@ -81,0 +86,4 @@
int iteration_;
public:
SerialLoopZoneComputeContext(const ComputeContext *parent, int output_node_id, int iteration);
Member

int -> int32_t

`int` -> `int32_t`
JacquesLucke marked this conversation as resolved
@ -101,1 +101,4 @@
}
if (ELEM(node.type, GEO_NODE_SERIAL_LOOP_INPUT, GEO_NODE_SERIAL_LOOP_OUTPUT)) {
aal::RelationsInNode &relations = scope.construct<aal::RelationsInNode>();
/* Add all possible relations for now. The inferencing algorithm has to be updated to detect a
Member

Feels more natural to me to write this as a "TODO" comment like
TODO: Add a smaller set of relations. For now, adding all possible relations is correct.

Feels more natural to me to write this as a "TODO" comment like `TODO: Add a smaller set of relations. For now, adding all possible relations is correct.`
JacquesLucke marked this conversation as resolved
@ -0,0 +47,4 @@
if (!output_node) {
return true;
}
NodeGeometrySerialLoopOutput &storage = *static_cast<NodeGeometrySerialLoopOutput *>(
Member

I'd go with auto here, keeps it on one line

I'd go with `auto` here, keeps it on one line
JacquesLucke marked this conversation as resolved
Hans Goudey reviewed 2023-06-28 14:17:48 +02:00
@ -1424,0 +1435,4 @@
IndexRange main_output_usages;
IndexRange border_link_usages;
Map<int, int> attribute_set_input_by_field_source_index;
Member

Could use some comments IMO, this name is pretty tricky to parse

Could use some comments IMO, this name is pretty tricky to parse
JacquesLucke marked this conversation as resolved
@ -1424,0 +1582,4 @@
Array<lf::ValueUsage> output_usages(body_outputs_num, lf::ValueUsage::Used);
Array<bool> set_outputs(body_outputs_num, false);
Array<bool> tmp_main_input_usages(loop_items_num);
Member

I think this would be easier to read if there were comments describing what each block of code did. Even if it's "obvious" when you read the code, it would help when skimming it. For example "Retrieve inputs from outside the loop" or something. "Destruct values that are not needed after the evaluation anymore" already does that here.

I think this would be easier to read if there were comments describing what each block of code did. Even if it's "obvious" when you read the code, it would help when skimming it. For example "Retrieve inputs from outside the loop" or something. "Destruct values that are not needed after the evaluation anymore" already does that here.
JacquesLucke marked this conversation as resolved
Jacques Lucke added 5 commits 2023-06-28 17:03:22 +02:00
Author
Member

@blender-bot build

@blender-bot build
Hans Goudey approved these changes 2023-06-28 19:15:55 +02:00
Hans Goudey left a comment
Member

This generally looks good to me.

For the timing overlay in the node editor, I wonder if it makes more sense to show the sum of all the node's time for all loop iterations.

This generally looks good to me. For the timing overlay in the node editor, I wonder if it makes more sense to show the sum of all the node's time for all loop iterations.
Member

Found a crash:

  1. Open the test file
  2. Attach the viewer to the rightmost "GraphColoring.Iteration" node group (ctrl+shift+click)

This is a little greedy graph coloring implementation i'm trying out. We probably want a built-in node for this stuff, but it's sufficiently loopy to work as a test.

Graph coloring is a necessary step for efficient constraint solving with Gauss-Seidel methods. It allows separating constraints into groups, within which each constraint is independent of all others. This way the solver can be parallelized as much as possible.

Found a crash: 1. Open the test file 1. Attach the viewer to the rightmost "GraphColoring.Iteration" node group (ctrl+shift+click) This is a little greedy graph coloring implementation i'm trying out. We probably want a built-in node for this stuff, but it's sufficiently loopy to work as a test. Graph coloring is a necessary step for efficient constraint solving with Gauss-Seidel methods. It allows separating constraints into groups, within which each constraint is independent of all others. This way the solver can be parallelized as much as possible.
Jacques Lucke added 3 commits 2023-06-29 12:36:11 +02:00
Member

Moving an item of the serial loop in the sidebar does not update the node sockets right away. After reloading the file the sockets get shuffled into the correct order.
image

Moving an item of the serial loop in the sidebar does not update the node sockets right away. After reloading the file the sockets get shuffled into the correct order. ![image](/attachments/b6d2a644-8063-4c94-89ec-f5df760bce5b)
Member

Duplicating the serial loop output node crashes Blender atm. Duplicating the input node un-pairs it.

The simulation zone nodes are both copied when either one of them is copied, and they remain paired. Would be worth implementing this for serial loops too, and possibly share some code.

Duplicating the serial loop output node crashes Blender atm. Duplicating the input node un-pairs it. The simulation zone nodes are both copied when either one of them is copied, and they remain paired. Would be worth implementing this for serial loops too, and possibly share some code.
Member

Loop-inside-loop seems to be slightly problematic: i implemented a bitonic sorting node group, which has an outer loop ("stage" from 0 to log2(n)) and an inner loop ("pass" descending from stage to 0). This works beautifully when it works, but about half the time the outer loop does not produce any Geometry.

Test file: sorting.blend

Sometimes (randomly) the geometry of the inner loop is not passed on to the outer loop. Tooltip says the inner loop Geometry output has all the points but the outer loop Geometry is empty. I could implement this with a single loop, but i assume nested loops should be working as well.
image

Loop-inside-loop seems to be slightly problematic: i implemented a [bitonic sorting](https://en.wikipedia.org/wiki/Bitonic_sorter) node group, which has an outer loop ("stage" from 0 to log2(n)) and an inner loop ("pass" descending from stage to 0). This works beautifully when it works, but about half the time the outer loop does not produce any Geometry. Test file: [sorting.blend](/attachments/54f499c3-e9fa-4156-90ab-4597f5e99446) <video src="/attachments/322d6bda-49d2-4da5-adf5-6fc3f466634b" title="2023-06-30 15-46-04.mp4" controls></video> Sometimes (randomly) the geometry of the inner loop is not passed on to the outer loop. Tooltip says the inner loop Geometry output has all the points but the outer loop Geometry is empty. I could implement this with a single loop, but i assume nested loops should be working as well. ![image](/attachments/ba42fa7b-5b87-46aa-a975-ac98e16ccc4b)
Author
Member

The simulation zone nodes are both copied when either one of them is copied, and they remain paired. Would be worth implementing this for serial loops too, and possibly share some code.

Are you sure that is implemented? Doesn't look like it.

> The simulation zone nodes are both copied when either one of them is copied, and they remain paired. Would be worth implementing this for serial loops too, and possibly share some code. Are you sure that is implemented? Doesn't look like it.
Jacques Lucke added 5 commits 2023-07-01 11:49:15 +02:00
Author
Member

This works beautifully when it works, but about half the time the outer loop does not produce any Geometry.

Thanks for discovering this. Nice thing was, that triggered an assert in a debug build. Should be fixed now.

> This works beautifully when it works, but about half the time the outer loop does not produce any Geometry. Thanks for discovering this. Nice thing was, that triggered an assert in a debug build. Should be fixed now.
Jacques Lucke added 1 commit 2023-07-01 12:04:53 +02:00
Member

The simulation zone nodes are both copied when either one of them is copied, and they remain paired. Would be worth implementing this for serial loops too, and possibly share some code.

Are you sure that is implemented? Doesn't look like it.

Hah, i should have checked. I thought i had implemented this but looks like that's not the case. Maybe was just an experiment in a branch. If the crash on copying outputs is fixed i guess that's good enough for now. Copying a single zone node is kinda useless right now because we don't yet have a "fix zone" operator, so maybe that could be added in future.

> > The simulation zone nodes are both copied when either one of them is copied, and they remain paired. Would be worth implementing this for serial loops too, and possibly share some code. > > Are you sure that is implemented? Doesn't look like it. Hah, i should have checked. I thought i had implemented this but looks like that's not the case. Maybe was just an experiment in a branch. If the crash on copying outputs is fixed i guess that's good enough for now. Copying a single zone node is kinda useless right now because we don't yet have a "fix zone" operator, so maybe that could be added in future.
Member

Works great and i didn't spot any more issues with the code.

For the timing overlay in the node editor, I wonder if it makes more sense to show the sum of all the node's time for all loop iterations.

I agree with Hans. The timings inside the loop are typically a factor of 1/N smaller than the timings outside, which makes it difficult to compare numbers and figure out which parts really are slow. Fast loops also fall into the "< 1ms" bracket so you don't get a whole lot of information. Ideally we'd also get some numbers about how often a particular node was executed, in case some branches are executed more frequently and skew the result. That can all happen later.

LGTM

Works great and i didn't spot any more issues with the code. > For the timing overlay in the node editor, I wonder if it makes more sense to show the sum of all the node's time for all loop iterations. I agree with Hans. The timings inside the loop are typically a factor of 1/N smaller than the timings outside, which makes it difficult to compare numbers and figure out which parts really are slow. Fast loops also fall into the "< 1ms" bracket so you don't get a whole lot of information. Ideally we'd also get some numbers about how often a particular node was executed, in case some branches are executed more frequently and skew the result. That can all happen later. LGTM
Lukas Tönne approved these changes 2023-07-01 13:40:43 +02:00
Jacques Lucke added 1 commit 2023-07-03 12:34:16 +02:00
Author
Member

FYI, I mainly waiting with merging this because we should have a more final sign off for the name "serial loop" (or discuss other alternatives).

FYI, I mainly waiting with merging this because we should have a more final sign off for the name "serial loop" (or discuss other alternatives).

As discussed today in the geometry-nodes daily meeting:

Name: Repeat (Zone)
Category: Utilities
Color: [0.181164, 0.082283, 0.028426, 0.200000]
Input/Output: The nodes will be "Repeat Input" and "Repeat Output"

Mockup of the color:
image

Rational:

  • Name: "Repeat" suggests the main target for this function, and it is more accesible/simple concept than Serial Loop (or even Loop)
  • Category: There is no other category that this belongs and we don't have many high level nodes in the Utilities category yet. The "Parallel Run" (formerly known as Parallel Loop, name still pending though) zone fits here as well.
  • Color: NLA/animation usess this yellowish color. NLA is the one place in Blender where we have the concept of loop, so it is nice to use a color in that hue.
  • Input/Output: This is the name we use already for Node Groups and Simulation. They behave the same, thus should be named the same.
As discussed today in the geometry-nodes daily meeting: Name: Repeat (Zone) Category: Utilities Color: [0.181164, 0.082283, 0.028426, 0.200000] Input/Output: The nodes will be "Repeat Input" and "Repeat Output" Mockup of the color: ![image](/attachments/99d0fe19-ab58-4081-8bca-02a353fd3d14) Rational: * Name: "Repeat" suggests the main target for this function, and it is more accesible/simple concept than Serial Loop (or even Loop) * Category: There is no other category that this belongs and we don't have many high level nodes in the Utilities category yet. The "Parallel Run" (formerly known as Parallel Loop, name still pending though) zone fits here as well. * Color: NLA/animation usess this yellowish color. NLA is the one place in Blender where we have the concept of loop, so it is nice to use a color in that hue. * Input/Output: This is the name we use already for Node Groups and Simulation. They behave the same, thus should be named the same.
Jacques Lucke added 4 commits 2023-07-04 18:08:06 +02:00
Jacques Lucke added 1 commit 2023-07-04 18:41:25 +02:00
increase integer ids to avoid crashing when loading files created with experimental branch
All checks were successful
buildbot/vexp-code-patch-coordinator Build done.
9dadf81f2e
Jacques Lucke changed title from Geometry Nodes: add support for serial loops to Geometry Nodes: new repeat zone 2023-07-04 18:48:27 +02:00
Author
Member

@blender-bot build

Note that due to the rename from Serial Loop Zone to Repeat Zone, files created with the experimental branch do not work out of the box anymore. I made it so that it doesn't crash when opening those files though. So one can manually replace the undefined nodes with a new Repeat Zone.

@blender-bot build Note that due to the rename from `Serial Loop Zone` to `Repeat Zone`, files created with the experimental branch do not work out of the box anymore. I made it so that it doesn't crash when opening those files though. So one can manually replace the undefined nodes with a new Repeat Zone.
Jacques Lucke requested review from Hans Goudey 2023-07-04 18:55:22 +02:00
Hans Goudey approved these changes 2023-07-04 19:01:52 +02:00
@ -353,0 +378,4 @@
class RepeatZoneItemAddOperator(RepeatZoneOperator, Operator):
"""Add a repeat item to the repeat zone"""
Member

I'd go with "item" instead of "repeat item" which sounds a bit unnatural but it's not a big deal either way.

I'd go with "item" instead of "repeat item" which sounds a bit unnatural but it's not a big deal either way.
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR109164) when ready.

It seems to be working as expected. I'm not crazy about the name iterations but I think it is fine.

It seems to be working as expected. I'm not crazy about the name `iterations` but I think it is fine.
Dalai Felinto approved these changes 2023-07-06 12:31:21 +02:00
Author
Member

@blender-bot build

@blender-bot build
Jacques Lucke added 1 commit 2023-07-11 12:46:24 +02:00
Merge branch 'main' into serial-loop
All checks were successful
buildbot/vexp-code-patch-coordinator Build done.
238f6ab98b
Jacques Lucke added 1 commit 2023-07-11 22:19:41 +02:00
Jacques Lucke merged commit 3d73b71a97 into main 2023-07-11 22:36:17 +02:00
Jacques Lucke deleted branch serial-loop 2023-07-11 22:36:18 +02:00
Contributor

Awesome. Is there any plan to add this to the compositor too?

Awesome. Is there any plan to add this to the compositor too?
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
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 project
No Assignees
7 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#109164
No description provided.