From badeba07170c7dd496ce808b1f18aca732f8014c Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Tue, 12 Nov 2024 09:55:49 -0800 Subject: [PATCH] perf: oiiotool --line --text --point --box speedups (#4518) 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 --- src/oiiotool/oiiotool.cpp | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/oiiotool/oiiotool.cpp b/src/oiiotool/oiiotool.cpp index a6305bbe4e..dc9fa00162 100644 --- a/src/oiiotool/oiiotool.cpp +++ b/src/oiiotool/oiiotool.cpp @@ -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 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 img) { \ @@ -4940,8 +4952,9 @@ OIIOTOOL_OP(contrast, 1, [&](OiiotoolOp& op, span img) { // --box // clang-format off -OIIOTOOL_OP(box, 1, nullptr, ([&](OiiotoolOp& op, span img) { - img[0]->copy(*img[1]); +OIIOTOOL_INPLACE_OP(box, 1, nullptr, ([&](OiiotoolOp& op, span 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)); @@ -4963,8 +4976,9 @@ OIIOTOOL_OP(box, 1, nullptr, ([&](OiiotoolOp& op, span img) { // --line -OIIOTOOL_OP(line, 1, [&](OiiotoolOp& op, span img) { - img[0]->copy(*img[1]); +OIIOTOOL_INPLACE_OP(line, 1, [&](OiiotoolOp& op, span 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 points; Strutil::extract_from_list_string(points, op.args(1)); @@ -4983,8 +4997,9 @@ OIIOTOOL_OP(line, 1, [&](OiiotoolOp& op, span img) { // --point -OIIOTOOL_OP(point, 1, [&](OiiotoolOp& op, span img) { - img[0]->copy(*img[1]); +OIIOTOOL_INPLACE_OP(point, 1, [&](OiiotoolOp& op, span 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 points; Strutil::extract_from_list_string(points, op.args(1)); @@ -5000,8 +5015,9 @@ OIIOTOOL_OP(point, 1, [&](OiiotoolOp& op, span img) { // --text -OIIOTOOL_OP(text, 1, [&](OiiotoolOp& op, span img) { - img[0]->copy(*img[1]); +OIIOTOOL_INPLACE_OP(text, 1, [&](OiiotoolOp& op, span 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);