patchGNU Octave - Patches: patch #8824, [octave forge] (image) added...

 
 

patch #8824: [octave forge] (image) added affine2d class

Submitter:  Guy B <motherboard>
Submitted:  Mon 07 Dec 2015 10:23:58 PM UTC
 
Category:  Forge : new feature Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Wed 29 Dec 2021 06:04:01 AM UTC, comment #18: 

I applied @Hartmut correction of comment #17 and pushed all the changes, using Carnë's cset.
(sorry - I did not notice that the patch # was missing :-().

Thanks guys, and sorry it took 6 years.

Avinoam Kalma <avinoam>
Project Member
Tue 28 Dec 2021 02:55:56 PM UTC, comment #17: 

Thanks for preparing this new version of the three files, Avinoam. I looked over them, and this looks very good.

Some minor points that came up are:

  • In the help string of affine2d and affine3d there is one occasion each of "isRigit". This is misspelled and should probably read "isRigid" with a D instead of the T.
  • in affine3d it would be nice to also have one blank line right before the 3-line comment of the function outputLimits.

After improving these tiny points, I would be happy to see those three files pushed to the image repository, finally.

Hartmut <hardy>
Sun 26 Dec 2021 06:48:25 AM UTC, comment #16: 

Thanks, @Hartmut for the comments.
I have added:
1. Documentation for affine2d, affine3d
2. Input check for outputLimits
3. Some BISTs (including that of comment #14

Please review the updated version of affine2d.m affine3d.m
(affine.m left unchanged)

(file #52568, file #52569)

Avinoam Kalma <avinoam>
Project Member
Fri 17 Dec 2021 11:15:59 AM UTC, comment #15: 

Thanks, @Hartmut for the detailed review.
I will prepare a corrected version.

Avinoam Kalma <avinoam>
Project Member
Thu 16 Dec 2021 01:47:06 PM UTC, comment #14: 

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...

Hartmut <hardy>
Wed 06 Oct 2021 05:39:11 AM UTC, comment #13: 

Please review the attached modified files.
Thanks

(file #52044, file #52045, file #52046)

Avinoam Kalma <avinoam>
Project Member
Tue 14 Sep 2021 04:38:30 PM UTC, comment #12: 

Thanks, @Carne for the file.
The final m-files are attached here.

Some things to correct:
1.  print_usage within classdef constructor breaks subsequent calls to the class (bug #60729)
2. The tolerances in BISTs should not be in the form of e^(-n), but 1e(-n)

(file #51907, file #51908, file #51909)

Avinoam Kalma <avinoam>
Project Member
Mon 13 Sep 2021 10:20:09 AM UTC, comment #11: 

Bitbucket closed all the mercurial repos so my previous work there was lost.  I'm attaching the changesets of the affine-classe branch I mentioned on comment #7 which already includes the Martin's patch from comment #9

(file #51904)

Carnë Draug <carandraug>
Project Member
Fri 03 Sep 2021 12:02:27 PM UTC, comment #10: 

@Martin: Could you please send the full code (not a patch), so I can check it?

Avinoam Kalma <avinoam>
Project Member
Fri 23 Nov 2018 01:17:28 AM UTC, comment #9: 

Hi, what Avinoam says makes sense to me. I'm attaching a patch with proposed changes and new BIST tests. I checked all my changes against online MATLAB docs. When I call affine2d or affine3d constructor with no parameters, it strangely fails but it shouldn't (I included two xtests demonstrating that). Is that a problem on my side?

(file #45513)

Martin Janda <tojtoj>
Sun 21 May 2017 03:42:39 PM UTC, comment #8: 

Consider the following script:

isTranslation(affine2d ([1 0 0; 0 -1 0; 0 0 1]))
isRigid(affine2d ([1 0 0; 0 -1 0; 0 0 1]))
isSimilarity(affine2d ([1 2 0; 3 4 0; 0 0 1]))
a = invert(affine2d ([1 2 0; 3 4 0; 10 20 1]))
b = affine2d(a.T)

The first 3 command give 1, which is wrong.
The last command fails with an error.

The reasons are:
isTranslation: the check is

check = all ((abs (abs (submatrix) - eye (this.Dimensionality)) < e^-10)(:));

which is wrong - the 2nd abs should be removed. It should be
check = all ((abs(submatrix - eye (this.Dimensionality)) < e^-10)(:));

Since I do not like those e^-10, I would prefer
check = all (abs (submatrix - eye (this.Dimensionality)) < eps)(:));
or even
check = !any (abs (submatrix - eye (this.Dimensionality)) > 0)(:));

Explanation: submatrix should be the Identity matrix (in the accuracy limits), so maybe the check should be different for
the diagonal entries (which should be 1), and the non-diagonal 
(which should be 0)

isRigid: The submatrix should be a rotation matrix, without
scale or shear, so its determinant should be 1:
check = abs ((det (submatrix)) -1) < eps;

isSimilarity: The submatrix should be a rotation matrix multiplied by constant, so the condition is something like

      s2 = submatrix*submatrix';
      ans = all((abs(s2 - s2(1,1)*eye(2)) < ones(2)*eps)(:));

s2 should be the Identity Matrix multiplied by a constant.

When I do
a = invert(affine2d ([1 2 0; 3 4 0; 10 20 1]))
a.T(3,3)-1 = -1.1102e-016
so a.T(3,3) is not 1, and b = affine2d(a.T) fails.
there should be something like
this.T(:,3) = [0; 0; 1];

BTW,  line 72 should be:
this.T = inv (this.T);

 
Avinoam Kalma <avinoam>
Project Member
Sat 20 May 2017 09:53:37 PM UTC, comment #7: 

I have reviewed this and made a new class named affine to implement all of affine2d and affine3d in order to reduce code duplication.  Only the outputLimits method is still implemented on affine2d and affine3d.  Anyone can give a hand there?  See the branch affine-classes or

https://bitbucket.org/carandraug/octave-image/commits/branch/affine-classes

@Avinoam

can you be more explicit what was the problems? There was a lot of changes from you, but only a few were required to pass the new tests.

@motherboard

Can I have a proper name and valid email for the copyright notice?

Carnë Draug <carandraug>
Project Member
Wed 12 Apr 2017 07:46:00 PM UTC, comment #6: 

I have fixed some problems with affine2d (the inverse was problematic).

Please review.

(file #40380)

Avinoam Kalma <avinoam>
Project Member
Wed 09 Dec 2015 07:38:34 AM UTC, comment #5: 

I mean, it is one of the possible arguments for imwarp (which are affine2d, affine3d or projective2d).
And this transform may be either built "by hand" by constructing it with the transformation matrix, or calculated using functions such as imregcorr, imregtform or estimateGeometricTransform.

since Matlab 2013a, it is no longer recommended to use imtransform, which used the transformation structure returned from maketform or cp2tform.

Guy B <motherboard>
Wed 09 Dec 2015 01:58:04 AM UTC, comment #4: 

What do you mean with?

> It is used in imwarp, and is returned from functions such as imregcorr, imregtform and estimateGeometricTransform.

Carnë Draug <carandraug>
Project Member
Tue 08 Dec 2015 01:54:06 PM UTC, comment #3: 

Added affine3d as well (very similar)

(file #35665)

Guy B <motherboard>
Tue 08 Dec 2015 01:47:19 PM UTC, comment #2: 

now the test is actually working...

(file #35664)

Guy B <motherboard>
Tue 08 Dec 2015 12:52:27 PM UTC, comment #1: 

fixed a bug in outputLimits, and added a test for it.

(file #35662)

Guy B <motherboard>
Mon 07 Dec 2015 10:23:58 PM UTC, original submission:  

affine2d class represents the affine transform of 2d vectors (as of matlab 2013a).
It is used in imwarp, and is returned from functions such as imregcorr, imregtform and estimateGeometricTransform.

Guy B <motherboard>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #52568:  affine2d.m added by avinoam (6KiB - application/octet-stream)
file #52569:  affine3d.m added by avinoam (6KiB - application/octet-stream)
file #52044:  affine.m added by avinoam (5KiB - application/octet-stream)
file #52045:  affine3d.m added by avinoam (4KiB - application/octet-stream)
file #52046:  affine2d.m added by avinoam (5KiB - application/octet-stream)
file #51907:  affine.m added by avinoam (5KiB - application/octet-stream)
file #51908:  affine3d.m added by avinoam (4KiB - application/octet-stream)
file #51909:  affine2d.m added by avinoam (5KiB - application/octet-stream)
file #51904:  affine-classes-branch.cset added by carandraug (50KiB - application/octet-stream)
file #45513:  affine_fix.patch added by tojtoj (6KiB - application/octet-stream)
file #40380:  affine2d.m added by avinoam (5KiB - application/octet-stream)
file #35665:  affine2dAnd3d.diff added by motherboard (5KiB - text/x-patch)
file #35664:  affine2d.diff added by motherboard (2KiB - text/x-patch)
file #35662:  affine2d.diff added by motherboard (1KiB - text/x-patch)
file #35658:  affine2d.diff added by motherboard (5KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by tojtoj (Updated the item)
  • -email is unavailable- added by tojtoj
  • -email is unavailable- added by avinoam (Updated the item)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by motherboard (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

     

     

     

    Follow 23 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-12-29 avinoam StatusIn Progress Done
        Open/ClosedOpen Closed
    2021-12-26 avinoam Attached File- Added affine2d.m, #52568
        Attached File- Added affine3d.m, #52569
    2021-10-06 avinoam Attached File- Added affine.m, #52044
        Attached File- Added affine3d.m, #52045
        Attached File- Added affine2d.m, #52046
    2021-09-14 avinoam Attached File- Added affine.m, #51907
        Attached File- Added affine3d.m, #51908
        Attached File- Added affine2d.m, #51909
        StatusNone In Progress
    2021-09-13 carandraug Attached File- Added affine-classes-branch.cset, #51904
    2019-03-07 mtmiller Carbon-CopyRemoved 80942 -
    2018-11-23 tojtoj Attached File- Added affine_fix.patch, #45513
        Carbon-Copy- Added -email is unavailable-
    2018-04-10 mtmiller CategoryNone Forge : new feature
        Summaryadded affine2d class [octave forge] (image) added affine2d class
    2017-04-12 avinoam Attached File- Added affine2d.m, #40380
        Carbon-Copy- Added -email is unavailable-
    2015-12-08 motherboard Attached File- Added affine2dAnd3d.diff, #35665
    2015-12-08 motherboard Attached File- Added affine2d.diff, #35664
    2015-12-08 motherboard Attached File- Added affine2d.diff, #35662
    2015-12-07 motherboard Attached File- Added affine2d.diff, #35658

    Back to the top


    Powered by Savane 3.9