[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
|