{"action":"created","issue":{"url":"https://api.github.com/repos/opensim-org/opensim-core/issues/1038","repository_url":"https://api.github.com/repos/opensim-org/opensim-core","labels_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/1038/labels{/name}","comments_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/1038/comments","events_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/1038/events","html_url":"https://github.com/opensim-org/opensim-core/pull/1038","id":161630539,"number":1038,"title":"Cleanup add geometry and attachment to frames","user":{"login":"chrisdembia","id":846001,"avatar_url":"https://avatars.githubusercontent.com/u/846001?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisdembia","html_url":"https://github.com/chrisdembia","followers_url":"https://api.github.com/users/chrisdembia/followers","following_url":"https://api.github.com/users/chrisdembia/following{/other_user}","gists_url":"https://api.github.com/users/chrisdembia/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisdembia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisdembia/subscriptions","organizations_url":"https://api.github.com/users/chrisdembia/orgs","repos_url":"https://api.github.com/users/chrisdembia/repos","events_url":"https://api.github.com/users/chrisdembia/events{/privacy}","received_events_url":"https://api.github.com/users/chrisdembia/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":21,"created_at":"2016-06-22T09:12:16Z","updated_at":"2016-06-22T18:11:07Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/opensim-org/opensim-core/pulls/1038","html_url":"https://github.com/opensim-org/opensim-core/pull/1038","diff_url":"https://github.com/opensim-org/opensim-core/pull/1038.diff","patch_url":"https://github.com/opensim-org/opensim-core/pull/1038.patch"},"body":"I am creating this PR for @aseth1 to replace #1037. The issue was that #1037 goes into master, even though it includes changes from a branch that has a separate PR.\r\n\r\nOriginal description from @aseth1:\r\n\r\nThis PR addresses most of the remaining issues of #172 and #914. The two main use cases it aims to support are:  1) adding geometry (e.g. bone mesh) that is fixed on a Frame which governs the transform of the geometry during visualization, and 2) add and move geometry that is not fixed to any Frame (e.g. representing experimental \"floating\" Markers) with the transform being an input.  \r\n\r\n`ModelComponent::addGeometry` was removed and `addComponent` can be used instead but one must either 1) connect the `Geometry` to `Frame` using its `Connector<Frame>` (via the convenience `setFrame()` does this) OR 2) satisfy the newly added `Geometry::Input<SimTK::Transform>` in which case the `Output` must be a transform of Geometry w.r.t and expressed in the `Ground`.  In the second case, you can add the Geometry to any Component like any other using `addComponent`.\r\n\r\nIt is also very handy to just *attach* `Geometry` directly to a `Frame`. Here *attach* specifically means fixed (not moving w.r.t.) to a `Frame`, which takes ownership and uses itself to satisfy the Geometry's `Connector<Frame>` (leaving the Input disconnected). The input `transform` can be used if there is no `Frame` satisfying the connector, in which case the `Geometry` is not *attached* to any Frame and is drawn according to its \"live\" transform w.r.t. Ground. \r\n\r\nThe current visualization code is dependent on (at least) decroative meshes being fixed to a mobilized body and my attempt to have all Geomtry display with its transform w.r.t ground was disastrous. @aymanhab assured that is was a very bad idea and that being fixed to a mobilized body was quite essential. A few floaty things seems OK.\r\n\r\nAt this  time there is one test failure `testVisualization` but it isn't clear what the test is doing and it may take awhile to figure out and debug. The exampleMain and Luxo example and other models appear to visualize fine, so we can at least begin the review process.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"35\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/opensim-org/opensim-core/1038)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/opensim-org/opensim-core/issues/comments/227829871","html_url":"https://github.com/opensim-org/opensim-core/pull/1038#issuecomment-227829871","issue_url":"https://api.github.com/repos/opensim-org/opensim-core/issues/1038","id":227829871,"user":{"login":"chrisdembia","id":846001,"avatar_url":"https://avatars.githubusercontent.com/u/846001?v=3","gravatar_id":"","url":"https://api.github.com/users/chrisdembia","html_url":"https://github.com/chrisdembia","followers_url":"https://api.github.com/users/chrisdembia/followers","following_url":"https://api.github.com/users/chrisdembia/following{/other_user}","gists_url":"https://api.github.com/users/chrisdembia/gists{/gist_id}","starred_url":"https://api.github.com/users/chrisdembia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chrisdembia/subscriptions","organizations_url":"https://api.github.com/users/chrisdembia/orgs","repos_url":"https://api.github.com/users/chrisdembia/repos","events_url":"https://api.github.com/users/chrisdembia/events{/privacy}","received_events_url":"https://api.github.com/users/chrisdembia/received_events","type":"User","site_admin":false},"created_at":"2016-06-22T18:11:06Z","updated_at":"2016-06-22T18:11:06Z","body":"\n\n\n\n\n\n\n\n*[OpenSim/Common/Component.h, line 2449 \\[r1\\]](https://reviewable.io:443/reviews/opensim-org/opensim-core/1038#-KKtIG3gti03_jkB32MB:-KKtIG3gti03_jkB32MC:-187437916) ([raw file](https://github.com/opensim-org/opensim-core/blob/ab3e357e69493b200be1659060cd6af7ebe32991/OpenSim/Common/Component.h#L2449)):*\n> ```objc\n>             else {\n>                 pathName = pathName + \"/\" + chan.second.getName();\n>             }\n> ```\n\nSeems there is a slightly better implementation:\n\n```cpp\nif (pathName.rfind(\"/\") != (pathName.length() - 1)) pathName += \"/\"\npathname += chan.second.getName()\n```\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/opensim-org/opensim-core/1038)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"chrisdembia"}}