WIP api(IBA): replace filter name or raw Filter2D* with FilterOpt

[lgritz]

Let's consider this a "design review", or a prototype, as I'm really not sure if this is the right design.

Inspired by Issue #4102.

Here's the gist:

There are a number of ImageBufAlgo functions that need to do filtering, and so need a variety of additional parameters like filtername and width, but sometimes other things, and they don't all keep up wth each other in terms of their parameters. Also, in addition to name/width, they also have additional varieties that take a Filter2D*, so that doubles the number of entry points. It's a little confusing. So this PR proposes the following:

• A new helper class FilterOpt that collects the different filtering parameters -- name and width, or Filter2D*, wrap mode, and other params TBD.
• The IBA functions that do filtering get cleaned up and take a FilterOpt& parameter. They have fewer varieties now. The old versions are scheduled for deprecation and moved to the unobtrusive end of imagebufalgo.h. So this doesn't break source compatibility for now, we'll more formally deprecate with warnings later, then for 3.0 remove the old call signatures entirely.
• filter.h cleaned up a bit to give Filter1D and Filter2D a new create_shared static method that returns a shared_ptr.

I think this is a step to making a more uniform and simple API for the IBA functions that take filtering controls. Here are a few examples of how the calls would look.

Old calls:

// specify filter by name and width
r = IBA::resize (src, "lanczos3", 6.0f, roi);

// if we have a Filter2D* already:
r = IBA::resize (src, filtptr, 6.0f, roi);

New calls, various varieties (all one API signature, I'm essentially showing off the various FilterOpt constructors):

r = IBA::resize (src, {}, roi); // default filter choice
r = IBA::resize (src, FilterOpt("lanczos3", 6.0f), roi);  // explicit
r = IBA::resize (src, "lanczos3", roi); // use name and its default width
r = IBA::resize (src, filtptr, roi); // constructs from Filter2D*

r = IBA::resize (src, { .name = "lanczos3", .width = 6.0f }, roi);
// ^^ This fancy syntax needs C++20 officially, but apparently works
// in gcc 8+ clang 10+, MSVS 19.21+, Apple clang 12+, and icx 2021.1,
// all versions we'll have as our minimums relatively shortly.
// Maybe this is the future, as the closest approximation to named
// parameter passing we can get in C++?

Does this seem appealing as an API? Maybe some people will love passing a block of related params as a struct, others will hate it?

What happens when we want to add additional options? Can we only do that when it's ok to break ABI and can never backport new fields? Should we use a PIMPL setup to hide the internals of FilerOpt so it's not part of the ABI (at the expense of pointer indirections to access the fields)? Embed a "version" number in the struct and on the library side use that to cast to the right one for the version it represents?

I expect we'll probably iterate on this design quite a bit before we get consensus. There is no rush on this, it's for master / 2.6.

[lgritz]

An alternate API choice to add to the discussion is using something like keyword arguments.

As background, remember that ParamValue is a utility class we use in ImageSpec and other places. PV is a name/type/value and has simple (string name, T type) constructors for important simple types. So this allows for the following possibility:

    // Function signature with some required essential parameters, and a catch-all
    // list of keyword/value options.
    ImageBuf IBL::myfunc(const ImageBuf& img, int essentialA, float essentialB,
                         cspan<ParamValue> options = {});

    // default call, no optional args:
    R = myfunc(img, 42, 3.14159f);
    // 2 optional args:
    R = myFunc(img, 42, 3.14159f, { { "offset", 0.75f }, { "iterations", 3 } });

So what I'm implying here is that most IBA functions have a few "essential" arguments that are required. Also sometimes they have optional args that override defaults for unusual cases, or arguments that we didn't realize we needed at first but add in subsequent releases (either breaking ABIs, or proliferating many overloaded versions of the function).

For the rarely-used options, or for expanding the exiting functions to have new functionality, or allowing experimental options to be added, we can use this mechanism without cluttering up the call signatures and allowing for future changes without altering ABIs.

You wouldn't want to do this for any old function, because obviously the internals of myfunc needs to walk the optional param list and extract the names/value that it recognizes. But I think it could be a good fit for IBA functions, where the meat of the function is an expensive operation on all pixels of the image, and in that case, a fixed-cost extraction of the arguments at the beginning is not going to add noticeable performance overhead, as it would for a small function that only runs a few instructions at its heart.

Compared to the status quo ("when you realize resize() could use a new parameter, you need to add a new version of resize, and eventually deprecate the old, at least one of which will break ABI and possibly API"), the scheme of using cspan<ParamValue> for an unspecified list of optional arguments has several advantages:

• Add new parameters without needing new function signatures, retire bad ideas without deprecating or removing entire public functions.
• No breaking changes to API or ABI
• This allows us to backport new features more easily in cases when changing the function signatures would have produced ABI changes that we can't allow in release branches.
• Easy to add experimental parameters that are subject to change, without ABI breaks or proliferation of function signatures.
• Possible for caller to assemble a list of options piece by piece and pass it to the function call.
• Optional args are self-documenting at the call site because they are expressed as name,value pairs, it's harder to have bugs such as accidentally swapping the relative order of two fixed-position arguments.

There are some potential down-sides as well:

• Caller needs to know the correct name of the arguments they try to pass, or it won't be recognized (a typo in the name could make it unknown).
• What should we do about unrecognized options: is that an error? a warning? silently ignore options it doesn't know?
• Maybe people hate this or think it's ugly?

[sfriedmapixar]

Personally I prefer the cspan idea. The RI spec had similar provisions for arbitrary keyword arguments, and that has saved us by allowing us to "bolt the landing gear on while still flying the plane" a number of times. It still takes care to deploy something new in a stable way, but it at least makes it possible.

[lgritz]

It's growing on me, too. I hope to have a prototype of it to post this weekend.

@sfriedmapixar (and others), I wonder if you have an opinion on the following, as pertains to uses the optional parameter passing for a function like resize() that takes optional filter parameters and currently comes in one flavor that takes a name/width for the filter, and the other allowing a passing of Filter2D* (which allow callers to implement their own custom filters from scratch, if they feel limited by our built-in selection; though it's entirely unclear to me if anybody needs or uses this). Passing a typed pointer like this is awkward and not especially type safe through the cpsan approach. So I can think of several approaches:

1. Make two versions of the call:
   • Choice A: one version that takes just the options list (you can specify name/width among the options), a second version that takes the Filter2D* additional explicit parameter.
   • Choice B: one version that takes filter+width explicitly, and a second version that takes Filter2D* explicitly.
2. One call only, based on cspan for all options including filtering, even for passing the pointer (awkward and not very type safe, but maybe it's rarely needed and only for experts?).
3. One call only with one "filter choice" parameter and one cspan for all other options, there the filter choice is a special lightweight struct that can be constructed from EITHER name+width, or Filter2D*. (Do we want another helper struct? Do we want to force passing any filter param explicitly rather than allow it to be entirely optional?)
4. Decide nobody uses the Filter2D* version at all, just don't worry about it for now and implement option 2 without any Filter2D* version, cross that bridge when and if people need this feature.

I think part of the philosophical question is whether we consider filter choice to be an essential explicit parameter (implying choice 3 is best), or if it's just another one of the defaulted options.

[lgritz]

Withdrawing this PR in favor of the approach of #4149