Dual pages: shown as 2 columns on a single page

Rework Dual pages code so that the view is considered
a single page number, so it looks more like 2-columns
on a single page.
This solves a few issues like:
- Page number and count are consistent between top
  and bottom status bars
- SkimTo -1/+1 doing nothing every other tap
- Statistics being wrong (like "Pages read" never
  going over half of the book page count)

frontend/apps/reader/modules/readercoptlistener.lua

@@ -10,16 +10,14 @@ local ReaderCoptListener = EventListener:new{}
 
 function ReaderCoptListener:onReadSettings(config)
     local view_mode = config:readSetting("copt_view_mode") or
-           G_reader_settings:readSetting("copt_view_mode")
+           G_reader_settings:readSetting("copt_view_mode") or 0 -- default to "page" mode
-    if view_mode == 0 then
+    local view_mode_name = view_mode == 0 and "page" or "scroll"
-        self.ui:registerPostReadyCallback(function()
+    -- Let crengine know of the view mode before rendering, as it can
-            self.view:onSetViewMode("page")
+    -- cause a rendering change (2-pages would become 1-page in
-        end)
+    -- scroll mode).
-    elseif view_mode == 1 then
+    self.ui.document:setViewMode(view_mode_name)
-        self.ui:registerPostReadyCallback(function()
+    -- ReaderView is the holder of the view_mode state
-            self.view:onSetViewMode("scroll")
+    self.view.view_mode = view_mode_name
-        end)
-    end
 
     -- crengine top status bar can only show author and title together
     self.title = G_reader_settings:readSetting("cre_header_title") or 1

frontend/apps/reader/modules/readerhighlight.lua

@@ -873,16 +873,20 @@ function ReaderHighlight:onHoldPan(_, ges)
                 elseif self.hold_pos.x <= screen_half_width and is_in_next_page_corner then
                     return true
                 end
-                local cur_page = self.ui.document:getCurrentPage()
+                -- To be able to browse half-page when 2 visible pages as 1 page number,
+                -- we must work with internal page numbers
+                local cur_page = self.ui.document:getCurrentPage(true)
                 local restore_page_mode_xpointer = self.ui.document:getXPointer() -- top of current page
+                self.ui.document.no_page_sync = true -- avoid CreDocument:drawCurrentViewByPage() to resync page
                 self.restore_page_mode_func = function()
+                    self.ui.document.no_page_sync = nil
                     self.ui.rolling:onGotoXPointer(restore_page_mode_xpointer, self.selected_text_start_xpointer)
                 end
                 if is_in_next_page_corner then -- bottom right corner in LTR UI
-                    self.ui.rolling:_gotoPage(cur_page + 1, true) -- no odd left page enforcement
+                    self.ui.rolling:_gotoPage(cur_page + 1, true, true) -- no odd left page enforcement
                     self.hold_pos.x = self.hold_pos.x - screen_half_width
                 else -- top left corner in RTL UI
-                    self.ui.rolling:_gotoPage(cur_page - 1, true) -- no odd left page enforcement
+                    self.ui.rolling:_gotoPage(cur_page - 1, true, true) -- no odd left page enforcement
                     self.hold_pos.x = self.hold_pos.x + screen_half_width
                 end
                 UIManager:setDirty(self.dialog, "ui")

frontend/apps/reader/modules/readerrolling.lua

@@ -558,7 +558,7 @@ function ReaderRolling:onResume()
 end
 
 function ReaderRolling:onGotoNextChapter()
-    local visible_page_count = self.ui.document:getVisiblePageCount()
+    local visible_page_count = self.ui.document:getVisiblePageNumberCount()
     local pageno = self.current_page + (visible_page_count > 1 and 1 or 0)
     local new_page
     if self.ui.document:hasHiddenFlows() then
@@ -664,7 +664,7 @@ function ReaderRolling:onGotoXPointer(xp, marker_xp)
                     -- In the middle margin, on the right of text
                     -- Same trick as below, assuming page2_x is equal to page 1 right x
                     screen_x = math.floor(Screen:getWidth() * 0.5)
-                    local page2_x = self.ui.document:getPageOffsetX(self.ui.document:getCurrentPage()+1)
+                    local page2_x = self.ui.document:getPageOffsetX(self.ui.document:getCurrentPage(true)+1)
                     marker_w = page2_x + marker_w - screen_x
                     screen_x = screen_x - marker_w
                 else
@@ -678,7 +678,7 @@ function ReaderRolling:onGotoXPointer(xp, marker_xp)
                     -- This is a bit tricky with how the middle margin is sized
                     -- by crengine (see LVDocView::updateLayout() in lvdocview.cpp)
                     screen_x = math.floor(Screen:getWidth() * 0.5)
-                    local page2_x = self.ui.document:getPageOffsetX(self.ui.document:getCurrentPage()+1)
+                    local page2_x = self.ui.document:getPageOffsetX(self.ui.document:getCurrentPage(true)+1)
                     marker_w = page2_x + marker_w - screen_x
                 end
             end
@@ -767,7 +767,7 @@ function ReaderRolling:onGotoViewRel(diff)
             self.ui:handleEvent(Event:new("EndOfBook"))
         end
     elseif self.view.view_mode == "page" then
-        local page_count = self.ui.document:getVisiblePageCount()
+        local page_count = self.ui.document:getVisiblePageNumberCount()
         local old_page = self.current_page
         -- we're in paged mode, so round up
         if diff > 0 then
@@ -970,8 +970,9 @@ function ReaderRolling:_gotoPercent(new_percent)
     end
 end
 
-function ReaderRolling:_gotoPage(new_page, free_first_page)
+function ReaderRolling:_gotoPage(new_page, free_first_page, internal)
-    if self.ui.document:getVisiblePageCount() > 1 and not free_first_page then
+    if self.ui.document:getVisiblePageCount() > 1 and not free_first_page
+            and (internal or self.ui.document:getVisiblePageNumberCount() == 2) then
         -- Ensure we always have the first of the two pages odd
         if self.odd_or_even_first_page == 1 then -- odd
             if band(new_page, 1) == 0 then
@@ -985,7 +986,7 @@ function ReaderRolling:_gotoPage(new_page, free_first_page)
             end
         end
     end
-    self.ui.document:gotoPage(new_page)
+    self.ui.document:gotoPage(new_page, internal)
     if self.view.view_mode == "page" then
         self.ui:handleEvent(Event:new("PageUpdate", self.ui.document:getCurrentPage()))
     else
@@ -1012,16 +1013,15 @@ end
 --]]
 
 function ReaderRolling:onSetVisiblePages(visible_pages)
-    -- crengine may decide to not ensure the value we request
+    -- By default, crengine may decide to not ensure the value we request
-    -- (for example, in 2-pages mode, it may stop being ensured
+    -- (for example, in 2-pages mode, it may stop being ensured when we
-    -- when we increase the font size up to a point where a line
+    -- increase the font size up to a point where a line would contain
-    -- would contain less that 20 glyphs).
+    -- less that 20 glyphs).
-    -- crengine may enforce visible_page=1 when:
+    -- But we have CreDocument:setVisiblePageCount() provide only_if_sane=false
-    --   - not in page mode but in scroll mode
+    -- so these checks are not done.
-    --   - screen w/h < 6/5
+    -- We nevertheless update the setting (that will be saved) with what
-    --   - w < 20*em
+    -- the user has requested - and not what crengine has enforced, and
-    -- We nevertheless update the setting (that will saved) with what
+    -- always query crengine for if it ends up ensuring it or not.
-    -- the user has requested - and not what crengine has enforced.
     self.visible_pages = visible_pages
     local prev_visible_pages = self.ui.document:getVisiblePageCount()
     self.ui.document:setVisiblePageCount(visible_pages)

frontend/document/credocument.lua

@@ -210,6 +210,13 @@ function CreDocument:setupDefaultView()
     self._document:setIntProperty("crengine.image.scaling.zoomout.inline.mode", 0)
     self._document:setIntProperty("crengine.image.scaling.zoomout.inline.scale", 1)
 
+    -- If switching to two pages on view, we want it to behave like two columns
+    -- and each view to be a single page number (instead of the default of two).
+    -- This ensures that page number and count are consistent between top and
+    -- bottom status bars, that SkimTo -1/+1 don't do nothing every other tap,
+    -- and that reading statistics do not see half of the pages never read.
+    self._document:setIntProperty("window.pages.two.visible.as.one.page.number", 1)
+
     -- set fallback font faces (this was formerly done in :init(), but it
     -- affects crengine calcGlobalSettingsHash() and would invalidate the
     -- cache from the main currently being read document when we just
@@ -312,8 +319,8 @@ function CreDocument:setHideNonlinearFlows(hide_nonlinear_flows)
     end
 end
 
-function CreDocument:getPageCount()
+function CreDocument:getPageCount(internal)
-    return self._document:getPages()
+    return self._document:getPages(internal)
 end
 
 -- Whether the document has any non-linear flow to care about
@@ -693,7 +700,14 @@ function CreDocument:drawCurrentViewByPos(target, x, y, rect, pos)
 end
 
 function CreDocument:drawCurrentViewByPage(target, x, y, rect, page)
-    self._document:gotoPage(page)
+    if not self.no_page_sync then
+        -- Avoid syncing page when this flag is set, when selecting text
+        -- across pages in 2-page mode and flipping half the screen
+        -- (currently only set by ReaderHighlight:onHoldPan())
+        -- self._document:gotoPage(page)
+        -- This allows this method to not be cached by cre call cache
+        self:gotoPage(page)
+    end
     self:drawCurrentView(target, x, y, rect)
 end
 
@@ -750,8 +764,9 @@ function CreDocument:getScreenPositionFromXPointer(xp)
     if self._view_mode == self.PAGE_VIEW_MODE then
         if self:getVisiblePageCount() > 1 then
             -- Correct coordinates if on the 2nd page in 2-pages mode
-            local next_page = self:getCurrentPage() + 1
+            -- getPageStartY() and getPageOffsetX() expects internal page numbers
-            if next_page <= self:getPageCount() then
+            local next_page = self:getCurrentPage(true) + 1
+            if next_page <= self:getPageCount(true) then
                 local next_top_y = self._document:getPageStartY(next_page)
                 if doc_y >= next_top_y then
                     screen_y = doc_y - next_top_y
@@ -831,9 +846,9 @@ function CreDocument:gotoPos(pos)
     self._document:gotoPos(pos)
 end
 
-function CreDocument:gotoPage(page)
+function CreDocument:gotoPage(page, internal)
     logger.dbg("CreDocument: goto page", page, "flow", self:getPageFlow(page))
-    self._document:gotoPage(page)
+    self._document:gotoPage(page, internal)
 end
 
 function CreDocument:gotoLink(link)
@@ -851,8 +866,8 @@ function CreDocument:goForward(link)
     self._document:goForward()
 end
 
-function CreDocument:getCurrentPage()
+function CreDocument:getCurrentPage(internal)
-    return self._document:getCurrentPage()
+    return self._document:getCurrentPage(internal)
 end
 
 function CreDocument:setFontFace(new_font_face)
@@ -1145,6 +1160,8 @@ function CreDocument:setTxtPreFormatted(enabled)
     self._document:setIntProperty("crengine.file.txt.preformatted", enabled)
 end
 
+-- get crengine internal visible page count (to be used when doing specific
+-- screen position handling)
 function CreDocument:getVisiblePageCount()
     return self._document:getVisiblePageCount()
 end
@@ -1154,6 +1171,11 @@ function CreDocument:setVisiblePageCount(new_count)
     self._document:setVisiblePageCount(new_count, false)
 end
 
+-- get visible page number count (to be used when only interested in page numbers)
+function CreDocument:getVisiblePageNumberCount()
+    return self._document:getVisiblePageNumberCount()
+end
+
 function CreDocument:setBatteryState(state)
     logger.dbg("CreDocument: set battery state", state)
     self._document:setBatteryState(state)
@@ -1519,6 +1541,7 @@ function CreDocument:setupCallCache()
             local cache_global = false
             local set_tag = nil
             local set_arg = nil
+            local set_arg2 = nil
             local is_cached = false
 
             -- Assume all set* may change rendering
@@ -1541,10 +1564,14 @@ function CreDocument:setupCallCache()
             elseif name == "findText" then add_buffer_trash = true
 
             -- These may change page/pos
-            elseif name == "gotoPage" then set_tag = "page" ; set_arg = 2
+            elseif name == "gotoPage" then set_tag = "page" ; set_arg = 2 ; set_arg2 = 3
             elseif name == "gotoPos" then set_tag = "pos" ; set_arg = 2
-            elseif name == "drawCurrentViewByPage" then set_tag = "page" ; set_arg = 6
             elseif name == "drawCurrentViewByPos" then set_tag = "pos" ; set_arg = 6
+            -- elseif name == "drawCurrentViewByPage" then set_tag = "page" ; set_arg = 6
+            -- drawCurrentViewByPage() has some tweaks when browsing half-pages for
+            -- text selection in two-pages mode: no need to wrap it, as it uses
+            -- internally 2 other functions that are themselves wrapped
+
             -- gotoXPointer() is for cre internal fixup, we always use gotoPage/Pos
             -- (goBack, goForward, gotoLink are not used)
 
@@ -1554,6 +1581,7 @@ function CreDocument:setupCallCache()
             elseif name == "getCurrentPage" then no_wrap = true
             elseif name == "getCurrentPos" then no_wrap = true
             elseif name == "getVisiblePageCount" then no_wrap = true
+            elseif name == "getVisiblePageNumberCount" then no_wrap = true
             elseif name == "getCoverPageImage" then no_wrap = true
             elseif name == "getDocumentFileContent" then no_wrap = true
             elseif name == "getHTMLFromXPointer" then no_wrap = true
@@ -1611,6 +1639,12 @@ function CreDocument:setupCallCache()
                 self[name] = function(...)
                     if do_log then logger.dbg("callCache:", name, "setting tag") end
                     local val = select(set_arg, ...)
+                    if set_arg2 then
+                        local val2 = select(set_arg2, ...)
+                        if val2 ~= nil then
+                            val = val .. tostring(val2)
+                        end
+                    end
                     self._callCacheSetCurrentTag(set_tag .. val)
                     return func(...)
                 end

frontend/ui/data/creoptions.lua

@@ -33,7 +33,7 @@ local CreOptions = {
             },
             {
                 name = "visible_pages",
-                name_text = _("Dual Pages"),
+                name_text = _("Two Columns"),
                 toggle = {_("off"), _("on")},
                 values = {1, 2},
                 default_value = 1,
@@ -55,8 +55,8 @@ local CreOptions = {
                         -- and Device.screen:getScreenMode() == "landscape"
                 end,
                 name_text_hold_callback = optionsutil.showValues,
-                help_text = _([[In landscape mode, you can choose to display one or two pages of the book on the screen.
+                help_text = _([[Render the document on half the screen width and display two pages at once with a single page number. This makes it look like two columns.
-Note that this may not be ensured under some conditions: in scroll mode, when a very big font size is used, or on devices with a very low aspect ratio.]]),
+This is disabled in scroll mode. Switching from page mode with two columns to scroll mode will cause the document to be re-rendered.]]),
             },
         }
     },