[jira] [Comment Edited] (FOP-2847) [PATCH] Support palette-based transparency via tRNS chunk of PNG images in PDF export

2019-03-27 Thread simon steiner (JIRA)


[ 
https://issues.apache.org/jira/browse/FOP-2847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16802755#comment-16802755
 ] 

simon steiner edited comment on FOP-2847 at 3/27/19 1:09 PM:
-

Can you submit 1 patch or 1 PR with 1 commit


was (Author: ssteiner1):
Next time submit 1 patch or 1 PR with 1 commit

> [PATCH] Support palette-based transparency via tRNS chunk of PNG images in 
> PDF export
> -
>
> Key: FOP-2847
> URL: https://issues.apache.org/jira/browse/FOP-2847
> Project: FOP
>  Issue Type: Bug
>  Components: renderer/pdf
>Affects Versions: 2.3
>Reporter: Tobias Hain
>Priority: Major
> Attachments: acrobat_reader_error_message.PNG, 
> broken_generated_pdf.pdf, pal_bk.png, pdf_trans_broken.png, 
> sample_image_palette_tRNS.png
>
>
> h4. TL;NR
> When using palette-based transparency via tRNS chunk in PNG images, the 
> generated PDFs are broken.
> h4. palette-based with transparency.
> {quote}*8.5.2. Palette-Based with Transparency*
> "The PNG spec forbids the use of a full alpha channel with palette-based 
> images, but it does allow `cheap alpha` via the transparency chunk, tRNS 
> [...] In effect, it transforms the palette from an RGB lookup table to an 
> RGBA table [...] a single PNG transparency entry should come first"
> {quote}
> [http://www.libpng.org/pub/png/book/chapter08.html#png.ch08.div.5.2]
> h4. general PDF encoding strategy
> Option 1:
> {quote}most libraries just undo the palettization and use a full color image 
> + mask.
> {quote}
> [https://forums.adobe.com/message/6148131#6148131]
> Option 2:
> {quote}SMask would be simpler since you could leave the palette in place
> {quote}
> [https://forums.adobe.com/message/6148557#6148557]
> h4. what does FOP do
> It creates a chroma-key mask:
> {code:java}
>  /Type /XObject
>   /ColorSpace [/Indexed /DeviceRGB 255 ]
>   /Mask [182 182 151 151 158 158]
> {code}
> Acrobat Reader even fails to read those files while PDF viewers embedded into 
> Chrome/Firefox read the file, but show visual glitches:
> !acrobat_reader_error_message.PNG!
> https://github.com/apache/fop/pull/49 fixes the Acrobat Reader error.
> FOP doesn't implement Option 1 yet. None of the ImageLoaders is prepared to 
> do that:
>  [https://wiki.apache.org/xmlgraphics-fop/HowTo/ImageLoaderRawPNG]
>  In fact the following results in missing Option 2:
> {code:java}
> public void setup(PDFDocument doc) {
>   RenderedImage ri = getImage().getRenderedImage();
>   // code deleted
>   Raster raster = GraphicsUtil.getAlphaRaster(ri);
>   if (raster != null) {
> AlphaRasterImage alphaImage = new AlphaRasterImage("Mask:" + getKey(), 
> raster);
> this.softMask = doc.addImage(null, alphaImage).makeReference();
>   }
> {code}
> [https://github.com/apache/fop/blob/ff452f3685a02715aee791d651b58dd1797ddfd4/fop-core/src/main/java/org/apache/fop/render/pdf/ImageRenderedAdapter.java]
> In the above code raster will be null and therefore FOP fails to initialize a 
> softMask. {{GraphicsUtil.getAlphaRaster()}} calls internally 
> {{ColorModel.getAlphaRaster()}}, which returns null:
> {quote}If this is an IndexColorModel which has alpha in the lookup table, 
> this method will return null since there is no spatially discrete alpha 
> channel.
> {quote}
> [https://docs.oracle.com/javase/8/docs/api/index.html?java/awt/image/ColorModel.html]
> The softMask Issue is fixed by:
> https://github.com/apache/fop/pull/50
> As a result an image such as !pal_bk.png! is rendered with a transparency 
> gradient instead of !pdf_trans_broken.png!
> This code will then add the chroma key mask to the image:
> {code:java}
> protected void populateXObjectDictionaryForIndexColorModel(PDFDictionary 
> dict, IndexColorModel icm) {
>   // code deleted
>   Integer index = getIndexOfFirstTransparentColorInPalette(icm);
>   if (index != null) {
> PDFArray mask = new PDFArray(dict);
> mask.add(index);
> mask.add(index);
> dict.put("Mask", mask);
>   }
> }
> {code}
> [https://github.com/apache/fop/blob/ff452f3685a02715aee791d651b58dd1797ddfd4/fop-core/src/main/java/org/apache/fop/render/pdf/AbstractImageAdapter.java]
> h4. why care to support?
> Libraries such as pngquant optimize images for web usage and low footprint 
> and can not be used directly with Apache FOP as source images:
> {quote}"*pngquant*: Lossy PNG optimizer, reducing a PNG image down to an 8 
> bit color palette with dithering. It will build indexed-color PNG's with 
> alpha transparency colors conveyed in the tRNS chunk."
> {quote}
> [https://imagemagick.org/Usage/formats/#png_non-im]
> h4. sample data
> {code}
> $ identify -verbose sample_image_palette_tRNS.png
>   Format: PNG (Portable Network Graphics)
>   Type: PaletteAlpha
>   png:IHDR.color_type  : 3 

[jira] [Comment Edited] (FOP-2847) [PATCH] Support palette-based transparency via tRNS chunk of PNG images in PDF export

2019-03-24 Thread Tobias Hain (JIRA)


[ 
https://issues.apache.org/jira/browse/FOP-2847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16794974#comment-16794974
 ] 

Tobias Hain edited comment on FOP-2847 at 3/24/19 8:29 PM:
---

{quote}Can you add a unit test{quote}
[~ssteiner1] Added unit tests, please review and pull. Three different pull 
requests, since issues are actually unrelated.
{code:java}
https://github.com/apache/fop/pull/50
+ FOP-2847: fix PNGs with index color model did not render SMask

https://github.com/apache/fop/pull/49
+ FOP-2847: fix Mask for PNG with IndexColorModel and transparent color
+ depends on pull#50 since it shares some mock data. Due to rebase pull/49 
incorporates pull/50 - so just pull this one.

https://github.com/apache/fop/pull/51
+ FOP-2847: optimize memory footprint when rendering images with IndexC…
+ trivial without side effects: no unit test required
{code}

The tests are written such that the same test is performed with the same image, 
but different colorspace models. Some of the tests will pass FOP trunk to also 
demonstrate the expected behavior (e.g. PNGs with ARGB will work fine already 
on FOP trunk). The pull request fixes this for other colorspace models.


was (Author: 7oby):
{quote}Can you add a unit test{quote}
[~ssteiner1] Added unit tests, please review and pull. Three different pull 
requests, since issues are actually unrelated.
{code:java}
https://github.com/apache/fop/pull/50
+ FOP-2847: fix PNGs with index color model did not render SMask

https://github.com/apache/fop/pull/49
+ FOP-2847: fix Mask for PNG with IndexColorModel and transparent color
+ depends on pull#50 since it shares some mock data

https://github.com/apache/fop/pull/51
+ FOP-2847: optimize memory footprint when rendering images with IndexC…
+ trivial without side effects: no unit test required
{code}

> [PATCH] Support palette-based transparency via tRNS chunk of PNG images in 
> PDF export
> -
>
> Key: FOP-2847
> URL: https://issues.apache.org/jira/browse/FOP-2847
> Project: FOP
>  Issue Type: Bug
>  Components: renderer/pdf
>Affects Versions: 2.3
>Reporter: Tobias Hain
>Priority: Major
> Attachments: acrobat_reader_error_message.PNG, 
> broken_generated_pdf.pdf, pal_bk.png, pdf_trans_broken.png, 
> sample_image_palette_tRNS.png
>
>
> h4. TL;NR
> When using palette-based transparency via tRNS chunk in PNG images, the 
> generated PDFs are broken.
> h4. palette-based with transparency.
> {quote}*8.5.2. Palette-Based with Transparency*
> "The PNG spec forbids the use of a full alpha channel with palette-based 
> images, but it does allow `cheap alpha` via the transparency chunk, tRNS 
> [...] In effect, it transforms the palette from an RGB lookup table to an 
> RGBA table [...] a single PNG transparency entry should come first"
> {quote}
> [http://www.libpng.org/pub/png/book/chapter08.html#png.ch08.div.5.2]
> h4. general PDF encoding strategy
> Option 1:
> {quote}most libraries just undo the palettization and use a full color image 
> + mask.
> {quote}
> [https://forums.adobe.com/message/6148131#6148131]
> Option 2:
> {quote}SMask would be simpler since you could leave the palette in place
> {quote}
> [https://forums.adobe.com/message/6148557#6148557]
> h4. what does FOP do
> It creates a chroma-key mask:
> {code:java}
>  /Type /XObject
>   /ColorSpace [/Indexed /DeviceRGB 255 ]
>   /Mask [182 182 151 151 158 158]
> {code}
> Acrobat Reader even fails to read those files while PDF viewers embedded into 
> Chrome/Firefox read the file, but show visual glitches:
> !acrobat_reader_error_message.PNG!
> https://github.com/apache/fop/pull/49 fixes the Acrobat Reader error.
> FOP doesn't implement Option 1 yet. None of the ImageLoaders is prepared to 
> do that:
>  [https://wiki.apache.org/xmlgraphics-fop/HowTo/ImageLoaderRawPNG]
>  In fact the following results in missing Option 2:
> {code:java}
> public void setup(PDFDocument doc) {
>   RenderedImage ri = getImage().getRenderedImage();
>   // code deleted
>   Raster raster = GraphicsUtil.getAlphaRaster(ri);
>   if (raster != null) {
> AlphaRasterImage alphaImage = new AlphaRasterImage("Mask:" + getKey(), 
> raster);
> this.softMask = doc.addImage(null, alphaImage).makeReference();
>   }
> {code}
> [https://github.com/apache/fop/blob/ff452f3685a02715aee791d651b58dd1797ddfd4/fop-core/src/main/java/org/apache/fop/render/pdf/ImageRenderedAdapter.java]
> In the above code raster will be null and therefore FOP fails to initialize a 
> softMask. {{GraphicsUtil.getAlphaRaster()}} calls internally 
> {{ColorModel.getAlphaRaster()}}, which returns null:
> {quote}If this is an IndexColorModel which has alpha in the lookup table, 
> this method will return null since there is no spatially discrete alpha 
> 

[jira] [Comment Edited] (FOP-2847) [PATCH] Support palette-based transparency via tRNS chunk of PNG images in PDF export

2019-03-23 Thread Tobias Hain (JIRA)


[ 
https://issues.apache.org/jira/browse/FOP-2847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16794974#comment-16794974
 ] 

Tobias Hain edited comment on FOP-2847 at 3/23/19 11:08 PM:


{quote}Can you add a unit test{quote}
[~ssteiner1] Added unit tests, please review and pull. Three different pull 
requests, since issues are actually unrelated.
{code:java}
https://github.com/apache/fop/pull/50
+ FOP-2847: fix PNGs with index color model did not render SMask

https://github.com/apache/fop/pull/49
+ FOP-2847: fix Mask for PNG with IndexColorModel and transparent color
+ depends on pull#50 since it shares some mock data

https://github.com/apache/fop/pull/51
+ FOP-2847: optimize memory footprint when rendering images with IndexC…
+ trivial without side effects: no unit test required
{code}


was (Author: 7oby):
{quote}Can you add a unit test\{quote}

[~ssteiner1]: Yes, just give me a couple of days to find a time slot to do so.

> [PATCH] Support palette-based transparency via tRNS chunk of PNG images in 
> PDF export
> -
>
> Key: FOP-2847
> URL: https://issues.apache.org/jira/browse/FOP-2847
> Project: FOP
>  Issue Type: Bug
>  Components: renderer/pdf
>Affects Versions: 2.3
>Reporter: Tobias Hain
>Priority: Major
> Attachments: acrobat_reader_error_message.PNG, 
> broken_generated_pdf.pdf, pal_bk.png, pdf_trans_broken.png, 
> sample_image_palette_tRNS.png
>
>
> h4. TL;NR
> When using palette-based transparency via tRNS chunk in PNG images, the 
> generated PDFs are broken.
> h4. palette-based with transparency.
> {quote}*8.5.2. Palette-Based with Transparency*
> "The PNG spec forbids the use of a full alpha channel with palette-based 
> images, but it does allow `cheap alpha` via the transparency chunk, tRNS 
> [...] In effect, it transforms the palette from an RGB lookup table to an 
> RGBA table [...] a single PNG transparency entry should come first"
> {quote}
> [http://www.libpng.org/pub/png/book/chapter08.html#png.ch08.div.5.2]
> h4. general PDF encoding strategy
> Option 1:
> {quote}most libraries just undo the palettization and use a full color image 
> + mask.
> {quote}
> [https://forums.adobe.com/message/6148131#6148131]
> Option 2:
> {quote}SMask would be simpler since you could leave the palette in place
> {quote}
> [https://forums.adobe.com/message/6148557#6148557]
> h4. what does FOP do
> It creates a chroma-key mask:
> {code:java}
>  /Type /XObject
>   /ColorSpace [/Indexed /DeviceRGB 255 ]
>   /Mask [182 182 151 151 158 158]
> {code}
> Acrobat Reader even fails to read those files while PDF viewers embedded into 
> Chrome/Firefox read the file, but show visual glitches:
> !acrobat_reader_error_message.PNG!
> https://github.com/apache/fop/pull/49 fixes the Acrobat Reader error.
> FOP doesn't implement Option 1 yet. None of the ImageLoaders is prepared to 
> do that:
>  [https://wiki.apache.org/xmlgraphics-fop/HowTo/ImageLoaderRawPNG]
>  In fact the following results in missing Option 2:
> {code:java}
> public void setup(PDFDocument doc) {
>   RenderedImage ri = getImage().getRenderedImage();
>   // code deleted
>   Raster raster = GraphicsUtil.getAlphaRaster(ri);
>   if (raster != null) {
> AlphaRasterImage alphaImage = new AlphaRasterImage("Mask:" + getKey(), 
> raster);
> this.softMask = doc.addImage(null, alphaImage).makeReference();
>   }
> {code}
> [https://github.com/apache/fop/blob/ff452f3685a02715aee791d651b58dd1797ddfd4/fop-core/src/main/java/org/apache/fop/render/pdf/ImageRenderedAdapter.java]
> In the above code raster will be null and therefore FOP fails to initialize a 
> softMask. {{GraphicsUtil.getAlphaRaster()}} calls internally 
> {{ColorModel.getAlphaRaster()}}, which returns null:
> {quote}If this is an IndexColorModel which has alpha in the lookup table, 
> this method will return null since there is no spatially discrete alpha 
> channel.
> {quote}
> [https://docs.oracle.com/javase/8/docs/api/index.html?java/awt/image/ColorModel.html]
> The softMask Issue is fixed by:
> https://github.com/apache/fop/pull/50
> As a result an image such as !pal_bk.png! is rendered with a transparency 
> gradient instead of !pdf_trans_broken.png!
> This code will then add the chroma key mask to the image:
> {code:java}
> protected void populateXObjectDictionaryForIndexColorModel(PDFDictionary 
> dict, IndexColorModel icm) {
>   // code deleted
>   Integer index = getIndexOfFirstTransparentColorInPalette(icm);
>   if (index != null) {
> PDFArray mask = new PDFArray(dict);
> mask.add(index);
> mask.add(index);
> dict.put("Mask", mask);
>   }
> }
> {code}
>