Add support for modifying link parent-child relationships through the UI

[gavanderhoorn]

Allow arbitrary re-ordering of links and joints using the property tree widget.

Options:

1. implement support for drag-and-drop
2. add UI elements (up/down-arrows?) that shift links around based on input from the user

[gavanderhoorn]

in both cases validity of the tree will need to be maintained. Two possible cases:

1. interchanging two adjacent links (across a single joint)
2. drag-and-dropping a link at some random position in the tree

Option 1 is relatively easy: parent becomes child.

Option 2 is more difficult: we can insert the link (together with a new joint) in the tree at the place where it is dropped, but what should be done with the joint-joint that is left where the link was dragged from. Perhaps only a joint-link pair should be drag-and-droppable (with joint always the upstream joint)?

[swhart115]

If I understand the last comment, option 2 seems to assume a different tree structure then what currently exists where things are listed as link-joint-link-joint-etc.

As it is implemented now, we have a list of Links and an expandable Chain of joints.

The Links have no "order" per se, so any drag-and-drop reordering should at most just change the location of the link in the underlying urdf, which would provide only a marginal, non-functional, change. Do we actually care about this?

Changing the order of joints in the Chain should not be too bad, the updated parent/child links for all joints down stream will need to be modified accordingly, and that should be it (possibly with some edge case adjustments).

Am I interpreting this correctly?

[gavanderhoorn]

The enhancement would indeed be to allow re-arranging the actual structure of the urdf, ie: changing parent-child relationships. As you noticed, re-ordering the links under Links does not really do that (and it doesn't even really change the order of link elements in the saved urdf, as the XML serialiser ignores that).

Changing the order of joints in the Chain should not be too bad, the updated parent/child links for all joints down stream will need to be modified accordingly [..]

I'm not sure I completely understand this: why do you think all downstream joints would need to be updated? I'd assume only immediate children of the changed joint would be affected? Or were you referring to those as downstream (and not all joints in the sub tree starting at the changed joint)?

[swhart115]

yes, sorry, that is what i meant: any downstream affected joints.

So then the goal of this ticket is to be able to move/adjust the order of joints in the Chain tree, but that is all?

[gavanderhoorn]

So then the goal of this ticket is to be able to move/adjust the order of joints in the Chain tree, but that is all?

Yes, that is the idea of the ticket. It's essentially an alternative way to do the same as in #48, but now with drag-and-drop, instead of dropdowns in the property browser widget.

[swhart115]

So let's ground this in an example, cause it's not clear to me what the resultant urdf should look like when we move a joint.

Let's say we have a chain that looks like this:
L1 -> J1 -> L2 -> J2 -> L3 -> J3 -> L4 -> J4 -> L5 -> J5 -> L6 -> J6 -> L7 -> J7 -> L8
and we want to move J5 between J2 and J3.
In this case we either 1) push down the J3 relationships, or 2) replace either J2 or J3(i.e., remove its child/parent link settings.)

1. would result in a new chain that looks like:
   L1 -> J1 -> L2 -> J2 -> L3 -> J5 -> L4 -> J3 -> L5 -> J4 -> L6 -> J6 -> L7 -> J7 -> L8
2. would result in a new chain that looks like:
   L1 -> J1 -> L2 -> J5 -> L3 -> J3 -> L4 -> J4 -> L5 -> N/A -> L6 -> J6 -> L7 -> J7 -> L8
   or
   L1 -> J1 -> L2 -> J2 -> L3 -> J5 -> L4 -> J4 -> L5 -> N/A -> L6 -> J6 -> L7 -> J7 -> L8

I'm not entirely sure what the desired/best resultant behavior is when doing this. 1) has the advantage of producing a valid chain, but the joints between where the joint was taken from and where it was inserted get affected in ways I don't think is appropriate (in the example, why should J3 and J4 change their parent/child links?). 2) has the advantage of "localizing" the changes, but results in "stranded" joints and links.

The other thing to consider is how we (could) drag and drop links. There isn't a way to do this right now cause the links in the tree view are just listed in isolation as mentioned in a previous comment. If the chains were displayed in terms of links instead of joints (which they arguably should be), then moving a link to a new location would require the user to set the requisite parent/child relationships of affected joints, but I also don't really see how to do this "automatically" unless the link takes its child joint with it. For example, consider moved L6 between L2 and L3 resulting in the following chain:
L1 -> J1 -> L2 -> J2 -> L6 -> J6 -> L3 -> J3 -> L4 -> J4 -> L5 -> J5 -> L7 -> J7 -> L8

Is what we ultimately want here a chain that has both joints and links and looks similar to the examples I wrote above? In this case, you could select some adjacent set of joints and links as a block and move them in their entirety to a new location, and then "rewiring" the parent and child relationships accordingly.

Thoughts?

[gavanderhoorn]

There are indeed two options (conceptually):

1. swap parent and child links: this allows the user to 'climb' up and down a tree / chain by repeatedly swapping parent for child (up) or child for parent (down) (if we're swapping 'parent').
2. drag-drop entire sub trees

Removing links or joints was never intended.

Perhaps we can make the idea of this issue/requirement more clear if we describe it as "re-parenting sub trees of the urdf". Dragging a link (and I agree with you, the chain (if we keep it) should display links) and dropping it somewhere else should update the parent of the dropped link to wherever it was dropped, and should take the entire sub tree starting at the dropped link with it.

This would be analogous to dragging and dropping directories in a file manager: all contents of the directory (files and any sub directories) will also be reparented (but basically because they only have one parent, namely the directory that was moved).

Conceptually, this would also cover the use-case where we want to be able to import and then attach 'sub urdfs/xacros', as is requested in #34.

[swhart115]

Ok, this is much clearer now. However, as stated, this suggests that both 1 and 2 are not currently possible with chains of Joints not Links. Should wait on this until we change the tree display accordingly? Should I make a ticket for this modification?

Also, thinking of it as a directory move mad this much clearer to me. Thanks

[gavanderhoorn]

This now good to go @swhart115. #73 fixed the tree.

[swhart115]

is there a preference for having up/down buttons for each Link item in the tree (on the right), or a single set of up/down arrows that lives outside the tree box and performs the operation on whatever link (or set of links) is selected?

[swhart115]

Aesthetics not withstanding, it would look something like this if we had up/down buttons for every link:

I can make it a little prettier (icons, square corners on the buttons, etc), but it won't look that much better, i'm guessing. ...and I think it doesn't looks very good.

[gavanderhoorn]

I agree this doesn't look good. But the buttons weren't a requirement, it was just meant as an example of how parent-child relationships could be manipulated through the gui.

From your earlier comments I understood that you wanted to go the drag-and-drop route. That would have my personal preference. Do you feel the buttons idea is better / more user-friendly?

[swhart115]

I was working on both drag and drop and buttons, as the ticket called for both.

Here's an alternative button option:

Of course these buttons would move the Link(s) that were selected in the tree.

[gavanderhoorn]

@swhart115 wrote:

I was working on both drag and drop and buttons, as the ticket called for both.

It'd be nice to have multiple ways to interact with the tree, but it's really not a requirement. I only listed buttons and drag-and-drop as options. I certainly did not mean that both are required. Seeing your prototypes, I feel drag-and-drop will make for a more intuitive UI actually.

[swhart115]

Ok, no problem. I'll focus on that then.

On Sun, Apr 24, 2016 at 7:46 AM G.A. vd. Hoorn notifications@github.com
wrote:

@swhart115 https://github.com/swhart115 wrote:

I was working on both drag and drop and buttons, as the ticket called for
both.

It'd be nice to have multiple ways to interact with the tree, but it's
really not a requirement. I only listed buttons and drag-and-drop as
options. I certainly did not mean that both are required. Seeing your
prototypes, I feel drag-and-drop will make for a more intuitive UI actually.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#26 (comment)

[swhart115]

I believe I have this working, with the exception of updating the Joint tree to reflect the new Link structure (post drag and drop). I'm still not convinced the Joints should be arranged in a tree structure, as we discussed above. I would think an arrangement where Links are arranged in a tree, Joints are listed flatly, and the parent/child joints are displayed as (text) properties of the Links might make more sense. Thoughts on this?

[gavanderhoorn]

I believe I have this working, with the exception of updating the Joint tree to reflect the new Link structure (post drag and drop). I'm still not convinced the Joints should be arranged in a tree structure, as we discussed above.

yeah, I've been thinking about that as well, and I think joints should be in a flat list. Having them in a tree doesn't really add anything (but that is just my personal opinion, so @Levi-Armstrong, any comments?).

Re-arranging parents/children should be done using the link tree then.

and the parent/child joints are displayed as (text) properties of the Links might make more sense.

I'm not sure I can picture this: "parent/child [..] displayed as (text) properties"? Is that different from what we have now (apart from the dropdowns)? Could you clarify?

Updating the tree post drag-drop should be a matter of shuffling the parent-child relationships of the QTreeWidgetItems around. That must unfortunately be done manually , as we lack a proper QTreeView (and data model) at the moment.

[swhart115]

what i mean is to add attributes to LinkProperty for "Parent" (that lists its parent joint), and "Children" (that lists all of its children joints). Although this info is not stored in the URDF, it is stored in the urdf::Link, and is the dual of the parent/joint properties of the joints that are displayed in our program.

I'm almost done with the re-shuffling (I have it working for updating the parent/child vars for the joints, and i'm working on updating the child_joint array and parent vars for the links, it's just nastier in terms of boost ptrs.) I hopefully will finish today and put in a PR for testing.

[swhart115]

I have this working with the exception of dealing with rearranging the root link, but I had a question on how to deal with this.

Some background: currently, you can drag links ON, ABOVE, or BELOW other links as it is processed in QtTreeWidgets. Dropping ON or BELOW will take the dragged link/subtree and make it a child of the dropped-on link and sibling to the dropped-on link's other (existing) children. Dropping ABOVE a link will make the dragged link/subtree a sibling to that link. This is all normal Qt drag-and-drop operation, and i have it working fine for everything except when the dropped-on location is the root (Links). Note that there is no default Qt behavior that inserts a link or subtree of links between two other links directly. This can only be accomplished by two drag-and-drop operations.

However, when dealing with the root link, it gets trickier because valid URDFs only allow one such root link. So there are two behaviors to consider:

1. When dropping a link/subtree ON or BELOW "Links", Qt will insert that link/subtree as a sibling of the previous root. This is not a valid URDF, but the user could then take one of those root siblings and re-attach it off of the other one to get a single root link again (and valid URDF).

2. When dropping a link/subtreeABOVE "Links", programmatically modify the QTreeWidget (and model) to make that link/subtree the new root link/subtree, with the previous root now child off of it. As I said, this is more difficult because it will require programmatically modifying the tree whereas other moves are processed automatically. It is also qualitatively different then how other drags-and-drops are handled.

Note that if we don't allow 1), then 2) would be the only way to create a new root link. Alternatively, we could allow 1), but not 2), and therefore allow temporarily invalid trees. I would vote for the latter option cause its easier to implement, even if the URDF is not valid.

Opinions?

[gavanderhoorn]

I think option 1 makes the most sense: we'd end up (temporarily) with a forest of two trees, but as you write, the user should rectify this by re-attaching one of the trees to the other.

It would be nice if we could give an indication to the user that this is what he/she must do. Right now this would probably just result in some warning being printed on the console, but some kind of message in the UI that the current model is invalid (maybe a statusbar updated? Taking the error message from the parser?) would be really needed. An option would be to create a signal (onRobotParseError() or something) that gets emitted whenever parsing the current model failed. A slot updating the status bar could then be connected to that and update the bar with a warning saying something like "/ ! \ the current model is not valid URDF and cannot be displayed" (or a similar message).

re: disallowing option 2: yes, I'd be in favour of that. Can we ignore the drop event if it is not on a child of Links? Alternatively: can we indicate the validity of a drop action to the user while he is dragging a subtree / link? A quick look at Drag and Drop - Drop Rectangles suggests we can. That would make it immediately clear where links / subtrees may be dropped (ie: not on Links).

[swhart115]

actually, as i think about it more, i'm not sure the insertion option ( 2) in my last comment ) makes sense because of the case where the dragged subtree has multiple leaf node links. In this case it isnt clear where the old root link should be moved to.

Given that this is the case, it really seems like allowing an invalid URDF structure (where there are multiple root links) is the most straight forward way of dealing with this, putting the impetus on the user to (then) fix the tree, or on the program to catch this on "save".

[swhart115]

sorry, didn't see your last post till after i posted my last post....

So i think we are in agreement. Is it alright if I finish with the implementation of 1) with just a command line output saying that the tree is invalid and push that through as a PR for the sake of the milestone 1 deadline? The embellishments could be further tickets/work.

In my implementation I already filter out "bad" drops.

Providing feedback during a drag is do-able is straightforward as well.

[gavanderhoorn]

@swhart115 wrote:

So i think we are in agreement. Is it alright if I finish with the implementation of 1) with just a command line output saying that the tree is invalid and push that through as a PR for the sake of the milestone 1 deadline? The embellishments could be further tickets/work.

I wouldn't necessarily call them 'embellishments': without those a user may become rather confused as to why his visualisation isn't updating. Especially if they don't check the console.

But let's leave the warning for later.

In my implementation I already filter out "bad" drops.

I wasn't necessarily referring to bad drops, but more real-time feedback showing whether a drop is allowed in a certain place in the tree.

[swhart115]

@gavanderhoorn Unfortunately, I can't reproduce the dropping of links outside the URDF tree issue you mentioned during the telecon. My mouse cursor changes to the "prohibited" icon when i leave the tree and won't register any of the (drop) events (and the links stay where they were).

[gavanderhoorn]

My mouse cursor changes to the "prohibited" icon when i leave the tree and won't register any of the (drop) events (and the links stay where they were).

yes, it's the same for me, unless I try to drag outside the application window (so onto my desktop fi). I get a cursor which is essentially a 'plus sign', and after releasing the mouse button, the links are removed from the tree and disappear.

[swhart115]

yeah, this isn't happening for me when i hover over the desktop or folders on my desktop or other applications or anything. This is odd. I'm assuming you are using standard Ubuntu?

Here is a quick video showing how I can't drop a link on my desktop:

nodrop.ogv.zip

[gavanderhoorn]

Just tried this again and can't reproduce it anymore, so let's ignore for now. If this becomes a problem we'll revisit it.

I also had the tree not updating sometimes after a drop, so perhaps I got it into a weird state or something.

[gavanderhoorn]

Fixed in #87 (as part of #97).