250531 - nsImageGTK::LockImagePixels(PR_TRUE) does not retrieve mAlphaBits

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Brian Ryner (not reading)]

Calling LockImagePixels(PR_TRUE) should allow the alpha mask to be accessed via
GetAlphaBits().  However, if the alpha mask has been stored onto a server-side
pixmap (mAlphaPixmap), LockImagePixels does not fetch the image data into
mAlphaBits.

Comment 1

[Brian Ryner (not reading)]

Attachment: patch

Comment 2

[Brian Ryner (not reading)]

Attachment: diff -w of the above

Comment 3

[tor]

What do you need this for?  As far as I know the only user of GetAlphaBits,
directly or indirectly, is imglib2 when decoding.

Comment 4

[Brian Ryner (not reading)]

I'm going to use it in bug 242254.

Comment 5

[tor]

Comment on attachment 152670 [details] [diff] [review]
diff -w of the above

> NS_IMETHODIMP
> nsImageGTK::LockImagePixels(PRBool aMaskPixels)
> {
>-  if (mOptimized && mImagePixmap) {
>+  if (!mOptimized)
>+    return NS_OK;

The mImagePixmap test is still needed, as 8-bit alpha images get marked
as optimized, yet don't have a backing pixmap.

Comment 6

[Brian Ryner (not reading)]

Attachment: new diff -w

add back mImagePixmap check when aMaskPixels is false.

Comment 7

[tor]

Comment on attachment 153335 [details] [diff] [review]
new diff -w

>+    mAlphaRowBytes = (mWidth + 7) / 8;
>+    // 32-bit align each row
>+    mAlphaRowBytes = (mAlphaRowBytes + 3) & ~0x3;

You don't need to recompute mAlphaRowBytes.  Remove that and r+sr+tor

Comment 8

[Brian Ryner (not reading)]

checked in.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

hm... is this sufficient? As is, gfxIImageFrame::LockAlphaData will fail because
GetHasAlphaMask returns false; and thus, LockImagePixels(PR_TRUE) is never called.

(The code at
http://lxr.mozilla.org/seamonkey/source/browser/components/shell/src/nsGNOMEShellService.cpp#357
does not deal with that very well (i.e. it crashes, because alphaBytesPerRow
will be uninitialized, and thus probably be a large number, and thus *maskPixel
causes a SIGSEGV))