Index: trunk/psModules/src/camera/pmFPACopy.c
===================================================================
--- trunk/psModules/src/camera/pmFPACopy.c	(revision 12564)
+++ trunk/psModules/src/camera/pmFPACopy.c	(revision 12589)
@@ -17,25 +17,13 @@
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////
-// File-static functions
-//////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
-// Bin a region down by specified factors in x and y
-static void binRegion(psRegion *region, // Region to be binned
-                      int xBin, int yBin // Binning in x and y
-                     )
-{
-    assert(region);
-    assert(xBin > 0);
-    assert(yBin > 0);
-
-    // Want to include the lower bound: 1 binned by 4 --> 0; 3 binned by 4 --> 0; 4 binned by 4 --> 1
-    region->x0 = (int)(region->x0 / xBin);
-    region->y0 = (int)(region->y0 / yBin);
-    // Want to exclude the upper bound: 4 binned by 4 --> 1; 5 binned by 4 --> 2; 7 binned by 4 --> 2
-    region->x1 = (int)((region->x1 + xBin - 1) / xBin);
-    region->y1 = (int)((region->y1 + yBin - 1) / yBin);
-}
-
-#if 1
+// File-static functions and macros
+//////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+// Copy the value for a concept
+#define COPY_CONCEPT(TARGET, SOURCE, NAME, TYPE) { \
+    psMetadataItem *targetItem = psMetadataLookup(TARGET, NAME); \
+    psMetadataItem *sourceItem = psMetadataLookup(SOURCE, NAME); \
+    targetItem->data.TYPE = sourceItem->data.TYPE; }
+
 // Find the blank (image-less) PHU, given a cell.
 static pmHDU *findBlankPHU(const pmCell *cell // The cell for which to find the PHU
@@ -58,5 +46,25 @@
     return NULL;
 }
-#endif
+
+// copy one of the psImage components of the readout
+static void readoutCopyComponent (psImage **target, psImage *source, psImageBinning *binning, bool xFlip, bool yFlip, bool pixels) {
+
+    if (!source) return;
+
+    if (*target) {
+	psFree(*target);
+    }
+    if (pixels) {
+	*target = psImageFlip(NULL, source, xFlip, yFlip);
+	return;
+    } 
+
+    // I have the fine image size, I know the binning factor, determine the ruff image size
+    binning->nXfine = source->numCols;
+    binning->nYfine = source->numRows;
+    psImageBinningSetRuffSize(binning, PS_IMAGE_BINNING_CENTER);
+    *target = psImageAlloc(binning->nXruff, binning->nYruff, source->type.type);
+    return;
+}
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -77,49 +85,51 @@
     assert(xBin > 0 && yBin > 0);
 
+    // XXX this is a programming / config error
+    if (pixels && (xBin != 1 || yBin != 1)) {
+        psError(PS_ERR_BAD_PARAMETER_VALUE, true, "Unable to copy pixels if binning is set\n");
+        return false;
+    }
+
+    // the binning structure carries the information no how to rebin the images if needed
+    psImageBinning *binning = psImageBinningAlloc();
+    binning->nXbin = xBin;
+    binning->nYbin = yBin;
+
     psArray *sourceReadouts = source->readouts; // The source readouts
     int numReadouts = sourceReadouts->n; // Number of readouts copied
 
-    // Copy the value for a concept
-    #define COPY_CONCEPT(TARGET, SOURCE, NAME, TYPE) \
-    psMetadataItem *targetItem = psMetadataLookup(TARGET, NAME); \
-    psMetadataItem *sourceItem = psMetadataLookup(SOURCE, NAME); \
-    targetItem->data.TYPE = sourceItem->data.TYPE;
-
     // Need to check/change CELL.XPARITY and CELL.YPARITY
-    bool mdok = true;                   // Status of MD lookup
+    bool mdokS = true;                   // Status of MD lookup
+    bool mdokT = true;                   // Status of MD lookup
     bool xFlip = false;                 // Switch parity in x?
     bool yFlip = false;                 // Switch parity in y?
-    {
-        int xParityTarget = psMetadataLookupS32(&mdok, target->concepts, "CELL.XPARITY"); // Target x parity
-        if (mdok && (xParityTarget == 1 || xParityTarget == -1))
-        {
-            int xParitySource = psMetadataLookupS32(&mdok, source->concepts, "CELL.XPARITY"); // Source parity
-            if (mdok && (xParitySource == 1 || xParitySource == -1) && xParityTarget != xParitySource) {
-                xFlip = true;
-            }
-        } else
-        {
-            // Use the source parity
-            COPY_CONCEPT(target->concepts, source->concepts, "CELL.XPARITY", S32);
-        }
-        int yParityTarget = psMetadataLookupS32(&mdok, target->concepts, "CELL.YPARITY"); // Target y parity
-        if (mdok && (yParityTarget == 1 || yParityTarget == -1))
-        {
-            int yParitySource = psMetadataLookupS32(&mdok, source->concepts, "CELL.YPARITY"); // Source parity
-            if (mdok && (yParitySource == 1 || yParitySource == -1) && yParityTarget != yParitySource) {
-                yFlip = true;
-            }
-        } else
-        {
-            // Use the source parity
-            COPY_CONCEPT(target->concepts, source->concepts, "CELL.YPARITY", S32);
-        }
-        psTrace("psModules.camera", 3, "xFlip: %d; yFlip: %d\n", xFlip, yFlip);
-    }
-
-    if (pixels && (xBin != 1 || yBin != 1)) {
-        psLogMsg(__func__, PS_LOG_WARN, "Unable to copy pixels if binning is set --- copy turned off.\n");
-        pixels = false;
-    }
+
+    // enforce the following conditions:
+    // CELL.XPARITY is required
+    // CELL.XPARITY must be +/- 1
+    int xParitySource = psMetadataLookupS32(&mdokS, source->concepts, "CELL.XPARITY"); // Source parity
+    int xParityTarget = psMetadataLookupS32(&mdokT, target->concepts, "CELL.XPARITY"); // Target x parity
+    assert (mdokS && mdokT);
+    assert (abs(xParitySource) == 1);
+    assert (abs(xParityTarget) == 1);
+    if (xParityTarget != xParitySource) {
+	xFlip = true;
+    } else {
+	// Use the source parity
+	COPY_CONCEPT(target->concepts, source->concepts, "CELL.XPARITY", S32);
+    }
+
+    int yParityTarget = psMetadataLookupS32(&mdokT, target->concepts, "CELL.YPARITY"); // Target y parity
+    int yParitySource = psMetadataLookupS32(&mdokS, source->concepts, "CELL.YPARITY"); // Source parity
+    assert (mdokS && mdokT);
+    assert (abs(yParitySource) == 1);
+    assert (abs(yParityTarget) == 1);
+    if (yParityTarget != yParitySource) {
+	yFlip = true;
+    } else {
+	// Use the source parity
+	COPY_CONCEPT(target->concepts, source->concepts, "CELL.YPARITY", S32);
+    }
+    psTrace("psModules.camera", 3, "xFlip: %d; yFlip: %d\n", xFlip, yFlip);
 
     // Perform deep copy of the images.  I would prefer *not* to do a deep copy, in the interests of speed (we
@@ -132,4 +142,5 @@
 
         // Copy attributes
+	// XXX is this correct under binning?
         targetReadout->col0 = sourceReadout->col0;
         targetReadout->row0 = sourceReadout->row0;
@@ -138,48 +149,8 @@
         targetReadout->data_exists = sourceReadout->data_exists;
 
-        // Copy image
-        if (sourceReadout->image) {
-            if (targetReadout->image) {
-                psFree(targetReadout->image);
-            }
-            if (pixels) {
-                targetReadout->image = psImageFlip(NULL, sourceReadout->image, xFlip, yFlip);
-            } else {
-                psImage *image = sourceReadout->image; // The source image
-                long binnedCols = (image->numCols + xBin - 1) / xBin;
-                long binnedRows = (image->numRows + yBin - 1) / yBin;
-                targetReadout->image = psImageAlloc(binnedCols, binnedRows, image->type.type);
-            }
-        }
-
-        // Copy mask
-        if (sourceReadout->mask) {
-            if (targetReadout->mask) {
-                psFree(targetReadout->mask);
-            }
-            if (pixels) {
-                targetReadout->mask = psImageFlip(NULL, sourceReadout->mask, xFlip, yFlip);
-            } else {
-                psImage *mask = sourceReadout->mask; // The source mask image
-                long binnedCols = (mask->numCols + xBin - 1) / xBin;
-                long binnedRows = (mask->numRows + yBin - 1) / yBin;
-                targetReadout->mask = psImageAlloc(binnedCols, binnedRows, mask->type.type);
-            }
-        }
-
-        // Copy weight
-        if (sourceReadout->weight) {
-            if (targetReadout->weight) {
-                psFree(targetReadout->weight);
-            }
-            if (pixels) {
-                targetReadout->weight = psImageFlip(NULL, sourceReadout->weight, xFlip, yFlip);
-            } else {
-                psImage *weight = sourceReadout->weight; // The source weight image
-                long binnedCols = (weight->numCols + xBin - 1) / xBin;
-                long binnedRows = (weight->numRows + yBin - 1) / yBin;
-                targetReadout->weight = psImageAlloc(binnedCols, binnedRows, weight->type.type);
-            }
-        }
+        // Copy all three image components (image, mask, weight)
+	readoutCopyComponent (&targetReadout->image,  sourceReadout->image,  binning, xFlip, yFlip, pixels);
+	readoutCopyComponent (&targetReadout->mask,   sourceReadout->mask,   binning, xFlip, yFlip, pixels);
+	readoutCopyComponent (&targetReadout->weight, sourceReadout->weight, binning, xFlip, yFlip, pixels);
 
         // Copy bias
@@ -187,16 +158,11 @@
             psListRemove(targetReadout->bias, PS_LIST_HEAD);
         }
+
         // Iterate over the biases
         psListIterator *biasIter = psListIteratorAlloc(sourceReadout->bias, PS_LIST_HEAD, false);
         psImage *bias = NULL;           // Bias image from iteration
         while ((bias = psListGetAndIncrement(biasIter))) {
-            psImage *biasCopy;          // Copy of the bias
-            if (pixels) {
-                biasCopy = psImageFlip(NULL, bias, xFlip, yFlip);
-            } else {
-                long binnedCols = (bias->numCols + xBin - 1) / xBin;
-                long binnedRows = (bias->numRows + yBin - 1) / yBin;
-                biasCopy = psImageAlloc(binnedCols, binnedRows, bias->type.type);
-            }
+            psImage *biasCopy = NULL;          // Copy of the bias
+	    readoutCopyComponent (&biasCopy, bias, binning, xFlip, yFlip, pixels);
             psListAdd(targetReadout->bias, PS_LIST_TAIL, biasCopy);
             psFree(biasCopy);           // Drop reference
@@ -216,19 +182,24 @@
     while ((conceptItem = psMetadataGetAndIncrement(conceptsIter))) {
         psString name = conceptItem->name; // Name of concept
-        if (strcmp(name, "CELL.TRIMSEC") != 0 && strcmp(name, "CELL.BIASSEC") != 0 &&
-                strcmp(name, "CELL.XPARITY") != 0 && strcmp(name, "CELL.YPARITY") != 0 &&
-                strcmp(name, "CELL.X0") != 0 && strcmp(name, "CELL.Y0") != 0) {
-            psMetadataItem *copy = psMetadataItemCopy(conceptItem); // Copy of the concept
-            psMetadataAddItem(target->concepts, copy, PS_LIST_TAIL, PS_META_REPLACE);
-            psFree(copy);               // Drop reference
-        }
+        if (!strcmp(name, "CELL.TRIMSEC")) continue;
+	if (!strcmp(name, "CELL.BIASSEC")) continue;
+	if (!strcmp(name, "CELL.XPARITY")) continue;
+	if (!strcmp(name, "CELL.YPARITY")) continue;
+	if (!strcmp(name, "CELL.X0")) continue;
+	if (!strcmp(name, "CELL.Y0")) continue;
+
+	psMetadataItem *copy = psMetadataItemCopy(conceptItem); // Copy of the concept
+	psMetadataAddItem(target->concepts, copy, PS_LIST_TAIL, PS_META_REPLACE);
+	psFree(copy);               // Drop reference
     }
     psFree(conceptsIter);
 
     // Need to update CELL.TRIMSEC and CELL.BIASSEC if we changed the binning and they exist already.
-    if (xBin != 1 || yBin != 1) {
+    // XXX this code seems to be very similar to pmConceptsUpdate
+    if ((binning->nXbin != 1) || (binning->nYbin != 1)) {
+	bool mdok = false;
         psRegion *trimsec = psMetadataLookupPtr(&mdok, target->concepts, "CELL.TRIMSEC"); // The trim section
         if (mdok && trimsec && !psRegionIsNaN(*trimsec)) {
-            binRegion(trimsec, xBin, yBin);
+            *trimsec = psImageBinningSetRuffRegion(binning, *trimsec);
         }
         psList *biassecs = psMetadataLookupPtr(&mdok, target->concepts, "CELL.BIASSEC"); // The bias sections
@@ -238,5 +209,5 @@
             while ((biassec = psListGetAndIncrement(biassecsIter))) {
                 if (!psRegionIsNaN(*biassec)) {
-                    binRegion(biassec, xBin, yBin);
+                    *biassec = psImageBinningSetRuffRegion(binning, *biassec);
                 }
             }
@@ -246,4 +217,5 @@
 
     // Need to update CELL.X0 and CELL.Y0 if we flipped
+    // XXX this section should probably use a common function consistent with psImageBinning
     if (xFlip) {
         int xZero = psMetadataLookupS32(NULL, source->concepts, "CELL.X0"); // CELL.X0 from source
@@ -254,9 +226,9 @@
         if (sourceBin == 0) {
             // Don't know the binning; assume it is unity
-            sourceBin = xBin;
-        }
-
-        psTrace("psModules.camera", 3, "CELL.X0: Before: %d After: %d\n", xZero,
-                xZero + (xSize - 1) * xParity * sourceBin);
+            sourceBin = binning->nXbin;
+        }
+
+	// XXX make sure this is consistent with the psImageBinning
+        psTrace("psModules.camera", 3, "CELL.X0: Before: %d After: %d\n", xZero, xZero + (xSize - 1) * xParity * sourceBin);
         psTrace("psModules.camera", 9, "(xParity: %d xBin: %d numCols: %d)\n", xParity, sourceBin, xSize);
 
@@ -277,9 +249,8 @@
         if (sourceBin == 0) {
             // Don't know the binning; assume it is unity
-            sourceBin = yBin;
-        }
-
-        psTrace("psModules.camera", 3, "CELL.Y0: Before: %d After: %d\n", yZero,
-                yZero + (ySize - 1) * yParity * sourceBin);
+            sourceBin = binning->nYbin;
+        }
+
+        psTrace("psModules.camera", 3, "CELL.Y0: Before: %d After: %d\n", yZero, yZero + (ySize - 1) * yParity * sourceBin);
         psTrace("psModules.camera", 9, "(yParity: %d yBin: %d numRows: %d)\n", yParity, sourceBin, ySize);
 
@@ -295,4 +266,5 @@
 
     // Update the binning concepts
+    // XXX this should probably be done with a common function using psImageBinning
     psMetadataItem *binItem = psMetadataLookup(target->concepts, "CELL.XBIN");
     binItem->data.S32 *= xBin;
@@ -314,5 +286,5 @@
     pmHDU *targetPHU = findBlankPHU(target); // The target PHU
     if (targetPHU && targetPHU != targetHDU) {
-//        pmHDU *sourcePHU = pmHDUGetHighest(source->parent->parent, source->parent, source); // A source HDU
+	// pmHDU *sourcePHU = pmHDUGetHighest(source->parent->parent, source->parent, source); // A source HDU
         pmHDU *sourcePHU = findBlankPHU(source); // The target PHU
         if (sourcePHU && sourcePHU->header) {
@@ -321,4 +293,5 @@
     }
 
+    psFree (binning);
     target->data_exists = true;
     return true;