api: start using optional keyword/value params for ImageBufAlgo (Acad…

…emySoftwareFoundation#4149)

Many IBA functions take lots of optional parameters, which are from time
to time augmented, leaving us with a clutter of semi-deprecated
versions.

This PR prototypes a new approach, which is to make IBA functions have
explicit parameters for their core/necessary controls, and the minor
optional behavioral control parameters are passed as keyword/value
argument lists. I think this has a few really big advantages over
explicit parameter lists with ever-growing optional parameters:

1. It makes the call sites very readable, since the keywords document
what the parameters do.

2. It allows us to add (or deprecate) optional parameters without ABI
breaking changes, and without proliferation of many versions of each
function.

A new class is added: ParamValueSpan, which as the name implies, is
simply a `span` of ParamValue structs (actually, it's a subclass that
inherits from `span<const ParamValue>`, and adds a bunch of helper
methods). Within the IBA namespace, this is aliased to KWArgs, to make a
short and self-documenting name to pass this collection of optional
parameters. This leads to IBA declarations that look like this:

    ImageBuf OIIO_API resize(const ImageBuf &src, KWArgs options = {},
                             ROI roi = {}, int nthreads = 0);

and calls that looks like this:

    resize(dst, src, { { "filtername", "lanczos3" },
                       { "filterwidth", 6.0f } });

In this first stab, I have converted IBA::resize(), fit(), and warp().
That seems like enough to evaluate whether we like the direction this is
going. If so, we'll tackle more IBA functions over time. I believe I've
done this all in a way that preserves API and ABI compatibility for now

A few other odds and ends that come along for the ride:

* Filter1D and Filter2D have a new create_shared static method that
returns a shared_ptr instead of raw pointer.

* Make all filter ctrs ok with 0 default filter width value, which means
to use the natural default width for that filter.

* IBA::make_kernel is extended so that if you pass 0 for width or hight,
it selects the a resolution using the default size for the named filter
kernel. This way, the caller doesn't need to know the correct extents of
the kernel, just the name.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Co-authored-by: Thiago Ize <ThiagoIze@users.noreply.github.com>

.github/workflows/ci.yml

@@ -188,7 +188,7 @@ jobs:
           # break the ABI. Basically, we will build that version as well as
           # the current one, and compare the resulting libraries.
           - desc: abi check
-            nametag: linux-vfx2023
+            nametag: abi-check
             runner: ubuntu-latest
             container: aswftesting/ci-osl:2023-clang15
             vfxyear: 2023

CMakeLists.txt

@@ -4,7 +4,7 @@
 
 cmake_minimum_required (VERSION 3.15)
 
-set (OpenImageIO_VERSION "2.6.0.3")
+set (OpenImageIO_VERSION "2.6.1.0")
 set (OpenImageIO_VERSION_OVERRIDE "" CACHE STRING
      "Version override (use with caution)!")
 mark_as_advanced (OpenImageIO_VERSION_OVERRIDE)

src/doc/imagebufalgo.rst

@@ -996,7 +996,7 @@ Shuffling channels
           // Resize the image to 640x480, using the default filter
           ImageBuf Src ("tahoe.exr");
           ROI roi (0, 640, 0, 480, 0, 1, /*chans:*/ 0, Src.nchannels());
-          ImageBuf Dst = ImageBufAlgo::resize (Src, "", 0, roi);
+          ImageBuf Dst = ImageBufAlgo::resize (Src, {}, roi);
 
        .. code-tab:: py
 
@@ -1060,14 +1060,14 @@ Shuffling channels
           // Resize to fit into a max of 640x480, preserving the aspect ratio
           ImageBuf Src ("tahoe.exr");
           ROI roi (0, 640, 0, 480, 0, 1, /*chans:*/ 0, Src.nchannels());
-          ImageBuf Dst = ImageBufAlgo::fit (Src, "", 0, true, roi);
+          ImageBuf Dst = ImageBufAlgo::fit (Src, {}, roi);
 
        .. code-tab:: py
 
           # Resize to fit into a max of 640x480, preserving the aspect ratio
           Src = ImageBuf("tahoe.exr")
           roi = ROI(0, 640, 0, 480, 0, 1, 0, Src.nchannels)
-          Dst = ImageBufAlgo.fit (Src, "", 0, True, roi)
+          Dst = ImageBufAlgo.fit (Src, roi=roi)
 
        .. code-tab:: bash oiiotool
 
@@ -1090,15 +1090,16 @@ Shuffling channels
                          -0.7071068, 0.7071068, 0,
                          20,        -8.284271,  1);
           ImageBuf Src ("tahoe.exr");
-          ImageBuf Dst = ImageBufAlgo::warp (dst, src, M, "lanczos3");
+          ParamValue options[] = { { "filtername", "lanczos3" } };
+          ImageBuf Dst = ImageBufAlgo::warp (dst, src, M, options);
 
        .. code-tab:: py
 
           M = ( 0.7071068, 0.7071068, 0,
                -0.7071068, 0.7071068, 0,
                 20,        -8.284271, 1)
           Src = ImageBuf("tahoe.exr")
-          Dst = ImageBufAlgo.warp (dst, src, M, "lanczos3");
+          Dst = ImageBufAlgo.warp (dst, src, M, filtername="lanczos3")
 
        .. code-tab:: bash oiiotool
 
@@ -1862,7 +1863,9 @@ Image arithmetic
           ImageBuf Compressed = ImageBufAlgo::rangecompress (Src);
 
           // 3. Now do the resize
-          ImageBuf Dst = ImageBufAlgo::resize (Compressed, "lanczos3", 6.0f,
+          ImageBuf Dst = ImageBufAlgo::resize (Compressed,
+                                               { { "filtername", "lanczos3" }
+                                                 { "filterwidth", 6.0f } },
                                                ROI(0, 640, 0, 480));
 
           // 4. Expand range to be linear again (operate in-place)
@@ -1877,7 +1880,8 @@ Image arithmetic
           Compressed = ImageBufAlgo.rangecompress (Src)
 
           # 3. Now do the resize
-          Dst = ImageBufAlgo.resize (Compressed, "lanczos3", 6.0, ROI(0, 640, 0, 480))
+          Dst = ImageBufAlgo.resize (Compressed, filtername="lanczos3",
+                                     filterwidth=6.0, ROI(0, 640, 0, 480))
 
           # 4. Expand range to be linear again (operate in-place)
           ImageBufAlgo.rangeexpand (Dst, Dst)

src/include/OpenImageIO/filter.h

@@ -5,6 +5,8 @@
 
 #pragma once
 
+#include <memory>
+
 #include <OpenImageIO/export.h>
 #include <OpenImageIO/oiioversion.h>
 #include <OpenImageIO/string_view.h>
@@ -30,6 +32,9 @@ class OIIO_UTIL_API FilterDesc {
 /// The filters are NOT expected to have their results normalized.
 class OIIO_UTIL_API Filter1D {
 public:
+    /// Alias for a shared pointer to a filter
+    using ref = std::shared_ptr<const Filter1D>;
+
     Filter1D(float width)
         : m_w(width)
     {
@@ -45,12 +50,22 @@ class OIIO_UTIL_API Filter1D {
     /// Return the name of the filter, e.g., "box", "gaussian"
     virtual string_view name(void) const = 0;
 
+    /// This static function allocates specific filter implementation for the
+    /// name you provide and returns it as a shared_ptr. It will be
+    /// automatically deleted when the last reference to it is released.
+    /// Example use:
+    ///        auto myfilt = Filter1D::create_shared("box", 1.0f);
+    /// If the name is not recognized, return an empty shared_ptr.
+    static ref create_shared(string_view filtername, float width);
+
     /// This static function allocates and returns an instance of the
     /// specific filter implementation for the name you provide.
     /// Example use:
-    ///        Filter1D *myfilt = Filter1::create ("box", 1);
+    ///        Filter1D *myfilt = Filter1D::create ("box", 1.0f);
     /// The caller is responsible for deleting it when it's done.
     /// If the name is not recognized, return NULL.
+    ///
+    /// Note that the create_shared method is preferred over create().
     static Filter1D* create(string_view filtername, float width);
 
     /// Destroy a filter that was created with create().
@@ -63,7 +78,7 @@ class OIIO_UTIL_API Filter1D {
     static const FilterDesc& get_filterdesc(int filternum);
 
 protected:
-    float m_w;
+    const float m_w;
 };
 
 
@@ -72,6 +87,9 @@ class OIIO_UTIL_API Filter1D {
 /// The filters are NOT expected to have their results normalized.
 class OIIO_UTIL_API Filter2D {
 public:
+    /// Alias for a shared pointer to a filter
+    using ref = std::shared_ptr<const Filter2D>;
+
     Filter2D(float width, float height)
         : m_w(width)
         , m_h(height)
@@ -103,25 +121,38 @@ class OIIO_UTIL_API Filter2D {
     /// Return the name of the filter, e.g., "box", "gaussian"
     virtual string_view name(void) const = 0;
 
+    /// This static function allocates specific filter implementation for the
+    /// name you provide and returns it as a shared_ptr. It will be
+    /// automatically deleted when the last reference do it is released.
+    /// Example use:
+    ///        auto myfilt = Filter2D::create_shared("box", 1.0f, 1.0f);
+    /// If the name is not recognized, return an empty shared_ptr.
+    static ref create_shared(string_view filtername, float width, float height);
+
     /// This static function allocates and returns an instance of the
     /// specific filter implementation for the name you provide.
     /// Example use:
-    ///        Filter2D *myfilt = Filter2::create ("box", 1, 1);
+    ///        Filter2D *myfilt = Filter2D::create ("box", 1.0f, 1.0f);
     /// The caller is responsible for deleting it when it's done.
     /// If the name is not recognized, return NULL.
+    ///
+    /// Note that the create_shared method is preferred over create().
     static Filter2D* create(string_view filtername, float width, float height);
 
     /// Destroy a filter that was created with create().
     static void destroy(Filter2D* filt);
 
+    /// Don't destroy a filter -- used for non-owning shared_ptr.
+    static void no_destroy(const Filter2D*) {}
+
     /// Get the number of filters supported.
     static int num_filters();
     /// Get the info for a particular index (0..num_filters()-1).
     static void get_filterdesc(int filternum, FilterDesc* filterdesc);
     static const FilterDesc& get_filterdesc(int filternum);
 
 protected:
-    float m_w, m_h;
+    const float m_w, m_h;
 };