ReaderLink: Minor logic simplification in onGoToPageLink (#9987)

Abort earlier if the nearest link is too far, instead of computing stuff and creating an object we'll never actually use.

Includes minor logging tweaks to vaguely related codepaths ;p.
This commit is contained in:
NiLuJe
2023-01-03 20:48:49 +01:00
committed by GitHub
parent a8b333e4f9
commit 4ce0058e2d
3 changed files with 51 additions and 58 deletions

View File

@@ -537,7 +537,7 @@ function ReaderLink:isXpointerCoherent(a_xpointer)
-- but easier to workaround here that way)
re_link_xpointer, re_a_xpointer = self.ui.document:getLinkFromPosition({x = screen_x+1, y = screen_y}) -- luacheck: no unused
if re_a_xpointer ~= a_xpointer then
logger.info("not coherent a_xpointer:", a_xpointer)
logger.info("noncoherent a_xpointer:", a_xpointer)
return false
end
end
@@ -563,8 +563,7 @@ function ReaderLink:getLinkFromGes(ges)
end
else
local link_xpointer, a_xpointer = self.ui.document:getLinkFromPosition(ges.pos)
logger.dbg("getLinkFromPosition link_xpointer:", link_xpointer)
logger.dbg("getLinkFromPosition a_xpointer:", a_xpointer)
logger.dbg("ReaderLink:getLinkFromPosition @", ges.pos.x, ges.pos.y, "from a_xpointer:", a_xpointer, "to link_xpointer:", link_xpointer)
-- On some documents, crengine may sometimes give a wrong a_xpointer
-- (in some Wikipedia saved as EPUB, it would point to some other <A>
@@ -580,8 +579,8 @@ function ReaderLink:getLinkFromGes(ges)
-- This link's source xpointer is more precise than a classic
-- xpointer to top of a page: we can show a marker at its
-- y-position in target page
-- (keep a_xpointer even if incoherent, might be needed for
-- footnote detection (better than nothing if incoherent)
-- (keep a_xpointer even if noncoherent, might be needed for
-- footnote detection (better than nothing if noncoherent)
return {
xpointer = link_xpointer,
marker_xpointer = link_xpointer,
@@ -703,12 +702,11 @@ end
-- (This is called by other modules (highlight, search) to jump to a xpointer,
-- they should not provide allow_footnote_popup=true)
function ReaderLink:onGotoLink(link, neglect_current_location, allow_footnote_popup)
logger.dbg("onGotoLink:", link)
local link_url
if self.ui.document.info.has_pages then
-- internal pdf links have a "page" attribute, while external ones have an "uri" attribute
if link.page then -- Internal link
logger.dbg("Internal link:", link)
logger.dbg("ReaderLink:onGotoLink: Internal link:", link)
if not neglect_current_location then
self:addCurrentLocationToStack()
end
@@ -724,7 +722,7 @@ function ReaderLink:onGotoLink(link, neglect_current_location, allow_footnote_po
-- Best to check that this link exists in document with the following,
-- which accepts both of the above legitimate xpointer as input.
if self.ui.document:isXPointerInDocument(link.xpointer) then
logger.dbg("Internal link:", link)
logger.dbg("ReaderLink:onGotoLink: Internal link:", link)
if allow_footnote_popup then
if self:showAsFootnotePopup(link, neglect_current_location) then
return true
@@ -763,7 +761,7 @@ function ReaderLink:onGotoLink(link, neglect_current_location, allow_footnote_po
end
link_url = link.xpointer -- external link
end
logger.dbg("External link:", link_url)
logger.dbg("ReaderLink:onGotoLink: External link:", link_url)
local is_http_link = link_url:find("^https?://") ~= nil
if is_http_link and self:onGoToExternalLink(link_url) then
@@ -876,10 +874,9 @@ end
--- Goes to link nearest to the gesture (or first link in page)
function ReaderLink:onGoToPageLink(ges, internal_links_only, max_distance)
local selected_link = nil
local selected_distance2 = nil
-- We use squares of distances all along the calculations, no need
-- to math.sqrt() them when comparing
local selected_link, selected_distance2
-- We use squared distances throughout the computations,
-- no need to math.sqrt() anything for comparisons.
if self.ui.document.info.has_pages then
local pos = self.view:screenToPageTransform(ges.pos)
if not pos then
@@ -901,7 +898,7 @@ function ReaderLink:onGoToPageLink(ges, internal_links_only, max_distance)
-- ["page"] = 347
-- },
local pos_x, pos_y = pos.x, pos.y
local shortest_dist = nil
local shortest_dist
for _, link in ipairs(links) do
if not internal_links_only or link.page then
local start_dist = (link.x0 - pos_x)^2 + (link.y0 - pos_y)^2
@@ -917,15 +914,16 @@ function ReaderLink:onGoToPageLink(ges, internal_links_only, max_distance)
end
if shortest_dist then
selected_distance2 = shortest_dist
if max_distance and selected_distance2 > max_distance^2 then
selected_link = nil
end
end
else
-- Getting segments on a page with many internal links is
-- a bit expensive. With larger_tap_area_to_follow_links=true,
-- this is done for each tap on screen (changing pages, showing
-- menu...). We might want to cache these links per page (and
-- clear that cache when page layout change).
-- If we care only about internal links, request them only
-- (and avoid that expensive segments work on external links)
-- Getting segments on a page with many internal links is a bit expensive.
-- With larger_tap_area_to_follow_links, this is done on every tap, page turn or not.
-- getPageLinks goes through the CRe call cache, so at least repeat calls are cheaper.
-- If we only care about internal links, we only request those.
-- That expensive segments work is always skipped on external links.
local links = self.ui.document:getPageLinks(internal_links_only)
if not links or #links == 0 then
return
@@ -976,7 +974,7 @@ function ReaderLink:onGoToPageLink(ges, internal_links_only, max_distance)
-- coordinates, and our code below may miss or give the wrong first
-- or nearest link...
local pos_x, pos_y = ges.pos.x, ges.pos.y
local shortest_dist = nil
local shortest_dist
for _, link in ipairs(links) do
-- link.uri may be an empty string with some invalid links: ignore them
if link.section or (link.uri and link.uri ~= "") then
@@ -1040,35 +1038,34 @@ function ReaderLink:onGoToPageLink(ges, internal_links_only, max_distance)
end
if shortest_dist then
selected_distance2 = shortest_dist
end
if selected_link then
logger.dbg("nearest selected_link", selected_link)
-- Check a_xpointer is coherent, use it as from_xpointer only if it is
local from_xpointer = nil
if selected_link.a_xpointer and self:isXpointerCoherent(selected_link.a_xpointer) then
from_xpointer = selected_link.a_xpointer
if max_distance and selected_distance2 > max_distance^2 then
logger.dbg("nearest link is further than max distance, ignoring it")
selected_link = nil
else
logger.dbg("nearest selected_link", selected_link)
-- Check if a_xpointer is coherent, use it as from_xpointer only if it is
local from_xpointer = nil
if selected_link.a_xpointer and self:isXpointerCoherent(selected_link.a_xpointer) then
from_xpointer = selected_link.a_xpointer
end
-- Make it a link as expected by onGotoLink
selected_link = {
xpointer = selected_link.section or selected_link.uri,
marker_xpointer = selected_link.section,
from_xpointer = from_xpointer,
-- (keep a_xpointer even if noncoherent, might be needed for
-- footnote detection (better than nothing if noncoherent)
a_xpointer = selected_link.a_xpointer,
-- keep the link y position, so we can keep its highlight shown
-- a bit more time if it was hidden by the footnote popup
link_y = selected_link.link_y,
}
end
-- Make it a link as expected by onGotoLink
selected_link = {
xpointer = selected_link.section or selected_link.uri,
marker_xpointer = selected_link.section,
from_xpointer = from_xpointer,
-- (keep a_xpointer even if incoherent, might be needed for
-- footnote detection (better than nothing if incoherent)
a_xpointer = selected_link.a_xpointer,
-- keep the link y position, so we can keep its highlight shown
-- a bit more time if it was hidden by the footnote popup
link_y = selected_link.link_y,
}
end
end
if selected_link then
if max_distance and selected_distance2 and selected_distance2 > max_distance^2 then
logger.dbg("nearest link is further than max distance, ignoring it")
else
return self:onGotoLink(selected_link, false, isFootnoteLinkInPopupEnabled())
end
return self:onGotoLink(selected_link, false, isFootnoteLinkInPopupEnabled())
end
end
@@ -1136,8 +1133,8 @@ function ReaderLink:selectRelPageLink(rel)
xpointer = selected_link.section or selected_link.uri,
marker_xpointer = selected_link.section,
from_xpointer = from_xpointer,
-- (keep a_xpointer even if incoherent, might be needed for
-- footnote detection (better than nothing if incoherent)
-- (keep a_xpointer even if noncoherent, might be needed for
-- footnote detection (better than nothing if noncoherent)
a_xpointer = selected_link.a_xpointer,
-- keep the link y position, so we can keep its highlight shown
-- a bit more time if it was hidden by the footnote popup