[squeak-dev] PNGReadWriter bugletNicolas Cellier nicolas.cellier.aka.nice at gmail.comMon Feb 27 14:12:34 UTC 2023
Hi Karl, Le lun. 27 févr. 2023 à 13:11, karl ramberg <karlramberg at gmail.com> a écrit : > In these methods: > > copyPixelsRGB: > copyPixelsRGB: at: by: > copyPixelsRGBA: > copyPixelsRGBA: at: by: > > Change to use rule 34 or 44 seems to work with both opaque and transparent > PNGs > Good, but they are not strictly the same, this would deserve some tests... > ... > tempForm displayOn: form at: 0 at y rule: 34. > ... > > Best, > Karl > > Note that we can continue using Form over in non interlaced copyPixelsRGB: and copyPixelsRGBA: I also notice that copyPixelsRGBA:at:by: uses paintAlpha rule (31) which is weird since this rule is using a constant alpha I think that I tested transparency back in 2014 with http://www.schaik.com/pngsuite/pngsuite_trn_png.html Unfortunately, these contain no example of interlaced mixed with transparency... Nicolas > On Mon, Feb 27, 2023 at 12:10 PM Nicolas Cellier < > nicolas.cellier.aka.nice at gmail.com> wrote: > >> 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/872f3834/attachment.html>
More information about the Squeak-dev mailing list |