Could you provide a document where the first page should be on the left, please? 

The document I was looking at: <http://commons.wikimedia.org/wiki/Image:Drei_Register_Arithmetischer_ahnfeng_zur_Practic.djvu>

I would really appreciate such an option.

I'm a subscriber to the e-paper version of Der Spiegel [1], a popular German weekly magazine. To protect their copyright, they prepend a single page to the original news paper, which has my name and a copyright notice printed on it. Due to this, dual view does not work as expected, as odd pages are on the right and even pages are on the left.

Due to obvious reasons, I can't provide you with a document, although I have a large number of them on my hard disk ;).

I guess there are more use cases out there, where such an option could be useful.

[1] <http://en.wikipedia.org/wiki/Der_Spiegel> 

Of course I wanted to say: odd pages are on the *left*, even pages are on the *right*. Sorry. 

And how does Acroread cope with this issue?

Alright, I installed acroread (v. 8.1.3) for you. I don't know how they do it, but it is displayed correctly there. Odd pages are on the right, not on the left. I'll attach screenshots in a minute.

Evince version is 2.24.1

If you need further information on the PDF files, I will try to provide you with this info. I would also send a sample PDF to an Evince developer. I don't want to attach it in public, though.

Created attachment 122803 [details]
Spiegel PDF in Adobe 8.1.3

Created attachment 122804 [details]
Spiegel PDF in Evince 2.24.1

Hm, that's the point, acroread somehow defines the right layout.

I did some research on this topic. 

Per the PDF specification [1], there is an optional entry in the "catalog dictionary". This entry is called "PageLayout" (page 73, table 28). The entry has four values: SinglePage, OneColumn, TwoColumnLeft, TwoColumnRight, TwoPageLeft, TwoPageRight. The last two values define, if the odd numbered pages are on the left (first value) or on the right (last value). However, the last two values are only supported since PDF version 1.5.

But: The Spiegel PDF documents are PDF version 1.4. So I suppose they don't have this entry or at least not one of the last two values. 

So why does acroread display this correctly anyway? Per [2] there was a change in default behaviour recently. If you select "Two Up", now the default display option seems to be TwoPageLeft. It must have been TwoPageRight before. You can select the old behaviour as described in the blog post. If you do this, the PDF is displayed like it is displayed in Evince.

[1] <http://www.adobe.com/devnet/acrobat/pdfs/PDF32000_2008.pdf>
[2] <http://blog.gilbertconsulting.com/2007/12/view-spreads-in-acrobat-8.html>

Thanks a lot for investigations! Well, it will require some work.

Is any work being done on this one? And if so, are you aiming to auto-detect the right setting from the document or a manually settable option (or both)?

Sorry to revisit an old bug but I'm also having this issue.

It would be very useful if this were addressed - are any of the developers looking into it?

I have a similar case with online articles from Stiftung Warentest.  The provide huge table spanning two facing pages.

I would appreciate a manual override for defining the oddness of the first page here.

*** Bug 620472 has been marked as a duplicate of this bug. ***

I view various short reports and scientific articles a lot and dual view is a way to see two pages simultaneously to my wide monitor.  For this it is irrelevant which pages would end up the left or right side in some printed version and I do not care the slightest.  Some document are never meant to be printed and still I want to see two pages at once.  Having a 4-page article split to 1+2+1 pages is quite annoying.

Created attachment 166698 [details] [review]
proposed patch

Patch that implements the option.  It is against evince 2.20.3 because I don't have the Gtk3 stuff required by git HEAD.

I tested it on some my documents but it needs more thorough testing.

Ping?

I use evince with this patch daily since I wrote it and I am quite happy with it.  Is anyone else still interested?

*** Bug 631564 has been marked as a duplicate of this bug. ***

I'm rather far down the food chain--I don't normally try to apply patches.  As the submitter of the duplicate above, I'm very interested.  I can report that, as an end user, the option needed is available in Foxit as:

View->Page Display->Show Cover Page During Facing (check or uncheck)

and in Adobe Reader as:

View->Page Display->Show Cover Page During Two-Up  (check or uncheck)

In an ideal world this attribute would travel with a document and not be specified globally, but it would still be necessary to override a document setting that was unsuitable for some purpose. (Comparing/Verifying formats on obverse and reverse of same page, possibly.)

(In reply to comment #18)
> Ping?
> 
> I use evince with this patch daily since I wrote it and I am quite happy with
> it.  Is anyone else still interested?
First of all, thanks for working on this. Patches are always welcome. However, 
if you wish this patch even to be considered for review, you need to produce a patch against current development branch (i.e. use branch master in git). 

Some general comments about the patch. It seems that it is quite involved the way you are changing the cache computations. As an advice, try to keep the modifications to a minimum (ask your self, do I need to really change the logic in that way?) since this will ease aceptance of your patch ( I could spot some modifications where I can't see why you are doing them, but there is no point on discussing issues on a patch that can't be applied to git master)
 
Also, although is not strictly related (after reading comment 11), It would be also good if your patch could also address the PageLayout option in PDF 1.5. 

If you need/want help on building evince from source, please join us in #evince on gimp.net

Created attachment 174590 [details] [review]
proposed patch

Patch updated to evince 2.32.0.

(In reply to comment #21)
> First of all, thanks for working on this. Patches are always welcome. However, 
> if you wish this patch even to be considered for review, you need to produce a
> patch against current development branch (i.e. use branch master in git). 

Hello, producing patch against git is quite a barrier for me.  Probably higher than having to patch evince for personal use forever.

> Some general comments about the patch. It seems that it is quite involved the
> way you are changing the cache computations. As an advice, try to keep the
> modifications to a minimum (ask your self, do I need to really change the logic
> in that way?) since this will ease aceptance of your patch ( I could spot some
> modifications where I can't see why you are doing them, but there is no point
> on discussing issues on a patch that can't be applied to git master)

I'll try to explain.  There are only two principal changes:
- a new setting determining if even pages should be on the left (reflected
  in various properties and struct fields)
- whenever it used to simply test the parity of page it now compares the result
  to the new setting

The only change to the cache are that the values are calculated and cached for both the even-left and odd-left modes now (otherwise it would sort of defeat the point of having a cache).

> Also, although is not strictly related (after reading comment 11), It would be
> also good if your patch could also address the PageLayout option in PDF 1.5. 
> 
> If you need/want help on building evince from source, please join us in #evince
> on gimp.net

I don't know anything about the PDF format.  I suppose such option can be used for the default choice of even/odd-left but I do not feel competent to implement it.

(In reply to comment #23)
> (In reply to comment #21)
> > First of all, thanks for working on this. Patches are always welcome. However, 
> > if you wish this patch even to be considered for review, you need to produce a
> > patch against current development branch (i.e. use branch master in git). 
> 
> Hello, producing patch against git is quite a barrier for me.  Probably higher
> than having to patch evince for personal use forever.
> 
No worries then, I will update the patch for git master.

(In reply to comment #23)
> The only change to the cache are that the values are calculated and cached for
> both the even-left and odd-left modes now (otherwise it would sort of defeat
> the point of having a cache).

OK, not really true, there is one more change in build_height_to_page() (I've written it some time ago): the introduction of page_heights[] that holds all the page heights.  I did this as the repeated ev_document_get_page_size() become somewhat unwieldy.  I can remove it to reduce the number of changes if you consider using ev_document_get_page_size() in each place a page height is requested to be better though I do not think it will make the code easier to understand.

(In reply to comment #25)
> (In reply to comment #23)
> > The only change to the cache are that the values are calculated and cached for
> > both the even-left and odd-left modes now (otherwise it would sort of defeat
> > the point of having a cache).
> 
> OK, not really true, there is one more change in build_height_to_page() (I've
> written it some time ago): the introduction of page_heights[] that holds all
> the page heights.  I did this as the repeated ev_document_get_page_size()
> become somewhat unwieldy.  I can remove it to reduce the number of changes if
> you consider using ev_document_get_page_size() in each place a page height is
> requested to be better though I do not think it will make the code easier to
> understand.

This is the change I was talking about. ev_document_get_page_size already has a cache of the page_sizes (indeed, ev_document_get_page_size access this cache). I'd really wait for Carlos opinion about that, but in the mean time, I think 
It would be better to factor out a static function like 

ev_view_dual_compute_page_heights (EvView *view, gboolean dual_page_even_left, **heights)

and call this function twice when computing the cache (I think most of the logic is the same for both cases)

(In reply to comment #23)
> The only change to the cache are that the values are calculated and cached for
> both the even-left and odd-left modes now (otherwise it would sort of defeat
> the point of having a cache).

I don't see the point of having the two caches calculated, as you will only change the odd-left/odd-right setting for a given pdf once. What do you think?

(In reply to comment #27)
> I don't see the point of having the two caches calculated, as you will only
> change the odd-left/odd-right setting for a given pdf once. What do you think?

Well, you are probably right (though if the user switches the view at least once both sets of height will be calculated anyway).

Having only one cache requires more code to know if even-left is on or off and make decisions based on it.  I'll have to look into it to see if it is fine or would propagate this knowledge to places where it does not make sense.

(In reply to comment #28)
> (In reply to comment #27)
> > I don't see the point of having the two caches calculated, as you will only
> > change the odd-left/odd-right setting for a given pdf once. What do you think?
> 
> Well, you are probably right (though if the user switches the view at least
> once both sets of height will be calculated anyway).

But in the given use case, only one of these calculations will be utilised per PDF; the first will be discarded if it is incorrect.

> Having only one cache requires more code to know if even-left is on or off and
> make decisions based on it.

This seems like the correct method to me. I've only had a cursory glance at the patch and don't know evince's code very well, but in general, duplicating code, functionality, etc. is an undesirable thing in software development.

I have some copyrighted PDFs from Paizo.com that are clearly intended to be viewed with the first page considered even. This is possibly quite common in eBook-style PDFs: the first page of the PDF is the front cover, the second is the title page; in the hardcopy version, there would be a blank inner cover page.

I'd like to add that it would be nice to read sheet music with first page on the left, to see the second page at the same time.  An option to control this behavior would be really nice.

Review of attachment 174590 [details] [review]:

I mostly agree with all the reviews, we need to patch updated to current git master and I think using only one cache is better, since this is not an option you would change more than once. Regarding the ui changes, I'm not sure about adding two dual modes, it's ok to consider them as two modes internally, but in the ui it could be an option that changes the way dual mode works like someone proposed.

Here is a new attempt at patching this issue, against git master.

Similarly to the previous proposal, I added a odd/even left flag to EvView (and document model) and adapted the interface accordingly (UI could be improved, possibly later as some other layout options could still be usefully added).

To minimize caching modification, I just rebuild the cache when user toggle the "Odd pages left" option. This is done by calling build_height_to_page() in ev_view_dual_even_left_changed_cb().

I still modified build_height_to_page() however. The reason is that where all other code was calling get_dual_even_left(), build_height_to_page() was computing dual_even_left directly based on the number of pages in document. This is no more possible as we need access to even/odd-left flag. I handled this by passing the full view instead of just view's document, and calling get_dual_even_left() as everywhere else.

In the process, I added git.mk to some Makefile.am files where it seemed missing (resulting in some built files not being ignored by git). I provide a separate patch for this minor modification.

Created attachment 201510 [details] [review]
Added some built files to git ignore list

Created attachment 201511 [details] [review]
Implemented dual view mode with odd left pages

Review of attachment 201511 [details] [review]:

Hey, thanks for the patch, this is indeed much better. We still need to figure out the correct UI for this,maybe following Adobe makes sense, even if I think it's no so obvious the UI they have. One quick comment, don't add the vim modelines, or, if Carlos agree we want modelines, add the modelines to all files in a separate patch.

Review of attachment 201510 [details] [review]:

Pushed to git master, thanks!

Review of attachment 201511 [details] [review]:

Yeah, this is much better, thank you very much. I'm still not sure about the UI, maybe someone from the design team could help us here. There's a problem with you current patch regarding the UI. The option is enabled even when dual page is not active. In such case enabling/disabling the odd left option takes no effect and it's not obvious that the option is only useful in dual mode. You should disable the action depending on dual mode action (in ev_window_setup_action_sensitivity()).

::: data/evince-ui.xml
@@ +42,3 @@
       <menuitem name="ViewContinuousMenu" action="ViewContinuous"/>
       <menuitem name="ViewDualMenu" action="ViewDual"/>
+      <menuitem name="ViewOddLeftMenu" action="ViewOddLeft"/>

This option only make sense in dual mode, so I prefer to use a name like ViewDualOddLeft

::: data/org.gnome.Evince.gschema.xml.in
@@ +41,3 @@
       <default>false</default>
     </key>
+    <key name="odd-left" type="b">

dual-odd-left

::: libview/ev-document-model.c
@@ +38,3 @@
 	guint continuous : 1;
 	guint dual_page  : 1;
+	guint odd_left   : 1;

dual_page_odd_left, I think you used this naming convention in previous patch, why did you change it in this one?

@@ +66,3 @@
 	PROP_CONTINUOUS,
 	PROP_DUAL_PAGE,
+	PROP_ODD_LEFT,

PROP_DUAL_PAGE_ODD_LEFT

@@ +126,3 @@
 		ev_document_model_set_dual_page (model, g_value_get_boolean (value));
 		break;
+	case PROP_ODD_LEFT:

Ditto.

@@ +127,3 @@
 		break;
+	case PROP_ODD_LEFT:
+		ev_document_model_set_odd_left (model, g_value_get_boolean (value));

EvDocumentModel is public API, so we should use a better name, ev_document_model_set_dual_page_odd_pages_left() it's longer but I think it's more clear.

@@ +171,3 @@
 		break;
+	case PROP_ODD_LEFT:
+		g_value_set_boolean (value, ev_document_model_get_odd_left (model));

Ditto.

@@ +252,3 @@
+					 g_param_spec_boolean ("odd-left",
+							       "Odd Pages Left",
+							       "Whether odd pages are displayed on left side",

Whether odd pages are displayed on the left side in dual mode

::: libview/ev-view.c
@@ +303,3 @@
 static void
+build_height_to_page (EvView		  *view,
+		      EvHeightToPageCache *cache)

Since now this method takes a view instead of the cache, I would rename it to something like ev_view_build_height_to_page_cache() to make it clear it's a view method.

@@ +397,3 @@
+				    gint     page,
+				    gdouble *height,
+				    gdouble *dual_height)

Same here, this was called ev_height_to_page_cache_ because it took a cache object, but now it's a view method, in this case I think we can even get rid of this method and include the code in ev_view_get_height_to_page() which is the only method that uses it.

@@ +5427,3 @@
 			  view);
+	g_signal_connect (view->model, "notify::odd-left",
+			  G_CALLBACK (ev_view_dual_even_left_changed_cb),

Even though you want to update dual_even_left variable internally, the property that changes is odd-left, so this callback should be ev_view_dual_odd_left_changed_cb

::: shell/ev-window.c
@@ +1142,3 @@
 	gboolean   continuous = FALSE;
 	gboolean   dual_page = FALSE;
+	gboolean   odd_left = FALSE;

dual_page_odd_left

@@ +1163,3 @@
 						 g_settings_get_boolean (settings, "dual-page"));
+		ev_document_model_set_odd_left (window->priv->model,
+						g_settings_get_boolean (settings, "odd-left"));

Do we want to update the odd-left property even when dual-page is FALSE? That would recompute the cache for nothing.

Created attachment 201926 [details] [review]
Proposed patch, take three

Thanks for your careful reviews. Here is my new attempt, addressing much (but not all) of the concerns Carlos and José raised.

I agree to (and implemented) the various suggestions relative to naming, which should make the code, API and options more clear. (Some names had changed between first and second proposal because these were not from the same person. As the first patch was against evince code more than one-year old, I felt it was easier/safer to restart from scratch which explain some variables were apparently needlessly renamed.)

The sensitivity of ViewDualOddLeft menu entry now depends of the state of ViewDual. This implied a change not only in ev_window_setup_action_sensitivity() but also in ev_window_update_dual_page_action().

I haven't touched the part updating the dual-odd-left property in setup_model_from_metadata(). My motive with this code was just to ensure model, view and meta-data were kept in sync. I agree it should avoid triggering a cache refresh when dual-page is false, but I could not devise an elegant solution, because I quite do not understand the exact cause of the problem in the first place. Could you elaborate a bit about when/why the cache is recomputed needlessly?

I've just pushed everything except the UI part, because I'm still not sure. My initial idea was to avoid adding more modes, but after trying current patch I find it more confusing than having just two dual modes.

I've finally split the dual mode into two: 

 - Dual (Even pages left)
 - Dual (Odd pages left)

I think it's easier to understand this way.