From 37a814f7255030c1859fbff97f634ad2b8b3e1f6 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 14 Sep 2024 10:34:07 -0700 Subject: [PATCH 1/5] 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; From ce82f61168f67856a8d6ba98ca00be2bb35da081 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 14 Sep 2024 10:34:43 -0700 Subject: [PATCH 2/5] api: IOProxy const method adjustments (#4415) With 3.0 approaching, it seems like the only time to make these changes that I made note of a while back: * It seems to me that `tell()` should be const because it isn't going to modify or change the state of the IOProxy. * It seems to me that `flush()` should NOT be const, because it probably does change internal state. Signed-off-by: Larry Gritz --- src/include/OpenImageIO/filesystem.h | 6 +++--- src/libutil/filesystem.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/include/OpenImageIO/filesystem.h b/src/include/OpenImageIO/filesystem.h index e29df72f9f..e54b2361a2 100644 --- a/src/include/OpenImageIO/filesystem.h +++ b/src/include/OpenImageIO/filesystem.h @@ -412,7 +412,7 @@ class OIIO_UTIL_API IOProxy { virtual const char* proxytype () const = 0; virtual void close () { } virtual bool opened () const { return mode() != Closed; } - virtual int64_t tell () { return m_pos; } + virtual int64_t tell() const { return m_pos; } // Seek to the position, returning true on success, false on failure. // Note the difference between this and std::fseek() which returns 0 on // success, and -1 on failure. @@ -440,7 +440,7 @@ class OIIO_UTIL_API IOProxy { // Return the total size of the proxy data, in bytes. virtual size_t size () const { return 0; } - virtual void flush () const { } + virtual void flush() { } Mode mode () const { return m_mode; } const std::string& filename () const { return m_filename; } @@ -491,7 +491,7 @@ class OIIO_UTIL_API IOFile : public IOProxy { size_t pread(void* buf, size_t size, int64_t offset) override; size_t pwrite(const void* buf, size_t size, int64_t offset) override; size_t size() const override; - void flush() const override; + void flush() override; // Access the FILE* FILE* handle() const { return m_file; } diff --git a/src/libutil/filesystem.cpp b/src/libutil/filesystem.cpp index e80deb7321..bc4cae0a7e 100644 --- a/src/libutil/filesystem.cpp +++ b/src/libutil/filesystem.cpp @@ -1309,7 +1309,7 @@ Filesystem::IOFile::size() const } void -Filesystem::IOFile::flush() const +Filesystem::IOFile::flush() { if (m_file) fflush(m_file); From 6aba6e99fd02cea879f87a0ec08f898db37ab49b Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 14 Sep 2024 10:35:20 -0700 Subject: [PATCH 3/5] build: No need for OCIO search to use PREFER_CONFIG (#4425) We no longer supply a FindOpenColorIO.cmake, so it makes no difference. But it was getting in the way of a weird case where a wrapping project wanted to supply one. Signed-off-by: Larry Gritz --- src/cmake/externalpackages.cmake | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cmake/externalpackages.cmake b/src/cmake/externalpackages.cmake index 955c301a9d..5e619a74d3 100644 --- a/src/cmake/externalpackages.cmake +++ b/src/cmake/externalpackages.cmake @@ -132,7 +132,6 @@ checked_find_package (Freetype checked_find_package (OpenColorIO REQUIRED VERSION_MIN 2.2 VERSION_MAX 2.9 - PREFER_CONFIG ) if (NOT OPENCOLORIO_INCLUDES) get_target_property(OPENCOLORIO_INCLUDES OpenColorIO::OpenColorIO INTERFACE_INCLUDE_DIRECTORIES) From a3c645302384ba8250ccd852bae4b3d140f3386b Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 14 Sep 2024 11:08:03 -0700 Subject: [PATCH 4/5] ci: Adjust ABI standard Signed-off-by: Larry Gritz --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6baad192bb..2cb49a21ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -213,7 +213,7 @@ jobs: pybind11_ver: v2.10.0 skip_tests: 1 # abi_check: v2.6.2.0 - abi_check: 1b839e5f78465c0aadca4e05e86982b762414ff4 + abi_check: 6aba6e99fd02cea879f87a0ec08f898db37ab49b setenvs: export OIIO_CMAKE_FLAGS="-DOIIO_BUILD_TOOLS=0 -DOIIO_BUILD_TESTS=0 -DUSE_PYTHON=0" USE_OPENCV=0 USE_FFMPEG=0 USE_PYTHON=0 USE_FREETYPE=0 CMAKE_BUILD_TYPE=RelWithDebInfo From 89cc7dc79426aef419e4ef87558aed93cc2f2f3e Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sun, 15 Sep 2024 14:23:04 -0700 Subject: [PATCH 5/5] docs: minor docs fixups to help with sphinx warnings and errors (#4427) Signed-off-by: Larry Gritz --- src/doc/Doxyfile | 2 ++ src/doc/builtinplugins.rst | 15 +++++++-------- src/doc/imagebuf.rst | 8 ++++---- src/doc/oiiotool.rst | 13 ++++++------- src/include/OpenImageIO/imagebuf.h | 2 +- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/doc/Doxyfile b/src/doc/Doxyfile index 504ac3d1a7..5d7e036c15 100644 --- a/src/doc/Doxyfile +++ b/src/doc/Doxyfile @@ -2576,3 +2576,5 @@ GENERATE_LEGEND = YES # This tag requires that the tag HAVE_DOT is set to YES. DOT_CLEANUP = YES + +CLANG_ASSISTED_PARSING = YES diff --git a/src/doc/builtinplugins.rst b/src/doc/builtinplugins.rst index 553ae6f433..8bc3f7e93f 100644 --- a/src/doc/builtinplugins.rst +++ b/src/doc/builtinplugins.rst @@ -1293,29 +1293,28 @@ control aspects of the writing itself: * - ``jpegxl:use_boxes`` - int (bool) - If nonzero, will enable metadata (Exif, XMP, jumb, iptc) writing to the - output file. Default is 1. + output file. Default is 1. * - ``jpegxl:compress_boxes`` - int (bool) - If nonzero, will enable metadata compression. Default is 1. * - ``jpegxl:exif_box`` - int (bool) - If nonzero, will enable Exif metadata writing to the output file. - Default is 1. + Default is 1. * - ``jpegxl:xmp_box`` - int (bool) - If nonzero, will enable XMP metadata writing to the output file. - Default is 1. + Default is 1. * - ``jpegxl:jumb_box`` - int (bool) - If nonzero, will enable JUMBF metadata writing to the output file. - Default is 0. - (dows not supported at this moment in OIIO) + Default is 0. (dows not supported at this moment in OIIO) * - ``jpegxl:iptc_box`` - int (bool) - If nonzero, will enable IPTC metadata writing to the output file. - Default is 0. - (Do not work as expected at this moment. Box is written but content - unreadable in exif readers) + Default is 0. + (Does not work as expected at this moment. Box is written but content + unreadable in exif readers.) .. _sec-bundledplugins-ffmpeg: diff --git a/src/doc/imagebuf.rst b/src/doc/imagebuf.rst index 461d56719f..7697e92c8d 100644 --- a/src/doc/imagebuf.rst +++ b/src/doc/imagebuf.rst @@ -47,8 +47,8 @@ Making an empty or uninitialized ImageBuf Constructing a readable ImageBuf -------------------------------- -.. doxygenfunction:: OIIO::ImageBuf::ImageBuf(string_view name, int subimage = 0, int miplevel = 0, ImageCache *imagecache = nullptr, const ImageSpec *config = nullptr, Filesystem::IOProxy *ioproxy = nullptr) -.. doxygenfunction:: OIIO::ImageBuf::reset(string_view name, int subimage = 0, int miplevel = 0, ImageCache *imagecache = nullptr, const ImageSpec *config = nullptr, Filesystem::IOProxy *ioproxy = nullptr) +.. doxygenfunction:: OIIO::ImageBuf::ImageBuf(string_view name, int subimage = 0, int miplevel = 0, std::shared_ptr imagecache = {}, const ImageSpec *config = nullptr, Filesystem::IOProxy *ioproxy = nullptr) +.. doxygenfunction:: OIIO::ImageBuf::reset(string_view name, int subimage = 0, int miplevel = 0, std::shared_ptr imagecache = {}, const ImageSpec *config = nullptr, Filesystem::IOProxy *ioproxy = nullptr) Constructing a writable ImageBuf @@ -271,9 +271,9 @@ Cons/Limitations: this approach, especially for operations where you expect inputs to be float typically. -.. doxygenfunction:: perpixel_op(const ImageBuf &src, bool (*op)(span, cspan), int prepflags = ImageBufAlgo::IBAprep_DEFAULT, int nthreads = 0) +.. doxygenfunction:: perpixel_op(const ImageBuf &src, function_view, cspan)> op, KWArgs options = {}) -.. doxygenfunction:: perpixel_op(const ImageBuf &srcA, const ImageBuf &srcB, bool (*op)(span, cspan, cspan), int prepflags = ImageBufAlgo::IBAprep_DEFAULT, int nthreads = 0) +.. doxygenfunction:: perpixel_op(const ImageBuf &srcA, const ImageBuf& srcB, function_view, cspan, cspan)> op, KWArgs options = {}) Examples: diff --git a/src/doc/oiiotool.rst b/src/doc/oiiotool.rst index ee9122e3af..20341f6fc8 100644 --- a/src/doc/oiiotool.rst +++ b/src/doc/oiiotool.rst @@ -241,7 +241,7 @@ contents of an expression may be any of: To illustrate how this works, consider the following command, which trims a four-pixel border from all sides and outputs a new image prefixed with -"cropped_", without needing to know the resolution or filename of the +"`cropped_`", without needing to know the resolution or filename of the original image:: oiiotool input.exr -cut "{TOP.width-2*4}x{TOP.height-2*4}+{TOP.x+4}+{TOP.y+4}" \ @@ -2781,8 +2781,7 @@ current top image. `max=` *vals* Specify the maximum range value(s), default 1.0. `scontrast=` *vals* - Specify sigmoidal contrast slope value(s), - default 1.0. + Specify sigmoidal contrast slope value(s), default 1.0. `sthresh=` *vals* Specify sigmoidal threshold value(s) giving the position of maximum slope, default 0.5. @@ -4113,11 +4112,11 @@ current top image. -fill:topleft=.1,.1,.1:topright=1,0,0:bottomleft=0,1,0:botromright=0,0,1 \ 640x480 -o gradient.tif - .. |textimg1| image:: figures/gradient.jpg + .. |gradimg1| image:: figures/gradient.jpg :width: 2.0 in - .. |textimg2| image:: figures/gradienth.jpg + .. |gradimg2| image:: figures/gradienth.jpg :width: 2.0 in - .. |textimg2| image:: figures/gradient4.jpg + .. |gradimg3| image:: figures/gradient4.jpg :width: 2.0 in .. @@ -4175,7 +4174,7 @@ current top image. :width: 2.0 in .. |textimg2| image:: figures/textcentered.jpg :width: 2.0 in - .. |textimg2| image:: figures/textshadowed.jpg + .. |textimg3| image:: figures/textshadowed.jpg :width: 2.0 in .. diff --git a/src/include/OpenImageIO/imagebuf.h b/src/include/OpenImageIO/imagebuf.h index e6ce497581..af31cf0675 100644 --- a/src/include/OpenImageIO/imagebuf.h +++ b/src/include/OpenImageIO/imagebuf.h @@ -1027,7 +1027,7 @@ class OIIO_API ImageBuf { /// spec in the ImageBuf to reset the "full" image size (a.k.a. /// "display window") to /// - /// [xbegin,xend) x [ybegin,yend) x [zbegin,zend)` + /// `[xbegin,xend) x [ybegin,yend) x [zbegin,zend)` /// /// This does not affect the size of the pixel data window. void set_full(int xbegin, int xend, int ybegin, int yend, int zbegin,