From 2697a30166a4e15e303fb2e29f246191e16357f1 Mon Sep 17 00:00:00 2001 From: poire-z Date: Mon, 20 Nov 2017 21:58:58 +0100 Subject: [PATCH] [fix] Optimize cre highlights onTap and drawing (#3508) drawXPointerSavedHighlight() and onTapXPointerSavedHighlight were looping thru all credocuments highlights, which was expensive. Now, we first check with cheaper getPageFromXPointer() the highlights are on the current page before doing more expensive stuff. Closes #3503. --- .../apps/reader/modules/readerhighlight.lua | 37 +++++++++++++---- frontend/apps/reader/modules/readerview.lua | 41 ++++++++++++++----- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua index a93675a3b..9b2aa5c35 100644 --- a/frontend/apps/reader/modules/readerhighlight.lua +++ b/frontend/apps/reader/modules/readerhighlight.lua @@ -178,7 +178,7 @@ function ReaderHighlight:onTapPageSavedHighlight(ges) if boxes then for index, box in pairs(boxes) do if inside_box(pos, box) then - logger.dbg("Tap on hightlight") + logger.dbg("Tap on highlight") return self:onShowHighlightDialog(page, i) end end @@ -189,18 +189,39 @@ function ReaderHighlight:onTapPageSavedHighlight(ges) end function ReaderHighlight:onTapXPointerSavedHighlight(ges) + local cur_page + -- In scroll mode, we'll need to check for highlights in previous or next + -- page too as some parts of them may be displayed + local neighbour_pages = self.view.view_mode ~= "page" and 1 or 0 local pos = self.view:screenToPageTransform(ges.pos) for page, _ in pairs(self.view.highlight.saved) do local items = self.view.highlight.saved[page] if items then for i = 1, #items do + if not cur_page then + cur_page = self.ui.document:getPageFromXPointer(self.ui.document:getXPointer()) + end local pos0, pos1 = items[i].pos0, items[i].pos1 - local boxes = self.ui.document:getScreenBoxesFromPositions(pos0, pos1) - if boxes then - for index, box in pairs(boxes) do - if inside_box(pos, box) then - logger.dbg("Tap on hightlight") - return self:onShowHighlightDialog(page, i) + -- document:getScreenBoxesFromPositions() is expensive, so we + -- first check this item is on current page + local page0 = self.ui.document:getPageFromXPointer(pos0) + local page1 = self.ui.document:getPageFromXPointer(pos1) + local start_page = math.min(page0, page1) + local end_page = math.max(page0, page1) + -- In scroll mode, we may be displaying cur_page and cur_page+1, so + -- we have to check the highlight start_page is <= cur_page+1. + -- Same thinking with highlight's end_page >= cur_page-1 as we may + -- be displaying a part of cur_page-1. + -- (A highlight starting on cur_page-17 and ending on cur_page+13 is + -- a highlight to consider) + if start_page <= cur_page + neighbour_pages and end_page >= cur_page - neighbour_pages then + local boxes = self.ui.document:getScreenBoxesFromPositions(pos0, pos1) + if boxes then + for index, box in pairs(boxes) do + if inside_box(pos, box) then + logger.dbg("Tap on highlight") + return self:onShowHighlightDialog(page, i) + end end end end @@ -561,7 +582,7 @@ function ReaderHighlight:saveHighlight() self.ui.bookmark:addBookmark(bookmark_item) end --[[ - -- disable exporting hightlights to My Clippings + -- disable exporting highlights to My Clippings -- since it's not portable and there is a better Evernote plugin -- to do the same thing if self.selected_text.text ~= "" then diff --git a/frontend/apps/reader/modules/readerview.lua b/frontend/apps/reader/modules/readerview.lua index 25626d0f7..8ad724dad 100644 --- a/frontend/apps/reader/modules/readerview.lua +++ b/frontend/apps/reader/modules/readerview.lua @@ -34,7 +34,7 @@ local ReaderView = OverlapGroup:extend{ bbox = nil, }, outer_page_color = Blitbuffer.gray(DOUTER_PAGE_COLOR/15), - -- hightlight with "lighten" or "underscore" or "invert" + -- highlight with "lighten" or "underscore" or "invert" highlight = { lighten_factor = 0.2, temp_drawer = "invert", @@ -473,21 +473,42 @@ function ReaderView:drawPageSavedHighlight(bb, x, y) end function ReaderView:drawXPointerSavedHighlight(bb, x, y) + local cur_page + -- In scroll mode, we'll need to check for highlights in previous or next + -- page too as some parts of them may be displayed + local neighbour_pages = self.view_mode ~= "page" and 1 or 0 for page, _ in pairs(self.highlight.saved) do local items = self.highlight.saved[page] if not items then items = {} end for j = 1, #items do + if not cur_page then + cur_page = self.ui.document:getPageFromXPointer(self.ui.document:getXPointer()) + end local item = items[j] local pos0, pos1 = item.pos0, item.pos1 - local boxes = self.ui.document:getScreenBoxesFromPositions(pos0, pos1) - if boxes then - for _, box in pairs(boxes) do - local rect = self:pageToScreenTransform(page, box) - if rect then - self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer) - end - end -- end for each box - end -- end if boxes + -- document:getScreenBoxesFromPositions() is expensive, so we + -- first check this item is on current page + local page0 = self.ui.document:getPageFromXPointer(pos0) + local page1 = self.ui.document:getPageFromXPointer(pos1) + local start_page = math.min(page0, page1) + local end_page = math.max(page0, page1) + -- In scroll mode, we may be displaying cur_page and cur_page+1, so + -- we have to check the highlight start_page is <= cur_page+1. + -- Same thinking with highlight's end_page >= cur_page-1 as we may + -- be displaying a part of cur_page-1. + -- (A highlight starting on cur_page-17 and ending on cur_page+13 is + -- a highlight to consider) + if start_page <= cur_page + neighbour_pages and end_page >= cur_page - neighbour_pages then + local boxes = self.ui.document:getScreenBoxesFromPositions(pos0, pos1) + if boxes then + for _, box in pairs(boxes) do + local rect = self:pageToScreenTransform(page, box) + if rect then + self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer) + end + end -- end for each box + end -- end if boxes + end end -- end for each highlight end -- end for all saved highlight end