Geometry Nodes: new repeat zone #109164
No reviewers
Labels
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
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#109164
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "JacquesLucke/blender:serial-loop"
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?
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.
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:
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
WIP: Geometry Nodes: add support for serial loopsto Geometry Nodes: add support for serial loopsAt 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, ...).
Yes, that's a good question. I don't really have a great answer yet. Right now I'd say yes.
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.
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.
This should copy the active type when possible. See
0f36f44db8
@ -81,0 +86,4 @@
int iteration_;
public:
SerialLoopZoneComputeContext(const ComputeContext *parent, int output_node_id, int iteration);
int
->int32_t
@ -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
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.
@ -0,0 +47,4 @@
if (!output_node) {
return true;
}
NodeGeometrySerialLoopOutput &storage = *static_cast<NodeGeometrySerialLoopOutput *>(
I'd go with
auto
here, keeps it on one line@ -1424,0 +1435,4 @@
IndexRange main_output_usages;
IndexRange border_link_usages;
Map<int, int> attribute_set_input_by_field_source_index;
Could use some comments IMO, this name is pretty tricky to parse
@ -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);
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.
@blender-bot build
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.
Found a crash:
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.
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.
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.
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.
Are you sure that is implemented? Doesn't look like it.
Thanks for discovering this. Nice thing was, that triggered an assert in a debug build. Should be fixed now.
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.
Works great and i didn't spot any more issues with the code.
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
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:
Rational:
Geometry Nodes: add support for serial loopsto Geometry Nodes: new repeat zone@blender-bot build
Note that due to the rename from
Serial Loop Zone
toRepeat 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.@ -353,0 +378,4 @@
class RepeatZoneItemAddOperator(RepeatZoneOperator, Operator):
"""Add a repeat item to the repeat zone"""
I'd go with "item" instead of "repeat item" which sounds a bit unnatural but it's not a big deal either way.
@blender-bot package
Package build started. Download here when ready.
It seems to be working as expected. I'm not crazy about the name
iterations
but I think it is fine.@blender-bot build
Awesome. Is there any plan to add this to the compositor too?