From 7a0e3d5e68e4d119cdd60060a01303380e497c74 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sat, 23 Oct 2021 21:12:56 +1100 Subject: [PATCH] readerhighlight: remove selected_word and use selected_text everywhere There were two ways of specifing selected text for a highlight depending on whether it was a "single word" or text selected using hold-and-pan. In addition to being a bit more complicated than is necessary, with the addition of the language support plugin system (where the "single word" selected might be expanded), it makes more sense to simply use the same logic and table structure for both cases. The dictionary lookup special case (hold-without-pan triggering a dictionary lookup by default) still works as before. In addition, this patch fixes a minor inefficiency during dictionary quick lookup -- before this patch, the highlight would be re-selected because the quick lookup window is run concurrently and tries to fetch ReaderHighlight.selected_text but this is set to nil immediately after triggering the lookup. This is unnecessary because :clear() will be called anyway when the quick pop-up closes, and so clearing this can be left until then. Signed-off-by: Aleksa Sarai --- .../apps/reader/modules/readerdictionary.lua | 18 ++--- .../apps/reader/modules/readerhighlight.lua | 80 ++++++++++++------- frontend/ui/widget/dictquicklookup.lua | 34 +++++--- 3 files changed, 83 insertions(+), 49 deletions(-) diff --git a/frontend/apps/reader/modules/readerdictionary.lua b/frontend/apps/reader/modules/readerdictionary.lua index 6a02b959a..e9c9f3dd3 100644 --- a/frontend/apps/reader/modules/readerdictionary.lua +++ b/frontend/apps/reader/modules/readerdictionary.lua @@ -398,8 +398,8 @@ function ReaderDictionary:addToMainMenu(menu_items) end end -function ReaderDictionary:onLookupWord(word, is_sane, box, highlight, link) - logger.dbg("dict lookup word:", word, box) +function ReaderDictionary:onLookupWord(word, is_sane, boxes, highlight, link) + logger.dbg("dict lookup word:", word, boxes) -- escape quotes and other funny characters in word word = self:cleanSelection(word, is_sane) logger.dbg("dict stripped word:", word) @@ -408,7 +408,7 @@ function ReaderDictionary:onLookupWord(word, is_sane, box, highlight, link) -- Wrapped through Trapper, as we may be using Trapper:dismissablePopen() in it Trapper:wrap(function() - self:stardictLookup(word, self.enabled_dict_names, not self.disable_fuzzy_search, box, link) + self:stardictLookup(word, self.enabled_dict_names, not self.disable_fuzzy_search, boxes, link) end) return true end @@ -447,7 +447,7 @@ function ReaderDictionary:onHtmlDictionaryLinkTapped(dictionary, link) -- Wrapped through Trapper, as we may be using Trapper:dismissablePopen() in it Trapper:wrap(function() - self:stardictLookup(word, {dictionary}, false, link_box, nil) + self:stardictLookup(word, {dictionary}, false, {link_box}, nil) end) end @@ -815,7 +815,7 @@ function ReaderDictionary:startSdcv(word, dict_names, fuzzy_search) return final_results end -function ReaderDictionary:stardictLookup(word, dict_names, fuzzy_search, box, link) +function ReaderDictionary:stardictLookup(word, dict_names, fuzzy_search, boxes, link) if word == "" then return end @@ -859,7 +859,7 @@ function ReaderDictionary:stardictLookup(word, dict_names, fuzzy_search, box, li lookup_cancelled = false, } } - self:showDict(word, nope, box, link) + self:showDict(word, nope, boxes, link) return end @@ -877,10 +877,10 @@ function ReaderDictionary:stardictLookup(word, dict_names, fuzzy_search, box, li return end - self:showDict(word, tidyMarkup(results), box, link) + self:showDict(word, tidyMarkup(results), boxes, link) end -function ReaderDictionary:showDict(word, results, box, link) +function ReaderDictionary:showDict(word, results, boxes, link) if results and results[1] then logger.dbg("showing quick lookup window", word, results) self.dict_window = DictQuickLookup:new{ @@ -893,7 +893,7 @@ function ReaderDictionary:showDict(word, results, box, link) -- selected link, if any selected_link = link, results = results, - word_box = box, + word_boxes = boxes, preferred_dictionaries = self.preferred_dictionaries, -- differentiate between dict and wiki is_wiki = self.is_wiki, diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua index 0092372ab..72d5bdb91 100644 --- a/frontend/apps/reader/modules/readerhighlight.lua +++ b/frontend/apps/reader/modules/readerhighlight.lua @@ -2,6 +2,7 @@ local BD = require("ui/bidi") local ButtonDialog = require("ui/widget/buttondialog") local Device = require("device") local Event = require("ui/event") +local Geom = require("ui/geometry") local InfoMessage = require("ui/widget/infomessage") local InputContainer = require("ui/widget/container/inputcontainer") local Notification = require("ui/widget/notification") @@ -436,6 +437,7 @@ function ReaderHighlight:clear(clear_id) self.restore_page_mode_func() self.restore_page_mode_func = nil end + self.is_word_selection = false self.selected_text_start_xpointer = nil if self.hold_pos then self.hold_pos = nil @@ -844,7 +846,16 @@ function ReaderHighlight:onHold(arg, ges) local ok, word = pcall(self.ui.document.getWordFromPosition, self.ui.document, self.hold_pos) if ok and word then logger.dbg("selected word:", word) - self.selected_word = word + -- Convert "word selection" table to "text selection" table because we + -- use text selections throughout readerhighlight. + self.is_word_selection = true + self.selected_text = { + text = word.word or "", + pos0 = word.pos0 or word.pos, + pos1 = word.pos1 or word.pos, + sboxes = word.sbox and { word.sbox }, + pboxes = word.pbox and { word.pbox }, + } local link = self.ui.link:getLinkFromGes(ges) self.selected_link = nil if link then @@ -852,15 +863,13 @@ function ReaderHighlight:onHold(arg, ges) self.selected_link = link end if self.ui.document.info.has_pages then - local boxes = {} - table.insert(boxes, self.selected_word.sbox) - self.view.highlight.temp[self.hold_pos.page] = boxes + self.view.highlight.temp[self.hold_pos.page] = self.selected_text.sboxes -- Unfortunately, getWordFromPosition() may not return good coordinates, -- so refresh the whole page UIManager:setDirty(self.dialog, "ui") else - -- With crengine, getWordFromPosition() does return good coordinates - UIManager:setDirty(self.dialog, "ui", self.selected_word.sbox) + -- With crengine, getWordFromPosition() does return good coordinates. + UIManager:setDirty(self.dialog, "ui", Geom.boundingBox(self.selected_text.sboxes)) end self:_resetHoldTimer() if word.pos0 then @@ -992,6 +1001,7 @@ function ReaderHighlight:onHoldPan(_, ges) local old_text = self.selected_text and self.selected_text.text self.selected_text = self.ui.document:getTextFromPositions(self.hold_pos, self.holdpan_pos) + self.is_word_selection = false if self.selected_text and self.selected_text.pos0 then if not self.selected_text_start_xpointer then @@ -1015,13 +1025,6 @@ function ReaderHighlight:onHoldPan(_, ges) logger.dbg("selected text:", self.selected_text) if self.selected_text then self.view.highlight.temp[self.hold_pos.page] = self.selected_text.sboxes - -- remove selected word if hold moves out of word box - if not self.selected_text.sboxes or #self.selected_text.sboxes == 0 then - self.selected_word = nil - elseif self.selected_word and not self.selected_word.sbox:contains(self.selected_text.sboxes[1]) or - #self.selected_text.sboxes > 1 then - self.selected_word = nil - end end UIManager:setDirty(self.dialog, "ui") end @@ -1035,18 +1038,36 @@ You can download language data files for version 3.04 from https://tesseract-ocr Copy the language data files for Tesseract 3.04 (e.g., eng.traineddata for English and spa.traineddata for Spanish) into koreader/data/tessdata]]) -function ReaderHighlight:lookup(selected_word, selected_link) +function ReaderHighlight:lookup(selected_text, selected_link) + -- convert sboxes to word boxes + local word_boxes = {} + for i, sbox in ipairs(selected_text.sboxes) do + word_boxes[i] = self.view:pageToScreenTransform(self.hold_pos.page, sbox) + end + -- if we extracted text directly - if selected_word.word and self.hold_pos then - local word_box = self.view:pageToScreenTransform(self.hold_pos.page, selected_word.sbox) - self.ui:handleEvent(Event:new("LookupWord", selected_word.word, false, word_box, self, selected_link)) + if selected_text.text and self.hold_pos then + self.ui:handleEvent(Event:new("LookupWord", selected_text.text, false, word_boxes, self, selected_link)) -- or we will do OCR - elseif selected_word.sbox and self.hold_pos then - local word = self.ui.document:getOCRWord(self.hold_pos.page, selected_word) - logger.dbg("OCRed word:", word) - if word and word ~= "" then - local word_box = self.view:pageToScreenTransform(self.hold_pos.page, selected_word.sbox) - self.ui:handleEvent(Event:new("LookupWord", word, false, word_box, self, selected_link)) + elseif selected_text.sboxes and self.hold_pos then + local text = self.ui.document:getOCRText(self.hold_pos.page, selected_text.sboxes) + if not text then + -- getOCRText is not implemented in some document backends, but + -- getOCRWord is implemented everywhere. As such, fall back to + -- getOCRWord. + text = "" + for _, sbox in ipairs(selected_text.sboxes) do + local word = self.ui.document:getOCRWord(self.hold_pos.page, { sbox = sbox }) + logger.dbg("OCRed word:", word) + -- @fixme This might produce incorrect results on RTL text. + if word and word ~= "" then + text = text .. word + end + end + end + logger.dbg("OCRed text:", text) + if text and text ~= "" then + self.ui:handleEvent(Event:new("LookupWord", text, false, word_boxes, self, selected_link)) else UIManager:show(InfoMessage:new{ text = info_message_ocr_text, @@ -1227,15 +1248,15 @@ function ReaderHighlight:onHoldRelease() end self.hold_last_tv = nil end - if self.selected_word then -- single-word selection + if self.is_word_selection then -- single-word selection if long_final_hold or G_reader_settings:isTrue("highlight_action_on_single_word") then - -- Force a 0-distance pan to have a self.selected_text with this word, - -- which will enable the highlight menu or action instead of dict lookup - self:onHoldPan(nil, {pos=self.hold_ges_pos}) + self.is_word_selection = false end end - if self.selected_text then + if self.is_word_selection then + self:lookup(self.selected_text, self.selected_link) + else local default_highlight_action = G_reader_settings:readSetting("default_highlight_action", "ask") if long_final_hold or default_highlight_action == "ask" then -- bypass default action and show popup if long final hold @@ -1257,9 +1278,6 @@ function ReaderHighlight:onHoldRelease() -- No self:onClose() to not remove the selected text -- which will have been the first search result end - elseif self.selected_word then - self:lookup(self.selected_word, self.selected_link) - self.selected_word = nil end return true end diff --git a/frontend/ui/widget/dictquicklookup.lua b/frontend/ui/widget/dictquicklookup.lua index fe8d57663..e4039acdd 100644 --- a/frontend/ui/widget/dictquicklookup.lua +++ b/frontend/ui/widget/dictquicklookup.lua @@ -50,8 +50,8 @@ local DictQuickLookup = InputContainer:new{ dict_index = 1, width = nil, height = nil, - -- box of highlighted word, quick lookup window tries to not hide the word - word_box = nil, + -- sboxes containing highlighted text, quick lookup window tries to not hide the word + word_boxes = nil, -- refresh_callback will be called before we trigger full refresh in onSwipe refresh_callback = nil, @@ -130,7 +130,7 @@ function DictQuickLookup:init() -- -- don't pass self.highlight to subsequent lookup, we want -- -- the first to be the only one to unhighlight selection -- -- when closed - -- Event:new("LookupWord", word, self.word_box)) + -- Event:new("LookupWord", word, true, {self.word_box})) -- end -- }, -- Allow selection of one or more words (see textboxwidget.lua) : @@ -619,16 +619,32 @@ function DictQuickLookup:init() local nb_lines = Math.round(self.definition_height / self.definition_line_height) self.definition_height = nb_lines * self.definition_line_height self.height = self.definition_height + others_height - if self.word_box then + if self.word_boxes and #self.word_boxes > 0 then -- Try to not hide the highlighted word. We don't want to always -- get near it if we can stay center, so that more context around -- the word is still visible with the dict result. -- But if we were to be drawn over the word, move a bit if possible. - local box = self.word_box - -- Don't stick to the box, ensure a minimal padding between box and window - local box_dict_padding = Size.padding.small - local word_box_top = box.y - box_dict_padding - local word_box_bottom = box.y + box.h + box_dict_padding + + -- In most cases boxes will be a single sbox, but handle multiple + -- sboxes by taking the top and bottom y values. + local word_box_top + local word_box_bottom + for _, box in ipairs(self.word_boxes) do + local box_top = box.y + local box_bottom = box.y + box.h + if not word_box_top or word_box_top > box_top then + word_box_top = box_top + end + if not word_box_bottom or word_box_bottom < box_bottom then + word_box_bottom = box_bottom + end + end + + -- Don't stick to the box, ensure a minimal padding between box and + -- window. + word_box_top = word_box_top - Size.padding.small + word_box_bottom = word_box_bottom + Size.padding.small + local half_visible_height = (avail_height - self.height) / 2 if word_box_bottom > half_visible_height and word_box_top <= half_visible_height + self.height then -- word would be covered by our centered window