Those changes had been made to the stable branch. I back them out but left them on the default branch.  They will be aprt of the next minor release but not part of the next patch release.

Avinoam wrote:

> imremap was created in http://hg.code.sf.net/p/octave/image/rev/1a4b84759d2c, Jan 2007.
>
> Maybe interp2 did not support (bi)cubic interpolation.

I already checked and it did exist in core back in 2006. But it had a few FIXME notes on the source at the time, and maybe those affected its use for image. Anyway, if you make the change to fix, please add a simple test that only passes after the fix you apply.

> Carnë makes a good point, there should always be a comment when special-casing something.

I have come to find that a test for that special case works best. Comments can become out of date.

> Is the original author still around to explain this special case?

He's no longer involved with Octave. I would guess that he wouldn't remember why the special case anyway, it's been 15 years.

---

Since you're talking about cleaning up imremap, I would advise in investing your time in implementing imwarp instead. That would be matlab compatible. imremap is Octave only and is meant to do a  very similar job. Eventually, imremap should be throw out and replaced with the Matlab compatible function, so you might as well do that now.

Sorry, preallocation should be:

sz = size (im);
sz(1:2) = size (XI);
warped = zeros(sz);

Obviously I haven't tested any of the code I'm suggesting... :)

Cris.

Would be nice to know why there was a special implementation for the cubic interpolation. This method already existed in core's interp2 back in 2006 when imremap was written. It may be working around a bug in core or maybe it's done for performance.

Anyway, we don't know why it was done so I guess it's fine to remove if it fixes a problem now. Can you add a minimal test case so that it will fail if someone tries to revert this in the future?

The problem seems to be from graybicubic call in grayinterp (imremap.m)

function [warped, valid] = grayinterp(im, XI, YI, interp, extrapval)
  if (strcmp(interp, "cubic"))
    warped = graybicubic(double(im), XI, YI, 0);
  else
    warped = interp2(double(im), XI, YI, interp, 0);
  endif
  valid = !isna(warped);
  warped(!valid) = extrapval;
endfunction

If we use interp2 in the case of cubic interpolation:

function [warped, valid] = grayinterp(im, XI, YI, interp, extrapval)
%  if (strcmp(interp, "cubic"))
%    warped = graybicubic(double(im), XI, YI, 0);
%  else
    warped = interp2(double(im), XI, YI, interp, 0);
%  endif
  valid = !isna(warped);
  warped(!valid) = extrapval;
endfunction

The result looks better.