bug 143046. Keep GIFs at original 8bit. patch from Alfred Kayser <alfredkayser@nl.ibm.com>. r=me sr=tor

gfx/idl/gfxIFormats.idl

@@ -97,4 +97,18 @@ interface gfxIFormats
    * BGRA   - packed RGBA image
    */
   const gfx_format BGRA   = 7;
+
+  /**
+   * PAL    - Palette based image data, all opaque colors
+   *		  PRUint32 colormap[256];
+   *		  PRUint8 pixels[width*height];
+   */
+  const gfx_format PAL    = 8;
+
+  /**
+   * PAL_A1 - Palette based image data, with transparency
+   *		  PRUint32 colormap[256];
+   *		  PRUint8 pixels[width*height];
+   */
+  const gfx_format PAL_A1 = 9;
 };

gfx/idl/gfxIImageFrame.idl

@@ -57,7 +57,7 @@ native nsRectRef(nsIntRect &);
  * @author Stuart Parmenter <pavlov@netscape.com>
  * @version 0.1
  */
-[scriptable, uuid(7292afa2-3b94-424d-97d5-51ce2f04c0fe)]
+[scriptable, uuid(9c37930b-cadd-453c-89e1-9ed456715b9c)]
 interface gfxIImageFrame : nsISupports
 {
   /**
@@ -132,6 +132,11 @@ interface gfxIImageFrame : nsISupports
   // XXX do we copy here?  lets not...
   void getImageData([array, size_is(length)] out PRUint8 bits, out unsigned long length);
 
+  /**
+   * Get Palette data pointer
+   */
+  void getPaletteData([array, size_is(length)] out gfx_color palette, out unsigned long length);
+
   /**
    * Lock image pixels before addressing the data directly
    */

gfx/src/shared/gfxImageFrame.cpp

@@ -40,15 +40,17 @@
 #include "gfxImageFrame.h"
 #include "nsIServiceManager.h"
 #include <limits.h>
+#include "prmem.h"
 
 NS_IMPL_ISUPPORTS2(gfxImageFrame, gfxIImageFrame, nsIInterfaceRequestor)
 
 gfxImageFrame::gfxImageFrame() :
-  mInitialized(PR_FALSE),
+  mImageData(nsnull),
-  mMutable(PR_TRUE),
   mTimeout(100),
   mDisposalMethod(0), /* imgIContainer::kDisposeNotSpecified */
-  mBlendMethod(1) /* imgIContainer::kBlendOver */
+  mBlendMethod(1), /* imgIContainer::kBlendOver */
+  mInitialized(PR_FALSE),
+  mMutable(PR_TRUE)
 {
   /* member initializers and constructor code */
 }
@@ -56,6 +58,8 @@ gfxImageFrame::gfxImageFrame() :
 gfxImageFrame::~gfxImageFrame()
 {
   /* destructor code */
+  PR_FREEIF(mImageData);
+  mInitialized = PR_FALSE;
 }
 
 /* void init (in PRInt32 aX, in PRInt32 aY, in PRInt32 aWidth, in PRInt32 aHeight, in gfx_format aFormat); */
@@ -83,11 +87,6 @@ NS_IMETHODIMP gfxImageFrame::Init(PRInt32 aX, PRInt32 aY, PRInt32 aWidth, PRInt3
     return NS_ERROR_FAILURE;
   }
 
-  if ( (aDepth != 8) && (aDepth != 24) ){
-    NS_ERROR("This Depth is not supported");
-    return NS_ERROR_FAILURE;
-  }
-
   /* reject over-wide or over-tall images */
   const PRInt32 k64KLimit = 0x0000FFFF;
   if ( aWidth > k64KLimit || aHeight > k64KLimit ){
@@ -103,17 +102,13 @@ NS_IMETHODIMP gfxImageFrame::Init(PRInt32 aX, PRInt32 aY, PRInt32 aWidth, PRInt3
   }
 #endif
 
-  nsresult rv;
-
   mOffset.MoveTo(aX, aY);
   mSize.SizeTo(aWidth, aHeight);
 
   mFormat = aFormat;
+  mDepth = aDepth;
 
-  mImage = do_CreateInstance("@mozilla.org/gfx/image;1", &rv);
+  PRBool needImage = PR_TRUE;
-  NS_ASSERTION(mImage, "creation of image failed");
-  if (NS_FAILED(rv)) return rv;
-
   nsMaskRequirements maskReq;
 
   switch (aFormat) {
@@ -137,16 +132,38 @@ NS_IMETHODIMP gfxImageFrame::Init(PRInt32 aX, PRInt32 aY, PRInt32 aWidth, PRInt3
     maskReq = nsMaskRequirements_kNeeds8Bit;
     break;
 
-  default:
+  case gfxIFormats::PAL:
-#ifdef DEBUG
+  case gfxIFormats::PAL_A1:
-    printf("unsupported gfx_format\n");
+    needImage = PR_FALSE;
-#endif
     break;
+
+  default:
+    NS_ERROR("unsupported gfx_format\n");
+    return NS_ERROR_FAILURE;
   }
 
-  rv = mImage->Init(aWidth, aHeight, aDepth, maskReq);
+  if (needImage) {
+    if (aDepth != 24) {
+      NS_ERROR("This Depth is not supported");
+      return NS_ERROR_FAILURE;
+    }
+
+    nsresult rv;
+    mImage = do_CreateInstance("@mozilla.org/gfx/image;1", &rv);
+    NS_ASSERTION(mImage, "creation of image failed");
     if (NS_FAILED(rv)) return rv;
   
+    rv = mImage->Init(aWidth, aHeight, aDepth, maskReq);
+    NS_ENSURE_SUCCESS(rv, rv);
+  } else {
+    if ((aDepth < 1) || (aDepth > 8)) {
+      NS_ERROR("This Depth is not supported\n");
+      return NS_ERROR_FAILURE;
+    }
+    mImageData = (PRUint8*)PR_MALLOC(PaletteDataLength() + ImageDataLength());
+    NS_ENSURE_TRUE(mImageData, NS_ERROR_OUT_OF_MEMORY);
+  }
+
   mInitialized = PR_TRUE;
   return NS_OK;
 }
@@ -170,7 +187,7 @@ NS_IMETHODIMP gfxImageFrame::SetMutable(PRBool aMutable)
 
   mMutable = aMutable;
 
-  if (!aMutable)
+  if (!aMutable && mImage)
     mImage->Optimize(nsnull);
 
   return NS_OK;
@@ -245,6 +262,7 @@ NS_IMETHODIMP gfxImageFrame::GetNeedsBackground(PRBool *aNeedsBackground)
     return NS_ERROR_NOT_INITIALIZED;
 
   *aNeedsBackground = (mFormat != gfxIFormats::RGB && 
+                       mFormat != gfxIFormats::PAL &&
                        mFormat != gfxIFormats::BGR) ||
                       !mImage->GetIsImageComplete();
   return NS_OK;
@@ -257,7 +275,7 @@ NS_IMETHODIMP gfxImageFrame::GetImageBytesPerRow(PRUint32 *aBytesPerRow)
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
-  *aBytesPerRow = mImage->GetLineStride();
+  *aBytesPerRow = mImage ? mImage->GetLineStride(): mSize.width;
   return NS_OK;
 }
 
@@ -267,7 +285,7 @@ NS_IMETHODIMP gfxImageFrame::GetImageDataLength(PRUint32 *aBitsLength)
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
-  *aBitsLength = mImage->GetLineStride() * mSize.height;
+  *aBitsLength = ImageDataLength();
   return NS_OK;
 }
 
@@ -277,8 +295,25 @@ NS_IMETHODIMP gfxImageFrame::GetImageData(PRUint8 **aData, PRUint32 *length)
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
-  *aData = mImage->GetBits();
+  NS_ASSERTION(mMutable, "trying to get data on an immutable frame");
-  *length = mImage->GetLineStride() * mSize.height;
+
+  *aData = mImage ? mImage->GetBits() : (mImageData + PaletteDataLength());
+  *length = ImageDataLength();
+
+  return NS_OK;
+}
+
+/* void getPaletteData ([array, size_is (length)] out PRUint32 palette, out unsigned long length); */
+NS_IMETHODIMP gfxImageFrame::GetPaletteData(gfx_color **aPalette, PRUint32 *length)
+{
+  if (!mInitialized)
+    return NS_ERROR_NOT_INITIALIZED;
+
+  if (!mImageData)
+    return NS_ERROR_FAILURE;
+
+  *aPalette = (gfx_color*)mImageData;
+  *length = PaletteDataLength();
 
   return NS_OK;
 }
@@ -289,6 +324,8 @@ NS_IMETHODIMP gfxImageFrame::LockImageData()
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
+  if (!mImage)
+    return NS_OK;
   return mImage->LockImagePixels(PR_FALSE);
 }
 
@@ -298,6 +335,8 @@ NS_IMETHODIMP gfxImageFrame::UnlockImageData()
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
+  if (!mImage)
+    return NS_OK;
   return mImage->UnlockImagePixels(PR_FALSE);
 }
 

gfx/src/shared/gfxImageFrame.h

@@ -71,14 +71,24 @@ protected:
   nsIntSize mSize;
 
 private:
-  nsCOMPtr<nsIImage> mImage;
+  PRUint32 PaletteDataLength() const {
+    return ((1 << mDepth) * sizeof(gfx_color));
+  }
 
-  PRPackedBool mInitialized;
+  PRUint32 ImageDataLength() const {
-  PRPackedBool mMutable;
+    return (mImage ? mImage->GetLineStride() : mSize.width) * mSize.height;
-  gfx_format   mFormat;
+  }
+
+  nsCOMPtr<nsIImage> mImage;
+  PRUint8*     mImageData;
 
   PRInt32      mTimeout; // -1 means display forever
   nsIntPoint   mOffset;
   PRInt32      mDisposalMethod;
+
+  gfx_format   mFormat;
+  gfx_depth    mDepth;
   PRInt8       mBlendMethod;
+  PRPackedBool mInitialized;
+  PRPackedBool mMutable;
 };

modules/libpr0n/decoders/gif/GIF2.h

@@ -102,7 +102,7 @@ typedef struct gif_struct {
     int ipass;                  /* Interlace pass; Ranges 1-4 if interlaced. */
     PRUintn rows_remaining;        /* Rows remaining to be output */
     PRUintn irow;                  /* Current output row, starting at zero */
-    PRUint32 *rowp;                 /* Current output pointer */
+    PRUint8 *rowp;                 /* Current output pointer */
 
     /* Parameters for image frame currently being decoded*/
     PRUintn x_offset, y_offset;    /* With respect to "screen" origin */
@@ -118,7 +118,7 @@ typedef struct gif_struct {
     int version;                /* Either 89 for GIF89 or 87 for GIF87 */
     PRUintn screen_width;       /* Logical screen width & height */
     PRUintn screen_height;
-    PRUint32 global_colormap_size;  /* Size of global colormap array. */
+    PRUint32 global_colormap_depth;  /* Depth of global colormap array. */
     int images_decoded;         /* Counts images for multi-part GIFs */
     int loop_count;             /* Netscape specific extension block to control
                                    the number of animation loops a GIF renders. */

modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp

@@ -93,7 +93,7 @@ mailing address.
  *
  * Note, the hold will never need to be bigger than 256 bytes to gather up in the hold,
  * as each GIF block (except colormaps) can never be bigger than 256 bytes.
- * Colormaps are directly copied in the resp. global_colormap or dynamically allocated local_colormap.
+ * Colormaps are directly copied in the resp. global_colormap or the local_colormap of the PAL image frame
  * So a fixed buffer in gif_struct is good enough.
  * This buffer is only needed to copy left-over data from one GifWrite call to the next
  */
@@ -242,7 +242,8 @@ nsresult nsGIFDecoder2::ProcessData(unsigned char *data, PRUint32 count, PRUint3
   nsresult rv = GifWrite(data, count);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  if (mImageFrame) {
+  // Flushing is only needed for first frame
+  if (!mGIFStruct.images_decoded && mImageFrame) {
     FlushImageData();
     mLastFlushedRow = mCurrentRow;
     mLastFlushedPass = mCurrentPass;
@@ -313,7 +314,7 @@ void nsGIFDecoder2::EndGIF()
 }
 
 //******************************************************************************
-void nsGIFDecoder2::BeginImageFrame()
+void nsGIFDecoder2::BeginImageFrame(gfx_depth aDepth)
 {
   mImageFrame = nsnull; // clear out our current frame reference
 
@@ -329,16 +330,23 @@ void nsGIFDecoder2::BeginImageFrame()
     }
   }
 
-  gfx_format format = gfxIFormats::RGB;
+  // Use correct format, RGB for first frame, PAL for following frames
-  if (mGIFStruct.is_transparent) {
+  // and include transparency to allow for optimization of opaque images
-    format = gfxIFormats::RGB_A1;  // XXX not really
+  gfx_format format;
+  if (mGIFStruct.images_decoded) {
+    // Image data is stored with original depth and palette
+    format = mGIFStruct.is_transparent ? gfxIFormats::PAL_A1 : gfxIFormats::PAL;
+  } else {
+    // Regardless of depth of input, image is decoded into 24bit RGB
+    format = mGIFStruct.is_transparent ? gfxIFormats::RGB_A1 : gfxIFormats::RGB;
+    aDepth = 24;
   }
 
   // initialize the frame and append it to the container
   mImageFrame = do_CreateInstance("@mozilla.org/gfx/image/frame;2");
   if (!mImageFrame || NS_FAILED(mImageFrame->Init(
         mGIFStruct.x_offset, mGIFStruct.y_offset, 
-        mGIFStruct.width, mGIFStruct.height, format, 24))) {
+        mGIFStruct.width, mGIFStruct.height, format, aDepth))) {
     mImageFrame = 0;
     return;
   }
@@ -350,7 +358,7 @@ void nsGIFDecoder2::BeginImageFrame()
     mObserver->OnStartFrame(nsnull, mImageFrame);
 
   PRUint32 imageDataLength;
-  mImageFrame->GetImageData((PRUint8 **)&mImageData, &imageDataLength);
+  mImageFrame->GetImageData(&mImageData, &imageDataLength);
 }
 
 
@@ -358,11 +366,10 @@ void nsGIFDecoder2::BeginImageFrame()
 void nsGIFDecoder2::EndImageFrame()
 {
   // First flush all pending image data 
-  FlushImageData();
-  mCurrentRow = mLastFlushedRow = -1;
-  mCurrentPass = mLastFlushedPass = 0;
-
   if (!mGIFStruct.images_decoded) {
+    // Only need to flush first frame
+    FlushImageData();
+
     // If the first frame is smaller in height than the entire image, send a
     // OnDataAvailable (Display Refresh) for the area it does not have data for.
     // This will clear the remaining bits of the placeholder. (Bug 37589)
@@ -374,6 +381,8 @@ void nsGIFDecoder2::EndImageFrame()
       mObserver->OnDataAvailable(nsnull, mImageFrame, &r);
     }
   }
+  mCurrentRow = mLastFlushedRow = -1;
+  mCurrentPass = mLastFlushedPass = 0;
 
   mGIFStruct.images_decoded++;
 
@@ -405,6 +414,13 @@ PRUint32 nsGIFDecoder2::OutputRow()
   int drow_start, drow_end;
   drow_start = drow_end = mGIFStruct.irow;
 
+  /* Protect against too much image data */
+  if ((PRUintn)drow_start >= mGIFStruct.height) {
+    NS_WARNING("GIF2.cpp::OutputRow - too much image data");
+    return 0;
+  }
+
+  if (!mGIFStruct.images_decoded) {
     /*
      * Haeberli-inspired hack for interlaced GIFs: Replicate lines while
      * displaying to diminish the "venetian-blind" effect as the image is
@@ -430,21 +446,25 @@ PRUint32 nsGIFDecoder2::OutputRow()
         drow_end = mGIFStruct.height - 1;
     }
 
-  /* Protect against too much image data */
+    // Row to process
-  if ((PRUintn)drow_start >= mGIFStruct.height) {
+    const PRUint32 bpr = sizeof(PRUint32) * mGIFStruct.width; 
-    NS_WARNING("GIF2.cpp::OutputRow - too much image data");
+    PRUint8 *rowp = mImageData + (mGIFStruct.irow * bpr);
-    return 0;
-  }
 
+    // Convert color indices to Cairo pixels
+    PRUint8 *from = rowp + mGIFStruct.width;
+    PRUint32 *to = ((PRUint32*)rowp) + mGIFStruct.width;
+    PRUint32 *cmap = mColormap;
+    for (PRUint32 c = mGIFStruct.width; c > 0; c--) {
+      *--to = cmap[*--from];
+    }
   
     // Duplicate rows
     if (drow_end > drow_start) {
       // irow is the current row filled
-    const PRUint32 width = mGIFStruct.width; 
-    PRUint32 *rgbRowIndex = mImageData + mGIFStruct.irow * width;
       for (int r = drow_start; r <= drow_end; r++) {
         if (r != mGIFStruct.irow) {
-        memcpy(mImageData + r * width, rgbRowIndex, width*sizeof(PRUint32));
+          memcpy(mImageData + (r * bpr), rowp, bpr);
+        }
       }
     }
   }
@@ -454,7 +474,6 @@ PRUint32 nsGIFDecoder2::OutputRow()
   if (mGIFStruct.ipass == 1)
     mLastFlushedPass = mGIFStruct.ipass;   // interlaced starts at 1
 
-
   if (!mGIFStruct.interlaced) {
     mGIFStruct.irow++;
   } else {
@@ -498,14 +517,18 @@ nsGIFDecoder2::DoLzw(const PRUint8 *q)
   PRUint8 *stackp   = mGIFStruct.stackp;
   PRUint8 *suffix   = mGIFStruct.suffix;
   PRUint8 *stack    = mGIFStruct.stack;
-  PRUint32 *rowp    = mGIFStruct.rowp;
+  PRUint8 *rowp     = mGIFStruct.rowp;
-  PRUint32 *rowend  = mImageData + (mGIFStruct.irow + 1) * mGIFStruct.width;
+
-  PRUint32 *cmap    = mColormap;
+  PRUint32 bpr = mGIFStruct.width;
+  if (!mGIFStruct.images_decoded) 
+    bpr *= sizeof(PRUint32);
+  PRUint8 *rowend   = mImageData + (bpr * mGIFStruct.irow) + mGIFStruct.width;
+
 #define OUTPUT_ROW()                                        \
   PR_BEGIN_MACRO                                            \
     if (!OutputRow())                                       \
       goto END;                                             \
-    rowp = mImageData + mGIFStruct.irow * mGIFStruct.width; \
+    rowp = mImageData + mGIFStruct.irow * bpr;              \
     rowend = rowp + mGIFStruct.width;                       \
   PR_END_MACRO
 
@@ -539,7 +562,7 @@ nsGIFDecoder2::DoLzw(const PRUint8 *q)
       }
 
       if (oldcode == -1) {
-        *rowp++ = cmap[suffix[code]];
+        *rowp++ = suffix[code];
         if (rowp == rowend)
           OUTPUT_ROW();
 
@@ -589,7 +612,7 @@ nsGIFDecoder2::DoLzw(const PRUint8 *q)
 
       /* Copy the decoded data out to the scanline buffer. */
       do {
-        *rowp++ = cmap[*--stackp];
+        *rowp++ = *--stackp;
         if (rowp == rowend)
           OUTPUT_ROW();
       } while (stackp > stack);
@@ -630,9 +653,6 @@ static void ConvertColormap(PRUint32 *aColormap, PRUint32 aColors)
   PRUint8 *from = ((PRUint8 *)aColormap) + 3 * aColors;
   PRUint32 *to = aColormap + aColors;
 
-  // Clear part after defined colors
-  memset(to, 0, (MAX_COLORS - aColors)*sizeof(PRUint32));
-
   // Convert color entries to Cairo format
   for (PRUint32 c = aColors; c > 0; c--) {
     from -= 3;
@@ -656,7 +676,7 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
   // If previous call to me left something in the hold first complete current block
   // Or if we are filling the colormaps, first complete the colormap
   PRUint8* p = (mGIFStruct.state == gif_global_colormap) ? (PRUint8*)mGIFStruct.global_colormap :
-               (mGIFStruct.state == gif_image_colormap) ? (PRUint8*)mGIFStruct.local_colormap :
+               (mGIFStruct.state == gif_image_colormap) ? (PRUint8*)mColormap :
                (mGIFStruct.bytes_in_hold) ? mGIFStruct.hold : nsnull;
   if (p) {
     // Add what we have sofar to the block
@@ -703,6 +723,8 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
     {
       // Make sure the transparent pixel is transparent in the colormap
       if (mGIFStruct.is_transparent) {
+        // Save old value so we can restore it later
+        if (mColormap == mGIFStruct.global_colormap)
             mOldColor = mColormap[mGIFStruct.tpixel];
         mColormap[mGIFStruct.tpixel] = 0;
       }
@@ -755,7 +777,7 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
 
       mGIFStruct.screen_width = GETINT16(q);
       mGIFStruct.screen_height = GETINT16(q + 2);
-      mGIFStruct.global_colormap_size = 2<<(q[4]&0x07);
+      mGIFStruct.global_colormap_depth = (q[4]&0x07) + 1;
 
       // screen_bgcolor is not used
       //mGIFStruct.screen_bgcolor = q[5];
@@ -765,7 +787,7 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
 
       if (q[4] & 0x80) { /* global map */
         // Get the global colormap
-        const PRUint32 size = 3*mGIFStruct.global_colormap_size;
+        const PRUint32 size = (3 << mGIFStruct.global_colormap_depth);
         if (len < size) {
           // Use 'hold' pattern to get the global colormap
           GETN(size, gif_global_colormap);
@@ -785,7 +807,7 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
     case gif_global_colormap:
       // Everything is already copied into global_colormap
       // Convert into Cairo colors including CMS transformation
-      ConvertColormap(mGIFStruct.global_colormap, mGIFStruct.global_colormap_size);
+      ConvertColormap(mGIFStruct.global_colormap, 1<<mGIFStruct.global_colormap_depth);
       GETN(1, gif_image_start);
       break;
 
@@ -928,6 +950,7 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
       break;
 
     case gif_image_header:
+    {
       /* Get image offsets, with respect to the screen origin */
       mGIFStruct.x_offset = GETINT16(q);
       mGIFStruct.y_offset = GETINT16(q + 2);
@@ -964,7 +987,13 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
         }
       }
 
-      BeginImageFrame();
+      /* Depth of colors is determined by colormap */
+      /* (q[8] & 0x80) indicates local colormap */
+      /* bits per pixel is (q[8]&0x07 + 1) when local colormap is set */
+      PRUint32 depth = mGIFStruct.global_colormap_depth;
+      if (q[8] & 0x80)
+        depth = (q[8]&0x07) + 1;
+      BeginImageFrame(depth);
       
       // handle allocation error
       if (!mImageFrame) {
@@ -992,28 +1021,33 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
 
       if (q[8] & 0x80) /* has a local colormap? */
       {
-        const int num_colors = 2 << (q[8] & 0x7);
+        mGIFStruct.local_colormap_size = 1 << depth;
-        const PRUint32 size = 3*num_colors;
+        if (mGIFStruct.images_decoded) {
+          // Copy directly into the palette of current frame,
+          // by pointing mColormap to that palette.
+          PRUint32 paletteSize;
+          mImageFrame->GetPaletteData(&mColormap, &paletteSize);
+        } else {
+          // First frame has local colormap, allocate space for it
+          // as the image frame doesn't have its own palette
           if (!mGIFStruct.local_colormap) {
             mGIFStruct.local_colormap = 
-			  (PRUint32*)PR_MALLOC(MAX_COLORS * sizeof(PRUint32));
+  			  (PRUint32*)PR_MALLOC(mGIFStruct.local_colormap_size * sizeof(PRUint32));
             if (!mGIFStruct.local_colormap) {
               mGIFStruct.state = gif_oom;
               break;
             }
           }
-
-        /* Switch to the new local palette after it loads */
-        mGIFStruct.local_colormap_size = num_colors;
           mColormap = mGIFStruct.local_colormap;
-
+        }
+        const PRUint32 size = 3 << depth;
         if (len < size) {
           // Use 'hold' pattern to get the image colormap
           GETN(size, gif_image_colormap);
           break;
         }
         // Copy everything, go to colormap state to do CMS correction
-        memcpy(mGIFStruct.local_colormap, buf, size);
+        memcpy(mColormap, buf, size);
         buf += size;
         len -= size;
         GETN(0, gif_image_colormap);
@@ -1021,14 +1055,21 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
       } else {
         /* Switch back to the global palette */
         mColormap = mGIFStruct.global_colormap;
+        if (mGIFStruct.images_decoded) {
+          // Copy global colormap into the palette of current frame
+          PRUint32 size;
+          mImageFrame->GetPaletteData(&mColormap, &size);
+          memcpy(mColormap, mGIFStruct.global_colormap, size);
+        }
       }
       GETN(1, gif_lzw_start);
+    }
     break;
 
     case gif_image_colormap:
       // Everything is already copied into local_colormap
       // Convert into Cairo colors including CMS transformation
-      ConvertColormap(mGIFStruct.local_colormap, mGIFStruct.local_colormap_size);
+      ConvertColormap(mColormap, mGIFStruct.local_colormap_size);
       GETN(1, gif_lzw_start);
       break;
 
@@ -1091,7 +1132,7 @@ nsresult nsGIFDecoder2::GifWrite(const PRUint8 *buf, PRUint32 len)
   if (len) {
     // Add what we have sofar to the block
     PRUint8* p = (mGIFStruct.state == gif_global_colormap) ? (PRUint8*)mGIFStruct.global_colormap :
-                 (mGIFStruct.state == gif_image_colormap) ? (PRUint8*)mGIFStruct.local_colormap :
+                 (mGIFStruct.state == gif_image_colormap) ? (PRUint8*)mColormap :
                  mGIFStruct.hold;
     memcpy(p, buf, len);
     mGIFStruct.bytes_to_consume -= len;

modules/libpr0n/decoders/gif/nsGIFDecoder2.h

@@ -76,7 +76,7 @@ private:
 
   void      BeginGIF();
   void      EndGIF();
-  void      BeginImageFrame();
+  void      BeginImageFrame(gfx_depth aDepth);
   void      EndImageFrame();
   void      FlushImageData();
   void      FlushImageData(PRUint32 fromRow, PRUint32 rows);
@@ -93,7 +93,7 @@ private:
   PRInt32 mCurrentRow;
   PRInt32 mLastFlushedRow;
 
-  PRUint32 *mImageData;      // Pointer to image data in Cairo format
+  PRUint8 *mImageData;       // Pointer to image data in either Cairo or 8bit format
   PRUint32 *mColormap;       // Current colormap to be used in Cairo format
   PRUint32 mOldColor;        // The old value of the transparent pixel
   PRUint8 mCurrentPass;

modules/libpr0n/src/imgContainer.cpp

@@ -791,28 +791,17 @@ nsresult imgContainer::DoComposite(gfxIImageFrame** aFrameToUse,
   if (prevFrameDisposalMethod == imgIContainer::kDisposeRestorePrevious &&
       !mAnim->compositingPrevFrame)
     prevFrameDisposalMethod = imgIContainer::kDisposeClear;
-
-  // Optimization: Skip compositing if the previous frame wants to clear the
-  //               whole image
-  if (prevFrameDisposalMethod == imgIContainer::kDisposeClearAll) {
-    aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
-    *aFrameToUse = aNextFrame;
-    return NS_OK;
-  }
-
   nsIntRect prevFrameRect;
   aPrevFrame->GetRect(prevFrameRect);
   PRBool isFullPrevFrame = (prevFrameRect.x == 0 && prevFrameRect.y == 0 &&
                             prevFrameRect.width == mSize.width &&
                             prevFrameRect.height == mSize.height);
 
-  // Optimization: Skip compositing if the previous frame is the same size as
+  // Optimization: DisposeClearAll if the previous frame is the same size as
   //               container and it's clearing itself
-  if (isFullPrevFrame && prevFrameDisposalMethod == imgIContainer::kDisposeClear) {
+  if (isFullPrevFrame && 
-    aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
+      (prevFrameDisposalMethod == imgIContainer::kDisposeClear))
-    *aFrameToUse = aNextFrame;
+    prevFrameDisposalMethod = imgIContainer::kDisposeClearAll;
-    return NS_OK;
-  }
 
   PRInt32 nextFrameDisposalMethod;
   nsIntRect nextFrameRect;
@@ -824,19 +813,25 @@ nsresult imgContainer::DoComposite(gfxIImageFrame** aFrameToUse,
 
   gfx_format nextFormat;
   aNextFrame->GetFormat(&nextFormat);
-  PRBool nextFrameHasAlpha = (nextFormat != gfxIFormats::RGB) &&
+  if (nextFormat != gfxIFormats::PAL && nextFormat != gfxIFormats::PAL_A1) {
-                             (nextFormat != gfxIFormats::BGR);
+    // Optimization: Skip compositing if the previous frame wants to clear the
+    //               whole image
+    if (prevFrameDisposalMethod == imgIContainer::kDisposeClearAll) {
+      aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
+      *aFrameToUse = aNextFrame;
+      return NS_OK;
+    }
   
     // Optimization: Skip compositing if this frame is the same size as the
     //               container and it's fully drawing over prev frame (no alpha)
     if (isFullNextFrame &&
         (nextFrameDisposalMethod != imgIContainer::kDisposeRestorePrevious) &&
-      !nextFrameHasAlpha) {
+        (nextFormat == gfxIFormats::RGB)) {
-
       aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
       *aFrameToUse = aNextFrame;
       return NS_OK;
     }
+  }
 
   // Calculate area that needs updating
   switch (prevFrameDisposalMethod) {
@@ -846,6 +841,11 @@ nsresult imgContainer::DoComposite(gfxIImageFrame** aFrameToUse,
       *aDirtyRect = nextFrameRect;
       break;
 
+    case imgIContainer::kDisposeClearAll:
+      // Whole image container is cleared
+      aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
+      break;
+
     case imgIContainer::kDisposeClear:
       // Calc area that needs to be redrawn (the combination of previous and
       // this frame)
@@ -891,42 +891,45 @@ nsresult imgContainer::DoComposite(gfxIImageFrame** aFrameToUse,
     needToBlankComposite = PR_TRUE;
   }
 
-  // Copy previous frame into compositingFrame before we put the new frame on top
+  // More optimizations possible when next frame is not transparent
-  // Assumes that the previous frame represents a full frame (it could be
+  PRBool doDisposal = PR_TRUE;
-  // smaller in size than the container, as long as the frame before it erased
+  if ((nextFormat == gfxIFormats::RGB)||(nextFormat == gfxIFormats::PAL)) {
-  // itself)
+    if (isFullNextFrame) {
-  // Note: Frame 1 never gets into DoComposite(), so (aNextFrameIndex - 1) will
+      // Optimization: No need to dispose prev.frame when 
-  // always be a valid frame number.
+      // next frame is full frame and not transparent.
-  if (mAnim->lastCompositedFrameIndex != aNextFrameIndex - 1 &&
+      doDisposal = PR_FALSE;
-      prevFrameDisposalMethod != imgIContainer::kDisposeRestorePrevious) {
+      // No need to blank the composite frame
-
-    // XXX If we had a method of drawing a section of a frame into another, we
-    //     could optimize further:
-    //     if aPrevFrameIndex == 1 && lastCompositedFrameIndex <> -1,
-    //     only firstFrameRefreshArea needs to be drawn back to composite
-    if (isFullPrevFrame) {
-      CopyFrameImage(aPrevFrame, mAnim->compositingFrame);
-    } else {
-      ClearFrame(mAnim->compositingFrame);
-      DrawFrameTo(aPrevFrame, mAnim->compositingFrame, prevFrameRect);
       needToBlankComposite = PR_FALSE;
+    } else {
+      if ((prevFrameRect.x >= nextFrameRect.x) &&
+          (prevFrameRect.y >= nextFrameRect.y) &&
+          (prevFrameRect.x + prevFrameRect.width <= nextFrameRect.x + nextFrameRect.width) &&
+          (prevFrameRect.y + prevFrameRect.height <= nextFrameRect.y + nextFrameRect.height)) {
+        // Optimization: No need to dispose prev.frame when 
+        // next frame fully overlaps previous frame.
+        doDisposal = PR_FALSE;  
+      }
     }      
   }
 
-  // Dispose of previous
+  if (doDisposal) {
+    // Dispose of previous: clear, restore, or keep (copy)
     switch (prevFrameDisposalMethod) {
       case imgIContainer::kDisposeClear:
         if (needToBlankComposite) {
           // If we just created the composite, it could have anything in it's
-        // buffers. Clear them
+          // buffer. Clear whole frame
           ClearFrame(mAnim->compositingFrame);
-        needToBlankComposite = PR_FALSE;
         } else {
-        // Blank out previous frame area (both color & Mask/Alpha)
+          // Only blank out previous frame area (both color & Mask/Alpha)
           ClearFrame(mAnim->compositingFrame, prevFrameRect);
         }
         break;
   
+      case imgIContainer::kDisposeClearAll:
+        ClearFrame(mAnim->compositingFrame);
+        break;
+  
       case imgIContainer::kDisposeRestorePrevious:
         // It would be better to copy only the area changed back to
         // compositingFrame.
@@ -940,6 +943,37 @@ nsresult imgContainer::DoComposite(gfxIImageFrame** aFrameToUse,
           ClearFrame(mAnim->compositingFrame);
         }
         break;
+      
+      default:
+        // Copy previous frame into compositingFrame before we put the new frame on top
+        // Assumes that the previous frame represents a full frame (it could be
+        // smaller in size than the container, as long as the frame before it erased
+        // itself)
+        // Note: Frame 1 never gets into DoComposite(), so (aNextFrameIndex - 1) will
+        // always be a valid frame number.
+        if (mAnim->lastCompositedFrameIndex != aNextFrameIndex - 1) {
+          gfx_format prevFormat;
+          aPrevFrame->GetFormat(&prevFormat);
+          if (isFullPrevFrame && 
+              prevFormat != gfxIFormats::PAL && prevFormat != gfxIFormats::PAL_A1) {
+            // Just copy the bits
+            CopyFrameImage(aPrevFrame, mAnim->compositingFrame);
+          } else {
+            if (needToBlankComposite) {
+              // Only blank composite when prev is transparent or not full.
+              if (!isFullPrevFrame ||
+                  (prevFormat != gfxIFormats::RGB && prevFormat != gfxIFormats::PAL)) {
+                ClearFrame(mAnim->compositingFrame);
+              }
+            }
+            DrawFrameTo(aPrevFrame, mAnim->compositingFrame, prevFrameRect);
+          }
+        }
+    }
+  } else if (needToBlankComposite) {
+    // If we just created the composite, it could have anything in it's
+    // buffers. Clear them
+    ClearFrame(mAnim->compositingFrame);
   }
 
   // Check if the frame we are composing wants the previous image restored afer
@@ -972,7 +1006,15 @@ nsresult imgContainer::DoComposite(gfxIImageFrame** aFrameToUse,
   aNextFrame->GetTimeout(&timeout);
   mAnim->compositingFrame->SetTimeout(timeout);
 
-  if (isFullNextFrame && mAnimationMode == kNormalAnimMode && mLoopCount != 0) {
+  // Tell the image that it is fully 'downloaded'.
+  nsIntRect r;
+  mAnim->compositingFrame->GetRect(r);
+  nsCOMPtr<nsIImage> img = do_GetInterface(mAnim->compositingFrame);
+  img->ImageUpdated(nsnull, nsImageUpdateFlags_kBitsChanged, &r);
+
+  // We don't want to keep composite images for 8bit frames...
+  if (isFullNextFrame && mAnimationMode == kNormalAnimMode && mLoopCount != 0 &&
+      nextFormat != gfxIFormats::PAL && nextFormat != gfxIFormats::PAL_A1) {
     // We have a composited full frame
     // Store the composited frame into the mFrames[..] so we don't have to
     // continuously re-build it
@@ -1007,10 +1049,6 @@ void imgContainer::ClearFrame(gfxIImageFrame *aFrame)
   gfxContext ctx(surf);
   ctx.SetOperator(gfxContext::OPERATOR_CLEAR);
   ctx.Paint();
-
-  nsIntRect r;
-  aFrame->GetRect(r);
-  img->ImageUpdated(nsnull, nsImageUpdateFlags_kBitsChanged, &r);
 }
 
 //******************************************************************************
@@ -1029,8 +1067,6 @@ void imgContainer::ClearFrame(gfxIImageFrame *aFrame, nsIntRect &aRect)
   ctx.SetOperator(gfxContext::OPERATOR_CLEAR);
   ctx.Rectangle(gfxRect(aRect.x, aRect.y, aRect.width, aRect.height));
   ctx.Fill();
-
-  img->ImageUpdated(nsnull, nsImageUpdateFlags_kBitsChanged, &aRect);
 }
 
 
@@ -1061,14 +1097,6 @@ PRBool imgContainer::CopyFrameImage(gfxIImageFrame *aSrcFrame,
   memcpy(aDataDest, aDataSrc, aDataLengthSrc);
   aDstFrame->UnlockImageData();
 
-  // Tell the image that it's data has been updated
-  nsCOMPtr<nsIImage> img(do_GetInterface(aDstFrame));
-  if (!img)
-    return PR_FALSE;
-  nsIntRect r;
-  aDstFrame->GetRect(r);
-  img->ImageUpdated(nsnull, nsImageUpdateFlags_kBitsChanged, &r);
-
   return PR_TRUE;
 }
 
@@ -1080,6 +1108,65 @@ nsresult imgContainer::DrawFrameTo(gfxIImageFrame *aSrc,
   if (!aSrc || !aDst)
     return NS_ERROR_NOT_INITIALIZED;
 
+  nsIntRect srcRect, dstRect;
+  aSrc->GetRect(srcRect);
+  aDst->GetRect(dstRect);
+
+  gfx_format format;
+  aSrc->GetFormat(&format);
+  if (format == gfxIFormats::PAL || format == gfxIFormats::PAL_A1) {
+    // dstRect must fully fit within destination image 
+    NS_ASSERTION((aDstRect.x >= 0) && (aDstRect.y >= 0) &&
+                 (aDstRect.x + aDstRect.width <= dstRect.width) &&
+                 (aDstRect.y + aDstRect.height <= dstRect.height),
+                "imgContainer::DrawFrameTo: Invalid aDstRect");
+    // dstRect size may be smaller than source, but not larger
+    NS_ASSERTION((aDstRect.width <= srcRect.width) &&
+                 (aDstRect.height <= srcRect.height),
+                 "imgContainer::DrawFrameTo: source and dest size must be equal");
+
+    if (NS_FAILED(aDst->LockImageData()))
+      return NS_ERROR_FAILURE;
+    // Get pointers to image data
+    PRUint32 size;
+    PRUint8 *srcPixels;
+    gfx_color *colormap;
+    gfx_color *dstPixels;
+
+    aSrc->GetImageData(&srcPixels, &size);
+    aDst->GetImageData((PRUint8**)&dstPixels, &size);
+    aSrc->GetPaletteData(&colormap, &size);
+    if (!srcPixels || !dstPixels || !colormap) {
+      aDst->UnlockImageData();
+      return NS_ERROR_FAILURE;
+    }
+
+    // Skip to the right offset
+    dstPixels += aDstRect.x + (aDstRect.y * dstRect.width);
+    const PRUint32 width = (PRUint32)aDstRect.width;
+    if (format == gfxIFormats::PAL) {
+      for (PRUint32 r = aDstRect.height; r > 0; --r) {
+        for (PRUint32 c = width; c > 0; --c) {
+          *dstPixels++ = colormap[*srcPixels++];
+        }
+        dstPixels += dstRect.width - width;
+      }
+    } else {
+      // With transparent source, skip transparent pixels
+      for (PRUint32 r = aDstRect.height; r > 0; --r) {
+        for (PRUint32 c = width; c > 0; --c) {
+          const PRUint32 color = colormap[*srcPixels++];
+          if (color)
+            *dstPixels = color;
+          dstPixels ++;
+        }
+        dstPixels += dstRect.width - width;
+      }
+    }
+    aDst->UnlockImageData();
+    return NS_OK;
+  }
+
   nsCOMPtr<nsIImage> srcImg(do_GetInterface(aSrc));
   nsRefPtr<gfxASurface> srcSurf;
   srcImg->GetSurface(getter_AddRefs(srcSurf));
@@ -1107,9 +1194,6 @@ nsresult imgContainer::DrawFrameTo(gfxIImageFrame *aSrc,
   // before we've even begun.
   dst.Translate(gfxPoint(aDstRect.x, aDstRect.y));
   dst.Rectangle(gfxRect(0, 0, aDstRect.width, aDstRect.height), PR_TRUE);
-
-  nsIntRect srcRect;
-  aSrc->GetRect(srcRect);
   dst.Scale(double(aDstRect.width) / srcRect.width, 
             double(aDstRect.height) / srcRect.height);
   dst.SetSource(srcSurf);