[poppler] Printing of certain PDF files does not work with "fit-to-page" because of wrong BoundingBox values in the PostScript

[Martin Pahl]

Thank you for reviewing and improving my patch. I confirm, after looking to 
your code and testing it with the example-all.pdf that this new patch solves 
the problems with the wrong PageBoundingBox. It leads to identical output as 
my patch.

You are right, I ignored the values of tx and ty prior to centering. I'm not 
sure about the reason anymore. x1 and y1 are usually 0 and clipping case is 
never used. At least I could not find any code in poppler using the setClip-
method of PSOutputDev. But of course that's no excuse for ignoring those 
values.

Regarding the problem with rotation values not being in the set of 
0,90,180,270 mentioned by William Bader maybe you should use 0 as default 
case. Although poppler always makes the assumption that rotate is within this 
set. Sometimes 0 rotation is the default and sometime 270 is used as default 
value (like in this patched method). 

And of course I also agree about using ceil and floor.

Regards,

Martin Pahl



Am Dienstag, 27. Oktober 2015, 22:47:27 schrieben Sie:
> On 25/10/15 02:58, Martin Pahl wrote:
> > Attached you find an example:
> > 
> > example.pdf: a box generated with xfig 321mm x 106mm
> > 
> > example.ps generated with pdftops -paper A4 example.pdf
> > example.ps:%%PageBoundingBox: 0 0 302 912
> > 
> > example-patched.ps generated with pdftops -paper A4 example.pdf, but with
> > the patched version.
> > example-patched.ps:%%PageBoundingBox: 158 0 437 842
> > 
> > You can view the postscript files with gv and select BBox. It's obvious
> > that the unpatched PageBoundingBox is totally wrong, because source
> > coordinates are used. The PageBoundingBox is bigger then the A4 paper
> > format. It's just the size of the original box (302 /72 dpi * 25,4mm =
> > 106,5mm; 912/72 dpi * 25,4mm = 321,73mm). But it must be the box scaled
> > down to A4 and shifted to the center.
> > 
> > Hope this example helps.
> 
> I've reviewed the patch and have the following comments.
> 
> It would have been a lot easier to review (and probably would have been
> reviewed earlier) if you avoided the unnecessary changes to convert
> if-else statements to case statements. Putting code style changes in a
> separate patch to the bug fix makes reviewing changes much easier.
> 
> + int(pbbty),
> + int(width * xScale + pbbtx + 0.5),
> 
> Using floor() and ceil() would be better and would make the code easier
> to understand.
> 
> You appear to be ignoring the value of tx and ty prior to centering
> calculations. What happens if tx and ty are non zero at this point? I
> would be more comfortable with the patch if the page bbox calculations
> used the exact same transformation as is output to PS.
> 
> I'm attaching a new patch that I think is a lot easier to understand. It
> uses the same transformation as is output to PS to calculate the
> bounding box.
> 
> > Regards,
> > 
> > Martin Pahl
> > 
> > Am Freitag, 23. Oktober 2015, 21:46:02 schrieb Adrian Johnson:
> >> On 21/10/15 03:32, Stefan Brandner wrote:
> >>> ------------------------------------------------------------------------
> >>> 
> >>> I am using the patch now for several month and I can prove it is working
> >>> fine. So if the code quality is ok for you Albert why not pushing it?
> >>> 
> >>> Regards
> >>> Stefan Brandner
> >> 
> >> I'll take a look at the patch sometime in the next week. It would help
> >> 
> >> if you:
> >>  - attach a PDF that reproduces the bug
> >>  - list the pdftops options you are using
> >>  - list the bounding box output you are getting and what you think
> >>  
> >>    it should be.
> >>> 
> >>> El Dijous, 7 de maig de 2015, a les 09:34:09, Martin Pahl va escriure:
> >>>> /Hi, />//>/I sent patches to fix the bug:
> >>>> />//>/https://bugs.freedesktop.org/show_bug.cgi?id=87161 />//>/Those
> >>>> patches were automatically sent to poppler-bugs mailing list. But>>
> >>> 
> >>> as I />/see no reaction to my patch submission (poppler-bugs seems to be
> >>> a
> >>> />/mailinglist without human interaction) I just want to ask, what is
> >>> the
> >>> />/right way to submit patches. / It is.
> >>> 
> >>> What we need is more people with time to review patches.
> >>> 
> >>> Cheers,
> >>> 
> >>>   Albert
> >>>> 
> >>>> /By the way this bug is really annoying as it makes all PDF viewers
> >>>> using
> >>>> />/poppler useless for printing documents that do not have the page
> >>>> size
> >>>> of>>
> >>> 
> >>> the />/output device (e.g. printer). On the other hand acroread is not
> >>> an
> >>> />/alternative anymore as Adobe has discontinued support for Linux.
> >>> />//>/Regards, />//>/Martin Pahl /
> >>> 
> >>> 
> >>> 
> >>> _______________________________________________
> >>> poppler mailing list
> >>> poppler at lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >> 
> >> _______________________________________________
> >> poppler mailing list
> >> poppler at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/poppler
> >> 
> >> 
> >> _______________________________________________
> >> poppler mailing list
> >> poppler at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/poppler