octave-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-bug-tracker] [bug #59623] [octave forge] (image) bwdist for quas


From: Hartmut
Subject: [Octave-bug-tracker] [bug #59623] [octave forge] (image) bwdist for quasi-euclidean case fails on i386
Date: Tue, 22 Dec 2020 15:48:32 -0500 (EST)
User-agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:84.0) Gecko/20100101 Firefox/84.0

Follow-up Comment #2, bug #59623 (project octave):

I have read a bit more in "the manual". Here is what I found, please correct
me if I am mistaken at any point. I reason about this piece of code in
bwdist.cc:


static float
quasi_euclidean (short x, short y)
{
  static const float sqrt2_1 = sqrt (2) - 1;
  return abs(x)>abs(y) ? (abs(x) + sqrt2_1 * abs(y)) :
                         (sqrt2_1 * abs(x) + abs(y)) ;
}

(The proposed patch would substitute the last four "abs" with "fabs" in this
code.)

* We are using C++11 in the image package.
* The two input variables (x and y) are of type "short", this is a shorthand
for "short signed integer" (having mostly 16 bits).
* The abs function should work fine with those signed integer input values, I
found just one corner case (intmin as input) where it would behave
unexpectedly.
* But the fabs function should also work fine with signed integer input
values, it just converts the input to float before working on it. Since in our
code we convert the output of abs to float anyways, I think this totally is
fine in our case.
* I have no clue why this test (in line 638 of bwdist.cc) hangs (with the
current code) on i386 systems. The input image (bw) in this test contains only
(0)s and (-2)s. But the two lines before we do nearly the identical test but
with (0)s and (3)s, without any trouble. And the content of the input image bw
should not matter at all, it is just "foreground" for nonzero elements and
"background" for zero elements.

I have tested the patch from comment #0. The resulting file bwdist.oct behaves
well, and all BIST tests still pass on my 64bit linux. The current BISTs also
contain several test of output values with the "quasi_euclidean" parameter. So
the changed piece of code is really tested by the current BISTs.

I suggest that I will PUSH the proposed patch from Rafel to the image repo
during the next couple of days. Please let me know if you have any objections.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59623>

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]