Add the ability to use multiple bad pixel masks at once

6 files changed, 225 insertions, 115 deletions

diff --git a/doc/man/crystfel_geometry.5 b/doc/man/crystfel_geometry.5
index 8797ef3d..02551a73 100644
--- a/doc/man/crystfel_geometry.5
+++ b/doc/man/crystfel_geometry.5
@@ -82,18 +82,6 @@ These statements specify the incident radiation wavelength.  You must include on
 Units should be specified after the value (or location).  These can be \fBm\fR or \fBA\fR for \fBwavelength\fR, \fBeV\fR or \fBkeV\fR for \fBphoton_energy\fR, and \fBV\fR or \fBkV\fR for \fBelectron_voltage\fR.  For \fBphoton_energy\fR, if no units are given then the value will be taken to be in eV.
 
 .PD 0
-.IP \fBmask_good\fR
-.IP \fBmask_bad\fR
-.PD
-Bitmasks for bad pixel masks. The pixel is considered good if all of the bits which are set in \fBmask_good\fR are set, \fIand\fR if none of the bits which are set in \fBmask_bad\fR are set. Example:
-.IP
-mask = /processing/hitfinder/masks
-.br
-mask_good = 0x27
-.br
-mask_bad = 0x00
-
-.PD 0
 .IP "\fBdetector_shift_x = \fInnn \fB[m|mm]"
 .IP "\fBdetector_shift_y = \fInnn \fB[m|mm]"
 .PD
@@ -230,12 +218,43 @@ The saturation value for the panel.  You can use this to exclude saturated peaks
 Mark pixels as "bad" if their values are respectively less than, more than or equal to the given value.  Note carefully that the inequalities are strict, not inclusive: "less than", not "less than or equal to".
 
 .PD 0
-.IP \fBmask\fR
-If you have a bad pixel mask, you can include it in the HDF5 file as data blocks with the same structure and size as the panel data.  You need to specify the location of each panel's mask data block using this property, and two bitmasks (see below).  The number of placeholders ('%') in the \fBmask\fR must be the same for all panels.  They will be substituted with the same values as used for the placeholders in the \fBdata\fR fields, although there may be fewer of them for \fBmask\fR than for \fBdata\fR.
-
+.IP \fBmaskN_data\fR
+.IP \fBmaskN_file\fR
+.IP \fBmaskN_goodbits\fR
+.IP \fBmaskN_badbits\fR
+.PD
+These specify the parameters for bad pixel mask number \fIN\fR.  You can have up to 8 bad pixel masks, numbered from 0 to 7 inclusive.  Placeholders ('%') in the location (\fBmaskN_data\fR) will be substituted with the same values as used for the placeholders in the image data, although there may be fewer of them for the masks than from the image data.
+.IP
+You can optionally give a filename for each mask with \fBmaskN_file\fR.  The filename may be specified as an absolute filename, or relative to the working directory.  If you don't specify a filename, the mask will be read from the same file as the image data.
+.IP
+A pixel will be considered bad unless \fIall\fR of the bits which are set in \fBgoodbits\fR are set.  A pixel will also be considered bad if \fIany\fR of the bits which are set in \fBbadbits\fR are set.  Note that pixels can additionally be marked as bad via other mechanisms as well (e.g. \fBno_index\fR or \fBbad\fR).
+.IP
+Example:
+.RS
 .PD 0
-.IP \fBmask_file\fR
-Use this option to specify that the bad pixel mask should be read from a different file to the image data.  The \fBmask\fR field, if it contains placeholders, will be expanded in exactly the same way as normal, it's just that the data will be read from the file you specify instead of the image data file.  The \fBmask_file\fR may be specified as an absolute filename, or relative to the working directory.
+.IP
+mask2_data = /data/bad_pixel_map
+.IP
+mask2_file = /home/myself/mybadpixels.h5
+.IP
+mask2_goodbits = 0x00
+.IP
+mask2_badbits = 0xff
+.RE
+.PD
+.IP
+There are some older mask directives which are still understood by this version of CrystFEL.  They are synonyms of the new directives as follows:
+.RS
+.PD 0
+.IP
+mask       ----->   mask0_data
+.IP
+mask_file  ----->   mask0_file
+.IP
+mask_good  ----->   mask0_goodbits
+.IP
+mask_bad   ----->   mask0_badbits
+.RE
 
 .PD 0
 .IP \fBsaturation_map\fR
diff --git a/libcrystfel/src/datatemplate.c b/libcrystfel/src/datatemplate.c
index ee95922d..4326aa33 100644
--- a/libcrystfel/src/datatemplate.c
+++ b/libcrystfel/src/datatemplate.c
@@ -63,6 +63,7 @@ static struct panel_template *new_panel(DataTemplate *det,
                                         struct panel_template *defaults)
 {
 	struct panel_template *new;
+	int i;
 
 	det->n_panels++;
 	det->panels = realloc(det->panels,
@@ -77,10 +78,12 @@ static struct panel_template *new_panel(DataTemplate *det,
 	/* Copy strings */
 	new->cnz_from = safe_strdup(defaults->cnz_from);
 	new->data = safe_strdup(defaults->data);
-	new->mask = safe_strdup(defaults->mask);
-	new->mask_file = safe_strdup(defaults->mask_file);
 	new->satmap = safe_strdup(defaults->satmap);
 	new->satmap_file = safe_strdup(defaults->satmap_file);
+	for ( i=0; i<MAX_MASKS; i++ ) {
+		new->masks[i].data_location = safe_strdup(defaults->masks[i].data_location);
+		new->masks[i].filename = safe_strdup(defaults->masks[i].filename);
+	}
 
 	return new;
 }
@@ -486,6 +489,67 @@ static int add_flag_value(struct panel_template *p,
 }
 
 
+static int parse_mask(struct panel_template *panel,
+                      const char *key_orig,
+                      const char *val)
+{
+	int n;
+	char *key;
+
+	if ( sscanf(key_orig, "mask%d_", &n) != 1 ) {
+		ERROR("Invalid mask directive '%s'\n", key_orig);
+		return 1;
+	}
+
+	key = strdup(key_orig);
+	if ( key == NULL ) return 1;
+
+	key[4] = '_';
+
+	if ( strcmp(key, "mask__file") == 0 ) {
+
+		panel->masks[n].filename = strdup(val);
+
+	} else if ( strcmp(key, "mask__data") == 0 ) {
+
+		if ( strncmp(val, "/", 1) != 0 ) {
+			ERROR("Invalid mask location '%s'\n", val);
+			free(key);
+			return 1;
+		}
+		panel->masks[n].data_location = strdup(val);
+
+	} else if ( strcmp(key, "mask__goodbits") == 0 ) {
+
+		char *end;
+		double v = strtod(val, &end);
+
+		if ( end != val ) {
+			panel->masks[n].good_bits = v;
+		} else {
+			free(key);
+			return 1;
+		}
+
+	} else if ( strcmp(key, "mask__badbits") == 0 ) {
+
+		char *end;
+		double v = strtod(val, &end);
+
+		if ( end != val ) {
+			panel->masks[n].bad_bits = v;
+		} else {
+			free(key);
+			return 1;
+		}
+
+	}
+
+	free(key);
+	return 0;
+}
+
+
 static int parse_field_for_panel(struct panel_template *panel, const char *key,
                                  const char *val, DataTemplate *det)
 {
@@ -531,15 +595,17 @@ static int parse_field_for_panel(struct panel_template *panel, const char *key,
 		free(panel->data);
 		panel->data = strdup(val);
 
+	} else if ( strcmp(key, "mask_bad") == 0 ) {
+		parse_field_for_panel(panel, "mask0_badbits", val, det);
+	} else if ( strcmp(key, "mask_good") == 0 ) {
+		parse_field_for_panel(panel, "mask0_goodbits", val, det);
 	} else if ( strcmp(key, "mask") == 0 ) {
-		if ( strncmp(val,"/",1) != 0 ) {
-			ERROR("Invalid mask location '%s'\n", val);
-			reject = -1;
-		}
-		panel->mask = strdup(val);
-
+		parse_field_for_panel(panel, "mask0_data", val, det);
 	} else if ( strcmp(key, "mask_file") == 0 ) {
-		panel->mask_file = strdup(val);
+		parse_field_for_panel(panel, "mask0_file", val, det);
+
+	} else if ( strncmp(key, "mask", 4) == 0 ) {
+		reject = parse_mask(panel, key, val);
 
 	} else if ( strcmp(key, "saturation_map") == 0 ) {
 		panel->satmap = strdup(val);
@@ -792,30 +858,7 @@ static int parse_toplevel(DataTemplate *dt,
                           int *n_rgc_defs,
                           struct panel_template *defaults)
 {
-
-	if ( strcmp(key, "mask_bad") == 0 ) {
-
-		char *end;
-		double v = strtod(val, &end);
-
-		if ( end != val ) {
-			dt->mask_bad = v;
-		} else {
-			return 1;
-		}
-
-	} else if ( strcmp(key, "mask_good") == 0 ) {
-
-		char *end;
-		double v = strtod(val, &end);
-
-		if ( end != val ) {
-			dt->mask_good = v;
-		} else {
-			return 1;
-		}
-
-	} else if ( strcmp(key, "detector_shift_x") == 0 ) {
+	if ( strcmp(key, "detector_shift_x") == 0 ) {
 		dt->shift_x_from = strdup(val);
 
 	} else if ( strcmp(key, "detector_shift_y") == 0 ) {
@@ -941,6 +984,37 @@ static int lookup_panel(const char *panel_name,
 }
 
 
+static int check_mask_and_satmap_placeholders(const DataTemplate *dt)
+{
+	int i;
+
+	for ( i=0; i<dt->n_panels; i++ ) {
+
+		int num_data_pl;
+		int num_satmap_pl;
+		int j;
+
+		num_data_pl = dt_num_path_placeholders(dt->panels[i].data);
+		num_satmap_pl = dt_num_path_placeholders(dt->panels[i].satmap);
+
+		if ( num_satmap_pl > num_data_pl ) return 1;
+
+		for ( j=0; j<MAX_MASKS; j++ ) {
+
+			int num_mask_pl;
+
+			/* Unused slot? */
+			if ( dt->panels[i].masks[j].data_location == NULL ) continue;
+
+			num_mask_pl = dt_num_path_placeholders(dt->panels[i].masks[j].data_location);
+			if ( num_mask_pl > num_data_pl ) return 1;
+		}
+	}
+
+	return 0;
+}
+
+
 DataTemplate *data_template_new_from_string(const char *string_in)
 {
 	DataTemplate *dt;
@@ -956,9 +1030,6 @@ DataTemplate *data_template_new_from_string(const char *string_in)
 	char *string;
 	char *string_orig;
 	size_t len;
-	int num_data_pl;
-	int num_mask_pl;
-	int num_satmap_pl;
 	struct panel_template defaults;
 
 	dt = calloc(1, sizeof(DataTemplate));
@@ -968,8 +1039,6 @@ DataTemplate *data_template_new_from_string(const char *string_in)
 	dt->panels = NULL;
 	dt->n_bad = 0;
 	dt->bad = NULL;
-	dt->mask_good = 0;
-	dt->mask_bad = 0;
 	dt->n_rigid_groups = 0;
 	dt->rigid_groups = NULL;
 	dt->n_rg_collections = 0;
@@ -1004,9 +1073,13 @@ DataTemplate *data_template_new_from_string(const char *string_in)
 	defaults.adu_scale_unit = ADU_PER_PHOTON;
 	for ( i=0; i<MAX_FLAG_VALUES; i++ ) defaults.flag_values[i] = 0;
 	for ( i=0; i<MAX_FLAG_VALUES; i++ ) defaults.flag_types[i] = FLAG_NOTHING;
+	for ( i=0; i<MAX_MASKS; i++ ) {
+		defaults.masks[i].data_location = NULL;
+		defaults.masks[i].filename = NULL;
+		defaults.masks[i].good_bits = 0;
+		defaults.masks[i].bad_bits = 0;
+	}
 	defaults.max_adu = +INFINITY;
-	defaults.mask = NULL;
-	defaults.mask_file = NULL;
 	defaults.satmap = NULL;
 	defaults.satmap_file = NULL;
 	defaults.data = strdup("/data/data");
@@ -1138,15 +1211,7 @@ DataTemplate *data_template_new_from_string(const char *string_in)
 		return NULL;
 	}
 
-	num_data_pl = dt_num_path_placeholders(dt->panels[0].data);
-	num_mask_pl = dt_num_path_placeholders(dt->panels[0].mask);
-	num_satmap_pl = dt_num_path_placeholders(dt->panels[0].satmap);
-
-	/* This is because the "data" path will be used to expand
-	 * the path to generate the event list */
-	if ( (num_mask_pl > num_data_pl)
-	  || (num_satmap_pl > num_data_pl) )
-	{
+	if ( check_mask_and_satmap_placeholders(dt) ) {
 		ERROR("Mask and saturation map paths must have fewer "
 		      "placeholders than image data path.\n");
 		reject = 1;
@@ -1154,6 +1219,7 @@ DataTemplate *data_template_new_from_string(const char *string_in)
 
 	for ( i=0; i<dt->n_panels; i++ ) {
 
+		int j;
 		struct panel_template *p = &dt->panels[i];
 		signed int dim_fs = find_dim(p->dims, DIM_FS);
 		signed int dim_ss = find_dim(p->dims, DIM_SS);
@@ -1231,29 +1297,15 @@ DataTemplate *data_template_new_from_string(const char *string_in)
 			reject = 1;
 		}
 
-		if ( (p->mask_file != NULL) && (p->mask == NULL) ) {
-			ERROR("You have specified 'mask_file' but not 'mask'.  "
-			      "'mask_file' will therefore have no effect.  "
-			      "(panel %s)\n", p->name);
-			reject = 1;
-		}
-
-		if ( dt_num_path_placeholders(p->data) != num_data_pl ) {
-			ERROR("Data locations for all panels must "
-			      "have the same number of placeholders\n");
-			reject = 1;
-		}
-
-		if ( dt_num_path_placeholders(p->mask) != num_mask_pl ) {
-			ERROR("Mask locations for all panels must "
-			      "have the same number of placeholders\n");
-			reject = 1;
-		}
-
-		if ( dt_num_path_placeholders(p->satmap) != num_satmap_pl ) {
-			ERROR("Satmap locations for all panels must "
-			      "have the same number of placeholders\n");
-			reject = 1;
+		for ( j=0; j<MAX_MASKS; j++ ) {
+			if ( (p->masks[j].filename != NULL)
+			  && (p->masks[j].data_location == NULL) )
+			{
+				ERROR("You have specified filename but not data"
+				      " location for mask %i of panel %s\n",
+				      j, p->name);
+				reject = 1;
+			}
 		}
 
 		/* The default rail direction */
@@ -1321,8 +1373,10 @@ DataTemplate *data_template_new_from_string(const char *string_in)
 
 	free(defaults.cnz_from);
 	free(defaults.data);
-	free(defaults.mask);
-	free(defaults.mask_file);
+	for ( i=0; i<MAX_MASKS; i++ ) {
+		free(defaults.masks[i].data_location);
+		free(defaults.masks[i].filename);
+	}
 
 	for ( rgi=0; rgi<n_rg_definitions; rgi++) {
 
@@ -1420,13 +1474,19 @@ void data_template_free(DataTemplate *dt)
 	free_all_rigid_group_collections(dt);
 
 	for ( i=0; i<dt->n_panels; i++ ) {
+
+		int j;
+
 		free(dt->panels[i].name);
 		free(dt->panels[i].data);
-		free(dt->panels[i].mask);
-		free(dt->panels[i].mask_file);
 		free(dt->panels[i].satmap);
 		free(dt->panels[i].satmap_file);
 		free(dt->panels[i].cnz_from);
+
+		for ( j=0; j<MAX_MASKS; j++ ) {
+			free(dt->panels[i].masks[j].filename);
+			free(dt->panels[i].masks[j].data_location);
+		}
 	}
 
 	free(dt->wavelength_from);
diff --git a/libcrystfel/src/datatemplate_priv.h b/libcrystfel/src/datatemplate_priv.h
index 60e8ead6..62911748 100644
--- a/libcrystfel/src/datatemplate_priv.h
+++ b/libcrystfel/src/datatemplate_priv.h
@@ -78,6 +78,28 @@ enum peak_layout
 #define DIM_UNDEFINED (-3)
 #define DIM_PLACEHOLDER (-4)
 
+
+/* Maximum number of masks per panel */
+#define MAX_MASKS (8)
+
+struct mask_template
+{
+	/** Location of mask data */
+	char *data_location;
+
+	/** Filename for mask data */
+	char *filename;
+
+	/** Bit mask for bad pixels
+	 * (pixel is bad if any of these are set) */
+	unsigned int bad_bits;
+
+	/** Bit mask for good pixels
+	 * (pixel cannot be good unless all of these are set) */
+	unsigned int good_bits;
+};
+
+
 /**
  * Represents one panel of a detector
  */
@@ -98,11 +120,8 @@ struct panel_template
 	/** The offset to be applied from \ref clen */
 	double cnz_offset;
 
-	/** Location of mask data */
-	char *mask;
-
-	/** Filename for mask data */
-	char *mask_file;
+	/** Mask definitions */
+	struct mask_template masks[MAX_MASKS];
 
 	/** Location of per-pixel saturation map */
 	char *satmap;
@@ -201,9 +220,6 @@ struct _datatemplate
 
 	double                     bandwidth;
 
-	unsigned int               mask_bad;
-	unsigned int               mask_good;
-
 	struct rigid_group       **rigid_groups;
 	int                        n_rigid_groups;
 
diff --git a/libcrystfel/src/image-hdf5.c b/libcrystfel/src/image-hdf5.c
index 5aa9afdd..ac987970 100644
--- a/libcrystfel/src/image-hdf5.c
+++ b/libcrystfel/src/image-hdf5.c
@@ -570,7 +570,8 @@ int image_hdf5_read(struct image *image,
 
 int image_hdf5_read_mask(struct panel_template *p,
                          const char *filename, const char *event,
-                         int *bad, int mask_good, int mask_bad)
+                         int *bad, const char *mask_location,
+                         int mask_good, int mask_bad)
 {
 	int p_w, p_h;
 	int *mask = NULL;
@@ -581,7 +582,7 @@ int image_hdf5_read_mask(struct panel_template *p,
 
 	if ( load_hdf5_hyperslab(p, filename, event,
 	                         (void *)&mask, H5T_NATIVE_INT,
-	                         sizeof(int), 1, p->mask) )
+	                         sizeof(int), 1, mask_location) )
 	{
 		ERROR("Failed to load mask data\n");
 		free(mask);
diff --git a/libcrystfel/src/image-hdf5.h b/libcrystfel/src/image-hdf5.h
index f468ff91..efb8a3b7 100644
--- a/libcrystfel/src/image-hdf5.h
+++ b/libcrystfel/src/image-hdf5.h
@@ -46,6 +46,7 @@ extern int image_hdf5_read(struct image *image,
 extern int image_hdf5_read_mask(struct panel_template *p,
                                 const char *filename,
                                 const char *event, int *bad,
+                                const char *mask_location,
                                 int mask_good, int mask_bad);
 
 extern ImageFeatureList *image_hdf5_read_peaks_cxi(const DataTemplate *dtempl,
diff --git a/libcrystfel/src/image.c b/libcrystfel/src/image.c
index c679fc6b..7e4a7549 100644
--- a/libcrystfel/src/image.c
+++ b/libcrystfel/src/image.c
@@ -809,11 +809,12 @@ static int load_mask(struct panel_template *p,
                      const char *mask_fn,
                      const char *ev,
                      int *bad,
+                     const char *mask_location,
                      unsigned int mask_good,
                      unsigned int mask_bad)
 {
 	if ( is_hdf5_file(mask_fn) ) {
-		image_hdf5_read_mask(p, mask_fn, ev, bad,
+		image_hdf5_read_mask(p, mask_fn, ev, bad, mask_location,
 		                     mask_good, mask_bad);
 
 	} else if ( is_cbf_file(mask_fn) ) {
@@ -872,19 +873,31 @@ static int create_badmap(struct image *image,
 			                    image->bad[i]);
 		}
 
-		/* Load mask (skip if panel is bad anyway) */
-		if ( (!no_mask_data) && (!p->bad) && (p->mask != NULL) )
-		{
-			const char *mask_fn;
+		/* Load masks (skip if panel is bad anyway) */
+		if ( (!no_mask_data) && (!p->bad) ) {
 
-			if ( p->mask_file == NULL ) {
-				mask_fn = image->filename;
-			} else {
-				mask_fn = p->mask_file;
-			}
+			int j;
 
-			load_mask(p, mask_fn, image->ev, image->bad[i],
-			          dtempl->mask_good, dtempl->mask_bad);
+			for ( j=0; j<MAX_MASKS; j++ ) {
+
+				const char *mask_fn;
+
+				if ( p->masks[j].data_location == NULL ) {
+					continue;
+				}
+
+				if ( p->masks[j].filename == NULL ) {
+					mask_fn = image->filename;
+				} else {
+					mask_fn = p->masks[j].filename;
+				}
+
+				load_mask(p, mask_fn, image->ev, image->bad[i],
+				          p->masks[j].data_location,
+				          p->masks[j].good_bits,
+				          p->masks[j].bad_bits);
+
+			}
 		}
 	}