From dd74194e0a1b2f722ef6acae08e8d395c958dc4c Mon Sep 17 00:00:00 2001
From: poire-z <poire-z@users.noreply.github.com>
Date: Tue, 22 Dec 2020 18:45:27 +0100
Subject: [PATCH] cre.getWordFromPosition(): fix a few issues

Drop the use of crengine's getWordFromPosition() which
is a bit unreliable: it may returns wrong coordinates,
or words from far away in the book (ie. when holding
in the margins).
Rely only on the robust getTextFromPositions() that
we already use for multi words selection.
Having good coordinates allows refreshing a smaller region
(the higlighted word, or the 2 lines if hyphenated).
---
 .../apps/reader/modules/readerhighlight.lua   |  6 +-
 frontend/document/credocument.lua             | 92 +++++++++++++++----
 2 files changed, 76 insertions(+), 22 deletions(-)

diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua
index 0d646b133..ba32f6f76 100644
--- a/frontend/apps/reader/modules/readerhighlight.lua
+++ b/frontend/apps/reader/modules/readerhighlight.lua
@@ -760,10 +760,10 @@ function ReaderHighlight:onHold(arg, ges)
             table.insert(boxes, self.selected_word.sbox)
             self.view.highlight.temp[self.hold_pos.page] = boxes
         end
-        UIManager:setDirty(self.dialog, "ui")
-        --- @todo only mark word?
         -- Unfortunately, CREngine does not return good coordinates
-        -- UIManager:setDirty(self.dialog, "partial", self.selected_word.sbox)
+        -- UIManager:setDirty(self.dialog, "ui")
+        -- But now it does:
+        UIManager:setDirty(self.dialog, "ui", self.selected_word.sbox)
         self:_resetHoldTimer()
         if word.pos0 then
             -- Remember original highlight start position, so we can show
diff --git a/frontend/document/credocument.lua b/frontend/document/credocument.lua
index 5b6ae0745..53b6693b8 100644
--- a/frontend/document/credocument.lua
+++ b/frontend/document/credocument.lua
@@ -494,33 +494,87 @@ function CreDocument:getImageFromPosition(pos, want_frames)
 end
 
 function CreDocument:getWordFromPosition(pos)
-    local word_box = self._document:getWordFromPosition(pos.x, pos.y)
-    logger.dbg("CreDocument: get word box", word_box)
-    local text_range = self._document:getTextFromPositions(pos.x, pos.y, pos.x, pos.y)
-    logger.dbg("CreDocument: get text range", text_range)
     local wordbox = {
-        word = text_range.text == "" and word_box.word or text_range.text,
         page = self._document:getCurrentPage(),
     }
-    if word_box.word then
-        wordbox.sbox = Geom:new{
-            x = word_box.x0, y = word_box.y0,
-            w = word_box.x1 - word_box.x0,
-            h = word_box.y1 - word_box.y0,
-        }
-    else
-        -- dummy word box
-        wordbox.sbox = Geom:new{
-            x = pos.x, y = pos.y,
-            w = 20, h = 20,
-        }
-    end
+    -- We use getTextFromPositions() which is more accurate.
+    -- In case some stuff is missing, we could fallback to use
+    -- the less accurate getWordFromPosition().
+    -- But it looks like getTextFromPositions() is just fine, and
+    -- when it fails, it's because there's no word at position.
+    -- So, we'll return nil below in case not all is found
+    local word_found = false
+    local box_found = false
+
+    local text_range = self._document:getTextFromPositions(pos.x, pos.y, pos.x, pos.y)
+    logger.dbg("CreDocument: get text range", text_range)
     if text_range then
-        -- add xpointers if found, might be useful for across pages highlighting
+        if text_range.text and text_range.text ~= "" then
+            wordbox.word = text_range.text
+            word_found = true
+        end
+        if text_range.pos0 and text_range.pos1 then
+            -- get segments from these pos, to build the overall box
+            local word_boxes = self._document:getWordBoxesFromPositions(text_range.pos0, text_range.pos1, true)
+            if #word_boxes > 0 then
+                local overall_box
+                for i = 1, #word_boxes do
+                    local line_box = word_boxes[i]
+                    if not overall_box then
+                        overall_box = line_box
+                    else
+                        if line_box.x0 < overall_box.x0 then overall_box.x0 = line_box.x0 end
+                        if line_box.y0 < overall_box.y0 then overall_box.y0 = line_box.y0 end
+                        if line_box.x1 > overall_box.x1 then overall_box.x1 = line_box.x1 end
+                        if line_box.y1 > overall_box.y1 then overall_box.y1 = line_box.y1 end
+                    end
+                end
+                wordbox.sbox = Geom:new{
+                    x = overall_box.x0,
+                    y = overall_box.y0,
+                    w = overall_box.x1 - overall_box.x0,
+                    h = overall_box.y1 - overall_box.y0,
+                }
+                box_found = true
+            end
+        end
+        -- add xpointers if any, might be useful for across pages highlighting
         wordbox.pos0 = text_range.pos0
         wordbox.pos1 = text_range.pos1
     end
+
+    -- Fully trust getTextFromPositions()
+    if word_found and box_found then
+        return wordbox
+    else
+        return nil
+    end
+
+    -- If we ever want to fallback to getWordFromPositions()
+    --[[
+    local word = self._document:getWordFromPosition(pos.x, pos.y)
+    logger.warn("CreDocument: get word box", word)
+    if not word_found then
+        wordbox.word = word.word
+    end
+    if not box_found then
+        if word.word then
+            wordbox.sbox = Geom:new{
+                x = word.x0,
+                y = word.y0,
+                w = word.x1 - word.x0,
+                h = word.y1 - word.y0,
+            }
+        else
+            -- dummy box
+            wordbox.sbox = Geom:new{
+                x = pos.x, y = pos.y,
+                w = 20, h = 20,
+            }
+        end
+    end
     return wordbox
+    ]]--
 end
 
 function CreDocument:getTextFromPositions(pos0, pos1)