[squeak-dev] PNGReadWriter buglet

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


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.
>
>
> 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/9ee4df28/attachment.html>


More information about the Squeak-dev mailing list