Use clamp wrap mode for IBA::resample()

[alister-chowdhury]

Description

This PR addresses #2480 and #2479

As mentioned in the tickets, I simply added a wrap argument.
This should hopefully not break any existing code, as I've added wrappers to match the previous signatures.

Checklist:

• I have read the contribution guidelines.
• If this is more extensive than a small change to existing code, I
  have previously submitted a Contributor License Agreement
  (individual, and if there is any way my
  employers might think my programming belongs to them, then also
  corporate).
• I have updated the documentation, if applicable.
• I have ensured that the change is tested somewhere in the testsuite
  (adding new test cases if necessary).
• My code follows the prevailing code style of this project.

[lgritz]

This is overall great and exactly what I hoped for. See the comments I made about parameter order (for uniformity with everything else), and to decide on the issue of link vs source back-compatibility (i.e., whether we need to backport or not).

You did resample and fit... maybe we should tackle resize in the same PR so they all continue to match?

[lgritz]

Thanks! This is very close to finished and I appreciate your sending the PR.

[alister-chowdhury]

• Switched roi and wrap ordering.
• Moved inline functions to the bottom under a deprecated heading (I'm in not specific hurry :))
• Added wrap to resize (with a default of clamp)

[alister-chowdhury]

I might be wrong, but the tests may be failing (it's somewhat hard for me to run them where I am currently), because I'm passing the wrap mode into resize via fit, rather than hardcoding the wrap to be WrapClamp.

If this is the case, I'm guessing, it would be preferable to hardcode WrapClamp in fit?

Alternatively we could remove resize from having the option wrap option all together.

[lgritz]

Hmmm, now this is all less obvious.

So the original problem you started with is that resample was getting black fringing because -- unlike resize, which was hard-coded to use clamp wrap mode -- resample was not specifying wrap and thus getting black wrap.

Fit was even weirder -- it looks like it was calling warp with black wrap for the "exact" mode, but calling resize (and thus implicitly using clamp) for the default "inexact" mode.

Now I don't know the right answer. Outstanding questions:

• resample -- Hard code to clamp to match resize? Or provide the option as you did?

• fit -- Was I thinking clearly when I wrote this, or did I just miss the asymmetry? Should it stay this way, or always use clamp, or allow a call-time option?

• resize -- is clamp always the desired result, or should it be an option?

I think that if more than one of them allow an optional user-set wrap mode, they should all have the same default, or it's confusing. People think of resample as a less exact and faster version of resample, and fit as a version of resize that forces an overall frame aspect ratio and letterboxes to make it right.

Was resize right all along and the others should just hard-code clamp as well to match (leaving only the fully general warp having a user-adjustable wrap mode)? Or should all of them have had the option all along? (and if the latter, what's the default that will give people the result they think is right in the widest range of cases?)

[lgritz]

I'm going to need to think about this one...

[lgritz]

I think... maybe the best thing to do is radically scale back the ambition of this PR, and just make it so that resample() hard-codes its call to interppixel to pass the 'WrapClamp' wrap mode, to match the behavior of resize(). I don't think that we need to change any of the public APIs or add new parameters, unless somebody make a strong case for the current behavior of resize or fit is not sensible or needs another parameter that uses will need to ponder every time they use it.

Sorry to ask you to back off much of the work you did. Sometimes that's just the way it works out. I think I've thrown away more of my own code that I've committed over the years.

[alister-chowdhury]

I have to admit, that was my initial thought, but figured someone somewhere may want black edges to contribute?

Would it be worth updating fit aswell (so as to stop asymmetry)?

That's absolutely fine, I'm the first to throw away everything in favour of a better solution :)

Fingers crossed this doesn't break any of the tests (or anyones existing code).

[lgritz]

I edited the subject title of the ticket so that it would be more clear what it ultimately did, for anybody browsing the closed PRs.