Comment 37 for bug 1242678

Created attachment 99100
Rotate documents correctly

Thank you for the review.

(In reply to comment #7)
> Comment on attachment 96165 [details] [review]
> Rotate documents correctly
>
> Review of attachment 96165 [details] [review]:
> -----------------------------------------------------------------
>
> Thanks for the patch!
>
> ::: libspectre/spectre-device.c
> @@ +206,5 @@
> > return SPECTRE_STATUS_RENDER_ERROR;
> > }
> >
> > + scaled_width = (int) ((width * rc->x_scale) + 0.5);
> > + scaled_height = (int) ((height * rc->y_scale) + 0.5);
>
> I think it would be clearer if we set these values depending on the
> orientation, and then we just use scale_width, scaled_height below.

Done.

> @@ +226,5 @@
> > rc->text_alpha_bits);
> > args[arg++] = graph_alpha = _spectre_strdup_printf ("-dGraphicsAlphaBits=%d",
> > rc->graphic_alpha_bits);
> > +
> > + if (rc->orientation == 0 || rc->orientation == 2) {
>
> Don't use magic numbers here, use NONE and LANDSCAPE instead.

Done.

> @@ +232,5 @@
> > + args[arg++] = resolution = _spectre_strdup_printf ("-r%fx%f",
> > + rc->x_scale * rc->x_dpi,
> > + rc->y_scale * rc->y_dpi);
> > + }
> > + else {
>
> } else {

Done.

> ::: libspectre/spectre-gs.c
> @@ +237,5 @@
> >
> > + switch (rotation) {
> > + default:
> > + tmp_xoffset = xoffset + x;
> > + tmp_yoffset = yoffset + y;
>
> I don't think we need the tmp_ variables, we can just modify the existing
> xoffset/yoffset.

I removed them but I had to reintroduce them back because of the next item.

> @@ +277,5 @@
> > doc->endsetup))
> > return FALSE;
> >
> > + if (rotation != 0) {
> > + set = _spectre_strdup_printf ("%d rotate", rotation);
>
> Where does this end up? After the setup and before the pages?

Yes, it ends up there.

> Isn't it recommended to apply the rotation after the translation? I think we could
> move this to spectre_gs_process, after the translate. We pass the rotation
> to spectre_gs_process and when it's != NONE we inject the rotate command
> there. What do you think?

I moved it there.

Unfortunately I've found a PostScript file on which the patch doesn't work. It shows the file with the original rotation for each rotation. It is due to "setpagedevice" and "PageSize" are employed there (see next attachment). I'm not sure what to do with it yet.

Regards

Marek