86600 - nsImageGTK::DrawToImage() missing functionality

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[tor]

nsImageGTK::DrawToImage() wasn't doing the composite into mImageBits.  This
caused certain animated gifs when scaled to turn all black.  The problem can
be seen on the dancing penguin on the upper right of the test url.  Patch
below adds the necessary guts.  Probably overengineered, as it seems
DrawToImage() is only ever called with a 1/0-bit alpha without scaling.

Side note: this sort of stuff should be taken care of by the image decoder,
not floating in the generic imgContainer and gfx.

Comment 1

[tor]

Attachment: smarter nsImageGTK::DrawToImage()

Comment 2

[tor]

*** Bug 75351 has been marked as a duplicate of this bug. ***

Comment 3

[Christopher Blizzard (:blizzard)]

+    scaledImage = (PRUint8 *)nsMemory::Alloc(3*aDWidth*aDHeight);
+    if (!scaledImage) {
+      nsMemory::Free(scaledImage);
+      return NS_OK;
+    } else

If the allocation of scaledImage failed don't try to free it.  <brendan>Also,
the else {} after the if {} is redundant since there's a return in the if
clause.  If you have braces around the if block you should have braces around
the else block as well.</brendan>  Plus, you're returning NS_OK even though it
did fail to draw the image.

+      if (mAlphaDepth==1)
+        alphaStride = (aDWidth+7)>>3;

Comment and/or macro?

+      scaledAlpha = (PRUint8 *)nsMemory::Alloc(alphaStride*aDHeight);
+      if (scaledAlpha)
+        RectStretch(0, 0, mWidth-1, mHeight-1, 0, 0, aDWidth-1, aDHeight-1,
+                    mAlphaBits, mAlphaRowBytes, scaledAlpha, alphaStride,
+                    mAlphaDepth);
+      else {
+        nsMemory::Free(scaledImage);
+        return NS_OK;
+      }

Please make it consistent with how you did it above.  That is, use:

if (!scaledAlpha) {
  nsMemory::Free(scaledImage);
  return NS_OK;
}

without the else clause.  Also, same comment about the return value.

+        if (alpha[x>>3] & (1<<(7-x&0x7))) {
+          dst[0] = src[0];
+          dst[1] = src[1];
+          dst[2] = src[2];
+          *(dest->mAlphaBits + (y+aDY)*dest->mAlphaRowBytes + ((aDX+x)>>3))
+            |= 1<<(7-(aDX+x)&0x7);

Comments, please!  Same with the 8: case below it.

Comment 4

[tor]

Attachment: deobfuscate

Comment 5

[Christopher Blizzard (:blizzard)]

r/sr=blizzard

Comment 6

[tor]

*** Bug 85137 has been marked as a duplicate of this bug. ***

Comment 7

[Stuart Parmenter]

r=pavlov

Comment 8

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 9

[tor]

Checked in 6/20.

Comment 10

[Chris Hiner]

*** Bug 86883 has been marked as a duplicate of this bug. ***

Comment 11

[John Morrison]

May God have mercy on us all. The 212 bug spam-o-rama is Now!