From fc51fd4a898a0c3de365979b2195fcf545165f0b Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Wed, 12 Sep 2018 12:16:14 -0500 Subject: [PATCH 1/4] Fix min/max String error bug When you provide an image of a type that can be negative, the Op can attempt to access the string of valid ascii image characters at a negative index, throwing an error. This bug was fixed and a test was added with the code that threw an error before, as well as making sure that the clamped ascii image is not the same as the unclamped image. --- .../imagej/ops/image/ascii/DefaultASCII.java | 6 +++- .../net/imagej/ops/image/ascii/ASCIITest.java | 35 ++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java b/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java index dcf247105..2c9193603 100644 --- a/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java +++ b/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java @@ -46,7 +46,7 @@ *

* Only the first two dimensions of the image are considered. *

- * + * * @author Curtis Rueden */ @Plugin(type = Ops.Image.ASCII.class) @@ -107,6 +107,8 @@ public static > String ascii( final Cursor cursor = image.localizingCursor(); final int[] pos = new int[image.numDimensions()]; final T tmp = image.firstElement().copy(); + final T zero = tmp.copy(); + zero.setZero(); while (cursor.hasNext()) { cursor.fwd(); cursor.localize(pos); @@ -115,6 +117,8 @@ public static > String ascii( // normalized = (value - min) / (max - min) tmp.set(cursor.get()); tmp.sub(min); + // NB: if a value is below min we set tmp to zero. + if (tmp.compareTo(zero) < 0) tmp.setZero(); final double normalized = tmp.getRealDouble() / span.getRealDouble(); final int charLen = CHARS.length(); diff --git a/src/test/java/net/imagej/ops/image/ascii/ASCIITest.java b/src/test/java/net/imagej/ops/image/ascii/ASCIITest.java index 3ceb8f2f2..37423cbec 100644 --- a/src/test/java/net/imagej/ops/image/ascii/ASCIITest.java +++ b/src/test/java/net/imagej/ops/image/ascii/ASCIITest.java @@ -40,8 +40,9 @@ /** * Tests {@link net.imagej.ops.Ops.Image.ASCII}. - * + * * @author Leon Yang + * @author Gabe Selzer */ public class ASCIITest extends AbstractOpTest { @@ -68,4 +69,36 @@ public void testDefaultASCII() { assertTrue(ascii.charAt(i * (width + 1) + width) == '\n'); } } + + @Test + public void testASCIIMinMax() { + // character set used in DefaultASCII, could be updated if necessary + final String CHARS = "#O*o+-,. "; + final int len = CHARS.length(); + final int width = 10; + final byte[] array = new byte[width * len]; + for (int i = 0; i < len; i++) { + for (int j = 0; j < width; j++) { + array[i * width + j] = (byte) (i * width + j); + } + } + final UnsignedByteType min = new UnsignedByteType(0); + final UnsignedByteType max = new UnsignedByteType(90); + final Img img = ArrayImgs.unsignedBytes(array, width, + len); + final String ascii = (String) ops.run(DefaultASCII.class, img, min, max); + for (int i = 0; i < len; i++) { + for (int j = 0; j < width; j++) { + assertTrue(ascii.charAt(i * (width + 1) + j) == CHARS.charAt(i)); + } + assertTrue(ascii.charAt(i * (width + 1) + width) == '\n'); + } + + // make sure that the clamped ASCII string is not equivalent to the + // unclamped String + final String asciiUnclamped = (String) ops.run(DefaultASCII.class, img); + + assertTrue(asciiUnclamped != ascii); + + } } From bd81a099ccbcdd75d219fdc551f402bdd3f4e979 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 17 Sep 2018 11:00:58 -0500 Subject: [PATCH 2/4] DefaultASCII: reorganize math Note: if the RealType of the image is not a type supporting floating point numbers, then div(span) on that type would cause a rounding error. This is the reason that normalized has to call getRealDouble() twice, although it seems it would be nicer if we used the Imglib2 Type Math. --- .../net/imagej/ops/image/ascii/DefaultASCII.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java b/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java index 2c9193603..474daf99f 100644 --- a/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java +++ b/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java @@ -117,13 +117,17 @@ public static > String ascii( // normalized = (value - min) / (max - min) tmp.set(cursor.get()); tmp.sub(min); - // NB: if a value is below min we set tmp to zero. - if (tmp.compareTo(zero) < 0) tmp.setZero(); final double normalized = tmp.getRealDouble() / span.getRealDouble(); final int charLen = CHARS.length(); - final int charIndex = (int) (charLen * normalized); - c[index] = CHARS.charAt(charIndex < charLen ? charIndex : charLen - 1); + int charIndex = (int) (charLen * normalized); + + // NB: clamp charIndex to [0, charLen) to prevent + // StringIndexOutOfBoundsExceptions + if (charIndex < 0) charIndex = 0; + if (charIndex >= charLen) charIndex = charLen - 1; + + c[index] = CHARS.charAt(charIndex); } return new String(c); From fb40484a8547b146cb9d6edd79938c41d2967918 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 17 Sep 2018 11:59:24 -0500 Subject: [PATCH 3/4] ASCIITest: use assertEquals instead of assertTrue --- .../net/imagej/ops/image/ascii/ASCIITest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/java/net/imagej/ops/image/ascii/ASCIITest.java b/src/test/java/net/imagej/ops/image/ascii/ASCIITest.java index 37423cbec..467e54add 100644 --- a/src/test/java/net/imagej/ops/image/ascii/ASCIITest.java +++ b/src/test/java/net/imagej/ops/image/ascii/ASCIITest.java @@ -29,7 +29,7 @@ package net.imagej.ops.image.ascii; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; import net.imagej.ops.AbstractOpTest; import net.imglib2.img.Img; @@ -64,9 +64,9 @@ public void testDefaultASCII() { final String ascii = (String) ops.run(DefaultASCII.class, img); for (int i = 0; i < len; i++) { for (int j = 0; j < width; j++) { - assertTrue(ascii.charAt(i * (width + 1) + j) == CHARS.charAt(i)); + assertEquals(ascii.charAt(i * (width + 1) + j), CHARS.charAt(i)); } - assertTrue(ascii.charAt(i * (width + 1) + width) == '\n'); + assertEquals(ascii.charAt(i * (width + 1) + width), '\n'); } } @@ -89,16 +89,16 @@ public void testASCIIMinMax() { final String ascii = (String) ops.run(DefaultASCII.class, img, min, max); for (int i = 0; i < len; i++) { for (int j = 0; j < width; j++) { - assertTrue(ascii.charAt(i * (width + 1) + j) == CHARS.charAt(i)); + assertEquals(ascii.charAt(i * (width + 1) + j), CHARS.charAt(i)); } - assertTrue(ascii.charAt(i * (width + 1) + width) == '\n'); + assertEquals(ascii.charAt(i * (width + 1) + width), '\n'); } - // make sure that the clamped ASCII string is not equivalent to the - // unclamped String + // make sure that the values of the min/max ascii are the same as the + // unclamped version (which will set the minimum and maximum to those of the + // data, which are the same as the ones that we set). final String asciiUnclamped = (String) ops.run(DefaultASCII.class, img); - - assertTrue(asciiUnclamped != ascii); + assertEquals(asciiUnclamped, ascii); } } From f717bad8491ead0fdcd0f9bed8c4ee9f3a974ed3 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 14 Jan 2019 11:14:57 -0600 Subject: [PATCH 4/4] Use helper Ops to simplify code --- .../imagej/ops/image/ascii/DefaultASCII.java | 50 +++++++++---------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java b/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java index 474daf99f..0e7a979ce 100644 --- a/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java +++ b/src/main/java/net/imagej/ops/image/ascii/DefaultASCII.java @@ -36,6 +36,7 @@ import net.imglib2.Cursor; import net.imglib2.IterableInterval; import net.imglib2.type.numeric.RealType; +import net.imglib2.type.numeric.real.DoubleType; import net.imglib2.util.Pair; import org.scijava.plugin.Parameter; @@ -79,24 +80,28 @@ public String calculate(final IterableInterval input) { if (min == null) min = minMax.getA(); if (max == null) max = minMax.getB(); } - return ascii(input, min, max); + + DoubleType minSource = new DoubleType(min.getRealDouble()); + DoubleType maxSource = new DoubleType(max.getRealDouble()); + DoubleType minTarget = new DoubleType(0); + DoubleType maxTarget = new DoubleType(CHARS.length()); + + IterableInterval converted = ops().convert().float64(input); + IterableInterval normalized = ops().image().normalize(converted, + minSource, maxSource, minTarget, maxTarget); + + return ascii(normalized); } // -- Utility methods -- - public static > String ascii( - final IterableInterval image, final T min, final T max) - { + public static String ascii(final IterableInterval image) { final long dim0 = image.dimension(0); final long dim1 = image.dimension(1); - // TODO: Check bounds. + final int w = (int) (dim0 + 1); final int h = (int) dim1; - // span = max - min - final T span = max.copy(); - span.sub(min); - // allocate ASCII character array final char[] c = new char[w * h]; for (int y = 1; y <= h; y++) { @@ -104,30 +109,21 @@ public static > String ascii( } // loop over all available positions - final Cursor cursor = image.localizingCursor(); + final Cursor cursor = image.localizingCursor(); final int[] pos = new int[image.numDimensions()]; - final T tmp = image.firstElement().copy(); - final T zero = tmp.copy(); - zero.setZero(); while (cursor.hasNext()) { cursor.fwd(); cursor.localize(pos); final int index = w * pos[1] + pos[0]; - // normalized = (value - min) / (max - min) - tmp.set(cursor.get()); - tmp.sub(min); - final double normalized = tmp.getRealDouble() / span.getRealDouble(); - - final int charLen = CHARS.length(); - int charIndex = (int) (charLen * normalized); - - // NB: clamp charIndex to [0, charLen) to prevent - // StringIndexOutOfBoundsExceptions - if (charIndex < 0) charIndex = 0; - if (charIndex >= charLen) charIndex = charLen - 1; - - c[index] = CHARS.charAt(charIndex); + // grab the value from the normalized image, convert it to an ASCII char. + // N.B. if the original value was at the max for the type range it will be + // equal to the length of the char array after normalization. Thus to + // prevent an exception when converting to ASCII we subtract one when the + // normalized image value is equal to the length. + int val = (int) cursor.get().getRealDouble(); + if (val == CHARS.length()) val--; + c[index] = CHARS.charAt(val); } return new String(c);