Nodes: Panel declarations for grouping sockets #108649
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#108649
Loading…
Reference in New Issue
No description provided.
Delete Branch "LukasTonne/blender:node-socket-categories"
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?
Adds an optional list of "categories" to node trees. Each socket can be
assigned a category. Sub-panels will be created in the future for these
categories.
Categories are stored in a list in node trees, next to socket declarations.
Each category has a name, but it does not have to be unique. In future
a category might also store whether it is visible by default and similar
information.
C API and RNA API are both added. Categories and their socket
assignments are accessible to users through another list in the "Group"
tab of the node editor sidebar.
Sockets are assigned a category by an index into the category list.
These indices are updated when categories are added, removed,
or moved in the list.
Sockets of the same category will remain together even when adding,
removing, or moving sockets or categories, renaming, etc.
A socket can be moved up or down within a category but each category
remains a contiguous block. Actual tree views may be created later.
BLI_listbase_sort_r
instead ofBLI_listbase_sort
to store userdata and avoid temporary category index storage for sorting (if still needed after switching to ListBase?)bNodeSocketPanel
tobNodePanel
bNodeSocket
for node tree interfaces, make serializable socket declarations, construct a tree structure reflecting "ownership" of sockets by panelsLooks good! Mostly just small style things inline. Thanks for using an array instead of a linked list.
@ -543,2 +543,4 @@
* \{ */
/**
* Run this after relevant changes to categories
This comment fits on one line
@ -552,0 +568,4 @@
/**
* Add a new socket category to the node tree.
* \param name Name of the new category.
There's meant to be a colon after the name here:
\param name:
@ -3758,2 +3808,4 @@
namespace blender::bke {
/* Fix socket category indices after changes. */
static void remap_socket_categories(bNodeTree &ntree, std::function<int(int)> index_fn)
Better to use
FunctionRef
thanstd::function
here I think, for the reasons mentioned in the header.Not needed any more, sockets now store category UID instead of simple index, so category changes don't require remapping.
@ -3760,0 +3829,4 @@
bNodeSocketCategory *old_categories_array = ntree->socket_categories_array;
const Span<bNodeSocketCategory> old_categories = ntree->socket_categories();
ntree->socket_categories_array = MEM_cnew_array<bNodeSocketCategory>(
ntree->socket_categories_num + 1, "socket categories");
IMO
__func__
for the allocation string is nicer than specifying one manually. It just takes up less visual space. I've also seen other developers preferring that too.@ -3760,0 +3833,4 @@
++ntree->socket_categories_num;
const MutableSpan<bNodeSocketCategory> new_categories = ntree->socket_categories_for_write();
for (const int i : new_categories.index_range().drop_back(1)) {
Don't have a strong preference, but this seems a bit more idiomatic and clear to me:
std::copy(old_categories.begin(), old_categories.end(), new_categories.data());
@ -3167,0 +3201,4 @@
{
bNodeTree *ntree = (bNodeTree *)ptr->owner_id;
bNodeSocketCategory *category = (bNodeSocketCategory *)value.data;
Super picky but this newline isn't helpful IMO, better to more closely connect the variable declaration and the null check.
Category declaration in node trees and sockets for grouping socketsto Nodes: Category declarations for grouping sockets@ -246,0 +269,4 @@
tree = snode.edit_tree
categories = tree.socket_categories
# Remember index to move the item
Period at the end of the comment.
@ -957,0 +1009,4 @@
active_category = tree.socket_categories.active
if active_category is not None:
layout.prop(active_category, "name")
I know I started this with the socket list, but I'm regretting it now. Maybe we can skip exposing the name property separate from the list. It should be clear that you can rename in the list.
Not sure, we may want to add more settings to categories, such as options to close categories by default, doc strings, etc.
Ideally i'd like to display the node tree declarations in a way that resembles the final node output, to make it more intuitive. The vertical UI layout of the node editor sidebar makes that a bit challenging. But lets say we could arrange it any way we like, then you'd have inputs on the left and outputs on the right. Categories would show in a kind of tree view matching the panels in modifiers. Socket details and category details would be displayed close to active element.
Hmm, all I mean is that there's no need to have the separate name property outside of the list. Not that displaying more options doesn't make sense.
For he other stuff, the inputs and outputs used to be side-by-side, but it doesn't work well with Blender's single-column UI layout paradigm. But that can be discussed more later.
I want to add a simple
int identifier
to categories, so that after adding/removing/moving categories i can remap the expand flag bits. Otherwise these category actions will mess up the open/closed state of panels, because the same flag index refers to a different panel.EDIT: The IDs have been added, but updating the
ui_expand_flag
after category changes is proving ugly: It requires a list of category ids along with the open/closed state flags, so that old flags can be mapped to categories and find their new indices. Decided to do this in a later PR, possibly after further improvements to the panel system. In the meantime panels might open or close unintentionally when changing categories, which seems acceptable.I think these should be called "panels" rather than "socket categories"? Not sure why we need another name for this.
And I would also call them just "panels" or "node panels" rather than "socket panels", to leave open the option of putting other things in there in the future.
I don't think we should be moving in this direction for DNA. With custom data layers we are running into the problem that Python API pointers are becoming invalid when they are deleted. This will have a similar problem.
Using "Panel" seems fine to me too. Another option is "Group", which at least assumes a bit less about how they'll be used in the future.
Not sure, like was discussed in #107500, I think the proper solution for that is to use some other identifier for RNA. Using an array simplifies any code that deals with this data, it's too bad if we can't do that anywhere. And we'll never remove that problem this way anyway, since we'll always have some arrays in DNA like attribute elements, etc.
Group is too confusing with node groups I think.
I didn't expect these to be used for anything except panels, do you have anything in mind?
I'm not sure identifiers are practical as a general solution to this. Every struct then would need to contain an identifier, lookup would be slow or you'd need to additionally maintain a hash map. And the RNA pointer to a node panel would potentially need to be a node node tree pointer + node identifier + panel identifier?
I'd rather not change the convention until we have a well defined alternative.
Hmm, I guess not, but I was thinking that it's a bit weird to drag "Panels" around if we present them in a tree layout like this:
I think the existing struct would work fine (though fair point that the alternative isn't defined very well at this point).
PointerRNA.data
could be the panel/category identifier, the rest of the struct would be the same. I don't think abNode
would be necessary there-- the panels are properties of the node tree.Using an array of pointers (
CategoryPanel **
) in DNA is probably a fine compromise too, gives us pointer stability but also type safety (Span<CategoryPanel *>
andSpan<const CategoryPanel *>
). It would just makes ownership unclear but can we really ask for that when we're using DNA? :PAn array of pointers if fine with me.
I'm ok with "panel" too. It's a UI term, but since UI layout is the only thing we're going to use it for i think that's justified. "Category" was just the first thing i could think of. "Group" is overused and confusing.
I can change this to linked lists, but this seems to be an unsettled argument? I like the simplicity of arrays, but can see how our python implementation struggles with them.
Do we actually have any guarantees regarding validity of objects in RNA? It seems kind of implicit right now that most objects remain valid until their ID block is freed (zero users, reload, undo).
There's no explicit rule, but it's certainly something that many scripts do and should be able to rely on.
abae706c9b
tof172423441
ebbd9629ac
to3d1aae86c5
@LukasTonne any reason they are called "node socket panells" rather than "node panels"?
The scope of this is a bit unclear. It started out as panels to organize sockets, if it's supposed to be a more general thing i can change that.
I can imagine in the future add-ons or native Blender nodes could add text or buttons in these panels. Some cases will be covered by making more things available as sockets like enums and color ramps, but that won't cover everything.
@LukasTonne Could you use an array of pointers in DNA rather than
ListBase
? That gives much better type safety without losing the pointer stability.@ -543,2 +543,4 @@
* \{ */
/** Run this after relevant changes to panels to ensure sockets remain sorted by panel. */
void ntreeEnsureSocketInterfacePanelOrder(bNodeTree *ntree);
This is currently in the public API because the
NODE_OT_tree_socket_move
operator works directly on DNA and needs to call this after it moves sockets. That operator should also use an API method so the update function does not need to be exposed. I suggest a separate fix.I've addressed the issues mentioned here:
bNodeSocketPanel
tobNodePanel
Do we want to hide this as "experimental" until we add actual panels and/or tree views?
I think that makes sense. Adding a poll for the new panel and an experimental option seems good enough for now.
Nodes: Category declarations for grouping socketsto Nodes: Panel declarations for grouping socketsJust two small comments, otherwise this looks good.
@ -3661,0 +3706,4 @@
}
/* Store panel index for sorting. */
blender::Map<const bNodePanel *, int> panel_index_map;
VectorSet
and VectorSet::index_of` are probably a more natural choice here, with a bit less boilerplate.@ -657,2 +674,4 @@
blender::Span<const bNodeSocket *> interface_inputs() const;
blender::Span<const bNodeSocket *> interface_outputs() const;
blender::Span<bNodePanel *> panels() const;
panels()
should returnSpan<const bNodePanel *>
, so aconst bNodeSocket *
doesn't give you a mutablebNodePanel
@blender-bot build
@blender-bot build