- Fixed the actual problem in canny instead of putting a bandaid on it

change.txt

@@ -19,7 +19,8 @@ Version : Alpha 0.17
   better results
   * Improved unit tests to explicitly test for this behavior
 - Canny edge detector would fail if threshold was zero and the image had no texture
-  * Improved unit tests for canny edge
+  * HysteresisEdge* code was using a value of 0 to mark traversed regions.  It now uses -1 and sanity checks
+    the lower threshold.
   * Thanks Lucaro for finding this bug
 
 - TODO make those two failed unit tests ago away

main/feature/src/boofcv/alg/feature/detect/edge/CannyEdge.java

@@ -102,18 +102,17 @@ public CannyEdge(BlurFilter<T> blur, ImageGradient<T, D> gradient, boolean saveT
 	 * a list of trace points then the output image is optional.
 	 * </p>
 	 * <p>
-	 * NOTE: Input and output can be the same instance if the image type allows it.
+	 * NOTE: Input and output can be the same instance, if the image type allows it.
 	 * </p>
 	 * @param input Input image. Not modified.
-	 * @param threshLow Lower threshold.
-	 * @param threshHigh Upper threshold.
+	 * @param threshLow Lower threshold. >= 0.
+	 * @param threshHigh Upper threshold. >= 0.
 	 * @param output (Might be option) Output binary image.  Edge pixels are marked with 1 and everything else 0.
 	 */
 	public void process(T input , float threshLow, float threshHigh , ImageUInt8 output ) {
 
-		// can't handle this case because it assumes edges are at most one pixel thick
-		if( threshLow <= 0 && threshHigh <= 0 )
-			throw new IllegalArgumentException("Both thresholds are <= 0.  This will make every pixel an edge!");
+		if( threshLow < 0 || threshHigh < 0 )
+			throw new IllegalArgumentException("Threshold must be >= zero!");
 
 		if( hysteresisMark != null ) {
 			if( output == null )

main/feature/src/boofcv/alg/feature/detect/edge/CannyEdgeDynamic.java

@@ -20,7 +20,6 @@
 
 import boofcv.abst.filter.blur.BlurFilter;
 import boofcv.abst.filter.derivative.ImageGradient;
-import boofcv.alg.misc.ImageMiscOps;
 import boofcv.alg.misc.ImageStatistics;
 import boofcv.struct.image.ImageSingleBand;
 import boofcv.struct.image.ImageUInt8;
@@ -56,16 +55,6 @@ protected void performThresholding(float threshLow, float threshHigh, ImageUInt8
 		threshLow = max*threshLow;
 		threshHigh = max*threshHigh;
 
-		// test for the pathological case
-		if( threshLow <= 0 && threshHigh <= 0 ) {
-			// return nothing
-			if( hysteresisPts != null )
-				hysteresisPts.getContours().clear();
-			if( output != null )
-				ImageMiscOps.fill(output,0);
-		} else {
-			super.performThresholding(threshLow, threshHigh, output);
-		}
+		super.performThresholding(threshLow, threshHigh, output);
 	}
-
 }

main/feature/src/boofcv/alg/feature/detect/edge/HysteresisEdgeTraceMark.java

@@ -36,6 +36,9 @@
  */
 public class HysteresisEdgeTraceMark {
 
+	// after an edge has been traversed it is set to this value
+	public static final float MARK_TRAVERSED = -1;
+
 	// reference to input intensity and direction images
 	private ImageFloat32 intensity; // intensity after edge non-maximum suppression
 	private ImageSInt8 direction; // 4-direction
@@ -62,6 +65,8 @@ public class HysteresisEdgeTraceMark {
 	 */
 	public void process( ImageFloat32 intensity , ImageSInt8 direction , float lower , float upper ,
 						 ImageUInt8 output ) {
+		if( lower < 0 )
+			throw new IllegalArgumentException("Lower must be >= 0!");
 		InputSanityCheck.checkSameShape(intensity,direction,output);
 
 		// set up internal data structures
@@ -95,7 +100,7 @@ protected void trace( int x , int y , int indexInten ) {
 		int indexOut = output.getIndex(x,y);
 		open.grow().set(x,y);
 		output.data[indexOut] = 1;
-		intensity.data[ indexInten ] = 0;
+		intensity.data[ indexInten ] = MARK_TRAVERSED;
 
 		while( open.size() > 0 ) {
 			active.set(open.removeTail());
@@ -127,15 +132,15 @@ protected void trace( int x , int y , int indexInten ) {
 				int bx = active.x-dx, by = active.y-dy;
 
 				if( intensity.isInBounds(fx,fy) && intensity.data[ indexForward ] >= lower ) {
-					intensity.data[ indexForward ] = 0;
+					intensity.data[ indexForward ] = MARK_TRAVERSED;
 					output.unsafe_set(fx, fy, 1);
 					active.set(fx, fy);
 					match = true;
 					indexInten = indexForward;
 					indexDir = prevIndexDir  + dy*intensity.stride + dx;
 				}
 				if( intensity.isInBounds(bx,by) && intensity.data[ indexBackward ] >= lower ) {
-					intensity.data[ indexBackward ] = 0;
+					intensity.data[ indexBackward ] = MARK_TRAVERSED;
 					output.unsafe_set(bx,by,1);
 					if( match ) {
 						open.grow().set(bx,by);
@@ -196,7 +201,7 @@ private boolean check( int x , int y , boolean match ) {
 		if( intensity.isInBounds(x,y)  ) {
 			int index = intensity.getIndex(x,y);
 			if( intensity.data[index] >= lower ) {
-				intensity.data[index] = 0;
+				intensity.data[index] = MARK_TRAVERSED;
 				output.unsafe_set(x,y,1);
 
 				if( match ) {

main/feature/src/boofcv/alg/feature/detect/edge/HysteresisEdgeTracePoints.java

@@ -43,6 +43,10 @@
  */
 public class HysteresisEdgeTracePoints {
 
+	// after an edge has been traversed it is set to this value.  This is also why the lower threshold
+	// must be >= 0
+	public static final float MARK_TRAVERSED = -1;
+
 	// reference to input intensity and direction images
 	private ImageFloat32 intensity; // intensity after edge non-maximum suppression
 	private ImageSInt8 direction; // 4-direction
@@ -70,6 +74,8 @@ public class HysteresisEdgeTracePoints {
 	 * @param upper Upper threshold.
 	 */
 	public void process( ImageFloat32 intensity , ImageSInt8 direction , float lower , float upper ) {
+		if( lower < 0 )
+			throw new IllegalArgumentException("Lower must be >= 0!");
 		InputSanityCheck.checkSameShape(intensity, direction);
 
 		// set up internal data structures
@@ -108,7 +114,7 @@ protected void trace( int x , int y , int indexInten ) {
 		int dx,dy;
 
 		addFirstSegment(x, y);
-		intensity.data[ indexInten ] = 0;
+		intensity.data[ indexInten ] = MARK_TRAVERSED;
 
 		while( open.size() > 0 ) {
 			EdgeSegment s = open.remove( open.size()-1 );
@@ -142,7 +148,7 @@ protected void trace( int x , int y , int indexInten ) {
 
 				// See if the forward point is in bounds and above the lower threshold
 				if( intensity.isInBounds(fx,fy) && intensity.data[ indexForward ] >= lower ) {
-					intensity.data[ indexForward ] = 0;
+					intensity.data[ indexForward ] = MARK_TRAVERSED;
 					p = queuePoints.grow();
 					p.set(fx,fy);
 					s.points.add(p);
@@ -152,7 +158,7 @@ protected void trace( int x , int y , int indexInten ) {
 				}
 				// See if the backwards point is in bounds and above the lower threshold
 				if( intensity.isInBounds(bx,by) && intensity.data[ indexBackward ] >= lower ) {
-					intensity.data[ indexBackward ] = 0;
+					intensity.data[ indexBackward ] = MARK_TRAVERSED;
 					if( match ) {
 						// a match was found in the forwards direction, so start a new segment here
 						startNewSegment(bx, by,s);
@@ -219,7 +225,7 @@ private boolean check( int x , int y , EdgeSegment parent , boolean match ) {
 		if( intensity.isInBounds(x,y)  ) {
 			int index = intensity.getIndex(x,y);
 			if( intensity.data[index] >= lower ) {
-				intensity.data[index] = 0;
+				intensity.data[index] = MARK_TRAVERSED;
 
 				if( !match ) {
 					Point2D_I32 p = queuePoints.grow();

main/feature/test/boofcv/alg/feature/detect/edge/TestCannyEdge.java

@@ -45,17 +45,31 @@ public class TestCannyEdge {
 	Random rand = new Random(234);
 
 	/**
-	 * Image has no texture and the sadistic user and specified a threshold of zero.  Exception should be
-	 * thrown because the threshold is zero
+	 * Image has no texture and the sadistic user and specified a threshold of zero.  Everything should
+	 * be an edge.
 	 */
-	@Test(expected=IllegalArgumentException.class)
+	@Test
 	public void canHandleNoTexture_and_zeroThresh() {
 		ImageUInt8 input = new ImageUInt8(width,height);
 		ImageUInt8 output = new ImageUInt8(width,height);
 
 		CannyEdge<ImageUInt8,ImageSInt16> alg = createCanny(true);
 
 		alg.process(input,0,0,output);
+
+		List<EdgeContour> contour = alg.getContours();
+		assertTrue(contour.size()>0);
+
+		int numEdgePixels = 0;
+		for( EdgeContour e : contour ) {
+			for( EdgeSegment s : e.segments ) {
+				numEdgePixels += s.points.size();
+			}
+		}
+		assertEquals(numEdgePixels,input.width*input.height);
+
+		for( int i = 0; i < output.data.length; i++ )
+			assertEquals(1,output.data[i]);
 	}
 
 	/**

main/feature/test/boofcv/alg/feature/detect/edge/TestCannyEdgeDynamic.java

@@ -31,6 +31,7 @@
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author Peter Abeles
@@ -42,7 +43,7 @@ public class TestCannyEdgeDynamic {
 
 	/**
 	 * Test the pathological case where the input image has no texture.  The threshold will be zero and the
-	 * edge intensity will be zero everywhere.  If not handled correctly things will blow up.
+	 * edge intensity will be zero everywhere.
 	 */
 	@Test
 	public void canHandleNoTexture() {
@@ -57,7 +58,7 @@ public void canHandleNoTexture() {
 		alg.process(input,0.075f,0.3f,output);
 
 		for( int i = 0; i < output.data.length; i++ ) {
-			assertEquals(0,output.data[i]);
+			assertEquals(1,output.data[i]);
 		}
 
 		// try it with a trace now
@@ -66,10 +67,18 @@ public void canHandleNoTexture() {
 		alg.process(input,0.075f,0.3f,output);
 
 		List<EdgeContour> contour = alg.getContours();
-		assertEquals(0,contour.size());
-		for( int i = 0; i < output.data.length; i++ ) {
-			assertEquals(0,output.data[i]);
+		assertTrue(contour.size() > 0);
+
+		int numEdgePixels = 0;
+		for( EdgeContour e : contour ) {
+			for( EdgeSegment s : e.segments ) {
+				numEdgePixels += s.points.size();
+			}
 		}
+		assertEquals(numEdgePixels,input.width*input.height);
+
+		for( int i = 0; i < output.data.length; i++ )
+			assertEquals(1,output.data[i]);
 	}
 
 	/**