fixes (and tests for) several problems in i_readgif_low()

diff --git a/Changes b/Changes
index 0c2c110eb22a121fb1d3d20f012443dbb22b312e..85f5cd7bd62cd2b9d1a76db99d39effa63148182 100644 (file)
--- a/Changes
+++ b/Changes
@@ -408,6 +408,7 @@ Revision history for Perl extension Imager.
         - Fix for i_gifread_low() for images with only a local and no global
           colormap and an abort if no colormap is present (which is legal
           in gif).
+        - several fixes to i_readgif_low()
 
 ~~~~~~~~~~~~~^ ^ ^~~~~~~~~~~~~~
 

diff --git a/gif.c b/gif.c
index e7c98c21ab786c59f29cccbf809ce5719b377a35..c26b7acc6a5ed4fd8ec5759e4f82fc0846e32d65 100644 (file)
--- a/gif.c
+++ b/gif.c
@@ -215,22 +215,23 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) {
        return NULL;
       }
 
-      if ( cmapcnt == 0) {
-       if (( ColorMap = (GifFile->Image.ColorMap ? GifFile->Image.ColorMap : GifFile->SColorMap) )) {
-         mm_log((1, "Adding local colormap\n"));
-         ColorMapSize = ColorMap->ColorCount;
+      if (( ColorMap = (GifFile->Image.ColorMap ? GifFile->Image.ColorMap : GifFile->SColorMap) )) {
+       mm_log((1, "Adding local colormap\n"));
+       ColorMapSize = ColorMap->ColorCount;
+       if ( cmapcnt == 0) {
          i_colortable_copy(colour_table, colours, ColorMap);
          cmapcnt++;
-       } else {
-         /* No colormap and we are about to read in the image - abandon for now */
-         mm_log((1, "Going in with no colormap\n"));
-         i_push_error(0, "Image does not have a local or a global color map");
-         /* we can't have allocated a colour table here */
-         i_img_destroy(im);
-         DGifCloseFile(GifFile);
-         return NULL;
        }
+      } else {
+       /* No colormap and we are about to read in the image - abandon for now */
+       mm_log((1, "Going in with no colormap\n"));
+       i_push_error(0, "Image does not have a local or a global color map");
+       /* we can't have allocated a colour table here */
+       i_img_destroy(im);
+       DGifCloseFile(GifFile);
+       return NULL;
       }
+      
       Row = GifFile->Image.Top; /* Image Position relative to Screen. */
       Col = GifFile->Image.Left;
       Width = GifFile->Image.Width;
@@ -253,7 +254,7 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) {
 
        for (Count = i = 0; i < 4; i++) for (j = Row + InterlacedOffset[i]; j < Row + Height; j += InterlacedJumps[i]) {
          Count++;
-         if (DGifGetLine(GifFile, &GifRow[Col], Width) == GIF_ERROR) {
+         if (DGifGetLine(GifFile, GifRow, Width) == GIF_ERROR) {
            gif_push_error();
            i_push_error(0, "Reading GIF line");
            if (colour_table && *colour_table) {
@@ -265,19 +266,19 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) {
            return NULL;
          }
          
-         for (x = 0; x < GifFile->SWidth; x++) {
+         for (x = 0; x < Width; x++) {
            ColorMapEntry = &ColorMap->Colors[GifRow[x]];
            col.rgb.r = ColorMapEntry->Red;
            col.rgb.g = ColorMapEntry->Green;
            col.rgb.b = ColorMapEntry->Blue;
-           i_ppix(im,x,j,&col);
+           i_ppix(im,Col+x,j,&col);
          }
          
        }
       }
       else {
        for (i = 0; i < Height; i++) {
-         if (DGifGetLine(GifFile, &GifRow[Col], Width) == GIF_ERROR) {
+         if (DGifGetLine(GifFile, GifRow, Width) == GIF_ERROR) {
            gif_push_error();
            i_push_error(0, "Reading GIF line");
            if (colour_table && *colour_table) {
@@ -289,12 +290,12 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) {
            return NULL;
          }
 
-         for (x = 0; x < GifFile->SWidth; x++) {
+         for (x = 0; x < Width; x++) {
            ColorMapEntry = &ColorMap->Colors[GifRow[x]];
            col.rgb.r = ColorMapEntry->Red;
            col.rgb.g = ColorMapEntry->Green;
            col.rgb.b = ColorMapEntry->Blue;
-           i_ppix(im, x, Row, &col);
+           i_ppix(im, Col+x, Row, &col);
          }
          Row++;
        }

diff --git a/t/t105gif.t b/t/t105gif.t
index 20dcf7cd1f62e76cb92389ac453f69a5163f138b..64e1c866f61f4d4e47fe95d2f92266d2aee5fb22 100644 (file)
--- a/t/t105gif.t
+++ b/t/t105gif.t
@@ -1,5 +1,5 @@
 $|=1;
-print "1..23\n";
+print "1..26\n";
 use Imager qw(:all);
 
 init_log("testout/t105gif.log",1);
@@ -312,6 +312,48 @@ EOS
       print "not ok 23 # failed to read image with only a local colour map";
     }
     close FH;
+
+    # image has global and local colour maps
+    open FH, "< testimg/screen2.gif"
+      or die "Cannot open testimg/screen2.gif: $!";
+    binmode FH;
+    my $ims = i_readgif(fileno(FH));
+    if ($ims) {
+      print "ok 24\n";
+    }
+    else {
+      print "not ok 24 # ",Imager::_error_as_msg(),"\n";
+    }
+    close FH;
+    open FH, "< testimg/expected.gif"
+      or die "Cannot open testimg/expected.gif: $!";
+    binmode FH;
+    my $ime = i_readgif(fileno(FH));
+    close FH;
+    if ($ime) {
+      print "ok 25\n";
+    }
+    else {
+      print "not ok 25 # ",Imager::_error_as_msg(),"\n";
+    }
+    if ($ims && $ime) {
+      if (i_img_diff($ime, $ims)) {
+       print "not ok 26 # mismatch ",i_img_diff($ime, $ims),"\n";
+       # save the bad one
+       open FH, "> testout/t105_screen2.gif"
+         or die "Cannot create testout/t105_screen.gif: $!";
+       binmode FH;
+       i_writegifmc($ims, fileno(FH), 7)
+         or print "# could not save t105_screen.gif\n";
+       close FH;
+      }
+      else {
+       print "ok 26\n";
+      }
+    }
+    else {
+      print "ok 26 # skipped\n";
+    }
 }
 
 sub test_readgif_cb {

diff --git a/testimg/expected.gif b/testimg/expected.gif
new file mode 100644 (file)
index 0000000..3409d38
Binary files /dev/null and b/testimg/expected.gif differ

diff --git a/testimg/screen2.gif b/testimg/screen2.gif
new file mode 100644 (file)
index 0000000..4dddf69
Binary files /dev/null and b/testimg/screen2.gif differ