[Octave-patch-tracker] [patch #9077] image package: new function imfill.m

[Carnë Draug]

Follow-up Comment #19, patch #9077 (project octave):

* line 22: You have added one of the two supported (but in Matlab
undocumented) syntax versions: imfill(I, 'holes'). Then it would probably a
good idea to add the second supported but undocumented version as well:
imfill(I, CONN, 'holes').
* line 28: two typos
* line 90: in this comment, we need to remove the first half: "imfill(BW,
CONN)", this is no valid syntax.

Done, done, and done. Thank you
http://hg.code.sf.net/p/octave/image/rev/c998fe65b8fb

* I have seen how you avoid my stupy "padding", very nice :)
* on several occasions you are calling the two helper functions check_loc and
the set_nonborder_pixels with the full variable img as parameter. Isn't this
creating a second memory copy of the (potentially huge) image data in img? I
was intentionally only calling check_loc with "size(img)" instead of "img" in
order to save this memory. And since your "fancy" way of avoiding the "stupid"
padding all around, was motivated by saving memory, shouldn't we try to avoid
this second full copy of the image data here as well? (But maybe my
understanding of when Octave will do a memory copy is not exhaustive...)

Two different cases:

* check_loc - This does not create a new copy of the image.  Octave does copy
on write https://en.wikipedia.org/wiki/Copy-on-write which means that if you
pass a variable to a function in the list of arguments, no copy is made until
that function changes the variable. Since check_loc never modifies the image
(it only uses img to get the size), no copy of the image is ever made.  There
was no technical need for that change, but it allows to use "ndims (img)"
instead of "numel (sz)" which is a bit clearer on the meaning.

* set_nonborder_pixels - This creates a copy of the image but it's a copy that
you will use. There is no extra copy when you return the created marker, and
you will need at the same time as the original (mask) when calling
imreconstruct.


  ## marker is a copy but we really need it to call imreconstruct
  marker = set_nonborder_pixels (mask, conn, false);
  imreconstruct (marker, mask, conn);

[...]

function marker = set_nonborder_pixels (img, conn, val)
  [...]
  marker = img;
  ## marker, is img, which is mask on the caller.  No copy has been made yet.
  marker(nonborder_idx{:}) = val;
  ## This triggered a copy.  This is returned to the caller, no new copy
  ## will be made
endfunction


But the issue of extra copies on the previous 2d solution using padarray is
not at that step.  The code now also makes a copy, and the extra memory used
for the padding is usually negligible (except for images with many dimensions
where some of the dimensions are "short").  The issue was the copy triggered
by the the unpadding.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?9077>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/