Stéphane Rollandin wrote:
> Hello,
>
> Working in Squeak 4.1, I stumbled upon a PNG image saving bug, which I 
> traced down to the fact that
>
> ((Color black pixelWordForDepth: 32) asColorOfDepth: 32)
>  = Color black
>
> is false.
>
> I found something looking like a fix, which is attached.
>
> Then I looked at Squeak 4.3 (update 11636) to see if someone else 
> already fixed that bug, and indeed Nicolas Cellier did, in December 
> 2010, by modifying Color>>pixelValueForDepth:
>
> But we still have
>
> ((Color black pixelWordForDepth: 32) asColorOfDepth: 32)
>  = Color black
>
> false, because black is being replaced with, as the comment says, the 
> "closest non-transparent black"
>
> At this point I guess I missed something, since I have no idea of what 
> this business of "transparent black" is about.
>
> Nicolas, could you please tell if my fix is correct ?
>
> Best,
>
>
> Stef

Hi Stef,

BitBlt supports Color transparent in depths < 32bpp, where no alpha 
channel is available. To do this, it considers any pixel with rgb = 0 as 
transparent and not black. So, the convention is to use the darkest 
possible blue instead of black.
See that the following draws nothing.
| f |
f := Form extent: 32 at 32 depth: 16.
f bits atAllPut: 16r00000000.
Display getCanvas image: f at: 10 at 10.    "This is the method for 
displaying a possibly translucent form in Cuis. Not sure about Squeak."

This "zero means transparent" trick is not needed in 32bpp, because real 
transparent is available (alpha=0), and especially because BitBlt 
considers transparent only pixels with _all_ the bits at zero. So, in 
32bpp, pixels with r=g=b=0 but alpha>0 are not considered transparent. 
There have been bugs historically with respect to this detail in the image.
See that this draws a black box
| f |
f := Form extent: 32 at 32 depth: 32.
f bits atAllPut: 16rFF000000.
Display getCanvas image: f at: 10 at 10    "This is the method for 
displaying a possibly translucent form in Cuis. Not sure about Squeak."
This shows that in the 32bpp case it is not needed to replace black by 
darkest blue, plain black can be used.

I think your fix is not correct, as Nicolas' code isn't either. It is 
true that both methods must match, but this special fake black is not 
needed for 32bpp.
After playing a bit with this today, I updated the Cuis code to reflect 
all this. Find it below. Thanks for bringing this up!

Cheers,
Juan Vuletich
--------------------------------------------
colorFromPixelValue: p depth: d
    "Convert a pixel value for the given display depth into a color."
    "Details: For depths of 8 or less, the pixel value is simply looked 
up in a table. For greater depths, the color components are extracted 
and converted into a color."
    "Warning: In BitBlt, a pixel with pixelValue = 0 is transparent.
    Squeak usually assumes that r=g=b=0 => transparent. But this is 
false if we have alpha (opacity).
    A color with r=g=b=0 and opacity = 255 is BLACK, not TRANSPARENT.
    Squeak also answers darkest possible blue when asked for black. 
Again, this is not needed in 32 bits (with alpha).
    The real rule is that pixelValue=0 means transparent.
    And that darkest blue must be used instead of black, but only for 
depths >8 and < 32 (no indexed colors, no alpha)
    This method is updated to reflect that."

    | r g b alpha |
    d = 8 ifTrue: [^ IndexedColors at: (p bitAnd: 16rFF) + 1].
    d = 4 ifTrue: [^ IndexedColors at: (p bitAnd: 16r0F) + 1].
    d = 2 ifTrue: [^ IndexedColors at: (p bitAnd: 16r03) + 1].
    d = 1 ifTrue: [^ IndexedColors at: (p bitAnd: 16r01) + 1].

    d = 32 ifTrue: [
        "eight bits per component; 8 bits of alpha"
        alpha := p bitShift: -24.
        alpha = 0 ifTrue: [ ^Color transparent ].
        r := (p bitShift: -16) bitAnd: 16rFF.
        g := (p bitShift: -8) bitAnd: 16rFF.
        b := p bitAnd: 16rFF.
        ^alpha < 255
            ifTrue: [ (Color r: r g: g b: b range: 255) alpha: alpha 
asFloat / 255.0 ]
            ifFalse: [ Color r: r g: g b: b range: 255 ]].

    "For the rest of the depths, pixelValue = 0 means transparent, and 
darkest blue is considered to be black."
    p = 0 ifTrue: [ ^Color transparent ].

    (d = 16) | (d = 15) ifTrue: [
        "five bits per component"
        r := (p bitShift: -10) bitAnd: 16r1F.
        g := (p bitShift: -5) bitAnd: 16r1F.
        b := p bitAnd: 16r1F.
        (r = 0 and: [ g = 0 and: [ b = 1]]) ifTrue: [
            ^Color black ].
        ^ Color r: r g: g b: b range: 31].

    d = 12 ifTrue: [
        "four bits per component"
        r := (p bitShift: -8) bitAnd: 16rF.
        g := (p bitShift: -4) bitAnd: 16rF.
        b := p bitAnd: 16rF.
        (r = 0 and: [ g = 0 and: [ b = 1]]) ifTrue: [
            ^Color black ].
        ^ Color r: r g: g b: b range: 15].

    d = 9 ifTrue: [
        "three bits per component"
        r := (p bitShift: -6) bitAnd: 16r7.
        g := (p bitShift: -3) bitAnd: 16r7.
        b := p bitAnd: 16r7.
        (r = 0 and: [ g = 0 and: [ b = 1]]) ifTrue: [
            ^Color black ].
        ^ Color r: r g: g b: b range: 7].

    self error: 'unknown pixel depth: ', d printString
---------------------
pixelValueForDepth: d
    "Returns an integer representing the bits that appear in a single 
pixel of this color in a Form of the given depth. The depth must be one 
of 1, 2, 4, 8, 16, or 32. Contrast with pixelWordForDepth: and 
bitPatternForDepth:, which return either a 32-bit word packed with the 
given pixel value or a multiple-word Bitmap containing a pattern. The 
inverse is the class message colorFromPixelValue:depth:"
    "Details: For depths of 8 or less, the result is a colorMap index. 
For depths of 16 and 32, it is a direct color value with 5 or 8 bits per 
color component."
    "Transparency: The pixel value zero is reserved for transparent. For 
depths greater than 8 and less than 32 (no Indexed colors, no real 
alpha), black maps to the darkest possible blue.
    Note that
        Color transparent class = TranslucentColor
    this special case is handled in TranslucentColor >> #pixelValueForDepth:
    "

    | bitBltFakeBlack val |
    d = 8 ifTrue: [^ self closestPixelValue8].  "common case"
    d < 8 ifTrue: [
        d = 4 ifTrue: [^ self closestPixelValue4].
        d = 2 ifTrue: [^ self closestPixelValue2].
        d = 1 ifTrue: [^ self closestPixelValue1]].

    d = 32 ifTrue: [
        "eight bits per component; top 8 bits set to all ones (opaque 
alpha)"
        val := LargePositiveInteger new: 4.
        val at: 3 put: ((rgb bitShift: -22) bitAnd: 16rFF).
        val at: 2 put: ((rgb bitShift: -12) bitAnd: 16rFF).
        val at: 1 put: ((rgb bitShift: -2) bitAnd: 16rFF).
        val at: 4 put: 16rFF.  "opaque alpha"
        ^ val normalize].

    "For the rest of the depths, pixelValue = 0 means transparent, and 
darkest blue is considered to be black."
    bitBltFakeBlack := 1.  "closest black that is not transparent in RGB 
- Not for depths <=8 (Indexed) or = 32 (RGBA)"

    d = 16 ifTrue: [
        "five bits per component; top bits ignored"
        val := (((rgb bitShift: -15) bitAnd: 16r7C00) bitOr:
             ((rgb bitShift: -10) bitAnd: 16r03E0)) bitOr:
             ((rgb bitShift: -5) bitAnd: 16r001F).
        ^ val = 0 ifTrue: [bitBltFakeBlack] ifFalse: [val]].

    d = 12 ifTrue: [  "for indexing a color map with 4 bits per color 
component"
        val := (((rgb bitShift: -18) bitAnd: 16r0F00) bitOr:
             ((rgb bitShift: -12) bitAnd: 16r00F0)) bitOr:
             ((rgb bitShift: -6) bitAnd: 16r000F).
        ^ val = 0 ifTrue: [bitBltFakeBlack] ifFalse: [val]].

    d = 9 ifTrue: [  "for indexing a color map with 3 bits per color 
component"
        val := (((rgb bitShift: -21) bitAnd: 16r01C0) bitOr:
             ((rgb bitShift: -14) bitAnd: 16r0038)) bitOr:
             ((rgb bitShift: -7) bitAnd: 16r0007).
        ^ val = 0 ifTrue: [bitBltFakeBlack] ifFalse: [val]].

    self error: 'unknown pixel depth: ', d printString