Comment 44 for bug 1242678

Revision history for this message
In , Carlos Garcia Campos (carlosgc) wrote :

Comment on attachment 99331
Rotate documents correctly

Review of attachment 99331:
-----------------------------------------------------------------

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.