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

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