{"action":"created","issue":{"url":"https://api.github.com/repos/RenderToolbox/RenderToolbox3/issues/46","repository_url":"https://api.github.com/repos/RenderToolbox/RenderToolbox3","labels_url":"https://api.github.com/repos/RenderToolbox/RenderToolbox3/issues/46/labels{/name}","comments_url":"https://api.github.com/repos/RenderToolbox/RenderToolbox3/issues/46/comments","events_url":"https://api.github.com/repos/RenderToolbox/RenderToolbox3/issues/46/events","html_url":"https://github.com/RenderToolbox/RenderToolbox3/pull/46","id":159306890,"number":46,"title":"Changes that let us run RTB3 with a PBRT docker container.","user":{"login":"tlian7","id":13054539,"avatar_url":"https://avatars.githubusercontent.com/u/13054539?v=3","gravatar_id":"","url":"https://api.github.com/users/tlian7","html_url":"https://github.com/tlian7","followers_url":"https://api.github.com/users/tlian7/followers","following_url":"https://api.github.com/users/tlian7/following{/other_user}","gists_url":"https://api.github.com/users/tlian7/gists{/gist_id}","starred_url":"https://api.github.com/users/tlian7/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tlian7/subscriptions","organizations_url":"https://api.github.com/users/tlian7/orgs","repos_url":"https://api.github.com/users/tlian7/repos","events_url":"https://api.github.com/users/tlian7/events{/privacy}","received_events_url":"https://api.github.com/users/tlian7/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-06-09T00:39:13Z","updated_at":"2016-10-14T17:38:40Z","closed_at":"2016-10-14T17:38:40Z","pull_request":{"url":"https://api.github.com/repos/RenderToolbox/RenderToolbox3/pulls/46","html_url":"https://github.com/RenderToolbox/RenderToolbox3/pull/46","diff_url":"https://github.com/RenderToolbox/RenderToolbox3/pull/46.diff","patch_url":"https://github.com/RenderToolbox/RenderToolbox3/pull/46.patch"},"body":"@benjamin-heasly  I think this is one of two ways we can potentially incorporate the PBRT docker container that Brian and I use into RTB3. Just to remind you since it's been a while, but this docker container allows users to render with PBRT without having to build it locally at all. \r\n\r\nThe first way is more straightforward, and I've included it in this pull request. Essentially there is a new function called `SetupPBRTDocker` which one can call in the scene script. It generates the command needed to run the docker container and then replaces the PBRT executable with this new command, so that nothing needs to be changed in the `RunPBRT.m` file or anywhere else.\r\n\r\nThe main drawback to this method is that in order to switch back to local PBRT one would have to rerun the configuration script to change the PBRT executable back to the local path.\r\n\r\nThe second way which I've also implemented (but did not generate a pull request for) is to make changes in the configuration file. This is more inline with what you suggested to me last year but is more messy and complicated.\r\n\r\nThe main problem with this method is that in order to generate the docker command, we need to know the working folder of the scene. To make things even more complicated, the docker command has slightly convoluted syntax that is hard to work around. For reference, it looks something like this:\r\n`docker run -t -i --rm -v [scene workingFolder]:[scene workingFolder] vistalab/pbrt pbrt --outfile [outfile] [input PBRT file]`\r\n\r\nWe could potentially keep things in the configuration file, but we would have to break things up. We would have to first change the PBRT executable to be `vistalab/pbrt pbrt` and pull the docker container in the configuration file. Then in the user's scene file, they would have to specify the front piece of the docker command:  `docker run -t -i --rm -v [scene workingFolder]:[scene workingFolder]` (we could potentially write a simple function that does this for them). Then in RunPBRT it would include this docker command before the PBRT executable.\r\n\r\nThe main advantage of doing it this way would be the choice of whether to use docker or not is kept in the configuration file. The downside is that it introduces several new things including this extra piece to the render command that has to be specified in the scene file if the user ran the configuration file with docker PBRT. To switch back to local PBRT, the user would still have to rerun the configuration file since the PBRT executable has to be changed.\r\n\r\nLet me know which way you think would be better for RTB. Thanks so much.\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/RenderToolbox/RenderToolbox3/issues/comments/253869847","html_url":"https://github.com/RenderToolbox/RenderToolbox3/pull/46#issuecomment-253869847","issue_url":"https://api.github.com/repos/RenderToolbox/RenderToolbox3/issues/46","id":253869847,"user":{"login":"benjamin-heasly","id":1950775,"avatar_url":"https://avatars.githubusercontent.com/u/1950775?v=3","gravatar_id":"","url":"https://api.github.com/users/benjamin-heasly","html_url":"https://github.com/benjamin-heasly","followers_url":"https://api.github.com/users/benjamin-heasly/followers","following_url":"https://api.github.com/users/benjamin-heasly/following{/other_user}","gists_url":"https://api.github.com/users/benjamin-heasly/gists{/gist_id}","starred_url":"https://api.github.com/users/benjamin-heasly/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/benjamin-heasly/subscriptions","organizations_url":"https://api.github.com/users/benjamin-heasly/orgs","repos_url":"https://api.github.com/users/benjamin-heasly/repos","events_url":"https://api.github.com/users/benjamin-heasly/events{/privacy}","received_events_url":"https://api.github.com/users/benjamin-heasly/received_events","type":"User","site_admin":false},"created_at":"2016-10-14T17:38:40Z","updated_at":"2016-10-14T17:38:40Z","body":"Hi Trisha,\r\n\r\nThanks a lot for this.  I'm sorry I left it hanging all summer.\r\n\r\nI'm finishing documentation for RenderToolbox4 which supports Docker explicitly.\r\n\r\nIt works much like your second suggestion.  Each renderer configuration has a field specifically for a Docker image name.  If Docker and the image are found, the rendering code will generate a Docker command that includes the folders to mount in the container.\r\n\r\nSo I would like to close this PR and consider it \"conceptually merged\".  OK with you?"}}
{"actor":{"display_login":"benjamin-heasly"}}