<div dir="ltr">Hi Karl,<br><div><div><br></div><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le lun. 27 févr. 2023 à 13:11, karl ramberg <<a href="mailto:karlramberg@gmail.com">karlramberg@gmail.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>In these methods:</div><div><br></div><div>copyPixelsRGB: <br></div><div>copyPixelsRGB: at: by: <br></div><div>copyPixelsRGBA:</div><div>copyPixelsRGBA: at: by: <br></div><div><br></div><div>Change to use rule 34 or 44 seems to work with both opaque and transparent PNGs<br></div></div></blockquote><div><br></div><div>Good, but they are not strictly the same, this would deserve some tests... <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div>...<br></div><div>tempForm displayOn: form at: 0@y rule: 34.</div><div>...</div><div><br></div><div>Best,</div><div>Karl<br></div></div><br></blockquote><div><br></div><div>
<div>Note that we can continue using Form over in non interlaced
copyPixelsRGB: and
copyPixelsRGBA: <br></div><div><br></div><div>I also notice that
copyPixelsRGBA:at:by: uses paintAlpha rule (31) which is weird</div><div>since this rule is using a constant alpha<br></div><div dir="ltr"><br></div><div>I think that I tested transparency back in 2014 with <a href="http://www.schaik.com/pngsuite/pngsuite_trn_png.html">http://www.schaik.com/pngsuite/pngsuite_trn_png.html</a></div><div>Unfortunately, these contain no example of interlaced mixed with transparency...</div><div><br></div><div>Nicolas<br></div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 27, 2023 at 12:10 PM Nicolas Cellier <<a href="mailto:nicolas.cellier.aka.nice@gmail.com" target="_blank">nicolas.cellier.aka.nice@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Ah, found it, obviously, this is usage of Form over rather than Form paint: it cannot work in interlaced...</div><div>We need to find the right rule that will also work with transparency...<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le lun. 27 févr. 2023 à 11:05, Nicolas Cellier <<a href="mailto:nicolas.cellier.aka.nice@gmail.com" target="_blank">nicolas.cellier.aka.nice@gmail.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le lun. 27 févr. 2023 à 09:29, Nicolas Cellier <<a href="mailto:nicolas.cellier.aka.nice@gmail.com" target="_blank">nicolas.cellier.aka.nice@gmail.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le lun. 27 févr. 2023 à 09:19, Nicolas Cellier <<a href="mailto:nicolas.cellier.aka.nice@gmail.com" target="_blank">nicolas.cellier.aka.nice@gmail.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le lun. 27 févr. 2023 à 07:15, tim Rowledge <<a href="mailto:tim@rowledge.org" target="_blank">tim@rowledge.org</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
> On 2023-02-26, at 3:21 PM, Vanessa Freudenberg <<a href="mailto:vanessa@codefrau.net" target="_blank">vanessa@codefrau.net</a>> wrote:<br>
> <br>
> 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.<br>
<br>
And in this evenings episode of the Hardy Drew series...<br>
<br>
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.<br>
<br>
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:<br>
<br>
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.<br>
<br></blockquote><div>Hi Tim,hi all,</div><div>err no, I make plenty of mistakes, as the average human does.</div><div>It's just that I intercept most of them before they go to the trunk.</div><div>Using (LargePositiveInteger new: ) without normalization is suspicious.<br></div><div>On 32 bits squeak, it has 1 chance out of 4 to be a SmallInteger in disguise, but on 64 bits, that's 100%<br></div><div>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...)<br></div><div>I'll have a look.</div><div><br></div></div></div></blockquote></div></div></blockquote><div><br></div><div>Ah OK, since opaque has leading 16rFF byte, normalize was not necessary in 32bits era.</div><div>Dave introduced the normalize fix later for 64bits:</div><div><br></div><div><a href="https://source.squeak.org/trunk/Graphics-dtl.377.diff" target="_blank">https://source.squeak.org/trunk/Graphics-dtl.377.diff</a></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
My hack working version - <br>
<br>
copyPixelsRGB: y at: startX by: incX<br>
"Handle interlaced RGB color mode (colorType = 2)"<br>
<br>
| i pixel tempForm tempBits xx loopsToDo |<br>
<br>
tempForm := Form extent: width@1 depth: 32.<br>
tempBits := tempForm bits.<br>
pixel := LargePositiveInteger new: 4.<br>
pixel at: 4 put: 16rFF.<br>
loopsToDo := width - startX + incX - 1 // incX.<br>
bitsPerChannel = 8 ifTrue: [<br>
i := (startX // incX * 3) + 1.<br>
xx := startX+1.<br>
1 to: loopsToDo do: [ :j |<br>
pixel<br>
at: 3 put: (thisScanline at: i);<br>
at: 2 put: (thisScanline at: i+1);<br>
at: 1 put: (thisScanline at: i+2).<br>
tempBits at: xx put: pixel normalize.<br>
i := i + 3.<br>
xx := xx + incX.<br>
]<br>
] ifFalse: [<br>
i := (startX // incX * 6) + 1.<br>
xx := startX+1.<br>
1 to: loopsToDo do: [ :j |<br>
pixel<br>
at: 3 put: (thisScanline at: i);<br>
at: 2 put: (thisScanline at: i+2);<br>
at: 1 put: (thisScanline at: i+4).<br>
tempBits at: xx put: pixel normalize.<br>
i := i + 6.<br>
xx := xx + incX.<br>
].<br>
].<br>
transparentPixelValue ifNotNil: [<br>
startX to: width-1 by: incX do: [ :x |<br>
(tempBits at: x+1) = transparentPixelValue ifTrue: [<br>
tempBits at: x+1 put: 0.<br>
].<br>
].<br>
].<br>
tempForm displayOn: form at: 0@y rule: Form paint.<br>
<br>
Yawn - time for bed.<br>
<br></blockquote></div></div></blockquote><div>I changed this method here:</div><div><br></div><div><a href="https://source.squeak.org/trunk/Graphics-nice.292.diff" target="_blank">https://source.squeak.org/trunk/Graphics-nice.292.diff</a></div><div> <br></div></div></div></blockquote>In a few words :<br></div><div class="gmail_quote">a tRNS chunk contains 3 16bits R-G-B pixel values that must be handled as fully transparent</div><div class="gmail_quote">But previously, we did not decode those properly
(see processTransparencyChunk)
!</div><div class="gmail_quote"><br></div><div class="gmail_quote"> And we did test for transparency **AFTER** converting the pixel to our internal 8bit channel,</div><div class="gmail_quote">which may convert many more pixels to transparent than specified by the transparency chunk...</div><div class="gmail_quote"><br></div><div class="gmail_quote"><a href="http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html" target="_blank">http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html</a> clearly states that we should tests for transparency first:<br><br>
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. <br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
tim<br>
--<br>
tim Rowledge; <a href="mailto:tim@rowledge.org" target="_blank">tim@rowledge.org</a>; <a href="http://www.rowledge.org/tim" rel="noreferrer" target="_blank">http://www.rowledge.org/tim</a><br>
Time to start the War on Errorism before stupidity finally gets us.<br>
<br>
<br>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div>
<br>
</blockquote></div>
<br>
</blockquote></div></div></div></div>