Address PR Comments.

guilt · SiegeLord · commit 0638d7c3bc40 · 2019-04-29T20:58:27.000-07:00
diff --git a/addons/image/freeimage.c b/addons/image/freeimage.c
@@ -15,19 +15,24 @@ ALLEGRO_DEBUG_CHANNEL("image")
 
 static bool freeimage_initialized = false;
 
+static void _fiio_al_error_handler(FREE_IMAGE_FORMAT fif, void *message) {
+   ALLEGRO_ERROR("FreeImage %s : %s\n", (fif == FIF_UNKNOWN)? "UNKNOWN" : FreeImage_GetFormatFromFIF(fif), message);
+}
+
 bool _al_init_fi(void)
 {
-   if(freeimage_initialized)
-       return true;
+   if (freeimage_initialized)
+      return true;
    FreeImage_Initialise(FALSE);
    _al_add_exit_func(_al_shutdown_fi, "_al_shutdown_fi");
+   FreeImage_SetOutputMessage(_fiio_al_error_handler);
    return true;
 }
 
 void _al_shutdown_fi(void)
 {
-   if(!freeimage_initialized)
-       return;
+   if (!freeimage_initialized)
+      return;
    FreeImage_DeInitialise();
    freeimage_initialized = false;
 }
@@ -38,16 +43,16 @@ static ALLEGRO_BITMAP *_al_fi_to_al_bitmap(FIBITMAP *fib) {
    width = FreeImage_GetWidth(fib);
    height = FreeImage_GetHeight(fib);
    bitmap = al_create_bitmap(width, height);
-   if(bitmap) {
+   if (bitmap) {
       ALLEGRO_LOCKED_REGION *a_lock = al_lock_bitmap(bitmap,
          ALLEGRO_PIXEL_FORMAT_ARGB_8888, ALLEGRO_LOCK_WRITEONLY);
       if (a_lock) {
          unsigned char *out = (unsigned char *)a_lock->data;
          int out_inc = a_lock->pitch - (width*4);
-         for(int j=height - 1; j > -1; --j) {
-            for(int i=0; i < width; ++i) {
-               RGBQUAD color;
-               if(FreeImage_GetPixelColor(fib, i, j, &color) == FALSE) {
+         for (int j=height - 1; j > -1; --j) {
+            for (int i=0; i < width; ++i) {
+               RGBQUAD color = { 0.0, 0.0, 0.0, 0.0 } ;
+               if (FreeImage_GetPixelColor(fib, i, j, &color) == FALSE) {
                   ALLEGRO_ERROR("Unable to get pixel data at %d,%d\n", i , j);
                }
                *out++ = (unsigned char) color.rgbBlue;
@@ -73,19 +78,21 @@ ALLEGRO_BITMAP *_al_load_fi_bitmap(const char *filename, int flags)
    ASSERT(freeimage_initialized == true);
 
    fif = FreeImage_GetFIFFromFilename(filename);
-   if(fif == FIF_UNKNOWN)
+   if (fif == FIF_UNKNOWN)
       fif = FreeImage_GetFileType(filename, 0);
-   if(fif == FIF_UNKNOWN)
-      return bitmap;
+   if (fif == FIF_UNKNOWN) {
+      ALLEGRO_WARN("Could not determine the file type for '%s'", filename);
+      return NULL;
+   }
 
    {
      FIBITMAP *fibRaw = FreeImage_Load(fif, filename, flags);
-     if(!fibRaw)
-       return bitmap;
+     if (!fibRaw)
+       return NULL;
      fib = FreeImage_ConvertTo32Bits(fibRaw);
      FreeImage_Unload(fibRaw);
-     if(!fib)
-       return bitmap;
+     if (!fib)
+       return NULL;
    }
 
    bitmap = _al_fi_to_al_bitmap(fib);
@@ -125,17 +132,19 @@ ALLEGRO_BITMAP *_al_load_fi_bitmap_f(ALLEGRO_FILE *f, int flags)
    fio.tell_proc  = _fiio_al_ftell;
 
    fif = FreeImage_GetFileTypeFromHandle(&fio, (fi_handle)f, 0);
-   if(fif == FIF_UNKNOWN)
-      return bitmap;
+   if (fif == FIF_UNKNOWN) {
+      ALLEGRO_WARN("Could not determine the file type for Allegro file.");
+      return NULL;
+   }
 
    {
      FIBITMAP *fibRaw = FreeImage_LoadFromHandle(fif, &fio, (fi_handle)f, 0);
-     if(!fibRaw)
-       return bitmap;
+     if (!fibRaw)
+       return NULL;
      fib = FreeImage_ConvertTo32Bits(fibRaw);
      FreeImage_Unload(fibRaw);
-     if(!fib)
-       return bitmap;
+     if (!fib)
+       return NULL;
    }
 
    bitmap = _al_fi_to_al_bitmap(fib);
@@ -158,8 +167,10 @@ bool _al_identify_fi(ALLEGRO_FILE *f)
    fio.tell_proc  = _fiio_al_ftell;
 
    fif = FreeImage_GetFileTypeFromHandle(&fio, (fi_handle)f, 0);
-   if(fif == FIF_UNKNOWN)
+   if (fif == FIF_UNKNOWN) {
+      ALLEGRO_WARN("Could not determine the file type for Allegro file.");
       return false;
+   }
 
    return true;
 }
diff --git a/addons/image/iio.c b/addons/image/iio.c
@@ -65,7 +65,7 @@ bool al_init_image_addon(void)
          for (i = 0; extensions[i]; i++) {
             success |= al_register_bitmap_loader(extensions[i], _al_load_fi_bitmap);
             success |= al_register_bitmap_loader_f(extensions[i], _al_load_fi_bitmap_f);
-            success |= al_register_bitmap_identifier(extensions[i], _al_load_fi_bitmap_f);
+            success |= al_register_bitmap_identifier(extensions[i], _al_identify_fi);
          }
       }
    }
diff --git a/cmake/FindFreeImage.cmake b/cmake/FindFreeImage.cmake
@@ -8,48 +8,48 @@
 #
 
 IF (WIN32)
-	FIND_PATH( FREEIMAGE_INCLUDE_PATH FreeImage.h
-		$ENV{FI_HOME}/include
-		${FREEIMAGE_ROOT_DIR}/include
-		${FREEIMAGE_ROOT_DIR}
-		DOC "The directory where FreeImage.h resides")
-	FIND_LIBRARY( FREEIMAGE_LIBRARY
-		NAMES FreeImage freeimage
-		PATHS
-		$ENV{FI_HOME}/lib/$ENV{PROCESSOR_ARCHITECTURE}
-		${FREEIMAGE_ROOT_DIR}/lib
-		${FREEIMAGE_ROOT_DIR}
-		DOC "The FreeImage library")
+    FIND_PATH( FREEIMAGE_INCLUDE_PATH FreeImage.h
+        $ENV{FI_HOME}/include
+        ${FREEIMAGE_ROOT_DIR}/include
+        ${FREEIMAGE_ROOT_DIR}
+        DOC "The directory where FreeImage.h resides")
+    FIND_LIBRARY( FREEIMAGE_LIBRARY
+        NAMES FreeImage freeimage
+        PATHS
+        $ENV{FI_HOME}/lib/$ENV{PROCESSOR_ARCHITECTURE}
+        ${FREEIMAGE_ROOT_DIR}/lib
+        ${FREEIMAGE_ROOT_DIR}
+        DOC "The FreeImage library")
 ELSE (WIN32)
-	FIND_PATH( FREEIMAGE_INCLUDE_PATH FreeImage.h
-		/usr/include
-		/usr/local/include
-		/sw/include
-		/opt/local/include
-		DOC "The directory where FreeImage.h resides")
-	FIND_LIBRARY( FREEIMAGE_LIBRARY
-		NAMES FreeImage freeimage
-		PATHS
-		/usr/lib64
-		/usr/lib
-		/usr/local/lib64
-		/usr/local/lib
-		/sw/lib
-		/opt/local/lib
-		DOC "The FreeImage library")
+    FIND_PATH( FREEIMAGE_INCLUDE_PATH FreeImage.h
+        /usr/include
+        /usr/local/include
+        /sw/include
+        /opt/local/include
+        DOC "The directory where FreeImage.h resides")
+    FIND_LIBRARY( FREEIMAGE_LIBRARY
+        NAMES FreeImage freeimage
+        PATHS
+        /usr/lib64
+        /usr/lib
+        /usr/local/lib64
+        /usr/local/lib
+        /sw/lib
+        /opt/local/lib
+        DOC "The FreeImage library")
 ENDIF (WIN32)
 
 SET(FREEIMAGE_LIBRARIES ${FREEIMAGE_LIBRARY})
 
 IF (FREEIMAGE_INCLUDE_PATH AND FREEIMAGE_LIBRARY)
-	SET( FREEIMAGE_FOUND TRUE CACHE BOOL "Set to TRUE if FreeImage is found, FALSE otherwise")
+    SET( FREEIMAGE_FOUND TRUE CACHE BOOL "Set to TRUE if FreeImage is found, FALSE otherwise")
 ELSE (FREEIMAGE_INCLUDE_PATH AND FREEIMAGE_LIBRARY)
-	SET( FREEIMAGE_FOUND FALSE CACHE BOOL "Set to TRUE if FreeImage is found, FALSE otherwise")
+    SET( FREEIMAGE_FOUND FALSE CACHE BOOL "Set to TRUE if FreeImage is found, FALSE otherwise")
 ENDIF (FREEIMAGE_INCLUDE_PATH AND FREEIMAGE_LIBRARY)
 
 MARK_AS_ADVANCED(
-	FREEIMAGE_FOUND
-	FREEIMAGE_LIBRARY
-	FREEIMAGE_LIBRARIES
-	FREEIMAGE_INCLUDE_PATH)
+    FREEIMAGE_FOUND
+    FREEIMAGE_LIBRARY
+    FREEIMAGE_LIBRARIES
+    FREEIMAGE_INCLUDE_PATH)
 

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ bool al_init_image_addon(void)`
`65`	`65`	`for (i = 0; extensions[i]; i++) {`
`66`	`66`	`success \|= al_register_bitmap_loader(extensions[i], _al_load_fi_bitmap);`
`67`	`67`	`success \|= al_register_bitmap_loader_f(extensions[i], _al_load_fi_bitmap_f);`
`68`		`- success \|= al_register_bitmap_identifier(extensions[i], _al_load_fi_bitmap_f);`
	`68`	`+ success \|= al_register_bitmap_identifier(extensions[i], _al_identify_fi);`
`69`	`69`	`}`
`70`	`70`	`}`
`71`	`71`	`}`