283091 - LockImagePixels doesn't regenerate alpha bits

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Mac, defect)

Description

[jhp (no longer active)]

The new Quartz code for nsImageMac doesn't regenerate mAlphaBits, and probably
regenerates mImageBits incorrectly.  This code path isn't normally hit, so it
wasn't a problem.  Luckily, Tim does make use of this for his SVG code, so he
can test this for me.  Patch coming up.

Comment 1

[jhp (no longer active)]

Attachment: patch

This patch makes it so LockImagePixels() properly regenerates mAlphaBits and
mImageBits, when necessary.  Also cleans up the ImageUpdated() function, which
closely mirrors LockImagePixels().  Lastly, decided to get rid of
PERF_PREMULT_ALPHA define and make it the norm, since there is no reason to
support the non-premultiplied alpha case.

Tim says he do some more testing on this patch, but in the meantime, have a
gander.

Comment 2

[tor]

Seems to work in my testing.

Comment 3

[Simon Fraser [no longer active]]

Comment on attachment 175082 [details] [diff] [review]
patch

Is this expected to have any impact on browser performance for most pages?

Comment 4

[jhp (no longer active)]

Actually, I removed a loop for images with no alpha info when creating the
CGImage, so it may actually speed things up a little.  The LockImagePixels()
function gets called rarely; that's why tor had to test it with his SVG code.

Comment 5

[tor]

Comment on attachment 175082 [details] [diff] [review]
patch

You've got a number of unchecked PR_Malloc()s that should be handled.

Comment 6

[jhp (no longer active)]

Attachment: patch v1.1

Now with malloc checks.  Also cleaned up the DrawTile code; no reason to run
SlowTile if DrawTile fails.

Comment 7

[tor]

Comment on attachment 175351 [details] [diff] [review]
patch v1.1

sr=tor if you fix the LockImagePixels leak.

Comment 8

[jhp (no longer active)]

Checked in. ->FIXED