|
Description
Stefan Brandner
2014-12-09 16:37:38 UTC
Changed component to qt4 frontend since the problem appears with okular. Yeah don't, pdftops has the same issue, hasn't it? yes, component general is right! I can confirm this bug with Opensuse 13.2 and a different file. Interestingly the wrongly suggested bounding box has the same dimensions, there seems to be no calculation done at all! /tmp/print-job-okular-bbox contains: %Produced by poppler pdftops version: 0.26.5 (http://poppler.freedesktop.org) %%Creator: LaTeX with Beamer class version 3.26 %%Title: vortrag-test.pdf %%LanguageLevel: 2 %%DocumentSuppliedResources: (atend) %%DocumentMedia: A4 595 842 0 () () %%BoundingBox: 0 0 595 842 Whereas ghostscript calculates the bounding box as following: gs -sDEVICE=bbox -dBATCH -dNOPAUSE /tmp/print-job-okular-bbox GPL Ghostscript 9.15 (2014-09-22) %%BoundingBox: 115 286 479 557 %%HiResBoundingBox: 115.991996 286.505991 478.835985 556.631983 Please fix this! Created attachment 114276 [details] [review] patch to version 0.32.0 of poppler to fix the calculation of the PageBoundingBox DSC comment Comment on attachment 114276 [details] [review] patch to version 0.32.0 of poppler to fix the calculation of the PageBoundingBox DSC comment To my understanding PageBoundingBox should reflect the actual bounding box of all elements on a page. The PageBoundingBox written by poppler is the width and height of the elements on a page before they are scaled and translated and to fit the page. The code is in file poppler/PSOutputDev.cc. I changed the code to first calculate scaling and translation and then write the transformed PageBoundingBox. Because the calculation was done after writing the PageBoundingBox in the original code it was neccessary to reorganize the code a litte bit. That's the reason why this patch is a little bit longer. I'm not sure, if I understood all the transformation in the file. I did not change anything in the calculation. I only changed the calculation of the PageBoundingBox. But somebody who has more understanding to the code should have a careful look to it. Created attachment 114277 [details] [review] patch to Opensuse 13.2 version 0.26.5 of poppler to fix the calculation of the PageBoundingBox DSC comment This is the patch for Opensuse 13.2 to calulate the PageBoundingBox. You have to apply this patch with rpmbuild to the original package. For the content of the patch look at my comment. Created attachment 114919 [details] [review] patch to version 2015-04-04 of poppler to fix the calculation of the PageBoundingBox DSC comment The calculation of the page bounding box seems to be more complicated than I thougt. So here is my second try. Created attachment 114920 [details] [review] patch to Opensuse 13.2 version 0.26.5 of poppler to fix the calculation of the PageBoundingBox DSC comment And here my second patch for the Opensuse 13.2 version 0.26.5 of poppler Created attachment 115615 [details] [review] patch to version 2015-05-03 of poppler to fix the calculation of the PageBoundingBox DSC comment The calculation of the page bounding box was still wrong. Hopefully it now works for any case. This patch is to the git version of poppler. For a patch to Opensuse 13.2 see my other patch. Created attachment 115616 [details] [review] patch to Opensuse 13.2 version 0.26.5 of poppler to fix the calculation of the PageBoundingBox DSC comment The calculation of the page bounding box was still wrong. Hopefully it now works for any case. This patch is to Opensuse 13.2 of poppler. For a patch to the git version see my other patch. Created attachment 119266 [details] [review] alternate patch by Adrian Johnson This is an alternate patch submitted to the poppler mailing list by Adrian Johnson on Oct 27. He added the text below. ========== 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. Created attachment 119271 [details]
simple multi-page example
The attached file is a simple example made by one of the people who reported the problem. If you run "pdftops -paper A4 example-all.pdf" and then open the resulting postscript pages in "gv" (or any viewer capable of toggling between an A4 view and a bounding box view), the bounding box view will show empty space or only a small portion of the image because pdftops with -paper shifts and scales the image to fit A4 but does not apply the same transform to the bounding box, so the bounding box remains at the lower left corner of the page.
The same also happens with the addition of the pdftops "-expand" option.
Some of the discussion on the poppler list mentioned applications that use poppler like okular and CUPS, but the core issue is that pdftops with -paper generates an incorrect PageBoundingBox comment.
Created attachment 119311 [details] [review] alternate patch I've updated the patch to add a default case to the 'switch (rotate)'. Adrian please commit, i trust you know what you're doing. I tried to commit but could not come up with a meaningful commit message. Pushed |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.