ImageViewer: Followup to #9529 (#9544)

* ImageViewer: Minor code cleanups * GestureDetector: Fix the `distance` field of `two_finger_pan` & `two_finger_swipe` gestures so that it's no longer the double of the actual distance traveled. Get rid of existing workarounds throughout the codebase that had to deal with this quirk.
2025-12-13 20:36:53 +01:00 · 2022-09-21 23:26:22 +02:00
parent ab4b4b31bd
commit b0d8919399
10 changed files with 145 additions and 94 deletions
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -110,7 +110,6 @@ read_globals = {
    "DCREREADER_CONFIG_WORD_EXPANSION_MORE",
    "DMINIBAR_CONTAINER_HEIGHT",
    "DGESDETECT_DISABLE_DOUBLE_TAP",
-    "FRONTLIGHT_SENSITIVITY_DECREASE",
    "DALPHA_SORT_CASE_INSENSITIVE",
    "KOBO_LIGHT_ON_START",
    "NETWORK_PROXY",
--- a/defaults.lua
+++ b/defaults.lua
@@ -215,10 +215,6 @@ DMINIBAR_CONTAINER_HEIGHT = 14  -- Larger means more padding at the bottom, at t
 -- no longer needed
 --DDICT_FONT_SIZE = 20

-- Frontlight decrease of sensitivity for two-fingered pan gesture,
-- e.g. 2 changes the sensitivity by 1/2, 3 by 1/3 etc.
-FRONTLIGHT_SENSITIVITY_DECREASE = 2
-
 -- Normally, KOReader will present file lists sorted in case insensitive manner
 -- when presenting an alphatically sorted list. So the Order is "A, b, C, d".
 -- You can switch to a case sensitive sort ("A", "C", "b", "d") by disabling
--- a/frontend/apps/reader/modules/readerfont.lua
+++ b/frontend/apps/reader/modules/readerfont.lua
@@ -26,7 +26,6 @@ local ReaderFont = InputContainer:new{
    -- default gamma from crengine's lvfntman.cpp
    gamma_index = nil,
    steps = {0,1,1,1,1,1,2,2,2,3,3,3,4,4,5},
-    gestureScale = Screen:getWidth() * FRONTLIGHT_SENSITIVITY_DECREASE,
 }

 function ReaderFont:init()
@@ -369,11 +368,24 @@ function ReaderFont:addToMainMenu(menu_items)
 end

 function ReaderFont:gesToFontSize(ges)
+    -- Dispatcher feeds us a number, not a gesture
    if type(ges) ~= "table" then return ges end
+
    if ges.distance == nil then
        ges.distance = 1
    end
-    local step = math.ceil(2 * #self.steps * ges.distance / self.gestureScale)
+    -- Compute the scaling based on the gesture's direction (for pinch/spread)
+    local step
+    if ges.direction and ges.direction == "vertical" then
+        step = math.ceil(2 * #self.steps * ges.distance / Screen:getHeight())
+    elseif ges.direction and ges.direction == "horizontal" then
+        step = math.ceil(2 * #self.steps * ges.distance / Screen:getWidth())
+    elseif ges.direction and ges.direction == "diagonal" then
+        local screen_diagonal = math.sqrt(Screen:getWidth()^2 + Screen:getHeight()^2)
+        step = math.ceil(2 * #self.steps * ges.distance / screen_diagonal)
+    else
+        step = math.ceil(2 * #self.steps * ges.distance / math.min(Screen:getWidth(), Screen:getHeight()))
+    end
    local delta_int = self.steps[step] or self.steps[#self.steps]
    return delta_int
 end
--- a/frontend/device/devicelistener.lua
+++ b/frontend/device/devicelistener.lua
@@ -83,10 +83,7 @@ if Device:hasFrontlight() then
            end
            local gestureScale
            local scale_multiplier
-            if ges.ges == "two_finger_swipe" then
-                -- for backward compatibility
-                scale_multiplier = FRONTLIGHT_SENSITIVITY_DECREASE * 0.8
-            elseif ges.ges == "swipe" then
+            if ges.ges == "two_finger_swipe" or ges.ges == "swipe" then
                scale_multiplier = 0.8
            else
                scale_multiplier = 1
--- a/frontend/device/gesturedetector.lua
+++ b/frontend/device/gesturedetector.lua
@@ -1061,13 +1061,18 @@ function Contact:handleTwoFingerPan(buddy_contact)
            w = 0,
            h = 0,
        }
+        -- Use midpoint of tstart and rstart as swipe start point
+        local start_point = tstart_pos:midpoint(rstart_pos)
+        local end_point = tend_pos:midpoint(rend_pos)
+        -- Compute the distance based on the start & end midpoints
+        local avg_distance = start_point:distance(end_point)
+        -- We'll also want to remember the span between both contacts on start & end for some gestures
        local start_distance = tstart_pos:distance(rstart_pos)
        local end_distance = tend_pos:distance(rend_pos)
        local ges_ev = {
            ges = "two_finger_pan",
-            -- Use midpoint of tstart and rstart as swipe start point
-            pos = tstart_pos:midpoint(rstart_pos),
-            distance = tpan_dis + rpan_dis,
+            pos = start_point,
+            distance = avg_distance,
            direction = tpan_dir,
            time = self.current_tev.timev,
        }
@@ -1078,6 +1083,11 @@ function Contact:handleTwoFingerPan(buddy_contact)
                ges_ev.ges = "outward_pan"
            end
            ges_ev.direction = gesture_detector.DIRECTION_TABLE[tpan_dir]
+            -- Use the the sum of both contacts' travel for the distance
+            ges_ev.distance = tpan_dis + rpan_dis
+            -- Some handlers might also want to know the distance between the two contacts on lift & down.
+            ges_ev.span = end_distance
+            ges_ev.start_span = start_distance
        elseif self.state == Contact.holdState then
            ges_ev.ges = "two_finger_hold_pan"
            -- Flag 'em for holdState to discriminate with two_finger_hold_release
--- a/frontend/ui/geometry.lua
+++ b/frontend/ui/geometry.lua
@@ -367,7 +367,7 @@ Returns the Euclidean distance between two geoms.
@tparam Geom rect_b
 ]]
 function Geom:distance(geom)
-    return math.sqrt(math.pow(self.x - geom.x, 2) + math.pow(self.y - geom.y, 2))
+    return math.sqrt((self.x - geom.x)^2 + (self.y - geom.y)^2)
 end

 --[[--
--- a/frontend/ui/widget/imageviewer.lua
+++ b/frontend/ui/widget/imageviewer.lua
@@ -103,7 +103,7 @@ function ImageViewer:init()
            w = Screen:getWidth(),
            h = Screen:getHeight(),
        }
-        local diagonal = math.sqrt( math.pow(Screen:getWidth(), 2) + math.pow(Screen:getHeight(), 2) )
+        local diagonal = math.sqrt(Screen:getWidth()^2 + Screen:getHeight()^2)
        self.ges_events = {
            Tap = { GestureRange:new{ ges = "tap", range = range } },
            -- Zoom in/out (Pinch & Spread are not triggered if user is too
@@ -636,11 +636,41 @@ function ImageViewer:onPanRelease(_, ges)
 end

 -- Zoom events
-function ImageViewer:onZoomIn(inc)
+function ImageViewer:_refreshScaleFactor()
    if self.scale_factor == 0 then
        -- Get the scale_factor made out for best fit
        self.scale_factor = self._scale_factor_0 or self._image_wg:getScaleFactor()
    end
+end
+
+function ImageViewer:_applyNewScaleFactor(new_factor)
+    -- Make sure self.scale_factor is up-to-date
+    self:_refreshScaleFactor()
+
+    -- We destroy ImageWidget on update, so only request this the first time,
+    -- in order to avoid jitter in the results given differing memory consumption at different zoom levels...
+    if not self._min_scale_factor or not self._max_scale_factor then
+        self._min_scale_factor, self._max_scale_factor = self._image_wg:getScaleFactorExtrema()
+    end
+    -- Clamp to sane values
+    new_factor = math.min(new_factor, self._max_scale_factor)
+    new_factor = math.max(new_factor, self._min_scale_factor)
+    if new_factor ~= self.scale_factor then
+        self.scale_factor = new_factor
+        self:update()
+    else
+        if self.scale_factor == self._min_scale_factor then
+            logger.dbg("ImageViewer: Hit the min scaling factor:", self.scale_factor)
+        elseif self.scale_factor == self._max_scale_factor then
+            logger.dbg("ImageViewer: Hit the max scaling factor:", self.scale_factor)
+        else
+            logger.dbg("ImageViewer: No change in scaling factor:", self.scale_factor)
+        end
+    end
+end
+
+function ImageViewer:onZoomIn(inc)
+    self:_refreshScaleFactor()

    if not inc then
        -- default for key zoom event
@@ -649,35 +679,14 @@ function ImageViewer:onZoomIn(inc)

    -- Compute new scale factor for rescaled image dimensions
    local new_factor = self.scale_factor * (1 + inc)
-
-    -- We destroy ImageWidget on update, so only request this the first time,
-    -- in order to avoid jitter in the results given differing memory consumption at different zoom levels...
-    if not self._max_scale_factor then
-        self._min_scale_factor, self._max_scale_factor = self._image_wg:getScaleFactorExtrema()
-    end
-    -- Clamp to sane values
-    new_factor = math.min(new_factor, self._max_scale_factor)
-    if new_factor ~= self.scale_factor then
-        self.scale_factor = new_factor
-        self:update()
-    else
-        if self.scale_factor == self._max_scale_factor then
-            logger.dbg("ImageViewer:onZoomIn: Hit the max scaling factor:", self.scale_factor)
-        else
-            logger.dbg("ImageViewer:onZoomIn: No change in scaling factor:", self.scale_factor)
-        end
-    end
+    self:_applyNewScaleFactor(new_factor)
    return true
 end

 function ImageViewer:onZoomOut(dec)
-    if self.scale_factor == 0 then
-        -- Get the scale_factor made out for best fit
-        self.scale_factor = self._scale_factor_0 or self._image_wg:getScaleFactor()
-    end
+    self:_refreshScaleFactor()

    if not dec then
-        -- default for key zoom event
        dec = 0.2
    elseif dec >= 0.75 then
        -- Larger reductions tend to be fairly jarring, so limit to 75%.
@@ -685,70 +694,90 @@ function ImageViewer:onZoomOut(dec)
        dec = 0.75
    end

-    -- Compute new scale factor for rescaled image dimensions
    local new_factor = self.scale_factor * (1 - dec)
-
-    if not self._min_scale_factor then
-        self._min_scale_factor, self._max_scale_factor = self._image_wg:getScaleFactorExtrema()
-    end
-    -- Clamp to sane values
-    new_factor = math.max(new_factor, self._min_scale_factor)
-    if new_factor ~= self.scale_factor then
-        self.scale_factor = new_factor
-        self:update()
-    else
-        if self.scale_factor == self._min_scale_factor then
-            logger.dbg("ImageViewer:onZoomOut: Hit the min scaling factor:", self.scale_factor)
-        else
-            logger.dbg("ImageViewer:onZoomOut: No change in scaling factor:", self.scale_factor)
-        end
-    end
+    self:_applyNewScaleFactor(new_factor)
    return true
 end

+--[[
+function ImageViewer:onZoomToHeight(height)
+    local new_factor = height / self._image_wg:getOriginalHeight()
+    self:_applyNewScaleFactor(new_factor)
+    return true
+end
+
+function ImageViewer:onZoomToWidth(width)
+    local new_factor = width / self._image_wg:getOriginalWidth()
+    self:_applyNewScaleFactor(new_factor)
+    return true
+end
+
+function ImageViewer:onZoomToDiagonal(d)
+    -- It's trigonometry time!
+    -- c.f., https://math.stackexchange.com/a/3369637
+    local r = self._image_wg:getOriginalWidth() / self._image_wg:getOriginalHeight()
+    local h = math.sqrt(d^2 / (r^2 + 1))
+    local w = h * r
+
+    -- Matches ImageWidget's best-fit computation in _render
+    local new_factor = math.min(w / self._image_wg:getOriginalWidth(), h / self._image_wg:getOriginalHeight())
+    self:_applyNewScaleFactor(new_factor)
+    return true
+end
+--]]
+
 function ImageViewer:onSpread(_, ges)
+    if not self._image_wg then
+        return
+    end
+
    -- We get the position where spread was done
    -- First, get center ratio we would have had if we did a pan to there,
    -- so we can have the zoom centered on there
-    if self._image_wg then
-        self._center_x_ratio, self._center_y_ratio = self._image_wg:getPanByCenterRatio(ges.pos.x - Screen:getWidth()/2, ges.pos.y - Screen:getHeight()/2)
-    end
-    -- Set some zoom increase value from pinch distance.
-    -- Making it relative to the smallest dimension between the currently scaled image or the Screen makes it less annoying
-    -- when approaching both very small scale factors (where the image dimensions are many times smaller than the screen),
+    self._center_x_ratio, self._center_y_ratio = self._image_wg:getPanByCenterRatio(ges.pos.x - Screen:getWidth()/2, ges.pos.y - Screen:getHeight()/2)
+    -- We compute a scaling percentage (which will *modify* the current scaling factor),
+    -- based on the gesture distance (it's the sum of the travel of both fingers).
+    -- Making this distance relative to the smallest dimension between
+    -- the currently scaled image or the Screen makes it less annoying when approaching both very small scale factors
+    -- (where the image dimensions are many times smaller than the screen),
    -- meaning using the image dimensions here takes less zoom steps to get it back to a sensible size;
    -- *and* large scale factors (where the image dimensions are larger than the screen),
    -- meaning using the screen dimensions here makes zoom steps, again, slightly more potent.
-    local inc
    if ges.direction == "vertical" then
-        inc = ges.distance / math.min(Screen:getHeight(), self._image_wg:getCurrentHeight())
+        local img_h = self._image_wg:getCurrentHeight()
+        local screen_h = Screen:getHeight()
+        self:onZoomIn(ges.distance / math.min(screen_h, img_h))
    elseif ges.direction == "horizontal" then
-        inc = ges.distance / math.min(Screen:getWidth(), self._image_wg:getCurrentWidth())
+        local img_w = self._image_wg:getCurrentWidth()
+        local screen_w = Screen:getWidth()
+        self:onZoomIn(ges.distance / math.min(screen_w, img_w))
    else
-        local tl = Geom:new{ x = 0, y = 0 }
-        local br = Geom:new{ x = Screen:getWidth() - 1, y = Screen:getHeight() - 1}
-        local screen_diag = tl:distance(br)
-        inc = ges.distance / math.min(screen_diag, self._image_wg:getCurrentDiagonal())
+        local img_d = self._image_wg:getCurrentDiagonal()
+        local screen_d = math.sqrt(Screen:getWidth()^2 + Screen:getHeight()^2)
+        self:onZoomIn(ges.distance / math.min(screen_d, img_d))
    end
-    self:onZoomIn(inc)
    return true
 end

 function ImageViewer:onPinch(_, ges)
-    -- With Pinch, unlike Spread, it feels more natural if we keep the same center point.
-    -- Set some zoom decrease value from pinch distance
-    local dec
-    if ges.direction == "vertical" then
-        dec = ges.distance / math.min(Screen:getHeight(), self._image_wg:getCurrentHeight())
-    elseif ges.direction == "horizontal" then
-        dec = ges.distance / math.min(Screen:getWidth(), self._image_wg:getCurrentWidth())
-    else
-        local tl = Geom:new{ x = 0, y = 0 }
-        local br = Geom:new{ x = Screen:getWidth() - 1, y = Screen:getHeight() - 1}
-        local screen_diag = tl:distance(br)
-        dec = ges.distance / math.min(screen_diag, self._image_wg:getCurrentDiagonal())
+    if not self._image_wg then
+        return
+    end
+
+    -- With Pinch, unlike Spread, it feels more natural if we keep the same center point.
+    if ges.direction == "vertical" then
+        local img_h = self._image_wg:getCurrentHeight()
+        local screen_h = Screen:getHeight()
+        self:onZoomOut(ges.distance / math.min(screen_h, img_h))
+    elseif ges.direction == "horizontal" then
+        local img_w = self._image_wg:getCurrentWidth()
+        local screen_w = Screen:getWidth()
+        self:onZoomOut(ges.distance / math.min(screen_w, img_w))
+    else
+        local img_d = self._image_wg:getCurrentDiagonal()
+        local screen_d = math.sqrt(Screen:getWidth()^2 + Screen:getHeight()^2)
+        self:onZoomOut(ges.distance / math.min(screen_d, img_d))
    end
-    self:onZoomOut(dec)
    return true
 end

--- a/frontend/ui/widget/imagewidget.lua
+++ b/frontend/ui/widget/imagewidget.lua
@@ -456,9 +456,20 @@ function ImageWidget:getCurrentHeight()
 end

 function ImageWidget:getCurrentDiagonal()
-    local tl = Geom:new{ x = 0, y = 0 }
-    local br = Geom:new{ x = self._bb:getWidth() - 1, y = self._bb:getHeight() - 1}
-    return tl:distance(br)
+    return math.sqrt(self._bb:getWidth()^2 + self._bb:getHeight()^2)
+end
+
+-- And now, getters for the original, unscaled dimensions.
+function ImageWidget:getOriginalWidth()
+    return self._img_w
+end
+
+function ImageWidget:getOriginalHeight()
+    return self._img_h
+end
+
+function ImageWidget:getOriginalDiagonal()
+    return math.sqrt(self._img_w^2 + self._img_h^2)
 end

 function ImageWidget:getPanByCenterRatio(x, y)
--- a/frontend/ui/widget/screenshoter.lua
+++ b/frontend/ui/widget/screenshoter.lua
@@ -14,10 +14,7 @@ local Screenshoter = InputContainer:new{
 }

 function Screenshoter:init()
-    local diagonal = math.sqrt(
-        math.pow(Screen:getWidth(), 2) +
-        math.pow(Screen:getHeight(), 2)
-    )
+    local diagonal = math.sqrt(Screen:getWidth()^2 + Screen:getHeight()^2)
    self.ges_events = {
        TapDiagonal = {
            GestureRange:new{
--- a/spec/unit/defaults_spec.lua
+++ b/spec/unit/defaults_spec.lua
@@ -8,7 +8,7 @@ describe("defaults module", function()

    it("should load all defaults from defaults.lua", function()
        Defaults:init()
-        assert.is_same(99, #Defaults.defaults_name)
+        assert.is_same(98, #Defaults.defaults_name)
    end)

    it("should save changes to defaults.persistent.lua", function()
@@ -16,7 +16,7 @@ describe("defaults module", function()
        os.remove(persistent_filename)

        -- To see indices and help updating this when new settings are added:
-        -- for i=1, 99 do print(i.." ".. Defaults.defaults_name[i]) end
+        -- for i=1, 98 do print(i.." ".. Defaults.defaults_name[i]) end

        -- not in persistent but checked in defaults
        Defaults.changed[18] = true
@@ -24,7 +24,7 @@ describe("defaults module", function()
        Defaults.changed[54] = true
        Defaults.changed[83] = true
        Defaults:saveSettings()
-        assert.is_same(99, #Defaults.defaults_name)
+        assert.is_same(98, #Defaults.defaults_name)
        assert.is_same("DTAP_ZONE_BACKWARD", Defaults.defaults_name[84])
        assert.is_same("DCREREADER_CONFIG_WORD_SPACING_LARGE", Defaults.defaults_name[48])
        assert.is_same("DCREREADER_CONFIG_H_MARGIN_SIZES_XXX_LARGE", Defaults.defaults_name[18])