> On 2023-02-26, at 3:21 PM, Vanessa Freudenberg <vanessa@codefrau.net> wrote:
>
> Can confirm that it's not a VM bug – in SqueakJS it works fine in 4.5, in 4.6 I see the same problem as in 6.0.
And in this evenings episode of the Hardy Drew series...
I thought I'd try saving the png fro ma non-squeak png reader (Preview in this case) to see what happens. Probably to no one's surprise the new version loads perfectly in Squeak. A *possibly* relevant factoid I noticed a moment ago during comparison debugging is that the problematic version is tagged as having 24 bits per pixel vs 32 for the good one.
Dang, no, the standard explains that. Color type 2 is indeed 24bpp, type 6 is 32bpp. Then again, it makes the difference between using #copyPixelsRGB: and #copyPixelsRGBA:
Right; it appears that #copyPixelsRGB: was modified in a way that causes this problem. The ' nice 5/10/2014 15:07' version made some fairly large changes to the prior 'nk 7/27/2004 17:18' implementation. A version with a couple of 'pixel normalize' replacing 'pixel' appears to work for all the examples I have to hand. The question remaining of course is what the change Nicolas made got wrong, because if we're honest, he rarely makes mistakes.
Hi Tim,hi all,
err no, I make plenty of mistakes, as the average human does.
It's just that I intercept most of them before they go to the trunk.
Using (LargePositiveInteger new: ) without normalization is suspicious.
On 32 bits squeak, it has 1 chance out of 4 to be a SmallInteger in disguise, but on 64 bits, that's 100%
So maybe we did not notice the bug before, or maybe it was an optimization at Integer side assuming some invariants (for example LargePositiveInteger isZero answer false or something...)
I'll have a look.
My hack working version -
copyPixelsRGB: y at: startX by: incX
"Handle interlaced RGB color mode (colorType = 2)"
| i pixel tempForm tempBits xx loopsToDo |
tempForm := Form extent: width@1 depth: 32.
tempBits := tempForm bits.
pixel := LargePositiveInteger new: 4.
pixel at: 4 put: 16rFF.
loopsToDo := width - startX + incX - 1 // incX.
bitsPerChannel = 8 ifTrue: [
i := (startX // incX * 3) + 1.
xx := startX+1.
1 to: loopsToDo do: [ :j |
pixel
at: 3 put: (thisScanline at: i);
at: 2 put: (thisScanline at: i+1);
at: 1 put: (thisScanline at: i+2).
tempBits at: xx put: pixel normalize.
i := i + 3.
xx := xx + incX.
]
] ifFalse: [
i := (startX // incX * 6) + 1.
xx := startX+1.
1 to: loopsToDo do: [ :j |
pixel
at: 3 put: (thisScanline at: i);
at: 2 put: (thisScanline at: i+2);
at: 1 put: (thisScanline at: i+4).
tempBits at: xx put: pixel normalize.
i := i + 6.
xx := xx + incX.
].
].
transparentPixelValue ifNotNil: [
startX to: width-1 by: incX do: [ :x |
(tempBits at: x+1) = transparentPixelValue ifTrue: [
tempBits at: x+1 put: 0.
].
].
].
tempForm displayOn: form at: 0@y rule: Form paint.
Yawn - time for bed.