bugGNU Octave - Bugs: bug #50153, image package: bwperim result on...


bug #50153: image package: bwperim result on 1x1 input incorrect

Submitted by:  Hartmut <hardy>
Submitted on:  Thu Jan 26 12:50:57 2017  
Category: Octave Forge PackageSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Matlab Compatibility
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: 4.2.0Operating System: Any

Add a New Comment (Rich MarkupRich Markup):

You are not logged in

Please log in, so followups can be emailed to you.


(Jump to the original submission Jump to the original submission)

Mon Apr 3 12:50:20 2017, comment #10:

Everyone's happy, so I pushed it http://hg.code.sf.net/p/octave/image/rev/c2a397b5bd8c

Carnë Draug <carandraug>
Project Member
Sun Apr 2 05:53:53 2017, comment #9:

Checking also Carnë's patch from comment #8, I got the
same results as Hartmut (comment #8), and the same conclusion.

Avinoam Kalma <avinoam>
Project Member
Sat Apr 1 21:37:49 2017, comment #8:

I have read through Carne's proposed patch in comment #7. To be honest, I didn't quite understand all in the convoluted section (as the code itself names it) where one check was dropped. But

  • the patch fixes the original issue in comment #0
  • all tests still pass
  • it adds some (passing) tests, including a new test for the issue in comment #0
  • the approach itself sound reasonable to me

So I would be happy to see this patch applied to the image repo. Thanks for working this out, Carne!

Hartmut <hardy>
Mon Mar 27 17:06:14 2017, comment #7:

The proposed patch would error on the cases where connectivity is just [0 0 0; 0 1 0; 0 0 0]. A very corner case but it hints that the issue is not 1x1 image. I think the issue is more generalised about not considering border elements in dimensions of length 1. This alternative patch


seems to fix the original issue plus several new ones (such as image of `true(1, 1, 6)`) This just removes a check that was just wrong. But maybe the check was there for some reason that we have no test for. I think the test was just wrong (mea culpa) so if no one can see anything wrong with it, I will push that cset.

Carnë Draug <carandraug>
Project Member
Mon Jan 30 20:45:06 2017, comment #6:

I have tested the (second) patch from comment #5 (file #39615):

  • The new test for the use case in comment #0 passes. So it fixes the original issue.
  • All other tests still pass.

I would support to commit this patch to the image repository. Thanks, Avinoam!

Hartmut <hardy>
Mon Jan 30 20:26:26 2017, comment #5:

Indeed there was a mistake in my patch, sorry.
Here is the fixed patch with a test.

(file #39615)

Avinoam Kalma <avinoam>
Project Member
Mon Jan 30 19:54:16 2017, comment #4:

I have tested Avinoam's patch from comment #1:

  • There is a typo in the line with the special case. The code there should probably read "perim = bw;", currently this is swapped in the patch.
  • Could you add a small test for the use case in comment #0?
Hartmut <hardy>
Mon Jan 30 19:11:31 2017, comment #3:

The fix from comment #1 attached as a patch.

(file #39614)

Avinoam Kalma <avinoam>
Project Member
Mon Jan 30 18:13:38 2017, comment #2:

I don't have any objections against a "quick fix" solution, that simply introduces a special case for single pixel inputs.

(@Avinoam: But I don't quite recognize the current code of bwperim.m in your proposed "patch" in comment #1.)

Hartmut <hardy>
Mon Jan 30 17:51:19 2017, comment #1:

I have check all 2x2 logical matrices, and some 2x3 and 3x3,
and it seems that bwperim is correct in all of them.
The only exception is 1x1, so what about a trivial check:

Avinoam Kalma <avinoam>
Project Member
Thu Jan 26 12:50:57 2017, original submission:

This happens with Octave 4.2.0 and the current image-2.6.1.

The result of this little script

returns twice a single 0 in Octave, but it returns twice a 1 in Matlab. The Matlab result makes more sense to me, since even a 1x1 region DOES have a perimeter, which is the pixel itself.

(I hit this when implementing a new feature for regionprops. But I will probably work around this bwperim issue in regionprops, to not create too many inter-dependencies of patches.)

Hartmut <hardy>


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

Attach File(s):

Attached Files
file #39615:  bwperim_v2.diff added by avinoam (735B - application/octet-stream)
file #39614:  bwperim.diff added by avinoam (505B - application/octet-stream)


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -unavailable- added by avinoam (Updated the item)
  • -unavailable- added by hardy (Submitted the item)
  • -unavailable- added by hardy (cc-ing people who care about the image package)
  • -unavailable- added by hardy (cc-ing people who care about the image package)

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.


    Please enter the title of George Orwell's famous dystopian book (it's a date):



    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon Apr 3 12:50:20 2017carandraugStatusPatch Reviewed=>Fixed
    Mon Jan 30 21:41:40 2017avinoamStatusConfirmed=>Patch Reviewed
    Mon Jan 30 20:26:26 2017avinoamAttached File-=>Added bwperim_v2.diff, #39615
    Mon Jan 30 19:11:31 2017avinoamAttached File-=>Added bwperim.diff, #39614
      Summaryimage package: bwperim result on 1x1input incorrect=>image package: bwperim result on 1x1 input incorrect
    Fri Jan 27 08:38:56 2017avinoamStatusNone=>Confirmed
    Thu Jan 26 12:50:57 2017hardyCarbon-Copy-=>Added avinoam
      Carbon-Copy-=>Added carandraug

    Back to the top

    Powered by Savane 3.1-cleanup1