Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul oiiotool internal idioms #2586

Merged
merged 2 commits into from
May 20, 2020

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented May 17, 2020

oiiotool dates from pre-C++11, and has grown by accretion. As a
result, the individual actions are implemented using multiple
different idioms, and there is a lot of complex boilerplate.

A recent PR added subimage selection widely to all commands that use
the idiom of an implementation that inherits from the OiiotoolOp
class. The convinced me that we want to migrate as many actions as
possible to this idiom. I also realized that the current use of
OiiotoolOp could be greatly simplified by using lambdas in most
cases, which weren't available in the pre-C++11 ancient times when
oiiotool was first being developed.

This patch:

  • Has the OiiotoolOp base class contain (and accept in its ctr) a
    std::function for the meat of the impl() and setup() methods.

  • If init() and setup() are the only bits that need customization for
    a particular action, they can just be passed as lambdas rather than
    needing to subclass OiiotoolOp.

  • This eliminates a huge amount of per-action boilerplate, many helper
    subclasses and macros.

  • In front of every action implemetation (however it is done), I
    inserted a comment with the command line argument (e.g.,
    --colormap) which is an aid for me to quickly find the
    implementation in this huge file.

This patch DOES NOT change any non-OiiotoolOp action idioms to
OiiotoolOp. That will come later, first I wanted to make these
improvements.

Signed-off-by: Larry Gritz lg@larrygritz.com

oiiotool dates from pre-C++11, and has grown by accretion. As a
result, the individual actions are implemented using multiple
different idioms, and there is a lot of complex boilerplate.

A recent PR added subimage selection widely to all commands that use
the idiom of an implementation that inherits from the OiiotoolOp
class. The convinced me that we want to migrate as many actions as
possible to this idiom. I also realized that the current use of
OiiotoolOp could be greatly simplified by using lambdas in most
cases, which weren't available in the pre-C++11 ancient times when
oiiotool was first being developed.

This patch:

* Has the OiiotoolOp base class contain (and accept in its ctr) a
  std::function for the meat of the impl() and setup() methods.

* If init() and setup() are the only bits that need customization for
  a particular action, they can just be passed as lambdas rather than
  needing to subclass OiiotoolOp.

* This eliminates a huge amount of per-action boilerplate, many helper
  subclasses and macros.

* In front of every action implemetation (however it is done), I
  inserted a comment with the command line argument (e.g.,
  `--colormap`) which is an aid for me to quickly find the
  implementation in this huge file.

This patch DOES NOT change any non-OiiotoolOp action idioms to
OiiotoolOp.  That will come later, first I wanted to make these
improvements.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz lgritz merged commit f161d93 into AcademySoftwareFoundation:master May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant