[squeak-dev] PNGReadWriter bugletNicolas Cellier nicolas.cellier.aka.nice at gmail.comMon Feb 27 11:09:50 UTC 2023
Ah, found it, obviously, this is usage of Form over rather than Form paint: it cannot work in interlaced... We need to find the right rule that will also work with transparency... Le lun. 27 févr. 2023 à 11:05, Nicolas Cellier < nicolas.cellier.aka.nice at gmail.com> a écrit : > > > Le lun. 27 févr. 2023 à 09:29, Nicolas Cellier < > nicolas.cellier.aka.nice at gmail.com> a écrit : > >> >> >> 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. >>> >>> > Ah OK, since opaque has leading 16rFF byte, normalize was not necessary in > 32bits era. > Dave introduced the normalize fix later for 64bits: > > https://source.squeak.org/trunk/Graphics-dtl.377.diff > > >>> >>>> 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 >> >> In a few words : > a tRNS chunk contains 3 16bits R-G-B pixel values that must be handled as > fully transparent > But previously, we did not decode those properly (see > processTransparencyChunk) ! > > And we did test for transparency **AFTER** converting the pixel to our > internal 8bit channel, > which may convert many more pixels to transparent than specified by the > transparency chunk... > > http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html clearly states > that we should tests for transparency first: > > Note: when dealing with 16-bit grayscale or truecolor data, it is > important to compare both bytes of the sample values to determine whether a > pixel is transparent. Although decoders may drop the low-order byte of the > samples for display, this must not occur until after the data has been > tested for transparency. For example, if the grayscale level 0x0001 is > specified to be transparent, it would be incorrect to compare only the > high-order byte and decide that 0x0002 is also transparent. > > > >>>> 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/20dd00c0/attachment.html>
More information about the Squeak-dev mailing list |