Skip to content

Commit

Permalink
perf: oiiotool --line --text --point --box speedups (#4518)
Browse files Browse the repository at this point in the history
These commands, which are intended to alter the top image on the stack,
were each allocating and copying an image unnecessarily for each
operation. By changing the implementation to the right calls that know
it doesn't need to make a copy (already a feature I'd implemented in the
OiiotoolOp helper class) greatly speeds them up.

---------

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz committed Nov 15, 2024
1 parent 540d855 commit baf85bd
Showing 1 changed file with 24 additions and 8 deletions.
32 changes: 24 additions & 8 deletions src/oiiotool/oiiotool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ using pvt::print_info_options;
op(); \
}

// Lke OIIOTOOL_OP, but designate the op as "inplace" -- which means it
// uses the input image itself as the destination.
#define OIIOTOOL_INPLACE_OP(name, ninputs, ...) \
static void action_##name(Oiiotool& ot, cspan<const char*> argv) \
{ \
if (ot.postpone_callback(ninputs, action_##name, argv)) \
return; \
OiiotoolOp op(ot, "-" #name, argv, ninputs, __VA_ARGS__); \
op.inplace(true); \
op(); \
}

// Canned setup for an op that uses one image on the stack.
#define UNARY_IMAGE_OP(name, impl) \
OIIOTOOL_OP(name, 1, [](OiiotoolOp& op, span<ImageBuf*> img) { \
Expand Down Expand Up @@ -4940,8 +4952,9 @@ OIIOTOOL_OP(contrast, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {

// --box
// clang-format off
OIIOTOOL_OP(box, 1, nullptr, ([&](OiiotoolOp& op, span<ImageBuf*> img) {
img[0]->copy(*img[1]);
OIIOTOOL_INPLACE_OP(box, 1, nullptr, ([&](OiiotoolOp& op, span<ImageBuf*> img) {
OIIO_ASSERT(img[0] == img[1]); // Assume we're operating in-place
// img[0]->copy(*img[1]); // what we'd do if we weren't in-place
const ImageSpec& Rspec(img[0]->spec());
int x1, y1, x2, y2;
string_view s(op.args(1));
Expand All @@ -4963,8 +4976,9 @@ OIIOTOOL_OP(box, 1, nullptr, ([&](OiiotoolOp& op, span<ImageBuf*> img) {


// --line
OIIOTOOL_OP(line, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {
img[0]->copy(*img[1]);
OIIOTOOL_INPLACE_OP(line, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {
OIIO_ASSERT(img[0] == img[1]); // Assume we're operating in-place
// img[0]->copy(*img[1]); // what we'd do if we weren't in-place
const ImageSpec& Rspec(img[0]->spec());
std::vector<int> points;
Strutil::extract_from_list_string(points, op.args(1));
Expand All @@ -4983,8 +4997,9 @@ OIIOTOOL_OP(line, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {


// --point
OIIOTOOL_OP(point, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {
img[0]->copy(*img[1]);
OIIOTOOL_INPLACE_OP(point, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {
OIIO_ASSERT(img[0] == img[1]); // Assume we're operating in-place
// img[0]->copy(*img[1]); // what we'd do if we weren't in-place
const ImageSpec& Rspec(img[0]->spec());
std::vector<int> points;
Strutil::extract_from_list_string(points, op.args(1));
Expand All @@ -5000,8 +5015,9 @@ OIIOTOOL_OP(point, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {


// --text
OIIOTOOL_OP(text, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {
img[0]->copy(*img[1]);
OIIOTOOL_INPLACE_OP(text, 1, [&](OiiotoolOp& op, span<ImageBuf*> img) {
OIIO_ASSERT(img[0] == img[1]); // Assume we're operating in-place
// img[0]->copy(*img[1]); // what we'd do if we weren't in-place
const ImageSpec& Rspec(img[0]->spec());
int x = op.options().get_int("x", Rspec.x + Rspec.width / 2);
int y = op.options().get_int("y", Rspec.y + Rspec.height / 2);
Expand Down

0 comments on commit baf85bd

Please sign in to comment.