Refactoring: Geometry Nodes: Enum items identifiers based on items names #117643

Open

Iliya Katushenock wants to merge 1 commits from mod_moder/blender:refactoring_enum_item_identifiers into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.

Iliya Katushenock commented

2024-01-29 21:19:29 +01:00

Member

Goal of this change is to be able to merge different enumerations by selection overlapped items.
Names of items is the way to compare them. While runtime of evaluation, identifiers of items need
to be in the same space as names, so them need to be based on the names. Names is unique in one
enumeration declaration, so name-based identifiers is unique too.
For old files, versioning code is added.

Goal of this change is to be able to merge different enumerations by selection overlapped items. Names of items is the way to compare them. While runtime of evaluation, identifiers of items need to be in the same space as names, so them need to be based on the names. Names is unique in one enumeration declaration, so name-based identifiers is unique too. For old files, versioning code is added.

Iliya Katushenock added the

label 2024-01-29 21:19:29 +01:00

Iliya Katushenock added 1 commit 2024-01-29 21:19:38 +01:00

ae83224427 init

Iliya Katushenock added this to the Nodes & Physics project 2024-01-29 21:19:48 +01:00

Hans Goudey requested review from Lukas Tönne 2024-01-31 20:40:22 +01:00

Lukas Tönne commented

2024-02-01 10:48:33 +01:00

Member

I'm not sure i understand the intention here: There is a static name VectorSet and the item identifier is now the index in this set rather than a fixed int. Wouldn't this pick a new identifier every time the name is changed? How does this set get initialized when loading a blend file? Does the name set keep growing during the same Blender session (static variable)? Why is this needed in the first place?

I'm not sure i understand the intention here: There is a static name `VectorSet` and the item identifier is now the index in this set rather than a fixed int. Wouldn't this pick a new identifier every time the name is changed? How does this set get initialized when loading a blend file? Does the name set keep growing during the same Blender session (static variable)? Why is this needed in the first place?

Iliya Katushenock commented

2024-02-01 13:59:25 +01:00

Author

Member

Wouldn't this pick a new identifier every time the name is changed?

Correct, identifier will be updated each time when name is changed. As i see, identifier is used only in evaluation pipeline, so its update shouldn't change something.

How does this set get initialized when loading a blend file?

I missed up that part, i'll have to add initialization part to blend-read function.

Does the name set keep growing during the same Blender session (static variable)?

Correct, not sure if this is problem, identifiers need to be used only internally, so there is should not be any problem.

Why is this needed in the first place?

Lets say we add Menu inputs for domains of different nodes. If two Evaluate on Domain will be connected to one group input, group interface have to display menu which is result of merging both enums as new one.
This might looking trivial, but if connected enums of different nodes (with different supported domains), result enums need to contains only shared items of source definitions.
So, for example:

One node domains (Eort Elements): Points, Edges, Faces, Curves.
Other one (Mesh to Points): Points, Edges, Faces.
Result will be: Points, Edges, Faces (Curves was not added).

To simplify evaluation part, int-representation of items need to be the same in any node (and if name is use as key to check overlapping), identifier need to be based on the name. So, previous examples now looks like:

One node domains (Eort Elements): 2, 6, 1, 3.
Other one (Mesh to Points): 2, 6, 1.
Result will be: 2, 6, 1 (3 was not added).

> Wouldn't this pick a new identifier every time the name is changed? Correct, identifier will be updated each time when name is changed. As i see, identifier is used only in evaluation pipeline, so its update shouldn't change something. > How does this set get initialized when loading a blend file? I missed up that part, i'll have to add initialization part to blend-read function. > Does the name set keep growing during the same Blender session (static variable)? Correct, not sure if this is problem, identifiers need to be used only internally, so there is should not be any problem. > Why is this needed in the first place? Lets say we add Menu inputs for domains of different nodes. If two `Evaluate on Domain` will be connected to one group input, group interface have to display menu which is result of merging both enums as new one. This might looking trivial, but if connected enums of different nodes (with different supported domains), result enums need to contains only shared items of source definitions. So, for example: - One node domains (Eort Elements): `Points, Edges, Faces, Curves`. - Other one (Mesh to Points): `Points, Edges, Faces`. - Result will be: `Points, Edges, Faces` (`Curves` was not added). To simplify evaluation part, int-representation of items need to be the same in any node (and if name is use as key to check overlapping), identifier need to be based on the name. So, previous examples now looks like: - One node domains (Eort Elements): `2, 6, 1, 3`. - Other one (Mesh to Points): `2, 6, 1`. - Result will be: `2, 6, 1` (`3` was not added).

Lukas Tönne commented

2024-02-01 14:21:03 +01:00

Member

If two Evaluate on Domain will be connected to one group input, group interface have to display mune which is result of merging both enums as new one.

Why does the group interface have to merge these enums? In the current design, connecting different enums results in a runtime error (red links). Only one type of menu connection is supported, changing this design is opening a big can of worms.

If i understand your motivation, you want to support built-in menus as well, with the complication that sometimes nodes use subsets of the overall enum. I don't have a good solution for that yet, but i don't think storing a static name list is going to work.

> If two Evaluate on Domain will be connected to one group input, group interface have to display mune which is result of merging both enums as new one. Why does the group interface have to merge these enums? In the current design, connecting different enums results in a runtime error (red links). Only one type of menu connection is supported, changing this design is opening a big can of worms. If i understand your motivation, you want to support _built-in_ menus as well, with the complication that sometimes nodes use subsets of the overall enum. I don't have a good solution for that yet, but i don't think storing a static name list is going to work.

Iliya Katushenock added a new dependency 2024-02-07 01:44:26 +01:00

#117609 Geometry Nodes: Enum socket for Evaluate on Domain node

Iliya Katushenock referenced this pull request

2024-02-10 17:23:14 +01:00

Geometry Nodes: Menu Switch Node #113445

This pull request has changes conflicting with the target branch.

source/blender/blenkernel/BKE_blender_version.h
source/blender/blenkernel/intern/node_enum_definition.cc
source/blender/blenloader/intern/versioning_400.cc
source/blender/makesdna/DNA_node_types.h
source/blender/nodes/geometry/nodes/node_geo_menu_switch.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u refactoring_enum_item_identifiers:mod_moder-refactoring_enum_item_identifiers

git checkout mod_moder-refactoring_enum_item_identifiers

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Blocks

#117609 Geometry Nodes: Enum socket for Evaluate on Domain node

blender/blender

Reference: blender/blender#117643

No description provided.