vikrambalye / dompdf

Automatically exported from code.google.com/p/dompdf
0 stars 0 forks source link

PNG alpha transparency check incorrect #304

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
The PNG transparency bit check in class.pdf.php seems to be incorrect. In this 
code, the header field is extracted and then bitwise-ANDed with 6. However, 
this causes any number which has the second and third bits set to match 
(relevant PNG image type codes are 2, 4 and 6; but only 4 and 6 have 
transparency).

This causes dompdf to follow the slow path (addImagePngAlpha() instead of the 
faster addImagePng()) for truecolor PNGs without alpha transparency (which have 
code 2, according to the spec: http://www.w3.org/TR/PNG/#11IHDR)

Simply ANDing with 4 will cause codes 4 and 6 to match, but nothing else. See 
the patch. This simple patch improved performance of 66% in my particular 
codebase. The transparent PNG tests still work.

Original issue reported on code.google.com by peter....@solide-ict.nl on 14 Jun 2011 at 2:12

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for that. We'll take a look and implement for the next release.

Original comment by eclecticgeek on 15 Jun 2011 at 4:57

GoogleCodeExporter commented 9 years ago
I fixed it in r404, to test if the image type is 3, 4 or 6, so the slow path 
will not be used when it is 2. This also fixes a few test cases (3 was not 
taken into account before), it's always good to read the specs ;) Thanks Peter

Original comment by fabien.menager on 19 Jun 2011 at 9:10

GoogleCodeExporter commented 9 years ago
I don't understand why the is_alpha check now includes 3. The spec says 3 means 
"indexed-colour", which means a palette (PLTE) chunk indicates the allowed 
colors, but the section on this PLTE chunk only mentions RGB but no alpha 
component.

I see a mention of an optional tRNS chunk which is allowed for color types 0, 2 
and 3 though. Perhaps the proper check is whether color type 4 or 6 is chosen 
OR a tRNS chunk is present. This is trick,y though. (but checking for 4, 6 or 3 
seems just wrong to me)

Original comment by peter....@solide-ict.nl on 20 Jun 2011 at 7:41

GoogleCodeExporter commented 9 years ago
Hello Peter, sorry for the very late reply, you are right, the check is still 
incorrect. I was misleaded by the tests in image_transparent_png that were 
better than before. 

The handling of PNG is very bad in CPDF, we need to improve it to be less 
complex and treat each case separatly (with/without palette, with/without 
alpha, etc).

Original comment by fabien.menager on 24 Jul 2011 at 2:16

GoogleCodeExporter commented 9 years ago

Original comment by eclecticgeek on 24 May 2013 at 3:00