Comment on attachment 96165
Rotate documents correctly

Review of attachment 96165:
-----------------------------------------------------------------

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.

@@ +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.

@@ +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 {

::: 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.

@@ +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? 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?

-- 
You received this bug notification because you are a member of Kubuntu
Bugs, which is subscribed to libspectre in Ubuntu.
https://bugs.launchpad.net/bugs/1242678

Title:
  evince cannot render some EPS files

To manage notifications about this bug go to:
https://bugs.launchpad.net/evince/+bug/1242678/+subscriptions

-- 
kubuntu-bugs mailing list
kubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kubuntu-bugs

Reply via email to