I've done some more testing and it turns out that this approach will not work as it is currently implemented. When the image is converted from an ARGB to a RGB image, the transparent pixels will be converted to black pixels and then when the JPEG compression occurs, those black pixels can blend with some of the other pixels. The generated mask is not able to compensate for this and, consequently, some black artifacts can appear along the edge of the image where the transition occurs from opaque pixels to transparent pixels.
I'll keep working on this and see if I can come up with a better solution. In the meantime, please do not expend the effort to evaluate incorporating my proposed feature into iText at this time. Steve On Dec 30, 2008, at 2:43 PM, Steven Case wrote: > In response to your six comments/questions: > > 1. I did not make the private method generic enough to work for any > image. It works for those images that are being processed by the > drawImage() method that might call CreateAlphaMask(). As such, at > that point in the process the getPixels() method is guaranteed to > return an int[]. If not, the ClassCastException would have been > thrown when the call to getInstance() had occurred. So, basically, > I handled it consistent with how it appeared to me the rest of the > code was dealing with getPixels(). > > 2. Similar to the previous question, the images have already been > put into standard RGB format so the alpha channel is in the most > significant byte of the int. > > 3. Agreed. I hadn't really paid any attention since my IDE takes > care of such spacing. > > 4. Agreed. The fully qualified names are there simply from doing a > cut and paste from other code within iText. > > 5. Agreed. I wasn't particularly concerned with how the exception > is record or if it is recorded. This is just a standard exception > handler that my IDE will generate. If the code is adopted then > there's no reason not to remove the logger call. > > 6. Since there is no javadoc in the rest of the code, I opted not > to generate any. > > Steve > > > On Dec 30, 2008, at 12:50 PM, Mark Storer wrote: > >> Lemme put my "code review" hat on... there we go. I hope this >> doesn't come >> across as harsh, but it *is* criticism (and a couple questions). >> That's what >> code reviews are for, and the goal is to produce better code (and >> hopefully a >> better coder), not to attack anyone, poke fun at them, disparage >> their >> parenting or personal hygiene, nor to accuse them of molesting farm >> animals. >> >> All that stuff is just a bonus. ;) >> >> 1 & 2 are potential bugs, the rest aren't that big a deal. >> >> 1) How do you know when pixelGrabber.getPixels() will return int[] >> vs byte[]? >> If it returns byte[], you're looking at a ClassCastException. I've >> never used >> a pixel grabber, so your code could be perfectly kosher, I just >> thought it was >> worth asking. >> >> 2) How do you know the order of 'colorants' in the integer? Your >> code assumes >> it to be in the high 8 bits of the int. Is this guaranteed? >> >> 3) createAlphaMask contains a mix of tabs and spaces. No Bueno. >> Using nothing >> but tabs is tolerable, but different editors *will* use different >> widths for >> tabs. Spaces will always be displayed consistently. Spaces good. >> Tabs bad. >> ;) >> >> 4) I'm surprised you needed to fully qualify >> java.awt.image.PixelGrabber and >> java.awt.image.ImageObserver.ABORT. ImageObserver in particular is >> specifically >> imported by name. >> >> 5) The use of Logger. createAlphaMask is the only place it's used >> inside this >> particular file. I checked several Important Classes (PdfReader, >> PdfWriter, >> PdfContentByte...) and didn't see it used anywhere. My private >> branch of >> paulo155 doesn't use it at all. It's not much use if no one knows >> to look for >> it. >> >> Which segues nicely into... >> >> 6) No javadoc. Neither do any of the other private methods in the >> 'implementation' section, but it could be used to clue people into >> the whole >> logger thing, removing at least part of my objection to its use. >> >> >> --Mark Storer >> PDF Guy >> Cardiff.com >> >> #include <disclaimer> >> typedef std::disclaimer<Cardiff> DisCard; >> >> >> ------------------------------------------------------------------------------ >> _______________________________________________ >> iText-questions mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/itext-questions >> >> Buy the iText book: http://www.1t3xt.com/docs/book.php >> > ------------------------------------------------------------------------------ _______________________________________________ iText-questions mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/itext-questions Buy the iText book: http://www.1t3xt.com/docs/book.php
