bug 143046. Keep GIFs at original 8bit. patch from Alfred Kayser <alfredkayser@nl.ibm.com>. r=me sr=tor
authorpavlov@pavlov.net
Wed, 07 Nov 2007 13:33:57 -0800
changeset 7671 652167adb6941da76728f787b61d1c701741f286
parent 7670 367385045cc54b070ccacf27360e13124e40d82a
child 7672 22ca36281f77a91b67a3dde5a4f9b061d6ef8147
push id1
push userbsmedberg@mozilla.com
push dateThu, 20 Mar 2008 16:49:24 +0000
treeherderautoland@61007906a1f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme, tor
bugs143046
milestone1.9b2pre
bug 143046. Keep GIFs at original 8bit. patch from Alfred Kayser <alfredkayser@nl.ibm.com>. r=me sr=tor
gfx/idl/gfxIFormats.idl
gfx/idl/gfxIImageFrame.idl
gfx/src/shared/gfxImageFrame.cpp
gfx/src/shared/gfxImageFrame.h
modules/libpr0n/decoders/gif/GIF2.h
modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp
modules/libpr0n/decoders/gif/nsGIFDecoder2.h
modules/libpr0n/src/imgContainer.cpp
--- a/gfx/idl/gfxIFormats.idl
+++ b/gfx/idl/gfxIFormats.idl
@@ -92,9 +92,23 @@ interface gfxIFormats
    * RGBA   - packed RGBA image
    */
   const gfx_format RGBA   = 6;
 
   /**
    * 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;
 };
--- a/gfx/idl/gfxIImageFrame.idl
+++ b/gfx/idl/gfxIImageFrame.idl
@@ -52,17 +52,17 @@ native nsRectRef(nsIntRect &);
  * gfxIImageFrame interface
  *
  * All x, y, width, height values are in pixels.
  *
  * @author Tim Rowley <tor@cs.brown.edu>
  * @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
 {
   /**
    * Create a new \a aWidth x \a aHeight sized image.
    *
    * @param aX The x-offset from the origin of the gfxIImageContainer parent.
    * @param aY The y-offset from the origin of the gfxIImageContainer parent.
    * @param aWidth The width of the image to create.
@@ -128,16 +128,21 @@ interface gfxIImageFrame : nsISupports
    * returns the number of bytes allocated for the image
    */
   readonly attribute unsigned long imageDataLength;
 
   // 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
    */
   void lockImageData();
 
   /**
    * Unlock image pixels
    */
   void unlockImageData();
--- a/gfx/src/shared/gfxImageFrame.cpp
+++ b/gfx/src/shared/gfxImageFrame.cpp
@@ -35,32 +35,36 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "gfxImageFrame.h"
 #include "nsIServiceManager.h"
 #include <limits.h>
+#include "prmem.h"
 
 NS_IMPL_ISUPPORTS2(gfxImageFrame, gfxIImageFrame, nsIInterfaceRequestor)
 
 gfxImageFrame::gfxImageFrame() :
-  mInitialized(PR_FALSE),
-  mMutable(PR_TRUE),
+  mImageData(nsnull),
   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 */
 }
 
 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); */
 NS_IMETHODIMP gfxImageFrame::Init(PRInt32 aX, PRInt32 aY, PRInt32 aWidth, PRInt32 aHeight, gfx_format aFormat,gfx_depth aDepth)
 {
   if (mInitialized)
     return NS_ERROR_FAILURE;
 
@@ -78,47 +82,38 @@ NS_IMETHODIMP gfxImageFrame::Init(PRInt3
     return NS_ERROR_FAILURE;
   }
   tmp = tmp * 4;
   if (tmp / 4 != aWidth * aHeight) {
     NS_WARNING("width or height too large");
     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 ){
     NS_WARNING("image too big");
     return NS_ERROR_FAILURE;
   }
   
 #if defined(XP_MACOSX)
   // CoreGraphics is limited to images < 32K in *height*, so clamp all surfaces on the Mac to that height
   if (aHeight > SHRT_MAX) {
     NS_WARNING("image too big");
     return NS_ERROR_FAILURE;
   }
 #endif
 
-  nsresult rv;
-
   mOffset.MoveTo(aX, aY);
   mSize.SizeTo(aWidth, aHeight);
 
   mFormat = aFormat;
+  mDepth = aDepth;
 
-  mImage = do_CreateInstance("@mozilla.org/gfx/image;1", &rv);
-  NS_ASSERTION(mImage, "creation of image failed");
-  if (NS_FAILED(rv)) return rv;
-
+  PRBool needImage = PR_TRUE;
   nsMaskRequirements maskReq;
 
   switch (aFormat) {
   case gfxIFormats::BGR:
   case gfxIFormats::RGB:
     maskReq = nsMaskRequirements_kNoMask;
     break;
 
@@ -132,25 +127,47 @@ NS_IMETHODIMP gfxImageFrame::Init(PRInt3
     maskReq = nsMaskRequirements_kNeeds1Bit;
     break;
 
   case gfxIFormats::BGR_A8:
   case gfxIFormats::RGB_A8:
     maskReq = nsMaskRequirements_kNeeds8Bit;
     break;
 
+  case gfxIFormats::PAL:
+  case gfxIFormats::PAL_A1:
+    needImage = PR_FALSE;
+    break;
+
   default:
-#ifdef DEBUG
-    printf("unsupported gfx_format\n");
-#endif
-    break;
+    NS_ERROR("unsupported gfx_format\n");
+    return NS_ERROR_FAILURE;
   }
 
-  rv = mImage->Init(aWidth, aHeight, aDepth, maskReq);
-  if (NS_FAILED(rv)) return rv;
+  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;
 }
 
 
 /* attribute boolean mutable */
 NS_IMETHODIMP gfxImageFrame::GetMutable(PRBool *aMutable)
@@ -165,17 +182,17 @@ NS_IMETHODIMP gfxImageFrame::GetMutable(
 
 NS_IMETHODIMP gfxImageFrame::SetMutable(PRBool aMutable)
 {
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
   mMutable = aMutable;
 
-  if (!aMutable)
+  if (!aMutable && mImage)
     mImage->Optimize(nsnull);
 
   return NS_OK;
 }
 
 /* readonly attribute PRInt32 x; */
 NS_IMETHODIMP gfxImageFrame::GetX(PRInt32 *aX)
 {
@@ -240,69 +257,91 @@ NS_IMETHODIMP gfxImageFrame::GetFormat(g
 
 /* readonly attribute boolean needsBackground; */
 NS_IMETHODIMP gfxImageFrame::GetNeedsBackground(PRBool *aNeedsBackground)
 {
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
   *aNeedsBackground = (mFormat != gfxIFormats::RGB && 
+                       mFormat != gfxIFormats::PAL &&
                        mFormat != gfxIFormats::BGR) ||
                       !mImage->GetIsImageComplete();
   return NS_OK;
 }
 
 
 /* readonly attribute unsigned long imageBytesPerRow; */
 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;
 }
 
 /* readonly attribute unsigned long imageDataLength; */
 NS_IMETHODIMP gfxImageFrame::GetImageDataLength(PRUint32 *aBitsLength)
 {
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
-  *aBitsLength = mImage->GetLineStride() * mSize.height;
+  *aBitsLength = ImageDataLength();
   return NS_OK;
 }
 
 /* void getImageData([array, size_is(length)] out PRUint8 bits, out unsigned long length); */
 NS_IMETHODIMP gfxImageFrame::GetImageData(PRUint8 **aData, PRUint32 *length)
 {
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
-  *aData = mImage->GetBits();
-  *length = mImage->GetLineStride() * mSize.height;
+  NS_ASSERTION(mMutable, "trying to get data on an immutable frame");
+
+  *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;
 }
 
 /* void lockImageData (); */
 NS_IMETHODIMP gfxImageFrame::LockImageData()
 {
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
+  if (!mImage)
+    return NS_OK;
   return mImage->LockImagePixels(PR_FALSE);
 }
 
 /* void unlockImageData (); */
 NS_IMETHODIMP gfxImageFrame::UnlockImageData()
 {
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
 
+  if (!mImage)
+    return NS_OK;
   return mImage->UnlockImagePixels(PR_FALSE);
 }
 
 /* attribute long timeout; */
 NS_IMETHODIMP gfxImageFrame::GetTimeout(PRInt32 *aTimeout)
 {
   if (!mInitialized)
     return NS_ERROR_NOT_INITIALIZED;
--- a/gfx/src/shared/gfxImageFrame.h
+++ b/gfx/src/shared/gfxImageFrame.h
@@ -66,19 +66,29 @@ public:
 
   gfxImageFrame();
   virtual ~gfxImageFrame();
 
 protected:
   nsIntSize mSize;
 
 private:
-  nsCOMPtr<nsIImage> mImage;
+  PRUint32 PaletteDataLength() const {
+    return ((1 << mDepth) * sizeof(gfx_color));
+  }
 
-  PRPackedBool mInitialized;
-  PRPackedBool mMutable;
-  gfx_format   mFormat;
+  PRUint32 ImageDataLength() const {
+    return (mImage ? mImage->GetLineStride() : mSize.width) * mSize.height;
+  }
+
+  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;
 };
--- a/modules/libpr0n/decoders/gif/GIF2.h
+++ b/modules/libpr0n/decoders/gif/GIF2.h
@@ -97,33 +97,33 @@ typedef struct gif_struct {
     int count;                  /* Remaining # bytes in sub-block */
     int bits;                   /* Number of unread bits in "datum" */
     int32 datum;                /* 32-bit input buffer */
 
     /* Output state machine */
     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 */
     PRUintn height, width;
     int tpixel;                 /* Index of transparent pixel */
     PRInt32 disposal_method;    /* Restore to background, leave in place, etc.*/
     PRUint32 *local_colormap;   /* Per-image colormap */
     int local_colormap_size;    /* Size of local colormap array. */
     PRUint32 delay_time;        /* Display time, in milliseconds,
                                    for this image in a multi-image GIF */
 
     /* Global (multi-image) state */
     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. */
 
     PRPackedBool progressive_display;    /* If TRUE, do Haeberli interlace hack */
     PRPackedBool interlaced;             /* TRUE, if scanlines arrive interlaced order */
     PRPackedBool is_transparent;         /* TRUE, if tpixel is valid */
 
--- a/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp
+++ b/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp
@@ -88,17 +88,17 @@ mailing address.
 #include "gfxPlatform.h"
 #include "lcms.h"
 
 /*
  * GETN(n, s) requests at least 'n' bytes available from 'q', at start of state 's'
  *
  * 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
  */
 #define GETN(n,s)                      \
   PR_BEGIN_MACRO                       \
     mGIFStruct.bytes_to_consume = (n); \
     mGIFStruct.state = (s);            \
   PR_END_MACRO
@@ -237,17 +237,18 @@ nsGIFDecoder2::FlushImageData()
 //******************************************************************************
 nsresult nsGIFDecoder2::ProcessData(unsigned char *data, PRUint32 count, PRUint32 *_retval)
 {
   // Push the data to the GIF decoder
   
   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;
   }
 
   *_retval = count;
 
   return NS_OK;
@@ -308,77 +309,85 @@ void nsGIFDecoder2::EndGIF()
   
   mImageContainer->SetLoopCount(mGIFStruct.loop_count);
   mImageContainer->DecodingComplete();
 
   mGIFOpen = PR_FALSE;
 }
 
 //******************************************************************************
-void nsGIFDecoder2::BeginImageFrame()
+void nsGIFDecoder2::BeginImageFrame(gfx_depth aDepth)
 {
   mImageFrame = nsnull; // clear out our current frame reference
 
   if (!mGIFStruct.images_decoded) {
     // Send a onetime OnDataAvailable (Display Refresh) for the first frame
     // if it has a y-axis offset.  Otherwise, the area may never be refreshed
     // and the placeholder will remain on the screen. (Bug 37589)
     if (mGIFStruct.y_offset > 0) {
       PRInt32 imgWidth;
       mImageContainer->GetWidth(&imgWidth);
       nsIntRect r(0, 0, imgWidth, mGIFStruct.y_offset);
       mObserver->OnDataAvailable(nsnull, mImageFrame, &r);
     }
   }
 
-  gfx_format format = gfxIFormats::RGB;
-  if (mGIFStruct.is_transparent) {
-    format = gfxIFormats::RGB_A1;  // XXX not really
+  // Use correct format, RGB for first frame, PAL for following frames
+  // and include transparency to allow for optimization of opaque images
+  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;
   }
 
   mImageFrame->SetFrameDisposalMethod(mGIFStruct.disposal_method);
   mImageContainer->AppendFrame(mImageFrame);
 
   if (mObserver)
     mObserver->OnStartFrame(nsnull, mImageFrame);
 
   PRUint32 imageDataLength;
-  mImageFrame->GetImageData((PRUint8 **)&mImageData, &imageDataLength);
+  mImageFrame->GetImageData(&mImageData, &imageDataLength);
 }
 
 
 //******************************************************************************
 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 (!mGIFStruct.images_decoded) {
     // 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)
     const PRUint32 realFrameHeight = mGIFStruct.height + mGIFStruct.y_offset;
     if (realFrameHeight < mGIFStruct.screen_height) {
       nsIntRect r(0, realFrameHeight, 
                   mGIFStruct.screen_width, 
 				  mGIFStruct.screen_height - realFrameHeight);
       mObserver->OnDataAvailable(nsnull, mImageFrame, &r);
     }
   }
+  mCurrentRow = mLastFlushedRow = -1;
+  mCurrentPass = mLastFlushedPass = 0;
 
   mGIFStruct.images_decoded++;
 
   // We actually have the timeout information before we get the lzw encoded 
   // image data, at least according to the spec, but we delay in setting the 
   // timeout for the image until here to help ensure that we have the whole 
   // image frame decoded before we go off and try to display another frame.
   mImageFrame->SetTimeout(mGIFStruct.delay_time);
@@ -400,66 +409,76 @@ void nsGIFDecoder2::EndImageFrame()
 
 //******************************************************************************
 // Send the data to the display front-end.
 PRUint32 nsGIFDecoder2::OutputRow()
 {
   int drow_start, drow_end;
   drow_start = drow_end = mGIFStruct.irow;
 
-  /*
-   * Haeberli-inspired hack for interlaced GIFs: Replicate lines while
-   * displaying to diminish the "venetian-blind" effect as the image is
-   * loaded. Adjust pixel vertical positions to avoid the appearance of the
-   * image crawling up the screen as successive passes are drawn.
-   */
-  if (mGIFStruct.progressive_display && mGIFStruct.interlaced && (mGIFStruct.ipass < 4)) {
-    /* ipass = 1,2,3 results in resp. row_dup = 7,3,1 and row_shift = 3,1,0 */
-    const PRUint32 row_dup = 15 >> mGIFStruct.ipass;
-    const PRUint32 row_shift = row_dup >> 1;
-
-    drow_start -= row_shift;
-    drow_end = drow_start + row_dup;
-
-    /* Extend if bottom edge isn't covered because of the shift upward. */
-    if (((mGIFStruct.height - 1) - drow_end) <= row_shift)
-      drow_end = mGIFStruct.height - 1;
-
-    /* Clamp first and last rows to upper and lower edge of image. */
-    if (drow_start < 0)
-      drow_start = 0;
-    if ((PRUintn)drow_end >= mGIFStruct.height)
-      drow_end = mGIFStruct.height - 1;
-  }
-
   /* 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
+     * loaded. Adjust pixel vertical positions to avoid the appearance of the
+     * image crawling up the screen as successive passes are drawn.
+     */
+    if (mGIFStruct.progressive_display && mGIFStruct.interlaced && (mGIFStruct.ipass < 4)) {
+      /* ipass = 1,2,3 results in resp. row_dup = 7,3,1 and row_shift = 3,1,0 */
+      const PRUint32 row_dup = 15 >> mGIFStruct.ipass;
+      const PRUint32 row_shift = row_dup >> 1;
   
-  // 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));
+      drow_start -= row_shift;
+      drow_end = drow_start + row_dup;
+  
+      /* Extend if bottom edge isn't covered because of the shift upward. */
+      if (((mGIFStruct.height - 1) - drow_end) <= row_shift)
+        drow_end = mGIFStruct.height - 1;
+  
+      /* Clamp first and last rows to upper and lower edge of image. */
+      if (drow_start < 0)
+        drow_start = 0;
+      if ((PRUintn)drow_end >= mGIFStruct.height)
+        drow_end = mGIFStruct.height - 1;
+    }
+
+    // Row to process
+    const PRUint32 bpr = sizeof(PRUint32) * mGIFStruct.width; 
+    PRUint8 *rowp = mImageData + (mGIFStruct.irow * bpr);
+
+    // 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
+      for (int r = drow_start; r <= drow_end; r++) {
+        if (r != mGIFStruct.irow) {
+          memcpy(mImageData + (r * bpr), rowp, bpr);
+        }
       }
     }
   }
 
   mCurrentRow = drow_end;
   mCurrentPass = mGIFStruct.ipass;
   if (mGIFStruct.ipass == 1)
     mLastFlushedPass = mGIFStruct.ipass;   // interlaced starts at 1
 
-
   if (!mGIFStruct.interlaced) {
     mGIFStruct.irow++;
   } else {
     static const PRUint8 kjump[5] = { 1, 8, 8, 4, 2 };
     do {
       // Row increments resp. per 8,8,4,2 rows
       mGIFStruct.irow += kjump[mGIFStruct.ipass];
       if (mGIFStruct.irow >= mGIFStruct.height) {
@@ -493,24 +512,28 @@ nsGIFDecoder2::DoLzw(const PRUint8 *q)
   int oldcode     = mGIFStruct.oldcode;
   const int clear_code = ClearCode();
   PRUint8 firstchar = mGIFStruct.firstchar;
   PRInt32 datum     = mGIFStruct.datum;
   PRUint16 *prefix  = mGIFStruct.prefix;
   PRUint8 *stackp   = mGIFStruct.stackp;
   PRUint8 *suffix   = mGIFStruct.suffix;
   PRUint8 *stack    = mGIFStruct.stack;
-  PRUint32 *rowp    = mGIFStruct.rowp;
-  PRUint32 *rowend  = mImageData + (mGIFStruct.irow + 1) * mGIFStruct.width;
-  PRUint32 *cmap    = mColormap;
+  PRUint8 *rowp     = mGIFStruct.rowp;
+
+  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
 
   for (const PRUint8* ch = q; count-- > 0; ch++)
   {
     /* Feed the next byte into the decoder's 32-bit input buffer. */
     datum += ((int32) *ch) << bits;
     bits += 8;
@@ -534,17 +557,17 @@ nsGIFDecoder2::DoLzw(const PRUint8 *q)
 
       /* Check for explicit end-of-stream code */
       if (code == (clear_code + 1)) {
         /* end-of-stream should only appear after all image data */
         return (mGIFStruct.rows_remaining == 0);
       }
 
       if (oldcode == -1) {
-        *rowp++ = cmap[suffix[code]];
+        *rowp++ = suffix[code];
         if (rowp == rowend)
           OUTPUT_ROW();
 
         firstchar = oldcode = code;
         continue;
       }
 
       int incode = code;
@@ -584,17 +607,17 @@ nsGIFDecoder2::DoLzw(const PRUint8 *q)
           codesize++;
           codemask += avail;
         }
       }
       oldcode = incode;
 
       /* Copy the decoded data out to the scanline buffer. */
       do {
-        *rowp++ = cmap[*--stackp];
+        *rowp++ = *--stackp;
         if (rowp == rowend)
           OUTPUT_ROW();
       } while (stackp > stack);
     }
   }
 
   END:
 
@@ -625,19 +648,16 @@ static void ConvertColormap(PRUint32 *aC
     if (transform)
       cmsDoTransform(transform, aColormap, aColormap, aColors);
   }
   // Convert from the GIF's RGB format to the Cairo format.
   // Work from end to begin, because of the in-place expansion
   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;
     *--to = GFX_PACKED_PIXEL(0xFF, from[0], from[1], from[2]);
   }
 }
 
 /******************************************************************************/
@@ -651,17 +671,17 @@ nsresult nsGIFDecoder2::GifWrite(const P
     return NS_ERROR_FAILURE;
 
   const PRUint8 *q = buf;
 
   // Add what we have sofar to the block
   // 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
     PRUint32 l = PR_MIN(len, mGIFStruct.bytes_to_consume);
     memcpy(p+mGIFStruct.bytes_in_hold, buf, l);
 
     if (l < mGIFStruct.bytes_to_consume) {
       // Not enough in 'buf' to complete current block, get more
@@ -698,17 +718,19 @@ nsresult nsGIFDecoder2::GifWrite(const P
       }
       GETN(1, gif_sub_block);
       break;
 
     case gif_lzw_start:
     {
       // Make sure the transparent pixel is transparent in the colormap
       if (mGIFStruct.is_transparent) {
-        mOldColor = mColormap[mGIFStruct.tpixel];
+        // Save old value so we can restore it later
+        if (mColormap == mGIFStruct.global_colormap)
+            mOldColor = mColormap[mGIFStruct.tpixel];
         mColormap[mGIFStruct.tpixel] = 0;
       }
 
       /* Initialize LZW parser/decoder */
       mGIFStruct.datasize = *q;
       const int clear_code = ClearCode();
       if (mGIFStruct.datasize > MAX_LZW_BITS ||
           clear_code >= MAX_BITS) {
@@ -750,27 +772,27 @@ nsresult nsGIFDecoder2::GifWrite(const P
        * frame into which images are rendered.  The
        * individual images can be smaller than the
        * screen size and located with an origin anywhere
        * within the screen.
        */
 
       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];
       // q[6] = Pixel Aspect Ratio
       //   Not used
       //   float aspect = (float)((q[6] + 15) / 64.0);
 
       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);
           break;
         }
         // Copy everything, go to colormap state to do CMS correction
         memcpy(mGIFStruct.global_colormap, buf, size);
         buf += size;
@@ -780,17 +802,17 @@ nsresult nsGIFDecoder2::GifWrite(const P
       }
 
       GETN(1, gif_image_start);
       break;
 
     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;
 
     case gif_image_start:
       switch (*q) {
         case GIF_TRAILER:
           mGIFStruct.state = gif_done;
           break;
@@ -923,16 +945,17 @@ nsresult nsGIFDecoder2::GifWrite(const P
   
         default:
           // 0,3-7 are yet to be defined netscape extension codes
           mGIFStruct.state = gif_error;
       }
       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);
 
       /* Get image width and height. */
       mGIFStruct.width  = GETINT16(q + 4);
       mGIFStruct.height = GETINT16(q + 6);
 
@@ -959,17 +982,23 @@ nsresult nsGIFDecoder2::GifWrite(const P
         mGIFStruct.height = mGIFStruct.screen_height;
         mGIFStruct.width = mGIFStruct.screen_width;
         if (!mGIFStruct.height || !mGIFStruct.width) {
           mGIFStruct.state = gif_error;
           break;
         }
       }
 
-      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) {
         mGIFStruct.state = gif_error;
         break;
       }
 
       if (q[8] & 0x40) {
@@ -987,53 +1016,65 @@ nsresult nsGIFDecoder2::GifWrite(const P
       mGIFStruct.irow = 0;
       mGIFStruct.rows_remaining = mGIFStruct.height;
       mGIFStruct.rowp = mImageData;
 
       /* bits per pixel is q[8]&0x07 */
 
       if (q[8] & 0x80) /* has a local colormap? */
       {
-        const int num_colors = 2 << (q[8] & 0x7);
-        const PRUint32 size = 3*num_colors;
-        if (!mGIFStruct.local_colormap) {
-          mGIFStruct.local_colormap = 
-			  (PRUint32*)PR_MALLOC(MAX_COLORS * sizeof(PRUint32));
+        mGIFStruct.local_colormap_size = 1 << depth;
+        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.state = gif_oom;
-            break;
+            mGIFStruct.local_colormap = 
+  			  (PRUint32*)PR_MALLOC(mGIFStruct.local_colormap_size * sizeof(PRUint32));
+            if (!mGIFStruct.local_colormap) {
+              mGIFStruct.state = gif_oom;
+              break;
+            }
           }
+          mColormap = mGIFStruct.local_colormap;
         }
-
-        /* 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);
         break;
       } 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;
+    }
+    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;
 
     case gif_sub_block:
       mGIFStruct.count = *q;
       if (mGIFStruct.count) {
         /* Still working on the same image: Process next LZW data block */
         /* Make sure there are still rows left. If the GIF data */
@@ -1086,16 +1127,16 @@ nsresult nsGIFDecoder2::GifWrite(const P
       return NS_ERROR_FAILURE;
   }
   
   // Copy the leftover into mGIFStruct.hold
   mGIFStruct.bytes_in_hold = 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;
   }
 
   return NS_OK;
 }
--- a/modules/libpr0n/decoders/gif/nsGIFDecoder2.h
+++ b/modules/libpr0n/decoders/gif/nsGIFDecoder2.h
@@ -71,34 +71,34 @@ public:
   nsresult ProcessData(unsigned char *data, PRUint32 count, PRUint32 *_retval);
 
 private:
   /* These functions will be called when the decoder has a decoded row,
    * frame size information, etc. */
 
   void      BeginGIF();
   void      EndGIF();
-  void      BeginImageFrame();
+  void      BeginImageFrame(gfx_depth aDepth);
   void      EndImageFrame();
   void      FlushImageData();
   void      FlushImageData(PRUint32 fromRow, PRUint32 rows);
 
   nsresult  GifWrite(const PRUint8 * buf, PRUint32 numbytes);
   PRUint32  OutputRow();
   PRBool    DoLzw(const PRUint8 *q);
 
   inline int ClearCode() const { return 1 << mGIFStruct.datasize; }
 
   nsCOMPtr<imgIContainer> mImageContainer;
   nsCOMPtr<gfxIImageFrame> mImageFrame;
   nsCOMPtr<imgIDecoderObserver> mObserver; // this is just qi'd from mRequest for speed
   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;
   PRUint8 mLastFlushedPass;
   PRPackedBool mGIFOpen;
 
   gif_struct mGIFStruct;
 };
--- a/modules/libpr0n/src/imgContainer.cpp
+++ b/modules/libpr0n/src/imgContainer.cpp
@@ -786,71 +786,71 @@ nsresult imgContainer::DoComposite(gfxII
   NS_ASSERTION(aFrameToUse, "imgContainer::DoComposite aFrameToUse is null");
   
   PRInt32 prevFrameDisposalMethod;
   aPrevFrame->GetFrameDisposalMethod(&prevFrameDisposalMethod);
 
   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) {
-    aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
-    *aFrameToUse = aNextFrame;
-    return NS_OK;
-  }
+  if (isFullPrevFrame && 
+      (prevFrameDisposalMethod == imgIContainer::kDisposeClear))
+    prevFrameDisposalMethod = imgIContainer::kDisposeClearAll;
 
   PRInt32 nextFrameDisposalMethod;
   nsIntRect nextFrameRect;
   aNextFrame->GetFrameDisposalMethod(&nextFrameDisposalMethod);
   aNextFrame->GetRect(nextFrameRect);
   PRBool isFullNextFrame = (nextFrameRect.x == 0 && nextFrameRect.y == 0 &&
                             nextFrameRect.width == mSize.width &&
                             nextFrameRect.height == mSize.height);
 
   gfx_format nextFormat;
   aNextFrame->GetFormat(&nextFormat);
-  PRBool nextFrameHasAlpha = (nextFormat != gfxIFormats::RGB) &&
-                             (nextFormat != gfxIFormats::BGR);
-
-  // 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) {
-
-    aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
-    *aFrameToUse = aNextFrame;
-    return NS_OK;
+  if (nextFormat != gfxIFormats::PAL && nextFormat != gfxIFormats::PAL_A1) {
+    // 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) &&
+        (nextFormat == gfxIFormats::RGB)) {
+      aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
+      *aFrameToUse = aNextFrame;
+      return NS_OK;
+    }
   }
 
   // Calculate area that needs updating
   switch (prevFrameDisposalMethod) {
     default:
     case imgIContainer::kDisposeNotSpecified:
     case imgIContainer::kDisposeKeep:
       *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)
       // XXX - This could be done with multiple framechanged calls
       //       Having prevFrame way at the top of the image, and nextFrame
       //       way at the bottom, and both frames being small, we'd be
       //       telling framechanged to refresh the whole image when only two
       //       small areas are needed.
@@ -886,65 +886,99 @@ nsresult imgContainer::DoComposite(gfxII
     if (NS_FAILED(rv)) {
       NS_WARNING("Failed to init compositingFrame!\n");
       mAnim->compositingFrame = nsnull;
       return rv;
     }
     needToBlankComposite = PR_TRUE;
   }
 
-  // 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 &&
-      prevFrameDisposalMethod != imgIContainer::kDisposeRestorePrevious) {
-
-    // 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);
+  // More optimizations possible when next frame is not transparent
+  PRBool doDisposal = PR_TRUE;
+  if ((nextFormat == gfxIFormats::RGB)||(nextFormat == gfxIFormats::PAL)) {
+    if (isFullNextFrame) {
+      // Optimization: No need to dispose prev.frame when 
+      // next frame is full frame and not transparent.
+      doDisposal = PR_FALSE;
+      // No need to blank the composite frame
+      needToBlankComposite = PR_FALSE;
     } else {
-      ClearFrame(mAnim->compositingFrame);
-      DrawFrameTo(aPrevFrame, mAnim->compositingFrame, prevFrameRect);
-      needToBlankComposite = PR_FALSE;
-    }
+      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
-  switch (prevFrameDisposalMethod) {
-    case imgIContainer::kDisposeClear:
-      if (needToBlankComposite) {
-        // If we just created the composite, it could have anything in it's
-        // buffers. Clear them
+  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
+          // buffer. Clear whole frame
+          ClearFrame(mAnim->compositingFrame);
+        } else {
+          // Only blank out previous frame area (both color & Mask/Alpha)
+          ClearFrame(mAnim->compositingFrame, prevFrameRect);
+        }
+        break;
+  
+      case imgIContainer::kDisposeClearAll:
         ClearFrame(mAnim->compositingFrame);
-        needToBlankComposite = PR_FALSE;
-      } else {
-        // Blank out previous frame area (both color & Mask/Alpha)
-        ClearFrame(mAnim->compositingFrame, prevFrameRect);
-      }
-      break;
-
-    case imgIContainer::kDisposeRestorePrevious:
-      // It would be better to copy only the area changed back to
-      // compositingFrame.
-      if (mAnim->compositingPrevFrame) {
-        CopyFrameImage(mAnim->compositingPrevFrame, mAnim->compositingFrame);
-
-        // destroy only if we don't need it for this frame's disposal
-        if (nextFrameDisposalMethod != imgIContainer::kDisposeRestorePrevious)
-          mAnim->compositingPrevFrame = nsnull;
-      } else {
-        ClearFrame(mAnim->compositingFrame);
-      }
-      break;
+        break;
+  
+      case imgIContainer::kDisposeRestorePrevious:
+        // It would be better to copy only the area changed back to
+        // compositingFrame.
+        if (mAnim->compositingPrevFrame) {
+          CopyFrameImage(mAnim->compositingPrevFrame, mAnim->compositingFrame);
+  
+          // destroy only if we don't need it for this frame's disposal
+          if (nextFrameDisposalMethod != imgIContainer::kDisposeRestorePrevious)
+            mAnim->compositingPrevFrame = nsnull;
+        } else {
+          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
   // it is done. Don't store it (again) if last frame wanted its image restored
   // too
   if ((nextFrameDisposalMethod == imgIContainer::kDisposeRestorePrevious) &&
       (prevFrameDisposalMethod != imgIContainer::kDisposeRestorePrevious)) {
     // We are storing the whole image.
@@ -967,17 +1001,25 @@ nsresult imgContainer::DoComposite(gfxII
   // blit next frame into it's correct spot
   DrawFrameTo(aNextFrame, mAnim->compositingFrame, nextFrameRect);
   // Set timeout of CompositeFrame to timeout of frame we just composed
   // Bug 177948
   PRInt32 timeout;
   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
     // Then set the previous frame's disposal to CLEAR_ALL so we just draw the
     // frame next time around
     if (CopyFrameImage(mAnim->compositingFrame, aNextFrame)) {
       aPrevFrame->SetFrameDisposalMethod(imgIContainer::kDisposeClearAll);
       mAnim->lastCompositedFrameIndex = -1;
@@ -1002,20 +1044,16 @@ void imgContainer::ClearFrame(gfxIImageF
   nsCOMPtr<nsIImage> img(do_GetInterface(aFrame));
   nsRefPtr<gfxASurface> surf;
   img->GetSurface(getter_AddRefs(surf));
 
   // Erase the surface to transparent
   gfxContext ctx(surf);
   ctx.SetOperator(gfxContext::OPERATOR_CLEAR);
   ctx.Paint();
-
-  nsIntRect r;
-  aFrame->GetRect(r);
-  img->ImageUpdated(nsnull, nsImageUpdateFlags_kBitsChanged, &r);
 }
 
 //******************************************************************************
 void imgContainer::ClearFrame(gfxIImageFrame *aFrame, nsIntRect &aRect)
 {
   if (!aFrame || aRect.width <= 0 || aRect.height <= 0) {
     return;
   }
@@ -1024,18 +1062,16 @@ void imgContainer::ClearFrame(gfxIImageF
   nsRefPtr<gfxASurface> surf;
   img->GetSurface(getter_AddRefs(surf));
 
   // Erase the destination rectangle to transparent
   gfxContext ctx(surf);
   ctx.SetOperator(gfxContext::OPERATOR_CLEAR);
   ctx.Rectangle(gfxRect(aRect.x, aRect.y, aRect.width, aRect.height));
   ctx.Fill();
-
-  img->ImageUpdated(nsnull, nsImageUpdateFlags_kBitsChanged, &aRect);
 }
 
 
 //******************************************************************************
 // Whether we succeed or fail will not cause a crash, and there's not much
 // we can do about a failure, so there we don't return a nsresult
 PRBool imgContainer::CopyFrameImage(gfxIImageFrame *aSrcFrame,
                                     gfxIImageFrame *aDstFrame)
@@ -1056,35 +1092,86 @@ PRBool imgContainer::CopyFrameImage(gfxI
   aDstFrame->GetImageData(&aDataDest, &aDataLengthDest);
   if (!aDataDest || !aDataSrc || aDataLengthDest != aDataLengthSrc) {
     aDstFrame->UnlockImageData();
     return PR_FALSE;
   }
   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;
 }
 
 //******************************************************************************
 nsresult imgContainer::DrawFrameTo(gfxIImageFrame *aSrc,
                                    gfxIImageFrame *aDst, 
                                    nsIntRect& aDstRect)
 {
   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));
 
   nsCOMPtr<nsIImage> dstImg(do_GetInterface(aDst));
   nsRefPtr<gfxASurface> dstSurf;
   dstImg->GetSurface(getter_AddRefs(dstSurf));
 
@@ -1102,19 +1189,16 @@ nsresult imgContainer::DrawFrameTo(gfxII
   
   dst.NewPath();
   dst.SetOperator(defaultOperator);
   // We don't use PixelSnappedRectangleAndSetPattern because if
   // these coords aren't already pixel aligned, we've lost
   // 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);
   dst.Paint();
 
   return NS_OK;
 }