Simulation nodes: UI for simulation state items in sidebar #106919
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
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
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
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
EEVEE & Viewport
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
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
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
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#106919
Loading…
Reference in New Issue
No description provided.
Delete Branch "LukasTonne/blender:geometry-nodes-simulation-item-ui"
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?
Selecting the input or output simulation node now shows a new panel in the node sidebar with a list of simulation state items. Items can be added, removed, or moved up and down the list. This updates the sockets on both simulation input and output nodes. This is similar to node group interfaces, except that simulation state items only have one list which generates both input and output sockets.
Resolves #105723
Resolve #105723: UI for simulation state items in sidebarto WIP Resolve #105723: UI for simulation state items in sidebar@ -572,6 +572,15 @@ class NodeTreeMainUpdater {
return true;
}
}
/* Check paired simulation zone nodes. */
Connecting the
__extend__
socket updates both input and output node, but only because of the blanketNTREE_CHANGED_LINK
handling which updates all nodes in the tree when a link is changed.Property changes of the output node don't update the paired input node on their own, so i had to add this special case check. Any better suggestions would be welcome.
@ -39,6 +39,13 @@ using namespace blender;
using blender::fn::ValueOrField;
using blender::nodes::SocketDeclarationPtr;
extern "C" void ED_node_type_draw_color(const char *idname, float *r_color);
Exposing socket type colors through
ED_node_type_draw_color
adds another bad level call. Since the same file already has other UI functions used this way it doesn't make things any worse at least.@ -55,2 +59,2 @@
if (STREQ(item.name, name)) {
return true;
for (const NodeSimulationItem &item : Span(args.sim->items, args.sim->items_num)) {
if (&item != args.item) {
This function is now also used to check for uniqueness of items that are already in the list. This check avoids detecting duplicates from the same socket.
@ -27,3 +27,2 @@
std::unique_ptr<decl::Geometry> decl = std::make_unique<decl::Geometry>();
decl->name = item.name;
decl->identifier = item.name;
decl->name = item.name ? item.name : "";
Assigning nullptr to a
std::string
is undefined behavior. We also ensure theitem.name
is never empty now, but it's better not to rely on that alone and check thechar *
as well.WIP Resolve #105723: UI for simulation state items in sidebarto WIP: Simulation nodes: UI for simulation state items in sidebarWIP: Simulation nodes: UI for simulation state items in sidebarto Simulation nodes: UI for simulation state items in sidebarI wonder if we could put the list in the existing
Node
sidebar tab instead of adding a new one. That might be a more consistent/expected place for it. What do you think about that?Would be a simple matter of changing the
bl_category = "Simulation"
of the panel. No strong opinion on this.@ -195,0 +224,4 @@
bl_label = "Add State Item"
bl_options = {'REGISTER', 'UNDO'}
default_socket_type = 'GEOMETRY'
Could support a type selection enum here, with a dropdown menu invoke.
@ -195,0 +258,4 @@
class NODE_OT_simulation_zone_item_move(SimulationZoneOperator, Operator):
'''Move a simulation state item up or down in list'''
"in the list"
SimulationStateItem
really doesn't feel like the right way to draw them in the UI. That information should probably only be accessible from the socket types. Could you retrieve the color from the socket type instead in Python?socket_type_items
enum items array being different from the existing socket items? If not, we could reuse the item array with a filter function.@ -28,2 +30,2 @@
decl->name = item.name;
decl->identifier = item.name;
decl->name = item.name ? item.name : "";
decl->identifier = buf;
How about
decl->identifier = "Item_" + std::to_string(item.identifier);
?Hah, i always forget how this number-to-string stuff works in C++, somehow find it easier to remember old BLI.
@ -55,2 +61,2 @@
if (STREQ(item.name, name)) {
return true;
for (const NodeSimulationItem &item : Span(args.sim->items, args.sim->items_num)) {
if (&item != args.item) {
This can be checked outside of the loop with
Span::contains_ptr()
I'm not sure what you mean here.
I'm checking the iterator here to avoid comparing it to itself, to prevent fake name collision. Otherwise having an existing item "XY" and entering the name "XY" would turn it into "XY.001".
Also note i've added C++ spans to the
NodeGeometrySimulationOutput
now to make this a bit nicer.Ah okay, didn't get the context fully here. Thanks.
@ -336,2 +322,4 @@
nodeRegisterType(&ntype);
}
bNode *NOD_geometry_simulation_output_find_node_by_data(bNodeTree *ntree,
NOD_geometry_simulation_output_find_node_by_data
is unused, and it doesn't seem like we should need this. If we do, I'd keep it local to RNA.Downside of putting this in RNA is that we don't have access to the C++ functions like
ntree->nodes_by_type
there.Removed the unused function though.
@ -338,0 +336,4 @@
bNode *NOD_geometry_simulation_output_find_node_by_item(bNodeTree *ntree,
const NodeSimulationItem *item)
{
for (bNode *node : ntree->nodes_by_type("GeometryNodeSimulationOutput")) {
These functions should call
ensure_topology_cache()
before they use it.@ -338,0 +367,4 @@
bool NOD_geometry_simulation_output_contains_item(NodeGeometrySimulationOutput *sim,
const NodeSimulationItem *item)
{
const int index = item - sim->items;
Implement with
Span::contains_ptr()
@ -338,0 +374,4 @@
NodeSimulationItem *NOD_geometry_simulation_output_get_active_item(
NodeGeometrySimulationOutput *sim)
{
if (sim->active_index >= 0 && sim->active_index < sim->items_num) {
A consistency suggestion: switch to return early in failure case, which is more common, and use
index_range.contains()
:@ -338,0 +392,4 @@
NodeSimulationItem *NOD_geometry_simulation_output_find_item(NodeGeometrySimulationOutput *sim,
const char *name)
{
for (const int i : blender::IndexRange(sim->items_num)) {
@ -338,0 +414,4 @@
{
NodeSimulationItem *old_items = sim->items;
sim->items = MEM_cnew_array<NodeSimulationItem>(sim->items_num + 1, __func__);
for (const int i : blender::IndexRange(0, index)) {
IndexRange(0, index)
->IndexRange(index)
Socket types are messy and complicated: the color there is a registerable property, which requires it to be an instance property and can't be a static method. Adding a property to a type can't be done with RNA, to my knowledge, it would have to be a custom-built python module, which seems overkill for this purpose.
We'd also need to have a direct way to map a
SimulationStateItem
to a socket class first, something like asocket_idname
method that returns a string and then do abpy.types
lookup. I would rather keep the RNA simple and do this internally.Seems generally fine, will leave the final review to hans.
For the color thing, I think I still prefer clarity/simplicity in the API and pushing the complexity to an RNA helper function. Just seems like
SimulationStateItem
should not be at all concerned with its color in the UI, that's the job of the node system socket types in general. But it's not a huge deal, and if you really prefer it that way that's okay.The rest of my comments are just small things.
@ -192,6 +193,97 @@ class NODE_OT_add_simulation_zone(NodeAddOperator, Operator):
return {'FINISHED'}
class SimulationZoneOperator():
What do you think about adding these operators to
geometry_nodes.py
instead ofnode.py
? They seem specific to geometry nodes.Didn't realize we had a dedicated operator py file for geonodes, will move it there.
@ -195,0 +208,4 @@
@classmethod
def poll(cls, context):
space = context.space_data
# needs active node editor and a tree to add nodes to
Comment style
@ -195,0 +233,4 @@
# Remember index to move the item
dst_index = min(node.active_index + 1, len(state_items))
# Empty name so it is based on the type only
Comment style: periods at the end of comments
@ -9790,0 +9973,4 @@
RNA_def_struct_ui_text(srna, "State Items", "Collection of simulation state items");
func = RNA_def_function(srna, "new", "rna_NodeGeometrySimulationOutput_items_new");
RNA_def_function_ui_description(func, "Add a state item to this simulation zone");
state item
->item
for UI text. "State" is mostly redundant for users in this context.@ -26,0 +36,4 @@
bool NOD_geometry_simulation_output_item_socket_type_supported(eNodeSocketDatatype socket_type);
/** Set a unique item name.
* @return True if the unique name differs from the original name.
Blender uses syntax like
\return
rather than@return
. Also for multi-line function docstring comments, the/**
should be on its own line at the start.@ -338,0 +324,4 @@
return blender::Span<NodeSimulationItem>(items, items_num);
}
blender::MutableSpan<NodeSimulationItem> NodeGeometrySimulationOutput::items_mutable_span()
I guess this is slightly different, but usually a method like this is called
items_for_write()
. Either way is fine with me.We already have
NodeSimulationItem *items
for the array, soSpan<NodeSimulationItem> items()
is a conflict, otherwiseitems
+items_for_write
would work.I opted for
items_span_for_write
now, if that's ok for you.@ -338,0 +501,4 @@
if (from_index == to_index) {
return;
}
else if (from_index < to_index) {
else after return
Problem is the
std_node_socket_colors
list is currently part of the editor code, so it requires this hacky bad-level call to access. I wouldn't mind getting rid of that, but that's out of scope here.