[poppler] Add trimbox support to pdftops

Albert Astals Cid aacid at kde.org
Fri Oct 29 16:30:37 PDT 2010


A Divendres, 29 d'octubre de 2010, Benjamin Adler va escriure:
> On 10/28/2010 10:16 PM, Albert Astals Cid wrote:
> > A Dijous, 28 d'octubre de 2010, Benjamin Adler va escriure:
> >> [...]
> >> Please tell me what you think!
> >> 
> >   * Use capital first letter for the enum values.
> 
> Done.
> 
> >   * Don't call the variables cropBox since it seems it contains the
> >   "cropBox",
> > 
> > call them boxToCrop or something like that
> 
> I opted for "pageBox" and hope thats ok.
> 
> >   * It also would be cool if you detected -cropbox and -crop MediaBox
> >   together
> > 
> > and told the user he is wrong and needs to fix his parameters.
> 
> Done.
> 
> I still don't know whether it was ok to change Page::textList() from
> 
> m_page->parentDoc->doc->displayPageSlice(
> 	output_dev, m_page->index + 1, 72, 72, rotation,
> 	false, false, false, -1, -1, -1, -1);
> 
> into
> 
> m_page->parentDoc->doc->displayPageSlice(
> 	output_dev, m_page->index + 1, 72, 72, rotation,
> 
> 	::Page::MediaBox, false, -1, -1, -1, -1);
> 
> because I don't really know what the desired behaviour is when
> "!useMediaBox && !crop" is passed.

I think false, true and false, false actually create a very similar if nto the 
same behaviour.

But that leads me to the fact that (even if noone that we know was using it), 
your current patch makes it impossible to get the same behaviour of having 
useMediaBox = true and crop = true. Since that gave you a page as big as the 
mediabox but the contents cropped to the cropbox, so i really think the 
display methods should have two parameters

Page::PageBox box, Page::PageBox cropBox

that mimic exactly what the Gfx constructor parameters do, and we'd probably 
need another value for the PageBox enum that would be something like NoBox so 
you can pass it to boxToCrop if you don't want any cropping (passing a null to 
Gfx).

What do you think?

> 
> Also, I created the two methods
> 
>    static Page::PageBox getPageBoxFromString(const char *pageBoxName,
> GBool* success) and
> 
>    PDFRectangle* getPDFRectangleOfPageBox(const Page::PageBox pageBox),
> 
> using the first in the utils and the second in Page itself. The latter
> is not private yet, should it be?
> 
> I personally feel that it'd be more beautiful to replace
> 
> 	Page::getMediaWidth()
> 	Page::getMediaHeight()
> 	Page::getCropWidth()
> 	Page::getCropHeight()
> 
> with
> 
> 	double Page::getPageBoxWidth(Page::PageBox)
> 	double Page::getPageBoxHeight(Page::PageBox)
> 
> and modify callers accordingly. Do you agree?

Please don't do any change to existing code unless it's totally necessary.

> 
> Should I add my name to the files I changed (mostly Page.cc/h)?

You don't need to worry about that i'll take care of it when commiting.

> 
> cheers,
> ben
> 
> P.S: Sorry for making this a two-patch-mess, I need to learn git.

That makes two of us ;-.)

Albert


More information about the poppler mailing list