PhilterPaper / Perl-PDF-Builder

Extended version of the popular PDF::API2 Perl-based PDF library for creating, reading, and modifying PDF documents
https://www.catskilltech.com/FreeSW/product/PDF%2DBuilder/title/PDF%3A%3ABuilder/freeSW_full
Other
6 stars 7 forks source link

[RT 118047] Adding 8bpp TIFF results in broken image #42

Closed PhilterPaper closed 6 years ago

PhilterPaper commented 7 years ago

Fri Sep 23 08:08:36 2016 ppisar [...] redhat.com - Ticket created (ssimms/pdfapi2#30) Subject: Adding 8bpp TIFF results in broken image

The following code creates a one-page PDF file and adds a TIFF image into the page:

#!/usr/bin/perl
use strict;
use warnings;
use PDF::Builder;

my $pdf = PDF::Builder->new(-file => 'out.pdf');
my $page = $pdf->page;
$page->mediabox(157, 196);

my $imgobj = $pdf->image_tiff('8.tiff');

my $gfx = $page->gfx;
$gfx->image($imgobj, 0, 0, 157, 196);

$pdf->save;
$pdf->end;

The problem is the resulting PDF renders the image broken if the TIFF image has 8 bit per pixel. It works fine if the image has only 1 bit per pixel. I attached the the two TIFF images and the resulting broken PDF file.

118047.zip out.pdf

# Fri Oct 07 14:49:34 2016 Jeffrey.Ratcliffe [...] gmail.com - Correspondence added

It's another lzw problem. 8.tiff is lzw compressed. If I uncompress it, (tiffcp -c none) PDF::Builder has no problem. # Fri Oct 07 14:49:35 2016 The RT System itself - Status changed from 'new' to 'open'

PhilterPaper commented 7 years ago

Fri Jul 14 11:44:50 2017 jffry [...] posteo.net - Correspondence added

The attached patch fixed this for me, albeit by adding the dependency Graphics::TIFF, which uses libtiff to do the decompression. If you were happy with this approach, RT 84665 [#2] could be fixed in the same way. I was unable to come up with a G3-encoded TIFF that the current version of PDF::Builder accepts. If you can find a way of generating one, then I can update the tests. Alternatively, I can simply implement G3 and G4 support without worrying about the old approach.

[wrong patch removed]

Fri Jul 14 11:46:31 2017 jffry [...] posteo.net - Correspondence added

0001-Use-libtiff-to-decode-image-data-in-TIFF-fixing-RT-1.patch.txt

PhilterPaper commented 7 years ago

Thu Jul 27 04:14:51 2017 jffry [...] posteo.net - Correspondence added

Because of the nature of libTIFF, it is not possible to pass an open filehandle.

PhilterPaper commented 7 years ago

Status update: Jeffrey has offered an extensive rewrite of TIFF support, a thin wrapper around the libtiff library. Unfortunately, this library is not by default available on Windows systems. We have been discussing how it might be ported over and made available in some manner. Apparently it's C/C++, which CPAN knows how to compile and link on an installation. If any reader out there has experience with providing such extensions to Perl, in a platform-independent manner, please chime in!

PhilterPaper commented 7 years ago

Fri, 28 Jul 2017 14:14:58 +0200 From: Petr Pisar <ppisar [...] redhat.com>

Thank you for the patch. I tested it successfully. All PDF-API2 tests pass for me as well as the reproducer I attached to original bug report. I obtained the TIFF image when debugging gscan2pdf tool. It was produced by ImageMagic >= 6.9.3-0. The procedure is at https://github.com/ImageMagick/ImageMagick/issues/277.

-- Petr <this was presumably on a Linux system, which is not going to work on a Windows system, as is. -- Mod.>

PhilterPaper commented 7 years ago

on Tue Sep 05 20:09:08 2017 steve [...] deefs.net - Correspondence added

Thanks for working on this!

Would it be possible to check whether Graphics::TIFF is installed at runtime, then use it if it's present? I'm hesitant to add a dependency that isn't pure-Perl. Then, if Graphics::TIFF isn't present and there's an attempt to work with a TIFF file that isn't supported by the existing code in PDF::API2, we could give an error recommending that Graphics::TIFF get installed for broader TIFF support.

PhilterPaper commented 6 years ago

Thu Nov 16 04:28:15 2017 jffry [...] posteo.net - Correspondence added

Of course, it is possible to check for Graphics::TIFF. My suggestion, however, would be that if it isn't present, then to completely disable the TIFF support, as the current code is hopelessly buggy. Just out of interest, is your problem with non-pure-Perl code Windows support or something else?

Thu Nov 16 04:29:31 2017 jffry [...] posteo.net - Cc RATCLIFFE added

PhilterPaper commented 6 years ago

Thu Nov 16 12:07:56 2017 steve [...] deefs.net - Correspondence added

Just out of interest, is your problem with non-pure-Perl code Windows support or something else?

Three things:

  1. Windows support.
  2. Ease of installation on any platform. Right now, it installs cleanly from CPAN with no additional steps. I'd prefer not to give that up.
  3. Backwards compatibility. The existing TIFF code is in use and working for some people's needs (based on bug reports, in any case, which are of the "it doesn't work in this case" variety vs. "it doesn't work at all"). If those people aren't in a position to install libtiff, I don't want the next version to break their code.

Since most TIFF-using people can probably benefit from using Graphics::TIFF, a simpler implementation might be to print a deprecation warning if Graphics::TIFF isn't installed and then fall back to the current code, and let it error or not as it currently does.

PhilterPaper commented 6 years ago

Fri Nov 17 04:07:19 2017 jffry [...] posteo.net - Correspondence added

I can see that I'm on my own here (apart from the OP), so I'll stop arguing about non-pure-Perl stuff.

How about this?

If Graphics::TIFF is installed, it uses it, otherwise it prints a warning and uses the legacy code. Subject: | optionally-use-libtiff-for-handling-TIFFs-fixing-RT-1.patch

(remove superseded code)

PhilterPaper commented 6 years ago

Fri Nov 17 04:10:37 2017 jffry [...] posteo.net - Correspondence added

Grr. git diff does include newly added files, so it's missed half the patch. Please try this.

0001-Optionally-use-libtiff-for-handling-TIFFs.patch.txt

PhilterPaper commented 6 years ago

Fri Nov 17 11:33:06 2017 $_ = 'spro^^%^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Fri Nov 17 04:10:37 2017, RATCLIFFE wrote: Hide quoted text

Grr. git diff does include newly added files, so it's missed half the patch.

I’ve run into that problem myself many times. What I now do is something like:

$ touch foo/bar 
$ git add foo/bar 
.... make changes .... 
$ git diff
PhilterPaper commented 6 years ago

I just pushed out to GitHub Jeffrey's changes as parallel modules TIFF_GT.pm and TIFF/File_GT.pm, leaving the original "pure Perl" code alone, as some may want to continue using it (filehandles not supported in the new version). Builder.pm sees if Graphics::TIFF is available, and if it is, will use it unless the -nouseGT option is specified. t/tiff.t will work with either version of TIFF support (the filehandle tests force the use of the old library). I will close this issue if Jeffrey has no major objections to the way I did it, which is a bit different than his proposal of a few days ago.

PhilterPaper commented 6 years ago

I think I have this sorted out, using Jeff's new Graphics::TIFF. Since Jeffrey hasn't said anything for a while, and the examples seem to be working OK, I'm going to call it a day an close this issue (RT 84665 also closed, with CCITT Group 3 and Group 4 compression now working).

I'm sure there are many TIFF parameter combinations which are not yet supported, but at least this issue and 84665 can be laid to rest. We'll just have to deal with other failing TIFFs one at a time.