diff --git a/base b/base index e8385ad08..ce27f6ed7 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit e8385ad081470ba8aab214a187887b7870c1ada2 +Subproject commit ce27f6ed762ec1046c94a540cc0d555eaeda9efd diff --git a/frontend/apps/reader/modules/readerrolling.lua b/frontend/apps/reader/modules/readerrolling.lua index 547375c2a..46bd7b1a6 100644 --- a/frontend/apps/reader/modules/readerrolling.lua +++ b/frontend/apps/reader/modules/readerrolling.lua @@ -1,4 +1,5 @@ local InputContainer = require("ui/widget/container/inputcontainer") +local ReaderPanning = require("apps/reader/modules/readerpanning") local Screen = require("ui/screen") local Device = require("ui/device") local Geom = require("ui/geometry") @@ -8,8 +9,27 @@ local GestureRange = require("ui/gesturerange") local UIManager = require("ui/uimanager") local DEBUG = require("dbg") local _ = require("gettext") -local ReaderPanning = require("apps/reader/modules/readerpanning") +--[[ + Rolling is just like paging in page-based documents except that + sometimes (in scroll mode) there is no concept of page number to indicate + current progress. + There are three kind of progress measurements for credocuments. + 1. page number (in page mode) + 2. progress percentage (in scroll mode) + 3. xpointer (in document dom structure) + We found that the first two measurements are not suitable for keeping a + record of the real progress. For example, when switching screen orientation + from portrait to landscape, or switching view mode from page to scroll, the + internal xpointer should not be used as the view dimen/mode is changed and + crengine's pagination mechanism will find a closest xpointer for the new view. + So if we change the screen orientation or view mode back, we cannot find the + original place since the internal xpointer is changed, which is counter- + intuitive as users didn't goto any other page. + The solution is that we keep a record of the internal xpointer and only change + it in explicit page turning. And use that xpointer for non-page-turning + rendering. +--]] local ReaderRolling = InputContainer:new{ old_doc_height = nil, old_page = nil, @@ -17,6 +37,7 @@ local ReaderRolling = InputContainer:new{ -- only used for page view mode current_page= nil, doc_height = nil, + xpointer = nil, panning_steps = ReaderPanning.panning_steps, show_overlap_enable = true, overlap = 20, @@ -152,36 +173,36 @@ function ReaderRolling:onReadSettings(config) self.show_overlap_enable = soe end local last_xp = config:readSetting("last_xpointer") + local last_per = config:readSetting("last_percent") if last_xp then table.insert(self.ui.postInitCallback, function() - self:gotoXPointer(last_xp) + self.xpointer = last_xp + self:gotoXPointer(self.xpointer) -- we have to do a real jump in self.ui.document._document to -- update status information in CREngine. - self.ui.document:gotoXPointer(last_xp) + self.ui.document:gotoXPointer(self.xpointer) end) - end -- we read last_percent just for backward compatibility - if not last_xp then - local last_per = config:readSetting("last_percent") - if last_per then - table.insert(self.ui.postInitCallback, function() - self:gotoPercent(last_per) - -- we have to do a real pos change in self.ui.document._document - -- to update status information in CREngine. - self.ui.document:gotoPos(self.current_pos) - end) + elseif last_per then + table.insert(self.ui.postInitCallback, function() + self:gotoPercent(last_per) + -- we have to do a real pos change in self.ui.document._document + -- to update status information in CREngine. + self.ui.document:gotoPos(self.current_pos) + self.xpointer = self.ui.document:getXPointer() + end) + else + if self.view.view_mode == "page" then + self.ui:handleEvent(Event:new("PageUpdate", 1)) end + self.xpointer = self.ui.document:getXPointer() end - if self.view.view_mode == "page" then - self.ui:handleEvent(Event:new("PageUpdate", self.ui.document:getCurrentPage())) - end - self.ui:handleEvent(Event:new("UpdatePos")) end function ReaderRolling:onSaveSettings() -- remove last_percent config since its deprecated self.ui.doc_settings:saveSetting("last_percent", nil) - self.ui.doc_settings:saveSetting("last_xpointer", self.ui.document:getXPointer()) + self.ui.doc_settings:saveSetting("last_xpointer", self.xpointer) self.ui.doc_settings:saveSetting("percent_finished", self:getLastPercent()) end @@ -247,7 +268,8 @@ function ReaderRolling:onResume() end function ReaderRolling:onDoubleTapForward() - local pageno = self.current_page + self.ui.document:getVisiblePageCount() + local visible_page_count = self.ui.document:getVisiblePageCount() + local pageno = self.current_page + (visible_page_count > 1 and 1 or 0) self:onGotoPage(self.ui.toc:getNextChapter(pageno, 0)) return true end @@ -268,6 +290,14 @@ function ReaderRolling:onGotoPercent(percent) return true end +function ReaderRolling:onGotoPage(number) + if number then + self:gotoPage(number) + end + self.xpointer = self.ui.document:getXPointer() + return true +end + function ReaderRolling:onGotoViewRel(diff) DEBUG("goto relative screen:", diff, ", in mode: ", self.view.view_mode) if self.view.view_mode == "scroll" then @@ -284,6 +314,7 @@ function ReaderRolling:onGotoViewRel(diff) local page_count = self.ui.document:getVisiblePageCount() self:gotoPage(self.current_page + diff*page_count) end + self.xpointer = self.ui.document:getXPointer() return true end @@ -292,6 +323,7 @@ function ReaderRolling:onPanning(args, key) local _, dy = unpack(args) DEBUG("key =", key) self:gotoPos(self.current_pos + dy * self.panning_steps.normal) + self.xpointer = self.ui.document:getXPointer() return true end @@ -303,6 +335,7 @@ end --[[ remember to signal this event when the document has been zoomed, font has been changed, or line height has been changed. + Note that xpointer should not be changed. --]] function ReaderRolling:onUpdatePos() UIManager:scheduleIn(0.1, function () self:updatePos() end) @@ -316,7 +349,7 @@ function ReaderRolling:updatePos() local new_height = self.ui.document.info.doc_height local new_page = self.ui.document.info.number_of_pages if self.old_doc_height ~= new_height or self.old_page ~= new_page then - self:gotoXPointer(self.ui.document:getXPointer()) + self:gotoXPointer(self.xpointer) self.old_doc_height = new_height self.old_page = new_page self.ui:handleEvent(Event:new("UpdateToc")) @@ -324,6 +357,7 @@ function ReaderRolling:updatePos() UIManager.repaint_all = true end +-- FIXME: there should no other way to update xpointer function ReaderRolling:onUpdateXPointer() local xp = self.ui.document:getXPointer() if self.view.view_mode == "page" then @@ -334,16 +368,20 @@ function ReaderRolling:onUpdateXPointer() return true end +--[[ + switching screen mode should not change current page number +--]] function ReaderRolling:onChangeViewMode() self.ui.document:_readMetadata() self.old_doc_height = self.ui.document.info.doc_height self.old_page = self.ui.document.info.number_of_pages self.ui:handleEvent(Event:new("UpdateToc")) - self:gotoXPointer(self.ui.document:getXPointer()) - if self.view.view_mode == "scroll" then - self.current_pos = self.ui.document:getCurrentPos() + if self.xpointer then + self:gotoXPointer(self.xpointer) else - self.current_page = self.ui.document:getCurrentPage() + table.insert(self.ui.postInitCallback, function() + self:gotoXPointer(self.xpointer) + end) end return true end @@ -357,15 +395,17 @@ function ReaderRolling:onRedrawCurrentView() return true end -function ReaderRolling:onSetDimensions() +function ReaderRolling:onSetDimensions(dimen) -- update listening according to new screen dimen if Device:isTouchDevice() then self:initGesListener() end + self.ui.document:setViewDimen(Screen:getSize()) end function ReaderRolling:onChangeScreenMode(mode) self.ui:handleEvent(Event:new("SetScreenMode", mode)) + self.ui.document:setViewDimen(Screen:getSize()) self:onChangeViewMode() self:onUpdatePos() end @@ -392,6 +432,10 @@ function ReaderRolling:gotoPos(new_pos) self.ui:handleEvent(Event:new("PosUpdate", new_pos)) end +function ReaderRolling:gotoPercent(new_percent) + self:gotoPos(new_percent * self.doc_height / 10000) +end + function ReaderRolling:gotoPage(new_page) self.ui.document:gotoPage(new_page) self.ui:handleEvent(Event:new("PageUpdate", self.ui.document:getCurrentPage())) @@ -405,17 +449,6 @@ function ReaderRolling:gotoXPointer(xpointer) end end -function ReaderRolling:gotoPercent(new_percent) - self:gotoPos(new_percent * self.doc_height / 10000) -end - -function ReaderRolling:onGotoPage(number) - if number then - self:gotoPage(number) - end - return true -end - --[[ currently we don't need to get page links on each page/pos update since we can check link on the fly when tapping on the screen diff --git a/frontend/document/credocument.lua b/frontend/document/credocument.lua index 41566c6a1..88ced31f7 100644 --- a/frontend/document/credocument.lua +++ b/frontend/document/credocument.lua @@ -330,6 +330,11 @@ function CreDocument:setViewMode(new_mode) end end +function CreDocument:setViewDimen(dimen) + DEBUG("CreDocument: set view dimen", dimen) + self._document:setViewDimen(dimen.w, dimen.h) +end + function CreDocument:setHeaderFont(new_font) if new_font then DEBUG("CreDocument: set header font", new_font) diff --git a/spec/unit/readerrolling_spec.lua b/spec/unit/readerrolling_spec.lua new file mode 100644 index 000000000..16efc6e6b --- /dev/null +++ b/spec/unit/readerrolling_spec.lua @@ -0,0 +1,109 @@ +require("commonrequire") +local DocumentRegistry = require("document/documentregistry") +local ReaderUI = require("apps/reader/readerui") +local Event = require("ui/event") +local DEBUG = require("dbg") + +describe("Readerrolling module", function() + local sample_epub = "spec/front/unit/data/juliet.epub" + local readerui = ReaderUI:new{ + document = DocumentRegistry:openDocument(sample_epub), + } + local rolling = readerui.rolling + describe("test in portrait screen mode", function() + it("should goto portrait screen mode", function() + readerui:handleEvent(Event:new("ChangeScreenMode", "portrait")) + end) + it("should goto certain page", function() + for i = 1, 10, 5 do + rolling:gotoPage(i) + assert.are.same(i, rolling.current_page) + end + end) + it("should goto relative page", function() + for i = 20, 40, 5 do + rolling:gotoPage(i) + rolling:onGotoViewRel(1) + assert.are.same(i + 1, rolling.current_page) + rolling:onGotoViewRel(-1) + assert.are.same(i, rolling.current_page) + end + end) + it("should goto next chapter", function() + local toc = readerui.toc + for i = 30, 50, 5 do + rolling:gotoPage(i) + rolling:onDoubleTapForward() + assert.are.same(toc:getNextChapter(i, 0), rolling.current_page) + end + end) + it("should goto previous chapter", function() + local toc = readerui.toc + for i = 60, 80, 5 do + rolling:gotoPage(i) + rolling:onDoubleTapBackward() + assert.are.same(toc:getPreviousChapter(i, 0), rolling.current_page) + end + end) + end) + describe("test in landscape screen mode", function() + it("should go to landscape screen mode", function() + readerui:handleEvent(Event:new("ChangeScreenMode", "landscape")) + end) + it("should goto certain page", function() + for i = 1, 10, 5 do + rolling:gotoPage(i) + assert.are.same(i, rolling.current_page) + end + end) + it("should goto relative page", function() + for i = 20, 40, 5 do + rolling:gotoPage(i) + rolling:onGotoViewRel(1) + assert.are.same(i + 1, rolling.current_page) + rolling:onGotoViewRel(-1) + assert.are.same(i, rolling.current_page) + end + end) + it("should goto next chapter", function() + local toc = readerui.toc + for i = 30, 50, 5 do + rolling:gotoPage(i) + rolling:onDoubleTapForward() + assert.are.same(toc:getNextChapter(i, 0), rolling.current_page) + end + end) + it("should goto previous chapter", function() + local toc = readerui.toc + for i = 60, 80, 5 do + rolling:gotoPage(i) + rolling:onDoubleTapBackward() + assert.are.same(toc:getPreviousChapter(i, 0), rolling.current_page) + end + end) + end) + describe("switching screen mode should not change current page number", function() + it("for portrait-landscape-portrait switching", function() + for i = 80, 100, 10 do + readerui:handleEvent(Event:new("ChangeScreenMode", "portrait")) + rolling:onGotoPage(i) + assert.are.same(i, rolling.current_page) + readerui:handleEvent(Event:new("ChangeScreenMode", "landscape")) + assert.are_not.same(i, rolling.current_page) + readerui:handleEvent(Event:new("ChangeScreenMode", "portrait")) + assert.are.same(i, rolling.current_page) + end + end) + it("for landscape-portrait-landscape switching", function() + for i = 110, 130, 10 do + readerui:handleEvent(Event:new("ChangeScreenMode", "landscape")) + rolling:onGotoPage(i) + assert.are.same(i, rolling.current_page) + readerui:handleEvent(Event:new("ChangeScreenMode", "portrait")) + assert.are_not.same(i, rolling.current_page) + readerui:handleEvent(Event:new("ChangeScreenMode", "landscape")) + assert.are.same(i, rolling.current_page) + end + end) + end) +end)