[Octave-patch-tracker] [patch #8824] [octave forge] (image) added affine2d class

[Hartmut]

Follow-up Comment #14, patch #8824 (project octave):

I have had a close look at the three files from comment #13. 

This is good in affine2d and affine3d:
* I tested the example code from the Matlab help page. And it produces the
same results as this Octave code, at least for bit I could check (mainly the
missing imwarp.m reduced my possibilities to compare results.)
* All methods of this class are implemented and they did something reasonably
when I tested them with 3 different types of tforms.
* All BIST tests pass, and those cover all input versions of the methods.

This should probably be improved in affine2d and affine3d:
* The help text is not display when doing for example "help affine2d", but
this is a different bug, namely bug #61521.
* The help text is a bit too short for my taste. I think it would be nice if
it mentioned at least the two possible syntax versions: tform = affine2d() and
tform = affine2d(M). (Same for affine3d.)
* The is no input check for the method outputLimits. This leads to an awkard
error when doing for example "[x y] = outputLimits(tform)". It should be
fairly easy to add this input check to the code of the method, there is a good
starting point in the comments of the method outputLimits of the parent class
in affine.m. (Same for affine3d.)
* In affine3d there is currently no BIST test to check the method invert. I
will propose a test code to add below.

Note: In the parent class affine.m is a method outputLimits that is neither
used by affine2d nor by affine3d. Nor is it tested anywhere. We could keep it,
to be use later by a 4d child class, or we could delete this method to reduce
code maintenance burden. I would tend to keep it.

Here is a small test that could be added as BIST to affine3d to test the
invert method there:


%!test
%! a = 23;
%! M = [cosd(a) 0 sind(a) 0;
%!      0       1      0  0;
%!     -sind(a) 0 cosd(a) 0;
%!      0       0      0  1];
%! tform = affine3d (M);
%! tform2 = invert (tform);
%! assert (tform.T * tform2.T, diag([1 1 1 1]), eps);


In total this looks like a very good foundation to move towards the modern
geometric transformation functions that Matlab introduced white some years
ago. Currently these "naked" classes affine2d and affine3d would not be of not
much use to any end user, since the following other functions are also still
missing in Octave: imregtform, imregcorr, fitgeotrans, randomAffine2d,
randomAffine3d, and mainly imwarp. But after having the two foundation classes
of this patch, it would be much easier to also generate the mentioned missing
functions as well.

@Avinoam: Could you add the above mentioned missing bits to those code files
and then prepare a final patch file? I would very much  like to see this code
pushed to the image repo in the next weeks, after those many years of
preparation and waiting...

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?8824>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/