From 37a814f7255030c1859fbff97f634ad2b8b3e1f6 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 14 Sep 2024 10:34:07 -0700 Subject: [PATCH] api: Adjust IBA::perpixel_op function signature (#4409) When I added perpixel_op recently, I didn't get the function signature right. It only would work for plain old function pointers, and would not work properly for "functors" or lambdas with capture. I should have used function_view which can do a light-weight wrapper of all three, so now I do. Also modified imagebufalgo_test to explicitly test all three forms to be sure they work. Also update function_view ever so slightly. --------- Signed-off-by: Larry Gritz --- CMakeLists.txt | 2 +- src/include/OpenImageIO/function_view.h | 5 +- src/include/OpenImageIO/imagebufalgo_util.h | 15 +++--- src/libOpenImageIO/imagebufalgo.cpp | 10 ++-- src/libOpenImageIO/imagebufalgo_test.cpp | 58 ++++++++++++++++++--- 5 files changed, 69 insertions(+), 21 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bf39062d4c..09e14e7a06 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,7 +4,7 @@ cmake_minimum_required (VERSION 3.15) -set (OpenImageIO_VERSION "2.6.5.1") +set (OpenImageIO_VERSION "2.6.6.0") set (OpenImageIO_VERSION_OVERRIDE "" CACHE STRING "Version override (use with caution)!") mark_as_advanced (OpenImageIO_VERSION_OVERRIDE) diff --git a/src/include/OpenImageIO/function_view.h b/src/include/OpenImageIO/function_view.h index 418ef9857d..5df9842a2e 100644 --- a/src/include/OpenImageIO/function_view.h +++ b/src/include/OpenImageIO/function_view.h @@ -77,7 +77,8 @@ template class function_view { template static Ret callback_fn(intptr_t callable, Params... params) { - return (*reinterpret_cast(callable))(params...); + return (*reinterpret_cast(callable))( + std::forward(params)...); } public: @@ -97,7 +98,7 @@ template class function_view { Ret operator()(Params... params) const { - return callback(callable, params...); + return callback(callable, std::forward(params)...); } operator bool() const { return callback; } diff --git a/src/include/OpenImageIO/imagebufalgo_util.h b/src/include/OpenImageIO/imagebufalgo_util.h index 1254fb1ab0..97924cc936 100644 --- a/src/include/OpenImageIO/imagebufalgo_util.h +++ b/src/include/OpenImageIO/imagebufalgo_util.h @@ -7,6 +7,7 @@ #include +#include #include #include @@ -610,6 +611,8 @@ inline TypeDesc type_merge (TypeDesc a, TypeDesc b, TypeDesc c) #define IBA_FIX_PERCHAN_LEN_DEF(av,len) \ IBA_FIX_PERCHAN_LEN (av, len, 0.0f, av.size() ? av.back() : 0.0f); +// clang-format on + /// Simple image per-pixel unary operation: Given a source image `src`, return @@ -658,22 +661,20 @@ inline TypeDesc type_merge (TypeDesc a, TypeDesc b, TypeDesc c) /// version of the operation that allows specialization to any other pixel /// data types // -OIIO_NODISCARD OIIO_API -ImageBuf -perpixel_op(const ImageBuf& src, bool(*op)(span, cspan), +OIIO_NODISCARD OIIO_API ImageBuf +perpixel_op(const ImageBuf& src, + function_view, cspan)> op, KWArgs options = {}); /// A version of perpixel_op that performs a binary operation, taking two /// source images and a 3-argument `op` function that receives a destination /// and two source pixels. -OIIO_NODISCARD OIIO_API -ImageBuf +OIIO_NODISCARD OIIO_API ImageBuf perpixel_op(const ImageBuf& srcA, const ImageBuf& srcB, - bool(*op)(span, cspan, cspan), + function_view, cspan, cspan)> op, KWArgs options = {}); } // end namespace ImageBufAlgo -// clang-format on OIIO_NAMESPACE_END diff --git a/src/libOpenImageIO/imagebufalgo.cpp b/src/libOpenImageIO/imagebufalgo.cpp index 53af5067ed..04fe1653ab 100644 --- a/src/libOpenImageIO/imagebufalgo.cpp +++ b/src/libOpenImageIO/imagebufalgo.cpp @@ -629,7 +629,8 @@ ImageBufAlgo::IBAprep(ROI& roi, ImageBuf& dst, cspan srcs, ImageBuf ImageBufAlgo::perpixel_op(const ImageBuf& src, - bool (*op)(span, cspan), KWArgs options) + function_view, cspan)> op, + KWArgs options) { using namespace ImageBufAlgo; ImageBuf result; @@ -680,9 +681,10 @@ ImageBufAlgo::perpixel_op(const ImageBuf& src, ImageBuf -ImageBufAlgo::perpixel_op(const ImageBuf& srcA, const ImageBuf& srcB, - bool (*op)(span, cspan, cspan), - KWArgs options) +ImageBufAlgo::perpixel_op( + const ImageBuf& srcA, const ImageBuf& srcB, + function_view, cspan, cspan)> op, + KWArgs options) { using namespace ImageBufAlgo; ImageBuf result; diff --git a/src/libOpenImageIO/imagebufalgo_test.cpp b/src/libOpenImageIO/imagebufalgo_test.cpp index 0d705e451f..b01bac67ee 100644 --- a/src/libOpenImageIO/imagebufalgo_test.cpp +++ b/src/libOpenImageIO/imagebufalgo_test.cpp @@ -1182,6 +1182,29 @@ test_yee() +// Raw function to reverse channels +bool +chan_reverse(span d, cspan s) +{ + for (size_t c = 0, nc = size_t(d.size()); c < nc; ++c) + d[c] = s[nc - 1 - c]; + return true; +} + +// Functor to reverse channels +class ChannelReverser { +public: + ChannelReverser() {} + bool operator()(span d, cspan s) + { + for (size_t c = 0, nc = size_t(d.size()); c < nc; ++c) + d[c] = s[nc - 1 - c]; + return true; + } +}; + + + template static void test_simple_perpixel() @@ -1191,12 +1214,33 @@ test_simple_perpixel() { print(" unary op\n"); ImageBuf src = filled_image({ 0.25f, 0.5f, 0.75f, 1.0f }, 4, 4, td); - ImageBuf result - = ImageBufAlgo::perpixel_op(src, [](span d, cspan s) { - for (size_t c = 0, nc = size_t(d.size()); c < nc; ++c) - d[c] = s[nc - 1 - c]; - return true; - }); + ImageBuf result; + // Test with raw function pointer + result = ImageBufAlgo::perpixel_op(src, chan_reverse); + OIIO_CHECK_EQUAL(result.spec().format, td); + for (ImageBuf::ConstIterator r(result); !r.done(); ++r) { + OIIO_CHECK_EQUAL(r[0], 1.0f); + OIIO_CHECK_EQUAL(r[1], 0.75f); + OIIO_CHECK_EQUAL(r[2], 0.5f); + OIIO_CHECK_EQUAL(r[3], 0.25f); + } + // Test with functor + result = ImageBufAlgo::perpixel_op(src, ChannelReverser()); + OIIO_CHECK_EQUAL(result.spec().format, td); + for (ImageBuf::ConstIterator r(result); !r.done(); ++r) { + OIIO_CHECK_EQUAL(r[0], 1.0f); + OIIO_CHECK_EQUAL(r[1], 0.75f); + OIIO_CHECK_EQUAL(r[2], 0.5f); + OIIO_CHECK_EQUAL(r[3], 0.25f); + } + // Test with lambda, including variable capture + float bias = 0.0; // Force capture of this variable + result = ImageBufAlgo::perpixel_op(src, [&](span d, + cspan s) { + for (size_t c = 0, nc = size_t(d.size()); c < nc; ++c) + d[c] = s[nc - 1 - c] + bias; + return true; + }); OIIO_CHECK_EQUAL(result.spec().format, td); for (ImageBuf::ConstIterator r(result); !r.done(); ++r) { OIIO_CHECK_EQUAL(r[0], 1.0f); @@ -1210,7 +1254,7 @@ test_simple_perpixel() ImageBuf srcA = filled_image({ 0.25f, 0.5f, 0.75f, 1.0f }, 4, 4, td); ImageBuf srcB = filled_image({ 1.0f, 2.0f, 3.0f, 4.0f }, 4, 4, td); ImageBuf result = ImageBufAlgo::perpixel_op( - srcA, srcB, [](span d, cspan a, cspan b) { + srcA, srcB, [&](span d, cspan a, cspan b) { for (size_t c = 0, nc = size_t(d.size()); c < nc; ++c) d[c] = a[c] + b[c]; return true;