[GRASS-SVN] r52232 - grass-addons/grass7/imagery/i.segment

svn_grass at osgeo.org svn_grass at osgeo.org
Tue Jun 26 15:53:05 PDT 2012


Author: momsen
Date: 2012-06-26 15:53:04 -0700 (Tue, 26 Jun 2012)
New Revision: 52232

Added:
   grass-addons/grass7/imagery/i.segment/flag.h
   grass-addons/grass7/imagery/i.segment/flag_clr_all.c
   grass-addons/grass7/imagery/i.segment/flag_create.c
   grass-addons/grass7/imagery/i.segment/flag_destroy.c
   grass-addons/grass7/imagery/i.segment/flag_get.c
   grass-addons/grass7/imagery/i.segment/flag_set.c
   grass-addons/grass7/imagery/i.segment/flag_unset.c
Modified:
   grass-addons/grass7/imagery/i.segment/create_isegs.c
   grass-addons/grass7/imagery/i.segment/iseg.h
   grass-addons/grass7/imagery/i.segment/main.c
   grass-addons/grass7/imagery/i.segment/open_files.c
   grass-addons/grass7/imagery/i.segment/parse_args.c
   grass-addons/grass7/imagery/i.segment/write_output.c
Log:
general clean up, using TRUE/FALSE instead of 1/0, and other suggestions from Hamish.

Modified: grass-addons/grass7/imagery/i.segment/create_isegs.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/create_isegs.c	2012-06-26 11:55:52 UTC (rev 52231)
+++ grass-addons/grass7/imagery/i.segment/create_isegs.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -4,7 +4,7 @@
 
 #include <stdlib.h>
 #include <float.h>		/* to get value of LDBL_MAX -> change this if there is a more usual grass way */
-							  /* #include <math.h>    *//* for sqrt() and pow() */
+										    /* #include <math.h>    *//* for sqrt() and pow() */
 #include <grass/gis.h>
 #include <grass/glocale.h>
 #include <grass/raster.h>
@@ -13,6 +13,7 @@
 #include "iseg.h"
 
 #define LINKM
+#define REVERSE
 
 int create_isegs(struct files *files, struct functions *functions)
 {
@@ -32,11 +33,10 @@
 	lower_bound = upper_bound = 0;
     }				/* just one time through loop */
     else {
-	if (Rast_read_range(files->bounds_map, files->bounds_mapset, &range) <
-	    0) {
-	    G_fatal_error(_("Unable to read fp range for raster map <%s>"),
+	if (Rast_read_range(files->bounds_map, files->bounds_mapset, &range) != 1) {	/* returns -1 on error, 2 on empty range, quiting either way. */
+	    G_fatal_error(_("No min/max found in raster map <%s>"),
 			  files->bounds_map);
-	}			/* TODO, still should close files? */
+	}
 	Rast_get_range_min_max(&range, &lower_bound, &upper_bound);	/* todo, faster way to do this?  maybe do it manually when open and write to the segment file? But it is just once.... */
     }
 
@@ -75,7 +75,6 @@
     G_verbose_message("writing scaled input (layer 1) to output file");
     G_verbose_message("weighted flag = %d", files->weighted);
     for (row = 0; row < files->nrows; row++) {
-	G_percent(row, files->nrows, 1);	/* TODO this didn't get displayed in the output??? Does it get erased when done? */
 	for (col = 0; col < files->ncols; col++) {
 	    /*files->out_val[0] = files->out_val[0]; *//*segment number *//* just copying the map for testing. */
 	    /* files->out_val[0] = col + row; */
@@ -85,6 +84,7 @@
 	    files->out_val[1] = 1;	/*processing flag */
 	    segment_put(&files->out_seg, (void *)files->out_val, row, col);
 	}
+	G_percent(row, files->nrows, 1);
     }
 
     /*speed test... showed a difference of 1min 9s for G_malloc and 34s for linkm. (with i < 2000000000   */
@@ -111,7 +111,7 @@
 #endif
 
     G_message("end speed test");
-    return 0;
+    return TRUE;
 }
 
 int ll_test(struct files *files, struct functions *functions)
@@ -230,13 +230,13 @@
 
     G_message("writing fake data to segmentation file");
     for (row = 0; row < files->nrows; row++) {
-	G_percent(row, files->nrows, 1);	/* TODO this didn't get displayed in the output??? Does it get erased when done? */
 	for (col = 0; col < files->ncols; col++) {
 	    /*files->out_val[0] = files->out_val[0]; *//*segment number *//* just copying the map for testing. */
 	    files->out_val[0] = col + row;
-	    files->out_val[1] = 1;	/*processing flag */
+	    files->out_val[1] = TRUE;	/*processing flag */
 	    segment_put(&files->out_seg, (void *)files->out_val, row, col);
 	}
+	G_percent(row, files->nrows, 1);
     }
 
     /*test how many pixels can be made and disposed of */
@@ -264,7 +264,7 @@
     G_message("after link_cleanup(Rkn_token)");
 
     G_message("end linked list test");
-    return 0;
+    return TRUE;
 }
 
 int test_pass_token(struct pixels **head, struct files *files)
@@ -282,7 +282,7 @@
 	G_message("Added: Rin %d: row: %d, col: %d", n,
 		  newpixel->row, newpixel->col);
     }
-    return 0;
+    return TRUE;
 
 }
 
@@ -327,7 +327,7 @@
 
 	threshold = functions->threshold;	/* TODO, consider making this a function of t. */
 
-	endflag = 1;
+	endflag = TRUE;
 
 	/* Set candidate flag to true/1 for all pixels TODO: for polygon/vector constraint, need to just set to true for those being processed */
 	if (files->bounds_map == NULL) {	/*normal processing */
@@ -338,7 +338,7 @@
 		    /* TODO: if we are starting from seeds...and only allow merges between unassigned pixels
 		     *  and seeds/existing segments, then this needs an if (and will be very inefficient)
 		     * maybe consider the sorted array, btree, map... but the number of seeds could still be high for a large map */
-		    files->out_val[1] = 1;	/*candidate pixel flag */
+		    files->out_val[1] = TRUE;	/*candidate pixel flag */
 		    segment_put(&files->out_seg, (void *)files->out_val, row,
 				col);
 
@@ -355,9 +355,9 @@
 				col);
 
 		    if (files->bounds_val == files->current_bound)	/*TODO could move this if statement one line up, and only set "1" flags if we can assume all flags are already zero.  (i.e. only get/put the ones we want to set to 1.) */
-			files->out_val[1] = 1;	/*candidate pixel flag */
+			files->out_val[1] = TRUE;	/*candidate pixel flag */
 		    else
-			files->out_val[1] = 0;
+			files->out_val[1] = FALSE;
 
 		    segment_put(&files->out_seg, (void *)files->out_val, row,
 				col);
@@ -373,24 +373,22 @@
 	/*process candidate pixels */
 	G_verbose_message("Row percent complete for pass number %d: ", t);
 	/*check each pixel, start the processing only if it is a candidate pixel */
-	
 	/* for validation, select one of the two... could make this IFDEF or input parameter */
 	/* reverse order 
+	 */
+#ifdef REVERSE
 	for (row = files->nrows; row >= 0; row--) {
-	    for (col = files->ncols; col >= 0 ; col--) {
-		G_percent(files->nrows-row, files->nrows, 1);
-end reverse order */
-/* "normal" order */
+	    for (col = files->ncols; col >= 0; col--) {
+#else
 	for (row = 0; row < files->nrows; row++) {
 	    for (col = 0; col < files->ncols; col++) {
-		G_percent(row, files->nrows, 1); /*end normal order */	/* TODO, can a message be included with G_percent? */
+#endif
 
-
 		G_debug(4,
 			"Next starting pixel from next row/col, not from Rk");
 
 		segment_get(&files->out_seg, (void *)files->out_val, row, col);	/*TODO small time savings - if candidate_count reaches zero, bail out of these loops too? */
-		if (files->out_val[1] == 1 && files->out_val != NULL) {	/* out_val[1] is the candidate pixel flag, want to process the 1's */ /*TODO MASK handling - checking if we have a segment ID already, in open_files() we will put nulls in [0] slot... better/faster way to do this?*/
+		if (files->out_val[1] == TRUE && files->out_val != NULL) {	/* out_val[1] is the candidate pixel flag, want to process the 1's *//*TODO MASK handling - checking if we have a segment ID already, in open_files() we will put nulls in [0] slot... better/faster way to do this? */
 		    G_debug(4, "going to free memory on linked lists...");
 		    /*free memory for linked lists */
 		    my_dispose_list(files->token, &Ri_head);
@@ -408,9 +406,9 @@
 		    newpixel->col = col;
 		    Ri_head = newpixel;
 
-		    pathflag = 1;
+		    pathflag = TRUE;
 
-		    //      while (pathflag == 1 && files->candidate_count > 0) {   /*if don't find mutual neighbors on first try, will use Rk as next Ri. */
+		    //      while (pathflag == TRUE && files->candidate_count > 0) {   /*if don't find mutual neighbors on first try, will use Rk as next Ri. */
 
 		    G_debug(4, "Next starting pixel: row, %d, col, %d",
 			    Ri_head->row, Ri_head->col);
@@ -436,13 +434,13 @@
 		    /* find segment neighbors */
 		    if (find_segment_neighbors
 			(&Ri_head, &Rin_head, &Ri_count, files,
-			 functions) != 0) {
+			 functions) != TRUE) {
 			G_fatal_error("find_segment_neighbors() failed");
 		    }		/* TODO - shouldn't be just fatal error - need to still close_files().  Just put that here then fatal error? */
 
 		    if (Rin_head == NULL) {
 			G_debug(4, "2a, Segment had no valid neighbors");	/*this could happen if there is a segment surrounded by pixels that have already been processed */
-			pathflag = 0;
+			pathflag = FALSE;
 			Ri_count = 0;
 			set_candidate_flag(Ri_head, 0, files);	/* TODO: error trap? */
 			files->candidate_count++;	/* already counted out Ri[0]; */
@@ -480,7 +478,7 @@
 			    G_debug(4,
 				    "simularity = %g for neighbor : row: %d, col %d.",
 				    tempsim, current->row, current->col);
-				    
+
 			    if (tempsim < Ri_similarity) {
 				Ri_similarity = tempsim;
 				Ri_bestn = current;	/*TODO want to point to the current pixel...confirm  when current changes need this to stay put! */
@@ -491,18 +489,20 @@
 			    }
 			}
 
-			if (Ri_bestn != NULL){
+			if (Ri_bestn != NULL) {
 			    G_debug(4,
 				    "Lowest Ri_similarity = %g, for neighbor pixel row: %d col: %d",
 				    Ri_similarity, Ri_bestn->row,
 				    Ri_bestn->col);
 
-			    segment_get(&files->out_seg, (void *)files->out_val, Ri_bestn->row, Ri_bestn->col);
-			    if (files->out_val[1] == 0)
+			    segment_get(&files->out_seg,
+					(void *)files->out_val, Ri_bestn->row,
+					Ri_bestn->col);
+			    if (files->out_val[1] == FALSE)
 				/* this check is important:
 				 * best neighbor is not a valid candidate, was already merged earlier in this time step */
 				Ri_similarity = threshold + 1;
-}
+			}
 
 			if (Ri_bestn != NULL && Ri_similarity < threshold) {	/* small TODO: should this be < or <= for threshold? */
 			    /* we'll have the neighbor pixel to start with. */
@@ -542,7 +542,7 @@
 
 			    for (current = Rkn_head; current != NULL; current = current->next) {	/* for each of Rk's neighbors */
 				tempsim = functions->calculate_similarity(Rk_head, current, files, functions);	/*TODO: need an error trap here, if something goes wrong with calculating similarity? */
-				
+
 				if (tempsim < Rk_similarity) {
 				    Rk_similarity = tempsim;
 				    break;	/* exit for Rk's neighbors loop here, we know that Ri and Rk aren't mutually best neighbors */
@@ -551,8 +551,8 @@
 
 			    if (Rk_similarity == Ri_similarity) {	/* so they agree, both are mutually most similar neighbors, none of Rk's other neighbors were more similar */
 				merge_values(Ri_head, Rk_head, Ri_count, Rk_count, files);	/* TODO error trap */
-				endflag = 0;	/* we've made at least one merge, so need another t iteration */
-				pathflag = 0;	/* go to next row,column pixel - end of Rk -> Ri chain since we found mutual best neighbors */
+				endflag = FALSE;	/* we've made at least one merge, so need another t iteration */
+				pathflag = FALSE;	/* go to next row,column pixel - end of Rk -> Ri chain since we found mutual best neighbors */
 			    }
 			    else {	/* they weren't mutually best neighbors */
 				G_debug(4,
@@ -583,27 +583,35 @@
 			     * thus Ri can not be the mutually best neighbor later on during this pass
 			     * unfortunately this does happen sometimes */
 			    set_candidate_flag(Ri_head, 0, files);	/* TODO: error trap? */
-			    files->candidate_count++; /*first pixel was already set*/
+			    files->candidate_count++;	/*first pixel was already set */
 			    G_debug(4,
 				    "3b Rk didn't didn't exist, was not valid candidate, or similarity was > threshold");
-				} /*end else - Ri's best neighbor was not a candidate */
+			}	/*end else - Ri's best neighbor was not a candidate */
 		    }		/* end else - Ri did have neighbors */
 		    //          }           /*end pathflag do loop */
 		}		/*end if pixel is candidate pixel */
 	    }			/*next column */
+#ifdef REVERSE
+	    G_percent(files->nrows - row, files->nrows, 1);
+#else
+	    G_percent(row, files->nrows, 1);	/* TODO, can a message be included with G_percent? */
+#endif
+	    /* TODO, the REVERSE version gets printed on a new line, and isnt' covered.  The else version is. ? */
+	    /* TODO, shows up in CLI, not in GUI */
+
 	}			/*next row */
 
 	/* finished one pass for processing candidate pixels */
 
 	G_debug(4, "Finished one pass, t was = %d", t);
 	t++;
-    } while (t <= functions->end_t && endflag == 0);
+    } while (t <= functions->end_t && endflag == FALSE);
     /*end t loop *//*TODO, should there be a max t that it can iterate for?  Include t in G_message? */
 
     /* free memory *//*TODO: anything ? */
 
 
-    return 0;
+    return TRUE;
 }
 
 int find_segment_neighbors(struct pixels **R_head,
@@ -639,7 +647,7 @@
     /*initialize data.... TODO: maybe remember min max row/col that was looked at each time, initialize in open_files, and reset smaller region at end of this functions */
     for (n = 0; n < files->nrows; n++) {
 	for (m = 0; m < files->ncols; m++) {
-	    val_no_check = 0;
+	    val_no_check = FALSE;
 	    segment_put(&files->no_check, &val_no_check, n, m);
 	}
     }
@@ -708,14 +716,14 @@
 		    "\twith pixel neigh %d, row: %d col: %d, val_no_check = %d",
 		    n, pixel_neighbors[n][0], pixel_neighbors[n][1],
 		    val_no_check);
-	    if (val_no_check == 0) {	/* want to check this neighbor */
+	    if (val_no_check == FALSE) {	/* want to check this neighbor */
 		val_no_check = 1;
 		segment_put(&files->no_check, &val_no_check, pixel_neighbors[n][0], pixel_neighbors[n][1]);	/* don't check it again */
 
 		segment_get(&files->out_seg, (void *)files->out_val, pixel_neighbors[n][0], pixel_neighbors[n][1]);	/*TODO : do I need a second "out_val" data structure? */
 
-		if (files->out_val[1] == 1 || files->out_val[1] == 0) {	/* all pixels, not just valid pixels */
-		/* TODO: use -1 for NULL/MASKED pixels? */
+		if (files->out_val[1] == TRUE || files->out_val[1] == FALSE) {	/* all pixels, not just valid pixels */
+		    /* TODO: use -1 for NULL/MASKED pixels? */
 
 		    G_debug(5, "\tfiles->out_val[0] = %d Ri_seg_ID = %d",
 			    files->out_val[0], Ri_seg_ID);
@@ -765,7 +773,7 @@
 	G_debug(5, "\t### end of pixel neighors");
     }				/* while to_check has more elements */
 
-    return 0;
+    return TRUE;
 }
 
 int find_four_pixel_neighbors(int p_row, int p_col, int pixel_neighbors[8][2],
@@ -805,7 +813,7 @@
 	pixel_neighbors[3][1] = p_col;
 
     /*TODO: seems there should be a more elegent way to do this... */
-    return 0;
+    return TRUE;
 }
 
 int find_eight_pixel_neighbors(int p_row, int p_col,
@@ -817,7 +825,7 @@
     /* get the 4 diagonal neighbors */
     G_warning("Diagonal neighbors Not Implemented");
     /*TODO... continue as above */
-    return 0;
+    return TRUE;
 }
 
 /* similarity / distance between two points based on their input raster values */
@@ -869,7 +877,7 @@
 
     segment_get(&files->out_seg, (void *)files->out_val, Ri_head->row,
 		Ri_head->col);
-    files->out_val[1] = 0;	/*candidate pixel flag, only one merge allowed per t iteration */
+    files->out_val[1] = FALSE;	/*candidate pixel flag, only one merge allowed per t iteration */
     /* if separate out candidate flag, can do all changes with helper function...otherwise remember: */
 
 
@@ -899,7 +907,7 @@
 
     files->candidate_count++;	/* had already counted down the starting pixel Ri[0] at the beginning... */
     G_debug(4, "line 522, \t\t\t\tcc = %d", files->candidate_count);
-    return 0;
+    return TRUE;
 }
 
 /* TODO.. helper function, maybe make more general? */
@@ -923,7 +931,7 @@
 	G_debug(4, "line 544, \t\t\t\tcc = %d", files->candidate_count);
 
     }
-    return 0;
+    return TRUE;
 }
 
 /* let memory manager know space is available again and reset head to NULL */
@@ -937,5 +945,5 @@
 	link_dispose(token, (VOID_T *) current);	/* remove "old" head */
     }
 
-    return 0;
+    return TRUE;
 }

Added: grass-addons/grass7/imagery/i.segment/flag.h
===================================================================
--- grass-addons/grass7/imagery/i.segment/flag.h	                        (rev 0)
+++ grass-addons/grass7/imagery/i.segment/flag.h	2012-06-26 22:53:04 UTC (rev 52232)
@@ -0,0 +1,77 @@
+#ifndef __FLAG_H__
+#define __FLAG_H__
+
+
+/* flag.[ch] is a set of routines which will set up an array of bits
+ ** that allow the programmer to "flag" cells in a raster map.
+ **
+ ** FLAG *
+ ** flag_create(nrows,ncols)
+ ** int nrows, ncols;
+ **     opens the structure flag.  
+ **     The flag structure will be a two dimensional array of bits the
+ **     size of nrows by ncols.  Will initalize flags to zero (unset).
+ **
+ ** flag_destroy(flags)
+ ** FLAG *flags;
+ **     closes flags and gives the memory back to the system.
+ **
+ ** flag_clear_all(flags)
+ ** FLAG *flags;
+ **     sets all values in flags to zero.
+ **
+ ** flag_unset(flags, row, col)
+ ** FLAG *flags;
+ ** int row, col;
+ **     sets the value of (row, col) in flags to zero.
+ **
+ ** flag_set(flags, row, col)
+ ** FLAG *flags;
+ ** int row, col;
+ **     will set the value of (row, col) in flags to one.
+ **
+ ** int
+ ** flag_get(flags, row, col)
+ ** FLAG *flags;
+ ** int row, col;
+ **     returns the value in flags that is at (row, col).
+ **
+ ** idea by Michael Shapiro
+ ** code by Chuck Ehlschlaeger
+ ** April 03, 1989
+ */
+#define FLAG struct _flagsss_
+FLAG {
+    int nrows, ncols, leng;
+    unsigned char **array;
+};
+
+#define FLAG_UNSET(flags,row,col) \
+	(flags)->array[(row)][(col)>>3] &= ~(1<<((col) & 7))
+
+#define FLAG_SET(flags,row,col) \
+	(flags)->array[(row)][(col)>>3] |= (1<<((col) & 7))
+
+#define FLAG_GET(flags,row,col) \
+	(flags)->array[(row)][(col)>>3] & (1<<((col) & 7))
+
+/* flag_clr_all.c */
+int flag_clear_all(FLAG *);
+
+/* flag_create.c */
+FLAG *flag_create(int, int);
+
+/* flag_destroy.c */
+int flag_destroy(FLAG *);
+
+/* flag_get.c */
+int flag_get(FLAG *, int, int);
+
+/* flag_set.c */
+int flag_set(FLAG *, int, int);
+
+/* flag_unset.c */
+int flag_unset(FLAG *, int, int);
+
+
+#endif /* __FLAG_H__ */

Added: grass-addons/grass7/imagery/i.segment/flag_clr_all.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/flag_clr_all.c	                        (rev 0)
+++ grass-addons/grass7/imagery/i.segment/flag_clr_all.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -0,0 +1,14 @@
+#include "flag.h"
+
+int flag_clear_all(FLAG * flags)
+{
+    register int r, c;
+
+    for (r = 0; r < flags->nrows; r++) {
+	for (c = 0; c < flags->leng; c++) {
+	    flags->array[r][c] = 0;
+	}
+    }
+
+    return 0;
+}

Added: grass-addons/grass7/imagery/i.segment/flag_create.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/flag_create.c	                        (rev 0)
+++ grass-addons/grass7/imagery/i.segment/flag_create.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -0,0 +1,27 @@
+#include <grass/gis.h>
+#include "flag.h"
+
+FLAG *flag_create(int nrows, int ncols)
+{
+    unsigned char *temp;
+    FLAG *new_flag;
+    register int i;
+
+    new_flag = (FLAG *) G_malloc(sizeof(FLAG));
+    new_flag->nrows = nrows;
+    new_flag->ncols = ncols;
+    new_flag->leng = (ncols + 7) / 8;
+    new_flag->array =
+	(unsigned char **)G_malloc(nrows * sizeof(unsigned char *));
+
+    temp =
+	(unsigned char *)G_malloc(nrows * new_flag->leng *
+				  sizeof(unsigned char));
+
+    for (i = 0; i < nrows; i++) {
+	new_flag->array[i] = temp;
+	temp += new_flag->leng;
+    }
+
+    return (new_flag);
+}

Added: grass-addons/grass7/imagery/i.segment/flag_destroy.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/flag_destroy.c	                        (rev 0)
+++ grass-addons/grass7/imagery/i.segment/flag_destroy.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -0,0 +1,11 @@
+#include <grass/gis.h>
+#include "flag.h"
+
+int flag_destroy(FLAG * flags)
+{
+    G_free(flags->array[0]);
+    G_free(flags->array);
+    G_free(flags);
+
+    return 0;
+}

Added: grass-addons/grass7/imagery/i.segment/flag_get.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/flag_get.c	                        (rev 0)
+++ grass-addons/grass7/imagery/i.segment/flag_get.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -0,0 +1,6 @@
+#include "flag.h"
+
+int flag_get(FLAG * flags, int row, int col)
+{
+    return (flags->array[row][col >> 3] & (1 << (col & 7)));
+}

Added: grass-addons/grass7/imagery/i.segment/flag_set.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/flag_set.c	                        (rev 0)
+++ grass-addons/grass7/imagery/i.segment/flag_set.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -0,0 +1,8 @@
+#include "flag.h"
+
+int flag_set(FLAG * flags, int row, int col)
+{
+    flags->array[row][col >> 3] |= (1 << (col & 7));
+
+    return 0;
+}

Added: grass-addons/grass7/imagery/i.segment/flag_unset.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/flag_unset.c	                        (rev 0)
+++ grass-addons/grass7/imagery/i.segment/flag_unset.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -0,0 +1,8 @@
+#include "flag.h"
+
+int flag_unset(FLAG * flags, int row, int col)
+{
+    flags->array[row][col >> 3] &= ~(1 << (col & 7));
+
+    return 0;
+}

Modified: grass-addons/grass7/imagery/i.segment/iseg.h
===================================================================
--- grass-addons/grass7/imagery/i.segment/iseg.h	2012-06-26 11:55:52 UTC (rev 52231)
+++ grass-addons/grass7/imagery/i.segment/iseg.h	2012-06-26 22:53:04 UTC (rev 52232)
@@ -34,9 +34,9 @@
     int nrows, ncols;
 
     /* files *//* TODO, for all map names, is this better for any reason, saw it in manual example: char name[GNAME_MAX]; */
-    char *out_name;		/* name of output raster map */  
+    char *out_name;		/* name of output raster map */
     char *seeds, *bounds_map, *bounds_mapset;	/* optional segment seeds and polygon constraints/boundaries */
-	char *out_band;		/* for debug */
+    char *out_band;		/* for debug */
 
     /* file processing */
     int nbands;			/* number of rasters in the image group */
@@ -84,8 +84,8 @@
     int (*find_pixel_neighbors) (int, int, int[8][2], struct files *);	/*parameters: row, col, pixel_neighbors */
     double (*calculate_similarity) (struct pixels *, struct pixels *, struct files *, struct functions *);	/*parameters: two points (row,col) to compare */
 
-	/* for debug */
-	int end_t;
+    /* for debug */
+    int end_t;
 };
 
 
@@ -102,7 +102,8 @@
 int ll_test(struct files *, struct functions *);
 int test_pass_token(struct pixels **, struct files *);
 int region_growing(struct files *, struct functions *);
-int find_segment_neighbors(struct pixels **, struct pixels **, int *, struct files *, struct functions *);
+int find_segment_neighbors(struct pixels **, struct pixels **, int *,
+			   struct files *, struct functions *);
 int set_candidate_flag(struct pixels *, int, struct files *);
 int merge_values(struct pixels *, struct pixels *, int, int, struct files *);
 int find_four_pixel_neighbors(int, int, int[][2], struct files *);

Modified: grass-addons/grass7/imagery/i.segment/main.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/main.c	2012-06-26 11:55:52 UTC (rev 52231)
+++ grass-addons/grass7/imagery/i.segment/main.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -37,29 +37,24 @@
     module->description =
 	_("Outputs a single segmented map (raster) based on input values in an image group.");
 
-    if (parse_args(argc, argv, &files, &functions) != 0)
-	G_debug(1, "Error in parse_args()");	/* TODO: should these be debug or G_fatal_error() or nested if statement? want to clean up mem and temp files */
+    if (parse_args(argc, argv, &files, &functions) != TRUE)
+	G_fatal_error(_("Error in parse_args()"));
 
     G_debug(1, "Main: starting open_files()");
-    if (open_files(&files) != 0)
-	G_debug(1, "Error in open_files()");
+    if (open_files(&files) != TRUE)
+	G_fatal_error(_("Error in open_files()"));
 
     G_debug(1, "Main: starting create_isegs()");
-    if (create_isegs(&files, &functions) != 0)
-	G_debug(1, "Error in create_isegs()");
+    if (create_isegs(&files, &functions) != TRUE)
+	G_fatal_error(_("Error in create_isegs()"));
 
     G_debug(1, "Main: starting write_output()");
-    if (write_output(&files) != 0)
-	G_debug(1, "Error in write_output()");
+    if (write_output(&files) != TRUE)
+	G_fatal_error(_("Error in write_output()"));
 
     G_debug(1, "Main: starting close_files()");
     close_files(&files);
 
-    /* TODO - G_fatal_error() called in sub routines - do I need to run close_files() before quitting?
-     * http://rackerhacker.com/2010/03/18/sigterm-vs-sigkill/
-     * "Standard C applications have a header file that contains the steps that the process should follow if it receives a particular signal. "
-     *  */
-
     G_done_msg("Number of segments created: TODO");
 
     exit(EXIT_SUCCESS);

Modified: grass-addons/grass7/imagery/i.segment/open_files.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/open_files.c	2012-06-26 11:55:52 UTC (rev 52231)
+++ grass-addons/grass7/imagery/i.segment/open_files.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -22,7 +22,7 @@
 
     /* ****** open the input rasters ******* */
 
-	/* TODO: I confirmed, the API does not check this.  Should this be checked in parse_args / validation ?  Or best to do here where I want to use the REF data? */
+    /* TODO: I confirmed, the API does not check this.  Should this be checked in parse_args / validation ?  Or best to do here where I want to use the REF data? */
     if (!I_get_group_ref(files->image_group, &Ref))
 	G_fatal_error(_("Unable to read REF file for group <%s>"),
 		      files->image_group);
@@ -47,15 +47,15 @@
 
     /* Get min/max values of each input raster for scaling */
 
-    if (files->weighted == 0) {	/*default, we will scale */
+    if (files->weighted == FALSE) {	/*default, we will scale */
 	for (n = 0; n < Ref.nfiles; n++) {
-	    if (Rast_read_fp_range
-		(Ref.file[n].name, Ref.file[n].mapset, &fp_range[n]) < 0)
-		G_fatal_error(_("Unable to read fp range for raster map <%s>"), Ref.file[n].name);	/* TODO, still should free memory and close files? */
+	    if (Rast_read_fp_range(Ref.file[n].name, Ref.file[n].mapset, &fp_range[n]) != 1)	/* returns -1 on error, 2 on empty range, quiting either way. */
+		G_fatal_error(_("No min/max found in raster map <%s>"),
+			      Ref.file[n].name);
 	    Rast_get_fp_range_min_max(&(fp_range[n]), &min[n], &max[n]);
 	}
 	G_debug(1, "scaling, for first layer, min: %f, max: %f",
-			  min[0], max[0]);
+		min[0], max[0]);
     }
 
     /* ********** find out file segmentation size ************ */
@@ -91,7 +91,7 @@
 
     if (segment_open
 	(&files->bands_seg, G_tempfile(), files->nrows, files->ncols, srows,
-	 scols, inlen, nseg) != 1)
+	 scols, inlen, nseg) != TRUE)
 	G_fatal_error("Unable to create input temporary files");
 
     /* G_debug(1, "finished segment_open(...bands_seg...)"); */
@@ -99,10 +99,10 @@
     /* TODO: signed integer gives a 2 billion segment limit, depending on how the initialization is done, this means 2 billion max input pixels. */
     if (segment_open
 	(&files->out_seg, G_tempfile(), files->nrows, files->ncols, srows,
-	 scols, outlen, nseg) != 1)
+	 scols, outlen, nseg) != TRUE)
 	G_fatal_error("Unable to create output temporary files");
 
-    if (segment_open(&files->no_check, G_tempfile(), files->nrows, files->ncols, srows, scols, sizeof(int), nseg) != 1)	/* todo could make this smaller ? just need 0 or 1 */
+    if (segment_open(&files->no_check, G_tempfile(), files->nrows, files->ncols, srows, scols, sizeof(int), nseg) != TRUE)	/* todo could make this smaller ? just need 0 or 1 */
 	G_fatal_error("Unable to create flag temporary files");
 
     /* load input bands to segment structure and initialize output segmentation file */
@@ -111,49 +111,48 @@
     files->bands_val = (double *)G_malloc(inlen);
     files->second_val = (double *)G_malloc(inlen);
     files->out_val = (int *)G_malloc(2 * sizeof(int));
-	s = 1; /* initial segment ID */
-	
+    s = 1;			/* initial segment ID */
+
     for (row = 0; row < files->nrows; row++) {
 	for (n = 0; n < Ref.nfiles; n++) {
 	    Rast_get_d_row(in_fd[n], inbuf[n], row);
 	}
 	for (col = 0; col < files->ncols; col++) {
-		/*tempval = 0; Doesn't work, no "null" for doubles in c */ /* want a number, not null */
-		null_check = 1; /*Assume there is data*/
+	    /*tempval = 0; Doesn't work, no "null" for doubles in c *//* want a number, not null */
+	    null_check = 1;	/*Assume there is data */
 	    for (n = 0; n < Ref.nfiles; n++) {
-		/*tempval += inbuf[n][col]; */ /* if mask/null, adding a null value should set tempval to NULL */
-		if(Rast_is_d_null_value(&inbuf[n][col]))
-			null_check=-1;
-		if (files->weighted == 1)
+		/*tempval += inbuf[n][col]; *//* if mask/null, adding a null value should set tempval to NULL */
+		if (Rast_is_d_null_value(&inbuf[n][col]))
+		    null_check = -1;
+		if (files->weighted == TRUE)
 		    files->bands_val[n] = inbuf[n][col];	/*unscaled */
 		else
 		    files->bands_val[n] = (inbuf[n][col] - min[n]) / (max[n] - min[n]);	/*scaled version */
 	    }
-	    segment_put(&files->bands_seg, (void *)files->bands_val, row,
-			col); /* store input bands */
-			
-		if (null_check != -1){ /*good pixel*/
+	    segment_put(&files->bands_seg, (void *)files->bands_val, row, col);	/* store input bands */
+
+	    if (null_check != -1) {	/*good pixel */
 		files->out_val[0] = s;	/*starting segment number TODO: for seeds this will be different */
-	    files->out_val[1] = 0;	/*flag */
-	}
-	else /*don't use this pixel*/
-	{
-		files->out_val[0] = -1;	/*starting segment number*/
-	    files->out_val[1] = -1;	/*flag */
-	}
-	    segment_put(&files->out_seg, (void *)files->out_val, row, col); /* initialize input */
+		files->out_val[1] = TRUE;	/*flag */
+	    }
+	    else {		/*don't use this pixel */
+
+		files->out_val[0] = -1;	/*starting segment number */
+		files->out_val[1] = -1;	/*flag */
+	    }
+	    segment_put(&files->out_seg, (void *)files->out_val, row, col);	/* initialize input */
 	    s++;		/* sequentially number all pixels with their own segment ID */
 	}
     }
 
     /* bounds/constraints */
     /* TODO: You should also handle NULL cells in the bounds
-	 * raster map, I would suggest to replace NULL with min(bounds) - 1 or
-	 +* max(bounds) + 1. */
+     * raster map, I would suggest to replace NULL with min(bounds) - 1 or
+     +* max(bounds) + 1. */
     if (files->bounds_map != NULL) {
 	if (segment_open
 	    (&files->bounds_seg, G_tempfile(), files->nrows, files->ncols,
-	     srows, scols, sizeof(int), nseg) != 1)
+	     srows, scols, sizeof(int), nseg) != TRUE)
 	    G_fatal_error("Unable to create bounds temporary files");
 
 	boundsbuf = Rast_allocate_c_buf();
@@ -184,7 +183,7 @@
     /* Free memory */
 
     for (n = 0; n < Ref.nfiles; n++) {
-	/* G_free(inbuf[n]); TODO - didn't have this line to start with, but should it be added? */
+	G_free(inbuf[n]);
 	Rast_close(in_fd[n]);
     }
 
@@ -195,5 +194,5 @@
     G_free(max);
     /* Need to clean up anything else? */
 
-    return 0;
+    return TRUE;
 }

Modified: grass-addons/grass7/imagery/i.segment/parse_args.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/parse_args.c	2012-06-26 11:55:52 UTC (rev 52231)
+++ grass-addons/grass7/imagery/i.segment/parse_args.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -13,8 +13,8 @@
     /* reference: http://grass.osgeo.org/programming7/gislib.html#Command_Line_Parsing */
 
     struct Option *group, *seeds, *bounds, *output, *method, *threshold;	/* Establish an Option pointer for each option */
-    struct Flag *diagonal, *weighted;										/* Establish a Flag pointer for each option */
-	struct Option *outband, *endt; 		/* debugging parameters... TODO: leave in code or remove?  hidden options? */
+    struct Flag *diagonal, *weighted;	/* Establish a Flag pointer for each option */
+    struct Option *outband, *endt;	/* debugging parameters... TODO: leave in code or remove?  hidden options? */
 
     /* required parameters */
     group = G_define_standard_option(G_OPT_I_GROUP);	/* TODO: OK to require the user to create a group?  Otherwise later add an either/or option to give just a single raster map... */
@@ -50,14 +50,12 @@
     /* Using raster for seeds, Low priority TODO: allow vector points/centroids seed input. */
     seeds = G_define_standard_option(G_OPT_R_INPUT);
     seeds->key = "seeds";
-    seeds->type = TYPE_STRING;
     seeds->required = NO;
     seeds->description = _("Optional raster map with starting seeds.");
 
     /* Polygon constraints. */
     bounds = G_define_standard_option(G_OPT_R_INPUT);
     bounds->key = "bounds";
-    bounds->type = TYPE_STRING;
     bounds->required = NO;
     bounds->description =
 	_("Optional bounding/constraining raster map, must be integer values, each area will be segmented independent of the others.");
@@ -70,17 +68,16 @@
 
     /* TODO input for distance function */
 
-	/* debug parameters */
+    /* debug parameters */
     endt = G_define_option();
     endt->key = "endt";
     endt->type = TYPE_INTEGER;
-    endt->required = YES; /* TODO, could put as optional, and if not supplied put in something large. */
+    endt->required = YES;	/* TODO, could put as optional, and if not supplied put in something large. */
     endt->description = _("Maximum number of time steps to complete.");
 
-    outband = G_define_standard_option(G_OPT_R_INPUT);
+    outband = G_define_standard_option(G_OPT_R_OUTPUT);
     outband->key = "final_mean";
-    outband->type = TYPE_STRING;
-    outband->required = YES;
+    outband->required = NO;
     outband->description =
 	_("debug - save band mean, currently implemented for only 1 band.");
 
@@ -100,20 +97,18 @@
 
     files->image_group = group->answer;
 
-    if (G_legal_filename(output->answer) == 1)
+    if (G_legal_filename(output->answer) == TRUE)
 	files->out_name = output->answer;	/* name of output raster map */
     else
 	G_fatal_error("Invalid output raster name.");
 
-    /* TODO: I'm assuming threshold is already validated as a number.  Is this OK, or is there a better way to cast the input? */
-    /* reference r.cost line 313 
-       if (sscanf(opt5->answer, "%d", &maxcost) != 1 || maxcost < 0)
-       G_fatal_error(_("Inappropriate maximum cost: %d"), maxcost); */
-    sscanf(threshold->answer, "%f", &functions->threshold);	/* Note: this threshold is scaled after we know more at the beginning of create_isegs() */
+    functions->threshold = atof(threshold->answer);	/* Note: this threshold is scaled after we know more at the beginning of create_isegs() */
+    if (weighted->answer == TRUE &&
+	(functions->threshold <= 0 || functions->threshold >= 1))
+	G_fatal_error(_("threshold should be >= 0 and <= 1"));	/* TODO OK to have fatal error here, seems this would be an invalid entry. */
 
     /* segmentation methods:  0 = debug, 1 = region growing */
     /* TODO, instead of string compare, does the Option structure have these already numbered? */
-
     if (strncmp(method->answer, "io_debug", 5) == 0)
 	functions->method = 0;
     else if (strncmp(method->answer, "region_growing", 10) == 0)
@@ -125,12 +120,12 @@
 
     G_debug(1, "segmentation method: %d", functions->method);
 
-    if (diagonal->answer == 0) {
+    if (diagonal->answer == FALSE) {
 	functions->find_pixel_neighbors = &find_four_pixel_neighbors;
 	functions->num_pn = 4;
 	G_debug(1, "four pixel neighborhood");
     }
-    else if (diagonal->answer == 1) {
+    else if (diagonal->answer == TRUE) {
 	functions->find_pixel_neighbors = &find_eight_pixel_neighbors;
 	functions->num_pn = 8;
 	G_debug(1, "eight (3x3) pixel neighborhood");
@@ -146,12 +141,13 @@
 	files->bounds_map = NULL;
     }
     else {			/* polygon constraints given */
-	files->bounds_map = bounds->answer;	
+	files->bounds_map = bounds->answer;
 	if ((files->bounds_mapset = G_find_raster(files->bounds_map, "")) == NULL) {	/* TODO, warning here:  parse_args.c:149:27: warning: assignment discards ‘const’ qualifier from pointer target type [enabled by default] */
 	    G_fatal_error(_("Segmentation constraint/boundary map not found."));
 	}
-	if (Rast_map_type(files->bounds_map, files->bounds_mapset) != CELL_TYPE) {
-		G_fatal_error(_("Segmentation constraint map must be CELL type (integers)"));
+	if (Rast_map_type(files->bounds_map, files->bounds_mapset) !=
+	    CELL_TYPE) {
+	    G_fatal_error(_("Segmentation constraint map must be CELL type (integers)"));
 	}
     }
 
@@ -159,13 +155,17 @@
     files->nrows = Rast_window_rows();
     files->ncols = Rast_window_cols();
 
-	/* debug help */
-    if (G_legal_filename(outband->answer) == 1)
-	files->out_band = outband->answer;	/* name of current means */
-    else
-	G_fatal_error("Invalid output raster name for means.");
+    /* debug help */
+    if (outband->answer == NULL)
+	files->out_band = NULL;
+    else {
+	if (G_legal_filename(outband->answer) == TRUE)
+	    files->out_band = outband->answer;	/* name of current means */
+	else
+	    G_fatal_error("Invalid output raster name for means.");
+    }
 
-    sscanf(endt->answer, "%d", &functions->end_t);	/* Note: this threshold is scaled after we know more at the beginning of create_isegs() */
+    functions->end_t = atoi(endt->answer);	/* Note: this threshold is scaled after we know more at the beginning of create_isegs() */
 
-    return 0;
+    return TRUE;
 }

Modified: grass-addons/grass7/imagery/i.segment/write_output.c
===================================================================
--- grass-addons/grass7/imagery/i.segment/write_output.c	2012-06-26 11:55:52 UTC (rev 52231)
+++ grass-addons/grass7/imagery/i.segment/write_output.c	2012-06-26 22:53:04 UTC (rev 52232)
@@ -7,65 +7,71 @@
 #include <grass/segment.h>	/* segmentation library */
 #include "iseg.h"
 
+/* TODO some time delay here with meanbuf, etc being processed.  I only put if statements on the actual files
+ * to try and keep the code more clear.  Need to see if this raster makes stats processing easier?  Or IFDEF it out?
+ */
+
 int write_output(struct files *files)
 {
-    int out_fd, mean_fd, row, col; /* mean_fd for validiating/debug of means, todo, could add array... */
+    int out_fd, mean_fd, row, col;	/* mean_fd for validiating/debug of means, todo, could add array... */
     CELL *outbuf;
     DCELL *meanbuf;
 
     outbuf = Rast_allocate_c_buf();	/*hold one row of data to put into raster */
-	meanbuf = Rast_allocate_d_buf();
+    meanbuf = Rast_allocate_d_buf();
 
     /* Todo: return codes are 1 for these, need to check and react to errors? programmer's manual didn't include it... */
 
     segment_flush(&files->out_seg);	/* force all data to disk */
-    segment_flush(&files->bands_seg); /* TODO use IFDEF or just delete for all these parts? for debug/validation output */
+    segment_flush(&files->bands_seg);	/* TODO use IFDEF or just delete for all these parts? for debug/validation output */
 
     G_debug(1, "preparing output raster");
     /* open output raster map */
     out_fd = Rast_open_new(files->out_name, CELL_TYPE);
+    if (files->out_band != NULL)
 	mean_fd = Rast_open_new(files->out_band, DCELL_TYPE);
-	
+
     G_debug(1, "start data transfer from segmentation file to raster");
     /* transfer data from segmentation file to raster */
     /* output segmentation file: each element includes the segment ID then processing flag(s).  So just need the first part of it. */
 
     for (row = 0; row < files->nrows; row++) {
-	G_percent(row, files->nrows, 1);
-	Rast_set_c_null_value(outbuf, files->ncols); /*set buffer to NULLs, only write those that weren't originally masked */
+	Rast_set_c_null_value(outbuf, files->ncols);	/*set buffer to NULLs, only write those that weren't originally masked */
 	Rast_set_d_null_value(meanbuf, files->ncols);
 	for (col = 0; col < files->ncols; col++) {
 	    segment_get(&files->out_seg, (void *)files->out_val, row, col);
+	    segment_get(&files->bands_seg, (void *)files->bands_val, row,
+			col);
 	    G_debug(5, "outval[0] = %i", files->out_val[0]);
-	    if(files->out_val[0] >= 0) /* only write positive segment ID's, using -1 as indicator of Null/Masked pixels.  TODO: OK to use -1 as flag for this? */
-	    {outbuf[col] = files->out_val[0];	/*just want segment assignment, not the other processing flag(s) */
-	//	meanbuf[col] = files->out_val[0];
+	    if (files->out_val[0] >= 0) {	/* only write positive segment ID's, using -1 as indicator of Null/Masked pixels.  TODO: OK to use -1 as flag for this? */
+		outbuf[col] = files->out_val[0];	/*just want segment assignment, not the other processing flag(s) */
+		meanbuf[col] = files->out_val[0];
+	    }
 	}
-		
-		segment_get(&files->bands_seg, (void *)files->bands_val, row, col);
-	    if(files->out_val[0] >= 0) 
-	    meanbuf[col] = files->bands_val[0];
-	    
-	}
 	Rast_put_row(out_fd, outbuf, CELL_TYPE);
-	Rast_put_row(mean_fd, meanbuf, DCELL_TYPE);
+	if (files->out_band != NULL)
+	    Rast_put_row(mean_fd, meanbuf, DCELL_TYPE);
+
+	G_percent(row, files->nrows, 1);
     }
 
-/* TODO: I don't understand the header/history/etc for raster storage.  Can/should we save any information about the segmentation
- * settings used to create the raster?  What the input image group was?  Anything else the statistics module would need?
- */
+    /* TODO: I don't understand the header/history/etc for raster storage.  Can/should we save any information about the segmentation
+     * settings used to create the raster?  What the input image group was?  Anything else the statistics module would need?
+     * check it with r.info to see what we have by default.
+     */
 
-/* TODO after we have a count of create segments... if(total segments == 0) G_warning(_("No segments were created. Verify threshold and region settings.")); */
+    /* TODO after we have a count of create segments... if(total segments == 0) G_warning(_("No segments were created. Verify threshold and region settings.")); */
 
     /* close and save file */
     Rast_close(out_fd);
-    Rast_close(mean_fd);
+    if (files->out_band != NULL)
+	Rast_close(mean_fd);
 
-	/* free memory */
-	G_free(outbuf);
-	G_free(meanbuf);
+    /* free memory */
+    G_free(outbuf);
+    G_free(meanbuf);
 
-    return 0;
+    return TRUE;
 }
 
 int close_files(struct files *files)
@@ -89,5 +95,5 @@
 
     /* anything else left to clean up? */
 
-    return 0;
+    return TRUE;
 }



More information about the grass-commit mailing list