{"action":"created","issue":{"url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1190","labels_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1190/labels{/name}","comments_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1190/comments","events_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1190/events","html_url":"https://github.com/ImageEngine/gaffer/pull/1190","id":57495143,"number":1190,"title":"Reimplemented Paths framework in C++","user":{"login":"johnhaddon","id":1133871,"avatar_url":"https://avatars.githubusercontent.com/u/1133871?v=3","gravatar_id":"","url":"https://api.github.com/users/johnhaddon","html_url":"https://github.com/johnhaddon","followers_url":"https://api.github.com/users/johnhaddon/followers","following_url":"https://api.github.com/users/johnhaddon/following{/other_user}","gists_url":"https://api.github.com/users/johnhaddon/gists{/gist_id}","starred_url":"https://api.github.com/users/johnhaddon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johnhaddon/subscriptions","organizations_url":"https://api.github.com/users/johnhaddon/orgs","repos_url":"https://api.github.com/users/johnhaddon/repos","events_url":"https://api.github.com/users/johnhaddon/events{/privacy}","received_events_url":"https://api.github.com/users/johnhaddon/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/ImageEngine/gaffer/labels/pr-externalDependency","name":"pr-externalDependency","color":"FFCCCC"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":14,"created_at":"2015-02-12T17:53:24Z","updated_at":"2015-02-27T10:53:23Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1190","html_url":"https://github.com/ImageEngine/gaffer/pull/1190","diff_url":"https://github.com/ImageEngine/gaffer/pull/1190.diff","patch_url":"https://github.com/ImageEngine/gaffer/pull/1190.patch"},"body":"This is the \"SceneHierarchy rewrite\" requested in #852. In reality, the SceneHierarchy code is barely touched, and it's the underlying Path and PathListingWidget classes which are rewritten in C++ for improved performance.\r\n\r\nPaths can still be implemented in Python (and several are still in Gaffer), but the performance critical Path, FileSystemPath and ScenePath are now implemented directly in C++. I've also replaced the old `Path.info()` method which returned a dictionary with a new pair of `Path::attributeNames()` and `Path::attribute()` methods. This allows a single attribute to be queried at a time (a requirement Ivan had a long while back) and works around the fact that we don't have a direct equivalent for dict in pure C++. Attributes must be derived from RunTimeTyped, which is the most flexible we can be within the Cortex framework, but is still more restrictive than the pure Python implementation. The old `info()` method isn't visible in C++ at all, but I've emulated it in the bindings to keep old Python code working, and also provided default implementations for the new attribute API that use the old `info()` method if its available. I hope this will ease the transition between the two implementations.\r\n\r\nThe PathListingWidget rewrite means that we must now link to Qt from GafferUI, and there are some details to be aware of there and which are documented in the comments for 6f4e139. I'll write a separate internal mail about the necessary environment changes at Image Engine. PathListingWidget columns are now implemented in C++ too, which means they have lost a little flexibility compared to their old Python counterparts, but they have much better performance. 808b1c0 and cbb6a3d demonstrate how to update code to deal with that.\r\n\r\nI don't think this is quite there yet, and I'm still waiting to see if the Travis tests are passing perfectly, but I wanted to get something up for discussion. In particular, I'd really like feedback on the following :\r\n\r\n- Is \"attribute\" a good name for \"something you can query about a Path\"? Are we worried about potential confusion with scene attributes? I didn't want to overload `info()` to avoid confusion with the old method, and also `infoNames()` sounds awful. I thought about \"property\", but that's got overlap with Qt. Any other ideas, or are you happy with attribute?\r\n- Many of the Path implementations which remain in Python are still implementing `info()` and relying on the emulation layer in the bindings to implement the attribute methods. That was useful to me in testing, but should I go through and replace them with the proper implementation. Or replace all but one, so we still have one testing the emulation?"},"comment":{"url":"https://api.github.com/repos/ImageEngine/gaffer/issues/comments/76375970","html_url":"https://github.com/ImageEngine/gaffer/pull/1190#issuecomment-76375970","issue_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1190","id":76375970,"user":{"login":"johnhaddon","id":1133871,"avatar_url":"https://avatars.githubusercontent.com/u/1133871?v=3","gravatar_id":"","url":"https://api.github.com/users/johnhaddon","html_url":"https://github.com/johnhaddon","followers_url":"https://api.github.com/users/johnhaddon/followers","following_url":"https://api.github.com/users/johnhaddon/following{/other_user}","gists_url":"https://api.github.com/users/johnhaddon/gists{/gist_id}","starred_url":"https://api.github.com/users/johnhaddon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johnhaddon/subscriptions","organizations_url":"https://api.github.com/users/johnhaddon/orgs","repos_url":"https://api.github.com/users/johnhaddon/repos","events_url":"https://api.github.com/users/johnhaddon/events{/privacy}","received_events_url":"https://api.github.com/users/johnhaddon/received_events","type":"User","site_admin":false},"created_at":"2015-02-27T10:53:23Z","updated_at":"2015-02-27T10:53:23Z","body":"I've pushed a fix for a crash which could occur if a plug was removed from its node, while being observed by a ScenePath which is then destroyed before the node is. I'm speculating that this will fix the crash that Ben observed in Caribou during testing, although I haven't reproduced his original problem.\r\n"}}