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

NodeView widget improvements #46

Open
4 tasks
sbordeyne opened this issue Dec 3, 2019 · 5 comments
Open
4 tasks

NodeView widget improvements #46

sbordeyne opened this issue Dec 3, 2019 · 5 comments

Comments

@sbordeyne
Copy link
Member

sbordeyne commented Dec 3, 2019

I have started merging the NodeView widget into ttkwidgets. This widget is intended to give a convenient interface to interact with, and display graphs (series of nodes connected to one another).

So far, the NodeView widget has the following features :

  • Node selection
  • Node creation
  • Connection creation

There's a lot working already, but it's a bit buggy at the moment, hence why I'd like some help. Bugs I've noticed include :

  • The bezier curves for connections don't show up properly, they are very wonky and do not display how the nodes relate to one another properly

  • Node dragging is also quite a bit wonky. It works, but the node is offset from the mouse pointer which makes it hard to use

  • Connections do not snap to the edge of the Nodes

  • It would be great if nodes would scale with the length of the text inside of it

  • The connection won't show if it is created when the node is created

  • The connections won't move with the nodes when the nodes are dragged

Also, the following features are desired :

  • Ability to export the graph into a dictionary. This dictionary should map nodes to other nodes, following the connections between them.

  • Ability to delete connections

  • Ability to delete nodes, and all connections to and from that node

  • Maybe some virtual events : <<NodeMoved>>, <<NodeSelected>>, <<ConnectionCreated>>, <<ConnectionDeleted>>, <<NodeDeleted>>, <<NodeCreated>>

@RedFantom
Copy link
Member

It's been a really long time since you've opened this issue, but I've looked at it now. Some of what you would like to implement is already in the ItemsCanvas widget, like the better dragging of items, thanks to Juliette's contributions.
I looked at combining the two widgets, but the ItemsCanvas is not set up in such a way that it may be easily extended to allow for more properties on an item. I think it would be best to build a completely new widget that does all the required things in just the one widget. Otherwise, there will be quite a bit of duplication and two widgets that do very similar things.

We could do the following:

  • Create a new widget, also called NodeView, following a structure similar to that of ItemsCanvas and the existing NodeView, but with a render function that handles all the drawing (and uses kwargs to allow only updating what has changed)
  • Use a dictionary to keep track of all the elements, using string identifiers in classic Tkinter style for the user to address and keep all the item ids and the like in the dictionary.
  • Add the sizing with text and allow multiple connections from each node.
  • Allow drawing the connections by clicking on the connection points and connecting each to a node.
  • Create the virtual events instead of using callbacks. Callbacks are messy. ItemsCanvas is a prime example of that.
  • Fix the bezier curves (and show a preview of them while creating the connections)

I would be up for that. I think it's a nice challenge. What do you think?

@sbordeyne
Copy link
Member Author

I quite like that. I already think that this should be a brand new widget (and it's quite useful to have for a variety of application, off the top of my head : designing state machines for visual scripting applications for instance)

I also agree on using virtual events, it's much cleaner than using callbacks because you can have an arbitrary number of callbacks bound to them. I already had some events mapped out.

Showing a preview of the bezier would be great, but there's a lot of calculations to do to create them, so I worry a bit about potential performance issues. If it runs smoothly, there's no reason not to implement it. Maybe we could add a config option to disable/enable bezier previews ?

Having multiple connections per node is great.

I still think the data should be able to be exported as a dictionary to make it easy to manipulate the data later on. It may be a good idea to make a dedicated tkinter-like variable for the GUI backend, that has a method to export as json/dict. That way the flow remains consistent with tkinter, while still providing an easy to understand interface to the node view widget.

@RedFantom
Copy link
Member

There would have to be a huge amount of state, from the node options to the connections. At least the following that I can see: coordinates, text, font, colors, number of connection points on the nodes, multi-connectable connection points...

There is the question of how to store these. This may be done in a simple connection matrix or a list instead. If I understand you correctly, you would like to allow the user to manipulate data (like create an instance of a Connection instance) and update the UI accordingly automatically.

That's a really modern concept, but it's not really how Tkinter works. Assuring the separation of data for each separate instance of a widget would be quite a bit harder. If it were done like Tkinter works, then there would be a way to request the options of a node just fine, but only through functions on the widget itself. There would not be separate classes holding the data. An example of that is the TimeLine widget.

There are things to be said for both implementations. For a widget as complicated as this would be, I think it would be worth it to try implementing a proper separation between data and UI.

@sbordeyne
Copy link
Member Author

I envision the data to be something like that :

[
{
"name" : "nodename", 
"position" : (x, y), 
"connect_to" : [1, 2],
"connect_from" : [],
"attributes":{<extra data defined by the programmer>}, 
"color" :"#38FD2A",
}, 
{similar dict}, 
{same}, 
] 

Each node would be put in a list whenever a new connection is made, the connect_to and connect_from keys are updated on the two connected nodes. Any extra data is held in the attributes key. That allows for easy manipulation of the data by the programmer, it's easy to export as JSON as well. What do you think?

@RedFantom
Copy link
Member

Yeah, that would basically be the list type of way to save the connections between nodes. It'll contain some data duplication, but having each node separately in an element like that would indeed work best.

Maybe I misunderstood earlier, but I thought you wanted to something like this:

class Node(object);
    def connect_to(self, other: Node):
        # instance attributes updated

and then when connect_to has been called the widget updates itself automatically, without any functions being called on the widget itself.

It's not impossible, it would require something like a dirty bit to be set on the data held so the widget knows what to redraw (Tkinter Canvases are too slow to redraw everything every cycle), but it's not really the Tkinter way. Then again, it's more Pythonic than the Tkinter-y way.

Honestly, I would be fine with either option. Consistency is extremely important, but I've been thinking about how many options this widget would have and how that would look: It might result in a bit of a learning curve with the widget.

I'm leaning closer to the path of consistency, though. What do you think?

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

No branches or pull requests

2 participants