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 Desktop
Packages, which is subscribed to evince in Ubuntu.
https://bugs.launchpad.net/bugs/1242678

Title:
  evince cannot render some EPS files

Status in Evince document viewer:
  New
Status in GS-GPL  -  GPL Ghostscript:
  Unknown
Status in libspectre:
  Confirmed
Status in “evince” package in Ubuntu:
  Confirmed
Status in “ghostscript” package in Ubuntu:
  Invalid
Status in “gnuplot” package in Ubuntu:
  Invalid
Status in “libspectre” package in Ubuntu:
  Triaged
Status in “evince” source package in Trusty:
  Confirmed
Status in “ghostscript” source package in Trusty:
  Invalid
Status in “gnuplot” source package in Trusty:
  Invalid
Status in “libspectre” source package in Trusty:
  Triaged

Bug description:
  I upgraded to Ubuntu Saucy,
  evince cannot render EPS files generated by gnuplot.
  (Just show an empty page.)

  
  [how to reproduce]

  1. Generate an EPS file using gnuplot by executing the following command.
  $ gnuplot -e "set terminal postscript; set output 'test.eps'; plot x;"

  2. Open the file "test.eps" just created.
  $ evince "test.eps"

  The following are what evince writes to the terminal.

  (evince:24176): EvinceDocument-CRITICAL **: ev_document_get_n_pages:
  assertion 'EV_IS_DOCUMENT (document)' failed

  (evince:24176): Gtk-WARNING **: drawing failure for widget `EvView':
  invalid matrix (not invertible)

  (evince:24176): Gdk-CRITICAL **: gdk_pixbuf_get_from_surface:
  assertion 'width > 0 && height > 0' failed

  (evince:24176): Gdk-CRITICAL **: gdk_pixbuf_get_from_surface:
  assertion 'width > 0 && height > 0' failed

  3. Then evince just shows an empty (white) page.

  ===

  If i rotate 90 or -90 degree by pressing Ctrl+Left or Ctrl+Right key,
  evince can show this file correctly.

  ===

  Okular, GIMP, inkscape and gs can show this EPS file.
  (may be rotated 90 degree, however)

  ProblemType: Bug
  DistroRelease: Ubuntu 13.10
  Package: evince 3.10.0-0ubuntu2
  ProcVersionSignature: Ubuntu 3.11.0-12.19-generic 3.11.3
  Uname: Linux 3.11.0-12-generic x86_64
  NonfreeKernelModules: nvidia
  ApportVersion: 2.12.5-0ubuntu2
  Architecture: amd64
  Date: Mon Oct 21 21:23:15 2013
  InstallationDate: Installed on 2013-09-07 (44 days ago)
  InstallationMedia: Ubuntu 13.04 "Raring Ringtail" - Release amd64 (20130424)
  MarkForUpload: True
  SourcePackage: evince
  UpgradeStatus: Upgraded to saucy on 2013-10-04 (16 days ago)

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

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

Reply via email to