[squeak-dev] PNGReadWriter buglet

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Mon Feb 27 08:29:28 UTC 2023


Le lun. 27 févr. 2023 à 09:19, Nicolas Cellier <
nicolas.cellier.aka.nice at gmail.com> a écrit :

>
>
> Le lun. 27 févr. 2023 à 07:15, tim Rowledge <tim at rowledge.org> a écrit :
>
>>
>>
>> > On 2023-02-26, at 3:21 PM, Vanessa Freudenberg <vanessa at 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 at 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 at y rule: Form paint.
>>
>> Yawn - time for bed.
>>
>> I changed this method here:

https://source.squeak.org/trunk/Graphics-nice.292.diff


>> tim
>> --
>> tim Rowledge; tim at rowledge.org; http://www.rowledge.org/tim
>> Time to start the War on Errorism before stupidity finally gets us.
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20230227/b0367412/attachment.html>


More information about the Squeak-dev mailing list