Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move default socket properties to update system #4515

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Durman
Copy link
Collaborator

@Durman Durman commented Jun 8, 2022

Addressed problem description

Initially it was conceived as performance improvement but on my test trees I was able to measure an improvement only with trees of 400+ nodes and only of 1%. Probably the improvement can be more significant on bigger lightweight trees. But also the new feature is introduced:

default linked property

We have sockets which store their default properties in their nodes. It involves using of socket.node api wich, as were said, takes O(len(tree.nodes)) time. Ve have alternative of storing default values in socket itself but switch all nodes to it will take a lot of time and most likely it will break work of Sverchok extensions.

The solution is to move extracting of default values to the update system to which fo find a socket owner is not a problem. Update system can read default value and put it into socket_data cache where it can found by the node. So the socket_data module will keep not only generated by the nodes data but also and default values now.

I have doubts of adding extra complexity to data management but it seems the less sockets and nodes use is_linked API the better.

Closes #4475

Preflight checklist

  • Code changes complete.
  • Code documentation complete.
  • Ready for merge.

@zeffii
Copy link
Collaborator

zeffii commented Jul 25, 2022

i have questions about this, or rather i'd like to make a suggestion.
I think moving from socket.prop_name = "..." to socket.user_prop; etc... is fine, but maybe we are offering up customizability of the slider needlessly.

 default_int_property: bpy.props.IntProperty(update=process_from_socket)
  • loses min / max / steps / description

I wonder if we could register a sverchok namespace/propertygroup and dynamically register properties in a single place, rather than inside a node, to facilitate a fast lookup + retain customizability of that slider.

@Durman
Copy link
Collaborator Author

Durman commented Jul 25, 2022

I'm not sure it's right place to discuss. The PR does not change the way to manage properties, it just changes the place to read them. And even this make things more complicated and I'm not in a hurry to merge this.

The main issue I guess is whether such a namespace can support efficient search of created properties. It can happen that it shares the same problem as keeping properties inside nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_prop_name
2 participants