Comment 53 for bug 1242678

Revision history for this message
In , Marek Kašík (mkasik) wrote :

Created attachment 100723
Rotate documents correctly

Hi,

I'm sorry for my late response, I was super busy with something else.

(In reply to comment #15)
> Comment on attachment 99331 [details] [review]
> Rotate documents correctly
>
> Review of attachment 99331 [details] [review]:
> -----------------------------------------------------------------
>
> This looks good to me, except the confusing NONE/LANDSCAPE thing. Adrian,
> does the PostScript part looks good to you?
>
> ::: libspectre/spectre-device.c
> @@ +206,4 @@
> > return SPECTRE_STATUS_RENDER_ERROR;
> > }
> >
> > + if (rc->orientation == NONE || rc->orientation == LANDSCAPE) {
>
> Why NONE and LANDSCAPE? shouldn't we invert width/height when orientation is
> LANDSCAPE and SEASCAPE? Ah, I know what's going on, you are using the
> internal values of the parser (my fault, I think I suggested it) but
> rc->orientation is actually a SpectreOrientation enum value, so you should
> use SPECTRE_ORIENTATION_PORTRAIT || SPECTRE_ORIENTATION_REVERSE_PORTRAIT
> that are 0 and 2 like NONE and LANDSCAPE in the internal parser.
>
> @@ +269,4 @@
> > args[arg++] = "-dNOPLATFONTS";
> >
> > if (rc->width != -1 && rc->height != -1) {
> > + if (rc->orientation == NONE || rc->orientation == LANDSCAPE) {
>
> Same here.

My brain was probably on a vacation :). I've fixed this so that it uses the SPECTRE_ORIENTATION_PORTRAIT and SPECTRE_ORIENTATION_REVERSE_PORTRAIT.

Regards

Marek