ENH: Generalize background image calculations to support 16/32-bit ra…

…diographs (#245)

Refactor the working 8-bit background image calculation into a generic function
to work on 8, 16,  and 32-bit radiograph images. 
Previously, any radiograph that was not 8-bit would use the same broken 16-bit
method.  This 16-bit method failed to correctly loop over the image because the
loop failed to take the size  of the datatype into account. This bug was originally
introduced in commit 9bbce52 ("background mask 1", 2018-04-03).

Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>

autoscoper/src/ui/AutoscoperMainWindow.cpp

@@ -902,8 +902,13 @@ void AutoscoperMainWindow::setPose(std::vector<double> pose, unsigned int volume
 void AutoscoperMainWindow::setBackground(double threshold)
 {
   if (background_threshold_ < 0){
-    for (xromm::Video &vi : tracker->trial()->videos)
-      vi.create_background_image();
+    for (xromm::Video& vi : tracker->trial()->videos) {
+      if (!vi.create_background_image()) {
+        std::cerr << "Error creating background image for video " << vi.dirname() << "\n"
+          << "Failed to set background threshold" << std::endl;
+        return;
+      }
+    }
 
     tracker->updateBackground();
   }

libautoscoper/src/Video.cpp

@@ -43,6 +43,9 @@
 #include <algorithm>
 #include <iostream>
 #ifdef WIN32
+#ifndef NOMINMAX // Otherwise windows.h defines min/max macros
+#define NOMINMAX
+#endif // NOMINMAX
 #include <windows.h>
 #include "Win32/dirent.h"
 #elif __APPLE__
@@ -140,61 +143,79 @@ Video::operator=(const Video& video)
     return *this;
 }
 
-int Video::create_background_image()
+bool Video::create_background_image()
 {
-  if (filenames_.size() < 2)
-    return -1;
-
+  if (filenames_.size() < 2) {
+    std::cerr << "Video::create_background_image(): Not enough images to create background image." << std::endl;
+    return false;
+  }
 
   if (background_) delete[] background_;
   background_ = new float[width()*height()];
   memset(background_, 0, width()*height()*sizeof(float));
 
-  //Read tmp_image
-  TiffImage* tmp_image = new TiffImage();
-  TIFFSetWarningHandler(0);
-  TIFF* tif;
+  for (size_t i = 0; i < filenames_.size(); i++){
+
+    // Read tmp_image
+    TIFFSetWarningHandler(0);
+    TIFF* tif = TIFFOpen(filenames_.at(i).c_str(), "r");
 
-  for (int i = 0; i < filenames_.size(); i++){
-    tif = TIFFOpen(filenames_.at(i).c_str(), "r");
     if (!tif) {
-      std::cerr << "Video::frame(): Unable to open image. " << std::endl;
-      return -2;
+      std::cerr << "Video::create_background_image(): Unable to open image: " << filenames_.at(i) << std::endl;
+      return false;
     }
 
-    tiffImageFree(tmp_image);
+    TiffImage* tmp_image = new TiffImage();
     tiffImageRead(tif, tmp_image);
     TIFFClose(tif);
 
-    if (tmp_image->bitsPerSample == 8){
-      unsigned char * ptr = reinterpret_cast<unsigned char*> (tmp_image->data);
-      float* ptr_b = background_;
-      for (; ptr < reinterpret_cast<unsigned char*>(tmp_image->data) + tmp_image->dataSize; ptr++, ptr_b++)
-      {
-        float val = *ptr;
-        if (val / 255 > *ptr_b)
-          *ptr_b = val / 255;
-      }
-    }
-    else
+    switch (tmp_image->bitsPerSample)
     {
-      unsigned short * ptr = static_cast<unsigned short*> (tmp_image->data);
-      float * ptr_b = background_;
-      for (; ptr < reinterpret_cast<unsigned short*>(tmp_image->data) + tmp_image->dataSize; ptr++, ptr_b++)
-      {
-        float val = *ptr;
-        if (val / 65535  > *ptr_b)
-          *ptr_b = val / 65535;
-      }
+    case 8:
+      create_background_image_internal<unsigned char>(tmp_image);
+      tiffImageFree(tmp_image);
+      break;
+    case 16:
+      create_background_image_internal<unsigned short>(tmp_image);
+      tiffImageFree(tmp_image);
+      break;
+    case 32:
+      create_background_image_internal<unsigned int>(tmp_image);
+      tiffImageFree(tmp_image);
+      break;
+    default:
+      std::cerr << "Video::create_background_image(): Unsupported bits per sample." << std::endl;
+      tiffImageFree(tmp_image);
+      return false;
     }
   }
 
-  tiffImageFree(tmp_image);
-
-  return 1;
+  return true;
 }
 
-
+template <typename T> void Video::create_background_image_internal(TiffImage* tmp_img) {
+  static_assert(
+    std::is_same<T, unsigned char>::value
+    || std::is_same<T, unsigned short>::value
+    || std::is_same<T, unsigned int>::value, "T must be of type unsigned char, unsigned short, or unsigned int");
+  constexpr unsigned int normalization_factor = std::numeric_limits<T>::max();
+  static_assert(
+    normalization_factor == 255
+    || normalization_factor == 65535
+    || normalization_factor == 4294967295, "normalization_factor must be one of 255, 65535 or 4294967295");
+  T* start = reinterpret_cast<T*>(tmp_img->data);
+  T* end = start + (tmp_img->dataSize / sizeof(T));
+  T* iter = start;
+  float* bg = background_;
+  while(iter < end) {
+    float val = *iter;
+    if (val / normalization_factor > *bg) {
+      *bg = val / normalization_factor;
+    }
+    iter++;
+    bg++;
+  }
+}
 
   void
 Video::set_frame(size_type i)

libautoscoper/src/Video.hpp

@@ -72,7 +72,7 @@ class Video
 
     // Accessors
 
-  int create_background_image();
+  bool create_background_image();
 
     const std::string& dirname() const { return dirname_; }
 
@@ -97,6 +97,7 @@ class Video
   const float* background() const { return background_; }
 
 private:
+    template <typename T> void create_background_image_internal(TiffImage* tmp_img);
 
     std::string dirname_;