opensim-org / opensim-gui

SimTK OpenSim graphical user interface and distribution.
Apache License 2.0
61 stars 32 forks source link

Navigator shows two offset frames with the same name #825

Open tkuchida opened 6 years ago

tkuchida commented 6 years ago

Steps to reproduce

Expected result

Each Component has a unique absolute path and, therefore, a unique name on each level of the tree.

Actual result

There are two items named "femur_r_offset" under the "femur_r" Body. The parent_frame of the patellofemoral_r Joint is "femur_r_offset", but which of the two "femur_r_offset" frames is not clear.

Environment and GUI version

AppVeyor artifact OpenSim-cedd8d22-2018-05-31-win64.exe

aymanhab commented 6 years ago

This boils down to how to allow editing joints. If we go down the ownership tree then the frames are not shown but joints can't be moved. Totally open to suggestions and the change is quite easy. As of now we go down from the top level model find all Frames whose BaseFrame is the Body and create frame nodes for them. In general, as long as we try to map the hierarchical ownership tree to flat list and not show full path names we'll likely have duplicates.

tkuchida commented 6 years ago

Anything would be an improvement over showing two identical items in the drop-down list.

aymanhab commented 6 years ago

Ok, may make more sense to move the frames under the joint. Will try this out.

On Thu, Jun 14, 2018, 12:50 PM Tom Uchida notifications@github.com wrote:

Anything would be an improvement over showing two identical items in the drop-down list.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/opensim-org/opensim-gui/issues/825#issuecomment-397417475, or mute the thread https://github.com/notifications/unsubscribe-auth/ADyccEbjx3xD7oS3SHff9C-bo9CT5Qn5ks5t8r6QgaJpZM4UXhOO .

aseth1 commented 6 years ago

Why do offset frames have to be "moved" anywhere? If you just display under the Component (Body or Joint) they belong to, doesn't this problem go away?

tkuchida commented 6 years ago

If you just display under the Component (Body or Joint) they belong to, doesn't this problem go away?

Only in the first screencap. Shown in the second screencap is the (single) drop-down box for specifying the parent_frame Socket by name. Uniqueness is guaranteed only for absolute paths, so either the drop-down box would need to be populated with absolute paths (which might be long), or there would need to be a separate field for specifying, e.g., the base Body (essentially the first part of the absolute path).

aseth1 commented 6 years ago

Popup lists should never be populated with identically named items - it should be an error case. The pop-up list could be populated with just component names if they are all unique, otherwise it could start working up the component's path until uniqueness is achieved between components with the same name. There has to be strategy that works because we know the component paths are unique.

tkuchida commented 6 years ago

Popup lists should never be populated with identically named items - it should be an error case.

I agree, and at least some APIs disallow identical entries in drop-down boxes because the user then has no way of knowing what they're selecting. Relying on the order of items in the drop-down box is also poor design because only the (non-unique) string is displayed when the drop-down box is collapsed. My guess is that you wouldn't have made this comment unless you thought my suggestions were incorrect. I agree that my previous comment was not worded precisely, so I have clarified below.

Suggestion 1: Populate a parent_frame drop-down box with absolute paths. Suggestion 2: Replace the current drop-down box with two drop-down boxes. The first box is populated with the absolute path to each Body. The second box is disabled if the first box is null; upon making a selection in the first box, the second box is enabled and populated with all frames whose base Body is the one selected in the first box.

aymanhab commented 6 years ago

Navigator issue has been fixed. Dropdowns are not fixed and are a side effect of using a flat list to represent a tree with duplicate names. Drop down contain objects rather than names so the names could be anything, we could find a place in the UI to show the full path names in the rare case that's needed. A dropdown with very long names that differ by one character are not sensible and are a disservice to the vast majority of users/models that don't have duplicates. Replacing dropdown with a text-field that users consciously populate maybe less friendly than current option to the majority. I'd lean to leave it as is but open to suggestions. For Rjagopal model we can make the names unique on our side.

tkuchida commented 6 years ago

I'd lean to leave it as is but open to suggestions.

I think it's bad design to populate a drop-down list with identical entries. My recommendation is to populate with full paths, which are guaranteed to be unique. Perhaps not a high priority for 4.0, particularly if the Rajagopal model is modified as you suggest.