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

 
 

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

Submitter:  Hartmut <hardy>
Submitted:  Thu 26 Jan 2017 12:50:57 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 4.2.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 03 Apr 2017 12:50:20 PM UTC, comment #10: 

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

 Carnë Draug <carandraug>
Group Member
Sun 02 Apr 2017 05:53:53 AM UTC, 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>
Group Member
Sat 01 Apr 2017 09:37:49 PM UTC, 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 27 Mar 2017 05:06:14 PM UTC, 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

https://bitbucket.org/carandraug/octave-image/commits/c2a397b5bd8c65d8e2122ee5b7bf9eea874254a9

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>
Group Member
Mon 30 Jan 2017 08:45:06 PM UTC, 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 30 Jan 2017 08:26:26 PM UTC, comment #5: 

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


(file #39615)

 Avinoam Kalma <avinoam>
Group Member
Mon 30 Jan 2017 07:54:16 PM UTC, 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 30 Jan 2017 07:11:31 PM UTC, comment #3: 

The fix from comment #1 attached as a patch.

(file #39614)

 Avinoam Kalma <avinoam>
Group Member
Mon 30 Jan 2017 06:13:38 PM UTC, 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 30 Jan 2017 05:51:19 PM UTC, 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:


  if (numel(bw) == 1)
    perim = bw;
  else
    perim = (! imerode (bw, conn)) & bw;
  end


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

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

The result of this little script

BW = true;
P = bwperim(BW)
Q = bwperim(BW, 8)


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 Files:
   
   
Comment:
   

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
  • -email is unavailable- added by avinoam (Updated the item)
  • -email is unavailable- added by hardy (Submitted the item)
  • -email is unavailable- added by hardy (cc-ing people who care about the image package)
  • -email is unavailable- added by hardy (cc-ing people who care about the image package)

    •  

    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 group members can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-04-03 carandraug StatusPatch Reviewed Fixed
        Open/ClosedOpen Closed
    2017-01-30 avinoam StatusConfirmed Patch Reviewed
    2017-01-30 avinoam Attached File- Added bwperim_v2.diff, #39615
    2017-01-30 avinoam Attached File- Added bwperim.diff, #39614
        Summaryimage package: bwperim result on 1x1input incorrect image package: bwperim result on 1x1 input incorrect
    2017-01-27 avinoam StatusNone Confirmed
    2017-01-26 hardy Carbon-Copy- Added avinoam
        Carbon-Copy- Added carandraug

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.13-df36.
    Corresponding source code