{"action":"created","comment":{"url":"https://api.github.com/repos/ImageEngine/gaffer/pulls/comments/77981628","id":77981628,"diff_hunk":"@@ -335,3 +385,139 @@ IECore::ConstCompoundObjectPtr LightToCamera::computeProcessedAttributes( const\n \n \treturn result;\n }\n+\n+void LightToCamera::hashSetNames( const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const\n+{\n+\tSceneElementProcessor::hashSetNames( context, parent, h );\n+\t// We always just add a __cameras set if it doesn't yet exist, without spending time checking\n+\t// whether it will be non-empty ( usually there will already be a __cameras set anyway )\n+}\n+\n+\n+IECore::ConstInternedStringVectorDataPtr LightToCamera::computeSetNames( const Gaffer::Context *context, const ScenePlug *parent ) const\n+{\n+\tConstInternedStringVectorDataPtr inputSetNamesData = inPlug()->setNamesPlug()->getValue();\n+\tconst std::vector<InternedString> &inNames = inputSetNamesData->readable();\n+\n+\tif( std::find( inNames.begin(), inNames.end(), g_camerasSetName ) != inNames.end() )\n+\t{\n+\t\treturn inputSetNamesData;\n+\t}\n+\n+\tInternedStringVectorDataPtr resultData = inputSetNamesData->copy();\n+\tresultData->writable().push_back( g_camerasSetName );\n+\treturn resultData;\n+}\n+\n+void LightToCamera::hashSet( const IECore::InternedString &setName, const Gaffer::Context *context, const ScenePlug *parent, IECore::MurmurHash &h ) const\n+{\n+\tif( setName != g_camerasSetName && setName != g_lightsSetName )\n+\t{\n+\t\th = inPlug()->setPlug()->hash();\n+\t\treturn;\n+\t}\n+\n+\tSceneElementProcessor::hashSet( setName, context, parent, h );\n+\tinPlug()->setPlug()->hash( h );\n+\n+\n+\t// This setup is copied from Prune\n+\n+\t// The sets themselves do not depend on the \"scene:path\"\n+\t// context entry - the whole point is that they're global.\n+\t// However, the PathFilter is dependent on scene:path, so\n+\t// we must remove the path before hashing in the filter in\n+\t// case we're computed from multiple contexts with different\n+\t// paths (from a SetFilter for instance). If we didn't do this,\n+\t// our different hashes would lead to huge numbers of redundant\n+\t// calls to computeSet() and a huge overhead in recomputing\n+\t// the same sets repeatedly.\n+\t//\n+\t// See further comments in acceptsInput\n+\tContextPtr c = filterContext( context );\n+\tc->remove( ScenePlug::scenePathContextName );\n+\tContext::Scope s( c.get() );\n+\tfilterPlug()->hash( h );\n+}\n+\n+\n+GafferScene::ConstPathMatcherDataPtr LightToCamera::computeSet( const IECore::InternedString &setName, const Gaffer::Context *context, const ScenePlug *parent ) const\n+{\n+\tConstPathMatcherDataPtr inputSetData = inPlug()->setPlug()->getValue();\n+\n+\tbool isLightSet = setName == g_lightsSetName;\n+\tbool isCameraSet = setName == g_camerasSetName;\n+\tif( !( isLightSet || isCameraSet ) )\n+\t{\n+\t\treturn inputSetData;\n+\t}\n+\n+\tConstPathMatcherDataPtr lightSetData;\n+\tif( isLightSet )\n+\t{\n+\t\tlightSetData = inputSetData;\n+\t}\n+\telse\n+\t{\n+\t\tContextPtr tmpContext = new Context( *Context::current(), Context::Borrowed );\n+\t\ttmpContext->set( ScenePlug::setNameContextName, g_lightsSetName );\n+\t\tContext::Scope scopedContext( tmpContext.get() );\n+\t\tlightSetData = inPlug()->setPlug()->getValue();\n+\t}\n+\n+\tconst PathMatcher &lightSet = lightSetData->readable();\n+\n+\n+\tPathMatcherDataPtr outputSetData = inputSetData->copy();\n+\tPathMatcher &outputSet = outputSetData->writable();\n+\n+\tContextPtr tmpContext = filterContext( context );\n+\tContext::Scope scopedContext( tmpContext.get() );\n+\n+\tfor( PathMatcher::RawIterator pIt = lightSet.begin(), peIt = lightSet.end(); pIt != peIt; )","path":"src/GafferScene/LightToCamera.cpp","position":477,"original_position":174,"commit_id":"197b643aa0f21a38124c2c8ffe51597bc5db9cd4","original_commit_id":"31d78f08461abe347a8ac74193513fb5c2fd4728","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},"body":"This logic looks wrong to me, and more complicated than necessary. Here's an alternative stab at it :\r\n\r\n```\r\nfor( PathMatcher::Iterator pIt = lightSet.begin(), peIt = lightSet.end(); pIt != peIt; ++pIt )\r\n{\r\n    const int m = filterPlug()->getValue();\r\n    if( m & Filter::ExactMatch )\r\n    {\r\n        if( isLightSet )\r\n        {\r\n            outputSet.removePath( *pIt );\r\n        }\r\n        else\r\n        {\r\n            outputSet.addPath( *pIt );\r\n        }\r\n    }\r\n}\r\n```\r\n\r\nHow does that look?","created_at":"2016-09-08T10:29:59Z","updated_at":"2016-09-08T10:29:59Z","html_url":"https://github.com/ImageEngine/gaffer/pull/1836#discussion_r77981628","pull_request_url":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1836","_links":{"self":{"href":"https://api.github.com/repos/ImageEngine/gaffer/pulls/comments/77981628"},"html":{"href":"https://github.com/ImageEngine/gaffer/pull/1836#discussion_r77981628"},"pull_request":{"href":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1836"}}},"pull_request":{"url":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1836","id":83560485,"html_url":"https://github.com/ImageEngine/gaffer/pull/1836","diff_url":"https://github.com/ImageEngine/gaffer/pull/1836.diff","patch_url":"https://github.com/ImageEngine/gaffer/pull/1836.patch","issue_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1836","number":1836,"state":"open","locked":false,"title":"Support looking through lights, round 2","user":{"login":"danieldresser","id":4043098,"avatar_url":"https://avatars.githubusercontent.com/u/4043098?v=3","gravatar_id":"","url":"https://api.github.com/users/danieldresser","html_url":"https://github.com/danieldresser","followers_url":"https://api.github.com/users/danieldresser/followers","following_url":"https://api.github.com/users/danieldresser/following{/other_user}","gists_url":"https://api.github.com/users/danieldresser/gists{/gist_id}","starred_url":"https://api.github.com/users/danieldresser/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danieldresser/subscriptions","organizations_url":"https://api.github.com/users/danieldresser/orgs","repos_url":"https://api.github.com/users/danieldresser/repos","events_url":"https://api.github.com/users/danieldresser/events{/privacy}","received_events_url":"https://api.github.com/users/danieldresser/received_events","type":"User","site_admin":false},"body":"Started trying out John's suggestion on #1830 \r\n\r\nLooks like it's basically working.  The LightToCamera node adds camera objects on lights, and adjusts transforms to get the projections right ( still need to set clipping planes properly ).\r\n\r\nNext step is how would this actually connect up to the scene view?  We wouldn't want to actually render this scene with extra cameras added to it and transforms adjusted, so we would need to merge the camera we want to render with back into the scene.\r\n\r\nOne possibility:  use a Group node to group the whole scene into a __cameraRoot group, set it's visibility to false, and then parent it back to /.  Then the scene view just needs to prepend /__cameraRoot on the front of the camera path, and it will pick up an accurate camera if you give it a light.\r\n\r\nPossible issues with this approach:\r\n* Requires support for rendering from invisible camera ( currently works fine with GL view, and I think we decided that the long term convention was going to be that rendering from invisible cameras is OK? )\r\n* Andrew thinks it's horrifying to have an extra copy of the scene under __cameraRoot in an internal processing network in the scene view.  I'm not sure if it's horrifying - I feel like relying on lazy evaluation to mean that we will just pick out the camera path we're interested in without evaluating extra stuff might be a gaffery thing to do?  It would be completely invisible to users anyway.\r\n* When looking through a light, you still want to see the light viewport visualiser ( so you can see penumbra angle at least, and more stuff on the IE visualisers ), so I would have to modify the way that SceneView::LookThrough::updateLookThroughCamera() sets m_view->hideFilter().  I'm not quite sure why this was needed in the first place, the camera visualiser doesn't seem to show up when the view location is right at its origin anyway.  Maybe I need to treat objects in the __lights set differently?\r\n\r\nOr maybe you've got a way better idea how this is all supposed to work?  Andrew suggested adding cameras as children of the lights instead of replacing them - I'm really hoping that if I need to do hierarchy manipulation, it will be doable using existing Gaffer nodes rather than implementing custom hierarchy manipulation.\r\n","created_at":"2016-09-01T02:23:30Z","updated_at":"2016-09-08T10:29:59Z","closed_at":null,"merged_at":null,"merge_commit_sha":"2246b4979ceb4227c2e8beb3f3846bc468bf5c75","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1836/commits","review_comments_url":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1836/comments","review_comment_url":"https://api.github.com/repos/ImageEngine/gaffer/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/1836/comments","statuses_url":"https://api.github.com/repos/ImageEngine/gaffer/statuses/197b643aa0f21a38124c2c8ffe51597bc5db9cd4","head":{"label":"danieldresser:lookThroughLight","ref":"lookThroughLight","sha":"197b643aa0f21a38124c2c8ffe51597bc5db9cd4","user":{"login":"danieldresser","id":4043098,"avatar_url":"https://avatars.githubusercontent.com/u/4043098?v=3","gravatar_id":"","url":"https://api.github.com/users/danieldresser","html_url":"https://github.com/danieldresser","followers_url":"https://api.github.com/users/danieldresser/followers","following_url":"https://api.github.com/users/danieldresser/following{/other_user}","gists_url":"https://api.github.com/users/danieldresser/gists{/gist_id}","starred_url":"https://api.github.com/users/danieldresser/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danieldresser/subscriptions","organizations_url":"https://api.github.com/users/danieldresser/orgs","repos_url":"https://api.github.com/users/danieldresser/repos","events_url":"https://api.github.com/users/danieldresser/events{/privacy}","received_events_url":"https://api.github.com/users/danieldresser/received_events","type":"User","site_admin":false},"repo":{"id":12474128,"name":"gaffer","full_name":"danieldresser/gaffer","owner":{"login":"danieldresser","id":4043098,"avatar_url":"https://avatars.githubusercontent.com/u/4043098?v=3","gravatar_id":"","url":"https://api.github.com/users/danieldresser","html_url":"https://github.com/danieldresser","followers_url":"https://api.github.com/users/danieldresser/followers","following_url":"https://api.github.com/users/danieldresser/following{/other_user}","gists_url":"https://api.github.com/users/danieldresser/gists{/gist_id}","starred_url":"https://api.github.com/users/danieldresser/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danieldresser/subscriptions","organizations_url":"https://api.github.com/users/danieldresser/orgs","repos_url":"https://api.github.com/users/danieldresser/repos","events_url":"https://api.github.com/users/danieldresser/events{/privacy}","received_events_url":"https://api.github.com/users/danieldresser/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/danieldresser/gaffer","description":"Gaffer is an open source application framework designed specifically for creating tools for use in visual effects production.","fork":true,"url":"https://api.github.com/repos/danieldresser/gaffer","forks_url":"https://api.github.com/repos/danieldresser/gaffer/forks","keys_url":"https://api.github.com/repos/danieldresser/gaffer/keys{/key_id}","collaborators_url":"https://api.github.com/repos/danieldresser/gaffer/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/danieldresser/gaffer/teams","hooks_url":"https://api.github.com/repos/danieldresser/gaffer/hooks","issue_events_url":"https://api.github.com/repos/danieldresser/gaffer/issues/events{/number}","events_url":"https://api.github.com/repos/danieldresser/gaffer/events","assignees_url":"https://api.github.com/repos/danieldresser/gaffer/assignees{/user}","branches_url":"https://api.github.com/repos/danieldresser/gaffer/branches{/branch}","tags_url":"https://api.github.com/repos/danieldresser/gaffer/tags","blobs_url":"https://api.github.com/repos/danieldresser/gaffer/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/danieldresser/gaffer/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/danieldresser/gaffer/git/refs{/sha}","trees_url":"https://api.github.com/repos/danieldresser/gaffer/git/trees{/sha}","statuses_url":"https://api.github.com/repos/danieldresser/gaffer/statuses/{sha}","languages_url":"https://api.github.com/repos/danieldresser/gaffer/languages","stargazers_url":"https://api.github.com/repos/danieldresser/gaffer/stargazers","contributors_url":"https://api.github.com/repos/danieldresser/gaffer/contributors","subscribers_url":"https://api.github.com/repos/danieldresser/gaffer/subscribers","subscription_url":"https://api.github.com/repos/danieldresser/gaffer/subscription","commits_url":"https://api.github.com/repos/danieldresser/gaffer/commits{/sha}","git_commits_url":"https://api.github.com/repos/danieldresser/gaffer/git/commits{/sha}","comments_url":"https://api.github.com/repos/danieldresser/gaffer/comments{/number}","issue_comment_url":"https://api.github.com/repos/danieldresser/gaffer/issues/comments{/number}","contents_url":"https://api.github.com/repos/danieldresser/gaffer/contents/{+path}","compare_url":"https://api.github.com/repos/danieldresser/gaffer/compare/{base}...{head}","merges_url":"https://api.github.com/repos/danieldresser/gaffer/merges","archive_url":"https://api.github.com/repos/danieldresser/gaffer/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/danieldresser/gaffer/downloads","issues_url":"https://api.github.com/repos/danieldresser/gaffer/issues{/number}","pulls_url":"https://api.github.com/repos/danieldresser/gaffer/pulls{/number}","milestones_url":"https://api.github.com/repos/danieldresser/gaffer/milestones{/number}","notifications_url":"https://api.github.com/repos/danieldresser/gaffer/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/danieldresser/gaffer/labels{/name}","releases_url":"https://api.github.com/repos/danieldresser/gaffer/releases{/id}","deployments_url":"https://api.github.com/repos/danieldresser/gaffer/deployments","created_at":"2013-08-30T00:23:37Z","updated_at":"2013-09-11T17:57:10Z","pushed_at":"2016-09-08T01:49:33Z","git_url":"git://github.com/danieldresser/gaffer.git","ssh_url":"git@github.com:danieldresser/gaffer.git","clone_url":"https://github.com/danieldresser/gaffer.git","svn_url":"https://github.com/danieldresser/gaffer","homepage":"","size":24098,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"ImageEngine:master","ref":"master","sha":"0660ba9dbea7952f783843e553415516b5865b9c","user":{"login":"ImageEngine","id":3976662,"avatar_url":"https://avatars.githubusercontent.com/u/3976662?v=3","gravatar_id":"","url":"https://api.github.com/users/ImageEngine","html_url":"https://github.com/ImageEngine","followers_url":"https://api.github.com/users/ImageEngine/followers","following_url":"https://api.github.com/users/ImageEngine/following{/other_user}","gists_url":"https://api.github.com/users/ImageEngine/gists{/gist_id}","starred_url":"https://api.github.com/users/ImageEngine/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ImageEngine/subscriptions","organizations_url":"https://api.github.com/users/ImageEngine/orgs","repos_url":"https://api.github.com/users/ImageEngine/repos","events_url":"https://api.github.com/users/ImageEngine/events{/privacy}","received_events_url":"https://api.github.com/users/ImageEngine/received_events","type":"Organization","site_admin":false},"repo":{"id":9043124,"name":"gaffer","full_name":"ImageEngine/gaffer","owner":{"login":"ImageEngine","id":3976662,"avatar_url":"https://avatars.githubusercontent.com/u/3976662?v=3","gravatar_id":"","url":"https://api.github.com/users/ImageEngine","html_url":"https://github.com/ImageEngine","followers_url":"https://api.github.com/users/ImageEngine/followers","following_url":"https://api.github.com/users/ImageEngine/following{/other_user}","gists_url":"https://api.github.com/users/ImageEngine/gists{/gist_id}","starred_url":"https://api.github.com/users/ImageEngine/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ImageEngine/subscriptions","organizations_url":"https://api.github.com/users/ImageEngine/orgs","repos_url":"https://api.github.com/users/ImageEngine/repos","events_url":"https://api.github.com/users/ImageEngine/events{/privacy}","received_events_url":"https://api.github.com/users/ImageEngine/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ImageEngine/gaffer","description":"Gaffer is an open source application framework designed specifically for creating tools for use in visual effects production.","fork":false,"url":"https://api.github.com/repos/ImageEngine/gaffer","forks_url":"https://api.github.com/repos/ImageEngine/gaffer/forks","keys_url":"https://api.github.com/repos/ImageEngine/gaffer/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ImageEngine/gaffer/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ImageEngine/gaffer/teams","hooks_url":"https://api.github.com/repos/ImageEngine/gaffer/hooks","issue_events_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/events{/number}","events_url":"https://api.github.com/repos/ImageEngine/gaffer/events","assignees_url":"https://api.github.com/repos/ImageEngine/gaffer/assignees{/user}","branches_url":"https://api.github.com/repos/ImageEngine/gaffer/branches{/branch}","tags_url":"https://api.github.com/repos/ImageEngine/gaffer/tags","blobs_url":"https://api.github.com/repos/ImageEngine/gaffer/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ImageEngine/gaffer/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ImageEngine/gaffer/git/refs{/sha}","trees_url":"https://api.github.com/repos/ImageEngine/gaffer/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ImageEngine/gaffer/statuses/{sha}","languages_url":"https://api.github.com/repos/ImageEngine/gaffer/languages","stargazers_url":"https://api.github.com/repos/ImageEngine/gaffer/stargazers","contributors_url":"https://api.github.com/repos/ImageEngine/gaffer/contributors","subscribers_url":"https://api.github.com/repos/ImageEngine/gaffer/subscribers","subscription_url":"https://api.github.com/repos/ImageEngine/gaffer/subscription","commits_url":"https://api.github.com/repos/ImageEngine/gaffer/commits{/sha}","git_commits_url":"https://api.github.com/repos/ImageEngine/gaffer/git/commits{/sha}","comments_url":"https://api.github.com/repos/ImageEngine/gaffer/comments{/number}","issue_comment_url":"https://api.github.com/repos/ImageEngine/gaffer/issues/comments{/number}","contents_url":"https://api.github.com/repos/ImageEngine/gaffer/contents/{+path}","compare_url":"https://api.github.com/repos/ImageEngine/gaffer/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ImageEngine/gaffer/merges","archive_url":"https://api.github.com/repos/ImageEngine/gaffer/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ImageEngine/gaffer/downloads","issues_url":"https://api.github.com/repos/ImageEngine/gaffer/issues{/number}","pulls_url":"https://api.github.com/repos/ImageEngine/gaffer/pulls{/number}","milestones_url":"https://api.github.com/repos/ImageEngine/gaffer/milestones{/number}","notifications_url":"https://api.github.com/repos/ImageEngine/gaffer/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ImageEngine/gaffer/labels{/name}","releases_url":"https://api.github.com/repos/ImageEngine/gaffer/releases{/id}","deployments_url":"https://api.github.com/repos/ImageEngine/gaffer/deployments","created_at":"2013-03-27T00:04:53Z","updated_at":"2016-09-08T04:06:21Z","pushed_at":"2016-09-08T01:49:34Z","git_url":"git://github.com/ImageEngine/gaffer.git","ssh_url":"git@github.com:ImageEngine/gaffer.git","clone_url":"https://github.com/ImageEngine/gaffer.git","svn_url":"https://github.com/ImageEngine/gaffer","homepage":"http://www.gafferhq.org","size":110646,"stargazers_count":228,"watchers_count":228,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":85,"mirror_url":null,"open_issues_count":203,"forks":85,"open_issues":203,"watchers":228,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1836"},"html":{"href":"https://github.com/ImageEngine/gaffer/pull/1836"},"issue":{"href":"https://api.github.com/repos/ImageEngine/gaffer/issues/1836"},"comments":{"href":"https://api.github.com/repos/ImageEngine/gaffer/issues/1836/comments"},"review_comments":{"href":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1836/comments"},"review_comment":{"href":"https://api.github.com/repos/ImageEngine/gaffer/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ImageEngine/gaffer/pulls/1836/commits"},"statuses":{"href":"https://api.github.com/repos/ImageEngine/gaffer/statuses/197b643aa0f21a38124c2c8ffe51597bc5db9cd4"}}}}
{"actor":{"display_login":"johnhaddon"}}