ImageViewer: Clamp zoom factor to sane values

frontend/cache.lua

@@ -6,6 +6,7 @@ local lfs = require("libs/libkoreader-lfs")
 local logger = require("logger")
 local lru = require("ffi/lru")
 local md5 = require("ffi/sha2").md5
+local util = require("util")
 
 local CanvasContext = require("document/canvascontext")
 if CanvasContext.should_restrict_JIT then
@@ -78,95 +79,6 @@ function Cache:_getDiskCache()
     return cached
 end
 
--- For documentation purposes, here's a battle-tested shell version of calcFreeMem
---[[
-    if grep -q 'MemAvailable' /proc/meminfo ; then
-        # We'll settle for 85% of available memory to leave a bit of breathing room
-        tmpfs_size="$(awk '/MemAvailable/ {printf "%d", $2 * 0.85}' /proc/meminfo)"
-    elif grep -q 'Inactive(file)' /proc/meminfo ; then
-        # Basically try to emulate the kernel's computation, c.f., https://unix.stackexchange.com/q/261247
-        # Again, 85% of available memory
-        tmpfs_size="$(awk -v low=$(grep low /proc/zoneinfo | awk '{k+=$2}END{printf "%d", k}') \
-            '{a[$1]=$2}
-            END{
-                printf "%d", (a["MemFree:"]+a["Active(file):"]+a["Inactive(file):"]+a["SReclaimable:"]-(12*low))*0.85;
-            }' /proc/meminfo)"
-    else
-        # Ye olde crap workaround of Free + Buffers + Cache...
-        # Take it with a grain of salt, and settle for 80% of that...
-        tmpfs_size="$(awk \
-            '{a[$1]=$2}
-            END{
-                printf "%d", (a["MemFree:"]+a["Buffers:"]+a["Cached:"])*0.80;
-            }' /proc/meminfo)"
-    fi
---]]
-
--- And here's our simplified Lua version...
-function Cache:_calcFreeMem()
-    local memtotal, memfree, memavailable, buffers, cached
-
-    local meminfo = io.open("/proc/meminfo", "r")
-    if meminfo then
-        for line in meminfo:lines() do
-            if not memtotal then
-                memtotal = line:match("^MemTotal:%s-(%d+) kB")
-                if memtotal then
-                    -- Next!
-                    goto continue
-                end
-            end
-
-            if not memfree then
-                memfree = line:match("^MemFree:%s-(%d+) kB")
-                if memfree then
-                    -- Next!
-                    goto continue
-                end
-            end
-
-            if not memavailable then
-                memavailable = line:match("^MemAvailable:%s-(%d+) kB")
-                if memavailable then
-                    -- Best case scenario, we're done :)
-                    break
-                end
-            end
-
-            if not buffers then
-                buffers = line:match("^Buffers:%s-(%d+) kB")
-                if buffers then
-                    -- Next!
-                    goto continue
-                end
-            end
-
-            if not cached then
-                cached = line:match("^Cached:%s-(%d+) kB")
-                if cached then
-                    -- Ought to be the last entry we care about, we're done
-                    break
-                end
-            end
-
-            ::continue::
-        end
-        meminfo:close()
-    else
-        -- Not on Linux?
-        return 0, 0
-    end
-
-    if memavailable then
-        -- Leave a bit of margin, and report 85% of that...
-        return math.floor(memavailable * 0.85) * 1024, memtotal * 1024
-    else
-        -- Crappy Free + Buffers + Cache version, because the zoneinfo approach is a tad hairy...
-        -- So, leave an even larger margin, and only report 75% of that...
-        return math.floor((memfree + buffers + cached) * 0.75) * 1024, memtotal * 1024
-    end
-end
-
 function Cache:insert(key, object)
     -- If this object is single-handledly too large for the cache, don't cache it.
     if not self:willAccept(object.size) then
@@ -240,18 +152,20 @@ end
 
 -- Terribly crappy workaround: evict half the cache if we appear to be redlining on free RAM...
 function Cache:memoryPressureCheck()
-    local memfree, memtotal = self:_calcFreeMem()
+    local memfree, memtotal = util.calcFreeMem()
 
     -- Nonsensical values? (!Linux), skip this.
-    if memtotal == 0 then
+    if memtotal == nil then
         return
     end
 
     -- If less that 20% of the total RAM is free, drop half the Cache...
     local free_fraction = memfree / memtotal
     if free_fraction < 0.20 then
         logger.warn(string.format("Running low on memory (~%d%%, ~%.2f/%d MiB), evicting half of the cache...",
-                                  free_fraction * 100, memfree / 1024 / 1024, memtotal / 1024 / 1024))
+                                  free_fraction * 100,
+                                  memfree / (1024 * 1024),
+                                  memtotal / (1024 * 1024)))
         self.cache:chop()
 
         -- And finish by forcing a GC sweep now...

frontend/document/doccache.lua

@@ -8,11 +8,12 @@ local DataStorage = require("datastorage")
 local lfs = require("libs/libkoreader-lfs")
 local logger = require("logger")
 local md5 = require("ffi/sha2").md5
+local util = require("util")
 
 local function calcCacheMemSize()
     local min = DGLOBAL_CACHE_SIZE_MINIMUM
     local max = DGLOBAL_CACHE_SIZE_MAXIMUM
-    local calc = Cache:_calcFreeMem() * (DGLOBAL_CACHE_FREE_PROPORTION or 0)
+    local calc = util.calcFreeMem() * (DGLOBAL_CACHE_FREE_PROPORTION or 0)
     return math.min(max, math.max(min, calc))
 end
 local doccache_size = calcCacheMemSize()

frontend/ui/widget/imageviewer.lua

@@ -550,18 +550,19 @@ function ImageViewer:onSwipe(_, ges)
     local distance = ges.distance
     local sq_distance = math.sqrt(distance*distance/2)
     if direction == "north" then
-        if ges.pos.x < Screen:getWidth() * 1/16 or ges.pos.x > Screen:getWidth() * 15/16 then
+        if ges.pos.x < Screen:getWidth() * 1/8 or ges.pos.x > Screen:getWidth() * 7/8 then
             -- allow for zooming with vertical swipe on screen sides
             -- (for devices without multi touch where pinch and spread don't work)
-            local inc = ges.distance / Screen:getHeight()
+            -- c.f., onSpread for details about the choice between screen & scaled images height.
+            local inc = ges.distance / math.min(Screen:getHeight(), self._image_wg:getCurrentHeight())
             self:onZoomIn(inc)
         else
             self:panBy(0, distance)
         end
     elseif direction == "south" then
-        if ges.pos.x < Screen:getWidth() * 1/16 or ges.pos.x > Screen:getWidth() * 15/16 then
+        if ges.pos.x < Screen:getWidth() * 1/8 or ges.pos.x > Screen:getWidth() * 7/8 then
             -- allow for zooming with vertical swipe on screen sides
-            local dec = ges.distance / Screen:getHeight()
+            local dec = ges.distance / math.min(Screen:getHeight(), self._image_wg:getCurrentHeight())
             self:onZoomOut(dec)
         elseif self.scale_factor == 0 then
             -- When scaled to fit (on initial launch, or after one has tapped
@@ -640,13 +641,32 @@ function ImageViewer:onZoomIn(inc)
         -- Get the scale_factor made out for best fit
         self.scale_factor = self._scale_factor_0 or self._image_wg:getScaleFactor()
     end
-    if not inc then inc = 0.2 end -- default for key zoom event
-    self.scale_factor = self.scale_factor + inc
-    -- Avoid excessive zoom by halving the increase if we go too high, and clamp the result
-    if self.scale_factor > 100 then
-        self.scale_factor = math.min((self.scale_factor - inc) + inc/2, 100)
+
+    if not inc then
+        -- default for key zoom event
+        inc = 0.2
+    end
+
+    -- 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:update()
     return true
 end
 
@@ -655,13 +675,34 @@ function ImageViewer:onZoomOut(dec)
         -- Get the scale_factor made out for best fit
         self.scale_factor = self._scale_factor_0 or self._image_wg:getScaleFactor()
     end
-    if not dec then dec = 0.2 end -- default for key zoom event
-    self.scale_factor = self.scale_factor - dec
-    -- Avoid excessive unzoom by halving the decrease if we go too low, and clamp the result
-    if self.scale_factor < 0.01 then
-        self.scale_factor = math.max(0.01, (self.scale_factor + dec) - dec/2)
+
+    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%.
+        -- (Also, we can't go above 1 because maths).
+        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:update()
     return true
 end
 
@@ -672,16 +713,41 @@ function ImageViewer:onSpread(_, ges)
     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
-    local inc = ges.distance / Screen:getWidth()
+    -- 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),
+    -- 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())
+    elseif ges.direction == "horizontal" then
+        inc = 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)
+        inc = ges.distance / math.min(screen_diag, self._image_wg:getCurrentDiagonal())
+    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 = ges.distance / Screen:getWidth()
+    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())
+    end
     self:onZoomOut(dec)
     return true
 end