nsImageGTK::LockImagePixels(PR_TRUE) does not retrieve mAlphaBits

RESOLVED FIXED

Status

Core Graveyard
GFX: Gtk
RESOLVED FIXED
14 years ago
10 years ago

People

(Reporter: Brian Ryner (not reading), Assigned: Brian Ryner (not reading))

Tracking

({fixed-aviary1.0})

Trunk
x86
Linux
fixed-aviary1.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

14 years ago
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.
(Assignee)

Comment 2

14 years ago
Created attachment 152670 [details] [diff] [review]
diff -w of the above
(Assignee)

Updated

14 years ago
Attachment #152670 - Flags: superreview?(tor)
Attachment #152670 - Flags: review?(tor)

Comment 3

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

Comment 4

14 years ago
I'm going to use it in bug 242254.

Comment 5

14 years ago
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.
(Assignee)

Updated

14 years ago
Attachment #152670 - Flags: superreview?(tor)
Attachment #152670 - Flags: review?(tor)
(Assignee)

Comment 6

14 years ago
Created attachment 153335 [details] [diff] [review]
new diff -w

add back mImagePixmap check when aMaskPixels is false.
(Assignee)

Updated

14 years ago
Attachment #152669 - Attachment is obsolete: true
Attachment #152670 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #153335 - Flags: superreview?(tor)
Attachment #153335 - Flags: review?(tor)

Comment 7

14 years ago
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
Attachment #153335 - Flags: superreview?(tor)
Attachment #153335 - Flags: superreview+
Attachment #153335 - Flags: review?(tor)
Attachment #153335 - Flags: review+
(Assignee)

Comment 8

14 years ago
checked in.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Keywords: fixed-aviary1.0
Resolution: --- → FIXED
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))
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.