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
>>> [email protected]
>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>
>> _______________________________________________
>> poppler mailing list
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>
>>
>> _______________________________________________
>> poppler mailing list
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/poppler
diff --git a/poppler/GfxState.cc b/poppler/GfxState.cc
index ab796f7..e8facd7 100644
--- a/poppler/GfxState.cc
+++ b/poppler/GfxState.cc
@@ -80,6 +80,22 @@ GBool Matrix::invertTo(Matrix *other) const
return gTrue;
}
+void Matrix::translate(double tx, double ty)
+{
+ double x0 = tx*m[0] + ty*m[2] + m[4];
+ double y0 = tx*m[1] + ty*m[3] + m[5];
+ m[4] = x0;
+ m[5] = y0;
+}
+
+void Matrix::scale(double sx, double sy)
+{
+ m[0] *= sx;
+ m[1] *= sx;
+ m[2] *= sy;
+ m[3] *= sy;
+}
+
void Matrix::transform(double x, double y, double *tx, double *ty) const
{
double temp_x, temp_y;
diff --git a/poppler/GfxState.h b/poppler/GfxState.h
index f018e93..f64b8e0 100644
--- a/poppler/GfxState.h
+++ b/poppler/GfxState.h
@@ -59,7 +59,12 @@ class Matrix {
public:
double m[6];
+ void init(double xx, double yx, double xy, double yy, double x0, double y0) {
+ m[0] = xx; m[1] = yx; m[2] = xy; m[3] = yy; m[4] = x0; m[5] = y0;
+ }
GBool invertTo(Matrix *other) const;
+ void translate(double tx, double ty);
+ void scale(double sx, double sy);
void transform(double x, double y, double *tx, double *ty) const;
double determinant() const { return m[0] * m[3] - m[1] * m[2]; }
double norm() const;
diff --git a/poppler/PSOutputDev.cc b/poppler/PSOutputDev.cc
index 7b93e3b..01a55c8 100644
--- a/poppler/PSOutputDev.cc
+++ b/poppler/PSOutputDev.cc
@@ -3762,38 +3762,19 @@ void PSOutputDev::startPage(int pageNum, GfxState *state, XRef *xrefA) {
}
}
}
- if (paperMatch) {
- paperSize = (PSOutPaperSize *)paperSizes->get(pagePaperSize[pageNum]);
- writePSFmt("%%PageMedia: {0:t}\n", paperSize->name);
- }
- if (rotate == 0 || rotate == 180) {
- writePSFmt("%%PageBoundingBox: 0 0 {0:d} {1:d}\n", width, height);
- } else {
- writePSFmt("%%PageBoundingBox: 0 0 {0:d} {1:d}\n", height, width);
- }
- writePSFmt("%%PageOrientation: {0:s}\n",
- landscape ? "Landscape" : "Portrait");
- writePS("%%BeginPageSetup\n");
- if (paperMatch) {
- writePSFmt("{0:d} {1:d} pdfSetupPaper\n", imgURX, imgURY);
- }
- writePS("pdfStartPage\n");
if (rotate == 0) {
imgWidth2 = imgWidth;
imgHeight2 = imgHeight;
} else if (rotate == 90) {
- writePS("90 rotate\n");
ty = -imgWidth;
imgWidth2 = imgHeight;
imgHeight2 = imgWidth;
} else if (rotate == 180) {
- writePS("180 rotate\n");
imgWidth2 = imgWidth;
imgHeight2 = imgHeight;
tx = -imgWidth;
ty = -imgHeight;
} else { // rotate == 270
- writePS("270 rotate\n");
tx = -imgHeight;
imgWidth2 = imgHeight;
imgHeight2 = imgWidth;
@@ -3837,6 +3818,58 @@ void PSOutputDev::startPage(int pageNum, GfxState *state, XRef *xrefA) {
}
tx += (rotate == 0 || rotate == 180) ? imgLLX : imgLLY;
ty += (rotate == 0 || rotate == 180) ? imgLLY : -imgLLX;
+
+ if (paperMatch) {
+ paperSize = (PSOutPaperSize *)paperSizes->get(pagePaperSize[pageNum]);
+ writePSFmt("%%PageMedia: {0:t}\n", paperSize->name);
+ }
+
+ // Create a matrix with the same transform that will be output to PS
+ Matrix m;
+ switch (rotate) {
+ case 0:
+ m.init(1, 0,
+ 0, 1,
+ 0, 0);
+ break;
+ case 90:
+ m.init(0, 1,
+ -1, 0,
+ 0, 0);
+ break;
+ case 180:
+ m.init(-1, 0,
+ 0, -1,
+ 0, 0);
+ break;
+ case 270:
+ m.init(0, -1,
+ 1, 0,
+ 0, 0);
+ break;
+ }
+ m.translate(tx, ty);
+ m.scale(xScale, yScale);
+
+ double bboxX1, bboxY1, bboxX2, bboxY2;
+ m.transform(0, 0, &bboxX1, &bboxY1);
+ m.transform(width, height, &bboxX2, &bboxY2);
+
+ writePSFmt("%%PageBoundingBox: {0:g} {1:g} {2:g} {3:g}\n",
+ floor(std::min(bboxX1, bboxX2)),
+ floor(std::min(bboxY1, bboxY2)),
+ ceil (std::max(bboxX1, bboxX2)),
+ ceil (std::max(bboxY1, bboxY2)));
+
+ writePSFmt("%%PageOrientation: {0:s}\n",
+ landscape ? "Landscape" : "Portrait");
+ writePS("%%BeginPageSetup\n");
+ if (paperMatch) {
+ writePSFmt("{0:d} {1:d} pdfSetupPaper\n", imgURX, imgURY);
+ }
+ writePS("pdfStartPage\n");
+ if (rotate)
+ writePSFmt("{0:d} rotate\n", rotate);
if (tx != 0 || ty != 0) {
writePSFmt("{0:.6g} {1:.6g} translate\n", tx, ty);
}
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
