{"action":"opened","number":1127,"pull_request":{"url":"https://api.github.com/repos/OpenImageIO/oiio/pulls/1127","id":33927705,"html_url":"https://github.com/OpenImageIO/oiio/pull/1127","diff_url":"https://github.com/OpenImageIO/oiio/pull/1127.diff","patch_url":"https://github.com/OpenImageIO/oiio/pull/1127.patch","issue_url":"https://api.github.com/repos/OpenImageIO/oiio/issues/1127","number":1127,"state":"open","locked":false,"title":"Overhaul of oiiotool.cpp -- express operations as OiiotoolOp subclasses.","user":{"login":"lgritz","id":504179,"avatar_url":"https://avatars.githubusercontent.com/u/504179?v=3","gravatar_id":"","url":"https://api.github.com/users/lgritz","html_url":"https://github.com/lgritz","followers_url":"https://api.github.com/users/lgritz/followers","following_url":"https://api.github.com/users/lgritz/following{/other_user}","gists_url":"https://api.github.com/users/lgritz/gists{/gist_id}","starred_url":"https://api.github.com/users/lgritz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lgritz/subscriptions","organizations_url":"https://api.github.com/users/lgritz/orgs","repos_url":"https://api.github.com/users/lgritz/repos","events_url":"https://api.github.com/users/lgritz/events{/privacy}","received_events_url":"https://api.github.com/users/lgritz/received_events","type":"User","site_admin":false},"body":"It's been bugging me for a while that all the operations in oiiotool (add, mul, etc.) are enormously repetitive, lots of similar boilerplate in all the ops: pop images off the stack, create a destination image, decode arguments and options, loop over subimages, time the whole thing. etc.\r\n\r\nAnd sometimes the repetition is inexact -- either plain old errors, or else that because oiiotool has grown in pieces, improvements to methodology get incorporated into newly added functions but not old ones.\r\n\r\nSo I've been contemplating a big refactor for some time, and this past weekend I got the chance to experiment, and I think I like it. The gist is that I define an OiiotoolOp base class that contains all the common boilerplate code. Then the individual operations subclass, overriding just the tiniest bits that tend to differ from op to op. This greatly reduces the amount of repeated code, I hope tends to make the code clearer by boiling down each op to \"do the standard thing except with THIS one line in the loop\", and also makes it much easier for new ideas or changes in methodology to get applied to nearly all ops with one change.\r\n\r\nI'm sorry to say, though, that the overhaul is so extensive that 'diff' just doesn't line anything up for oiiotool.cpp, you can't easily see the difference side by side. You'll just have to look at the file in its new form and try to understand what it's doing. Github doesn't even attempt to show a diff, ugh. As an example, though, I'll present 'invert' done the old way:\r\n\r\n    static int\r\n    action_invert (int argc, const char *argv[])\r\n    {\r\n        if (ot.postpone_callback (1, action_invert, argc, argv))\r\n            return 0;\r\n        Timer timer (ot.enable_function_timing);\r\n        string_view command = ot.express (argv[0]);\r\n    \r\n        ot.read ();\r\n        ImageRecRef A = ot.pop();\r\n        ot.push (new ImageRec (*A, ot.allsubimages ? -1 : 0,\r\n                               ot.allsubimages ? -1 : 0, true, true/* copy pixels*/));\r\n    \r\n        for (int s = 0, subimages = ot.curimg->subimages(); s < subimages; ++s) {\r\n            for (int m = 0, miplevels = ot.curimg->miplevels(s); m < miplevels; ++m) {\r\n                ImageBuf &Rib ((*ot.curimg)(s,m));\r\n                // invert the first three channels only, spare alpha\r\n                ROI roi = Rib.roi();\r\n                roi.chend = std::min (3, Rib.spec().nchannels);\r\n                bool ok = ImageBufAlgo::invert (Rib, Rib, roi);\r\n                if (! ok)\r\n                    ot.error (command, Rib.geterror());\r\n            }\r\n        }\r\n    \r\n        ot.function_times[command] += timer();\r\n        return 0;\r\n    }\r\n\r\nand the new way for the same functionality:\r\n\r\n    class OpInvert : public OiiotoolOp {\r\n    public:\r\n        OpInvert (Oiiotool &ot, string_view opname, int argc, const char *argv[])\r\n            : OiiotoolOp (ot, opname, argc, argv, 1) {}\r\n        virtual int impl (ImageBuf **img) {\r\n            // invert the first three channels only, spare alpha\r\n            ROI roi = img[1]->roi();\r\n            roi.chend = std::min (3, roi.chend);\r\n            return ImageBufAlgo::invert (*img[0], *img[1], roi, 0);\r\n        }\r\n    };\r\n    \r\n    OP_CUSTOMCLASS (invert, OpInvert, 1);","created_at":"2015-04-23T06:33:17Z","updated_at":"2015-04-23T06:33:17Z","closed_at":null,"merged_at":null,"merge_commit_sha":null,"assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/OpenImageIO/oiio/pulls/1127/commits","review_comments_url":"https://api.github.com/repos/OpenImageIO/oiio/pulls/1127/comments","review_comment_url":"https://api.github.com/repos/OpenImageIO/oiio/pulls/comments{/number}","comments_url":"https://api.github.com/repos/OpenImageIO/oiio/issues/1127/comments","statuses_url":"https://api.github.com/repos/OpenImageIO/oiio/statuses/4790f15697e7d185a32bc5b46d5234cb3345d12f","head":{"label":"lgritz:lg-oiiotooloverhaul","ref":"lg-oiiotooloverhaul","sha":"4790f15697e7d185a32bc5b46d5234cb3345d12f","user":{"login":"lgritz","id":504179,"avatar_url":"https://avatars.githubusercontent.com/u/504179?v=3","gravatar_id":"","url":"https://api.github.com/users/lgritz","html_url":"https://github.com/lgritz","followers_url":"https://api.github.com/users/lgritz/followers","following_url":"https://api.github.com/users/lgritz/following{/other_user}","gists_url":"https://api.github.com/users/lgritz/gists{/gist_id}","starred_url":"https://api.github.com/users/lgritz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lgritz/subscriptions","organizations_url":"https://api.github.com/users/lgritz/orgs","repos_url":"https://api.github.com/users/lgritz/repos","events_url":"https://api.github.com/users/lgritz/events{/privacy}","received_events_url":"https://api.github.com/users/lgritz/received_events","type":"User","site_admin":false},"repo":{"id":1322330,"name":"oiio","full_name":"lgritz/oiio","owner":{"login":"lgritz","id":504179,"avatar_url":"https://avatars.githubusercontent.com/u/504179?v=3","gravatar_id":"","url":"https://api.github.com/users/lgritz","html_url":"https://github.com/lgritz","followers_url":"https://api.github.com/users/lgritz/followers","following_url":"https://api.github.com/users/lgritz/following{/other_user}","gists_url":"https://api.github.com/users/lgritz/gists{/gist_id}","starred_url":"https://api.github.com/users/lgritz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lgritz/subscriptions","organizations_url":"https://api.github.com/users/lgritz/orgs","repos_url":"https://api.github.com/users/lgritz/repos","events_url":"https://api.github.com/users/lgritz/events{/privacy}","received_events_url":"https://api.github.com/users/lgritz/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/lgritz/oiio","description":"OpenImageIO","fork":true,"url":"https://api.github.com/repos/lgritz/oiio","forks_url":"https://api.github.com/repos/lgritz/oiio/forks","keys_url":"https://api.github.com/repos/lgritz/oiio/keys{/key_id}","collaborators_url":"https://api.github.com/repos/lgritz/oiio/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/lgritz/oiio/teams","hooks_url":"https://api.github.com/repos/lgritz/oiio/hooks","issue_events_url":"https://api.github.com/repos/lgritz/oiio/issues/events{/number}","events_url":"https://api.github.com/repos/lgritz/oiio/events","assignees_url":"https://api.github.com/repos/lgritz/oiio/assignees{/user}","branches_url":"https://api.github.com/repos/lgritz/oiio/branches{/branch}","tags_url":"https://api.github.com/repos/lgritz/oiio/tags","blobs_url":"https://api.github.com/repos/lgritz/oiio/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/lgritz/oiio/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/lgritz/oiio/git/refs{/sha}","trees_url":"https://api.github.com/repos/lgritz/oiio/git/trees{/sha}","statuses_url":"https://api.github.com/repos/lgritz/oiio/statuses/{sha}","languages_url":"https://api.github.com/repos/lgritz/oiio/languages","stargazers_url":"https://api.github.com/repos/lgritz/oiio/stargazers","contributors_url":"https://api.github.com/repos/lgritz/oiio/contributors","subscribers_url":"https://api.github.com/repos/lgritz/oiio/subscribers","subscription_url":"https://api.github.com/repos/lgritz/oiio/subscription","commits_url":"https://api.github.com/repos/lgritz/oiio/commits{/sha}","git_commits_url":"https://api.github.com/repos/lgritz/oiio/git/commits{/sha}","comments_url":"https://api.github.com/repos/lgritz/oiio/comments{/number}","issue_comment_url":"https://api.github.com/repos/lgritz/oiio/issues/comments{/number}","contents_url":"https://api.github.com/repos/lgritz/oiio/contents/{+path}","compare_url":"https://api.github.com/repos/lgritz/oiio/compare/{base}...{head}","merges_url":"https://api.github.com/repos/lgritz/oiio/merges","archive_url":"https://api.github.com/repos/lgritz/oiio/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/lgritz/oiio/downloads","issues_url":"https://api.github.com/repos/lgritz/oiio/issues{/number}","pulls_url":"https://api.github.com/repos/lgritz/oiio/pulls{/number}","milestones_url":"https://api.github.com/repos/lgritz/oiio/milestones{/number}","notifications_url":"https://api.github.com/repos/lgritz/oiio/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/lgritz/oiio/labels{/name}","releases_url":"https://api.github.com/repos/lgritz/oiio/releases{/id}","created_at":"2011-02-02T23:16:23Z","updated_at":"2015-04-22T23:52:53Z","pushed_at":"2015-04-23T06:16:30Z","git_url":"git://github.com/lgritz/oiio.git","ssh_url":"git@github.com:lgritz/oiio.git","clone_url":"https://github.com/lgritz/oiio.git","svn_url":"https://github.com/lgritz/oiio","homepage":"http://www.openimageio.org","size":95114,"stargazers_count":5,"watchers_count":5,"language":"C++","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":5,"default_branch":"master"}},"base":{"label":"OpenImageIO:master","ref":"master","sha":"b2aad56360fdca8a53db9c4409cbca334b1c16a7","user":{"login":"OpenImageIO","id":547887,"avatar_url":"https://avatars.githubusercontent.com/u/547887?v=3","gravatar_id":"","url":"https://api.github.com/users/OpenImageIO","html_url":"https://github.com/OpenImageIO","followers_url":"https://api.github.com/users/OpenImageIO/followers","following_url":"https://api.github.com/users/OpenImageIO/following{/other_user}","gists_url":"https://api.github.com/users/OpenImageIO/gists{/gist_id}","starred_url":"https://api.github.com/users/OpenImageIO/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/OpenImageIO/subscriptions","organizations_url":"https://api.github.com/users/OpenImageIO/orgs","repos_url":"https://api.github.com/users/OpenImageIO/repos","events_url":"https://api.github.com/users/OpenImageIO/events{/privacy}","received_events_url":"https://api.github.com/users/OpenImageIO/received_events","type":"User","site_admin":false},"repo":{"id":1322284,"name":"oiio","full_name":"OpenImageIO/oiio","owner":{"login":"OpenImageIO","id":547887,"avatar_url":"https://avatars.githubusercontent.com/u/547887?v=3","gravatar_id":"","url":"https://api.github.com/users/OpenImageIO","html_url":"https://github.com/OpenImageIO","followers_url":"https://api.github.com/users/OpenImageIO/followers","following_url":"https://api.github.com/users/OpenImageIO/following{/other_user}","gists_url":"https://api.github.com/users/OpenImageIO/gists{/gist_id}","starred_url":"https://api.github.com/users/OpenImageIO/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/OpenImageIO/subscriptions","organizations_url":"https://api.github.com/users/OpenImageIO/orgs","repos_url":"https://api.github.com/users/OpenImageIO/repos","events_url":"https://api.github.com/users/OpenImageIO/events{/privacy}","received_events_url":"https://api.github.com/users/OpenImageIO/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/OpenImageIO/oiio","description":"OpenImageIO","fork":false,"url":"https://api.github.com/repos/OpenImageIO/oiio","forks_url":"https://api.github.com/repos/OpenImageIO/oiio/forks","keys_url":"https://api.github.com/repos/OpenImageIO/oiio/keys{/key_id}","collaborators_url":"https://api.github.com/repos/OpenImageIO/oiio/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/OpenImageIO/oiio/teams","hooks_url":"https://api.github.com/repos/OpenImageIO/oiio/hooks","issue_events_url":"https://api.github.com/repos/OpenImageIO/oiio/issues/events{/number}","events_url":"https://api.github.com/repos/OpenImageIO/oiio/events","assignees_url":"https://api.github.com/repos/OpenImageIO/oiio/assignees{/user}","branches_url":"https://api.github.com/repos/OpenImageIO/oiio/branches{/branch}","tags_url":"https://api.github.com/repos/OpenImageIO/oiio/tags","blobs_url":"https://api.github.com/repos/OpenImageIO/oiio/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/OpenImageIO/oiio/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/OpenImageIO/oiio/git/refs{/sha}","trees_url":"https://api.github.com/repos/OpenImageIO/oiio/git/trees{/sha}","statuses_url":"https://api.github.com/repos/OpenImageIO/oiio/statuses/{sha}","languages_url":"https://api.github.com/repos/OpenImageIO/oiio/languages","stargazers_url":"https://api.github.com/repos/OpenImageIO/oiio/stargazers","contributors_url":"https://api.github.com/repos/OpenImageIO/oiio/contributors","subscribers_url":"https://api.github.com/repos/OpenImageIO/oiio/subscribers","subscription_url":"https://api.github.com/repos/OpenImageIO/oiio/subscription","commits_url":"https://api.github.com/repos/OpenImageIO/oiio/commits{/sha}","git_commits_url":"https://api.github.com/repos/OpenImageIO/oiio/git/commits{/sha}","comments_url":"https://api.github.com/repos/OpenImageIO/oiio/comments{/number}","issue_comment_url":"https://api.github.com/repos/OpenImageIO/oiio/issues/comments{/number}","contents_url":"https://api.github.com/repos/OpenImageIO/oiio/contents/{+path}","compare_url":"https://api.github.com/repos/OpenImageIO/oiio/compare/{base}...{head}","merges_url":"https://api.github.com/repos/OpenImageIO/oiio/merges","archive_url":"https://api.github.com/repos/OpenImageIO/oiio/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/OpenImageIO/oiio/downloads","issues_url":"https://api.github.com/repos/OpenImageIO/oiio/issues{/number}","pulls_url":"https://api.github.com/repos/OpenImageIO/oiio/pulls{/number}","milestones_url":"https://api.github.com/repos/OpenImageIO/oiio/milestones{/number}","notifications_url":"https://api.github.com/repos/OpenImageIO/oiio/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/OpenImageIO/oiio/labels{/name}","releases_url":"https://api.github.com/repos/OpenImageIO/oiio/releases{/id}","created_at":"2011-02-02T23:03:28Z","updated_at":"2015-04-22T23:51:25Z","pushed_at":"2015-04-22T23:52:31Z","git_url":"git://github.com/OpenImageIO/oiio.git","ssh_url":"git@github.com:OpenImageIO/oiio.git","clone_url":"https://github.com/OpenImageIO/oiio.git","svn_url":"https://github.com/OpenImageIO/oiio","homepage":"http://www.openimageio.org","size":122625,"stargazers_count":353,"watchers_count":353,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":164,"mirror_url":null,"open_issues_count":81,"forks":164,"open_issues":81,"watchers":353,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/OpenImageIO/oiio/pulls/1127"},"html":{"href":"https://github.com/OpenImageIO/oiio/pull/1127"},"issue":{"href":"https://api.github.com/repos/OpenImageIO/oiio/issues/1127"},"comments":{"href":"https://api.github.com/repos/OpenImageIO/oiio/issues/1127/comments"},"review_comments":{"href":"https://api.github.com/repos/OpenImageIO/oiio/pulls/1127/comments"},"review_comment":{"href":"https://api.github.com/repos/OpenImageIO/oiio/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/OpenImageIO/oiio/pulls/1127/commits"},"statuses":{"href":"https://api.github.com/repos/OpenImageIO/oiio/statuses/4790f15697e7d185a32bc5b46d5234cb3345d12f"}},"merged":false,"mergeable":null,"mergeable_state":"unknown","merged_by":null,"comments":0,"review_comments":0,"commits":1,"additions":1637,"deletions":2484,"changed_files":3}}