A random assortment of fixes (#9513)

* Android: Make sure sdcv can find the STL
* DocCache: Be less greedy when serializing to disk, and only do that for the *current* document ;).
* CanvasContext: Explicitly document API quirks.
* Fontlist: Switch the on-disk Persist format to zstd (it's ever so slightly faster).
* Bump base for https://github.com/koreader/koreader-base/pull/1515 (fix #9506)

base

@@ -1 +1 @@
-Subproject commit d196c25aa670dc7e6aca92b6454f6410ca6e0fcd
+Subproject commit 03693cf2a09e3f3063a220a4944d3232f080a468

frontend/apps/reader/modules/readerdictionary.lua

@@ -16,6 +16,8 @@ local NetworkMgr = require("ui/network/manager")
 local SortWidget = require("ui/widget/sortwidget")
 local Trapper = require("ui/trapper")
 local UIManager = require("ui/uimanager")
+local ffi = require("ffi")
+local C = ffi.C
 local ffiUtil  = require("ffi/util")
 local logger = require("logger")
 local time = require("ui/time")
@@ -770,7 +772,16 @@ function ReaderDictionary:rawSdcv(words, dict_names, fuzzy_search, lookup_progre
         -- definition found, sdcv will output some message on stderr, and
         -- let stdout empty) by appending an "echo":
         cmd = cmd .. "; echo"
+        -- NOTE: Bionic doesn't support rpath, but does honor LD_LIBRARY_PATH...
+        --       Give it a shove so it can actually find the STL.
+        if Device:isAndroid() then
+            C.setenv("LD_LIBRARY_PATH", "./libs", 1)
+        end
         local completed, results_str = Trapper:dismissablePopen(cmd, lookup_progress_msg)
+        if Device:isAndroid() then
+            -- NOTE: It's unset by default, so this is perfectly fine.
+            C.unsetenv("LD_LIBRARY_PATH")
+        end
         lookup_cancelled = not completed
         if results_str and results_str ~= "\n" then -- \n is when lookup was cancelled
             -- sdcv can return multiple results if we passed multiple words to

frontend/apps/reader/readerui.lua

@@ -761,9 +761,9 @@ function ReaderUI:onClose(full_refresh)
     if self.dialog ~= self then
         self:saveSettings()
     end
-    -- Serialize the most recently displayed page for later launch
-    DocCache:serialize()
     if self.document ~= nil then
+        -- Serialize the most recently displayed page for later launch
+        DocCache:serialize(self.document.file)
         logger.dbg("closing document")
         self:notifyCloseDocument()
     end

frontend/cache.lua

@@ -233,64 +233,6 @@ function Cache:willAccept(size)
     return size*4 < self.size*3
 end
 
-function Cache:serialize()
-    if not self.disk_cache then
-        return
-    end
-
-    -- Calculate the current disk cache size
-    local cached_size = 0
-    local sorted_caches = {}
-    for _, file in pairs(self.cached) do
-        table.insert(sorted_caches, {file=file, time=lfs.attributes(file, "access")})
-        cached_size = cached_size + (lfs.attributes(file, "size") or 0)
-    end
-    table.sort(sorted_caches, function(v1, v2) return v1.time > v2.time end)
-
-    -- Only serialize the second most recently used cache item (as the MRU would be the *hinted* page).
-    local mru_key
-    local mru_found = 0
-    for key, item in self.cache:pairs() do
-        -- Only dump cache items that actually request persistence
-        if item.persistent and item.dump then
-            mru_key = key
-            mru_found = mru_found + 1
-            if mru_found >= 2 then
-                -- We found the second MRU item, i.e., the *displayed* page
-                break
-            end
-        end
-    end
-    if mru_key then
-        local cache_full_path = self.cache_path .. md5(mru_key)
-        local cache_file_exists = lfs.attributes(cache_full_path)
-
-        if not cache_file_exists then
-            logger.dbg("Dumping cache item", mru_key)
-            local cache_item = self.cache:get(mru_key)
-            local cache_size = cache_item:dump(cache_full_path)
-            if cache_size then
-                cached_size = cached_size + cache_size
-            end
-        end
-    end
-
-    -- Allocate the same amount of storage to the disk cache than the memory cache
-    while cached_size > self.size do
-        -- discard the least recently used cache
-        local discarded = table.remove(sorted_caches)
-        if discarded then
-            cached_size = cached_size - lfs.attributes(discarded.file, "size")
-            os.remove(discarded.file)
-        else
-            logger.warn("Cache accounting is broken")
-            break
-        end
-    end
-    -- We may have updated the disk cache's content, so refresh its state
-    self:refreshSnapshot()
-end
-
 -- Blank the cache
 function Cache:clear()
     self.cache:clear()

frontend/device/kobo/device.lua

@@ -35,12 +35,12 @@ local function checkStandby()
         return no
     end
     local mode = f:read()
-    logger.dbg("Kobo: available power states", mode)
+    logger.dbg("Kobo: available power states:", mode)
     if mode and mode:find("standby") then
-        logger.dbg("Kobo: standby state allowed")
+        logger.dbg("Kobo: standby state is supported")
         return yes
     end
-    logger.dbg("Kobo: standby state not allowed")
+    logger.dbg("Kobo: standby state is unsupported")
     return no
 end
 

frontend/document/canvascontext.lua

@@ -37,6 +37,15 @@ The following key is required for a device object:
 function CanvasContext:init(device)
     self.device = device
     self.screen = device.screen
+    -- NOTE: These work because they don't actually require accessing the Device object itself,
+    --       as opposed to more dynamic methods like the Screen ones we handle properly later...
+    --       By which I mean when one naively calls CanvasContext:isKindle(), it calls
+    --       device.isKindle(CanvasContext), whereas when one calls Device:isKindle(), it calls
+    --       Device.isKindle(Device).
+    --       In the latter case, self is sane, but *NOT* in the former.
+    --       TL;DR: The methods assigned below must *never* access self.
+    --              (Or programmers would have to be careful to call them through CanvasContext as functions,
+    --              and not methods, which is clunky, error-prone, and unexpected).
     self.isAndroid = device.isAndroid
     self.isDesktop = device.isDesktop
     self.isEmulator = device.isEmulator
@@ -44,7 +53,7 @@ function CanvasContext:init(device)
     self.isPocketBook = device.isPocketBook
     self.should_restrict_JIT = device.should_restrict_JIT
     self.hasSystemFonts = device.hasSystemFonts
-    self:setColorRenderingEnabled(device.screen.isColorEnabled())
+    self:setColorRenderingEnabled(device.screen:isColorEnabled())
 
     -- NOTE: At 32bpp, Kobo's fb is BGR, not RGB. Handle the conversion in MuPDF if needed.
     if device:hasBGRFrameBuffer() then
@@ -54,7 +63,7 @@ function CanvasContext:init(device)
 
     -- This one may be called by a subprocess, and would crash on Android when
     -- calling android.isEink() which is only allowed from the main thread.
-    local hasEinkScreen = device.hasEinkScreen()
+    local hasEinkScreen = device:hasEinkScreen()
     self.hasEinkScreen = function() return hasEinkScreen end
 
     self.canHWDither = device.canHWDither

frontend/document/doccache.lua

@@ -5,7 +5,9 @@
 local Cache = require("cache")
 local CanvasContext = require("document/canvascontext")
 local DataStorage = require("datastorage")
+local lfs = require("libs/libkoreader-lfs")
 local logger = require("logger")
+local md5 = require("ffi/sha2").md5
 
 local function calcCacheMemSize()
     local min = DGLOBAL_CACHE_SIZE_MINIMUM
@@ -13,23 +15,22 @@ local function calcCacheMemSize()
     local calc = Cache:_calcFreeMem() * (DGLOBAL_CACHE_FREE_PROPORTION or 0)
     return math.min(max, math.max(min, calc))
 end
-local cache_size = calcCacheMemSize()
-
+local doccache_size = calcCacheMemSize()
 
 local function computeCacheSize()
-    local mb_size = cache_size / 1024 / 1024
+    local mb_size = doccache_size / 1024 / 1024
 
     -- If we end up with a not entirely ridiculous cache size, use that...
     if mb_size >= 8 then
         logger.dbg(string.format("Allocating a %dMB budget for the global document cache", mb_size))
-        return cache_size
+        return doccache_size
     else
         return nil
     end
 end
 
 local function computeCacheSlots()
-    local mb_size = cache_size / 1024 / 1024
+    local mb_size = doccache_size / 1024 / 1024
 
     --- ...otherwise, effectively disable the cache by making it single slot...
     if mb_size < 8 then
@@ -51,4 +52,65 @@ local DocCache = Cache:new{
     cache_path = DataStorage:getDataDir() .. "/cache/",
 }
 
+function DocCache:serialize(doc_path)
+    if not self.disk_cache then
+        return
+    end
+
+    -- Calculate the current disk cache size
+    local cached_size = 0
+    local sorted_caches = {}
+    for _, file in pairs(self.cached) do
+        table.insert(sorted_caches, {file=file, time=lfs.attributes(file, "access")})
+        cached_size = cached_size + (lfs.attributes(file, "size") or 0)
+    end
+    table.sort(sorted_caches, function(v1, v2) return v1.time > v2.time end)
+
+    -- Rewind a bit in order to serialize the currently *displayed* page for the current document,
+    -- as the actual MRU item would be the most recently *hinted* page, which wouldn't be helpful ;).
+    if doc_path then
+        local mru_key
+        local mru_found = 0
+        for key, item in self.cache:pairs() do
+            -- Only dump items that actually request persistence and match the current document.
+            if item.persistent and item.dump and item.doc_path == doc_path then
+                mru_key = key
+                mru_found = mru_found + 1
+                if mru_found >= (1 + DHINTCOUNT) then
+                    -- We found the right item, i.e., the *displayed* page
+                    break
+                end
+            end
+        end
+        if mru_key then
+            local cache_full_path = self.cache_path .. md5(mru_key)
+            local cache_file_exists = lfs.attributes(cache_full_path)
+
+            if not cache_file_exists then
+                logger.dbg("Dumping cache item", mru_key)
+                local cache_item = self.cache:get(mru_key)
+                local cache_size = cache_item:dump(cache_full_path)
+                if cache_size then
+                    cached_size = cached_size + cache_size
+                end
+            end
+        end
+    end
+
+    -- Allocate the same amount of storage to the disk cache than the memory cache
+    while cached_size > self.size do
+        -- discard the least recently used cache
+        local discarded = table.remove(sorted_caches)
+        if discarded then
+            cached_size = cached_size - lfs.attributes(discarded.file, "size")
+            os.remove(discarded.file)
+        else
+            logger.warn("Cache accounting is broken")
+            break
+        end
+    end
+    -- We may have updated the disk cache's content, so refresh its state
+    self:refreshSnapshot()
+end
+
 return DocCache

frontend/document/document.lua

@@ -445,6 +445,7 @@ function Document:renderPage(pageno, rect, zoom, rotation, gamma, render_mode, h
     -- prepare cache item with contained blitbuffer
     tile = TileCacheItem:new{
         persistent = true,
+        doc_path = self.file,
         created_ts = os.time(),
         excerpt = size,
         pageno = pageno,

frontend/document/koptinterface.lua

@@ -285,6 +285,7 @@ function KoptInterface:getCachedContext(doc, pageno)
         self.last_context_size = fullwidth * fullheight + 3072 -- estimation
         DocCache:insert(hash, ContextCacheItem:new{
             persistent = true,
+            doc_path = doc.file,
             size = self.last_context_size,
             kctx = kc
         })
@@ -411,6 +412,7 @@ function KoptInterface:renderOptimizedPage(doc, pageno, rect, zoom, rotation, re
         -- prepare cache item with contained blitbuffer
         local tile = TileCacheItem:new{
             persistent = true,
+            doc_path = doc.file,
             excerpt = Geom:new{
                 x = 0, y = 0,
                 w = fullwidth,
@@ -576,6 +578,7 @@ function KoptInterface:getNativeTextBoxes(doc, pageno)
             kc = self:createContext(doc, pageno)
             DocCache:insert(kctx_hash, ContextCacheItem:new{
                 persistent = true,
+                doc_path = doc.file,
                 size = self.last_context_size or self.default_context_size,
                 kctx = kc,
             })

frontend/document/tilecacheitem.lua

@@ -19,6 +19,7 @@ function TileCacheItem:totable()
         excerpt = self.excerpt,
         created_ts = self.created_ts,
         persistent = self.persistent,
+        doc_path = self.doc_path,
         bb = {
             w = self.bb.w,
             h = self.bb.h,
@@ -56,6 +57,7 @@ function TileCacheItem:fromtable(t)
     self.excerpt = t.excerpt
     self.created_ts = t.created_ts
     self.persistent = t.persistent
+    self.doc_path = t.doc_path
     self.bb = Blitbuffer.fromstring(t.bb.w, t.bb.h, t.bb.fmt, t.bb.data, t.bb.stride, t.bb.rotation, t.bb.inverse)
 end
 

frontend/fontlist.lua

@@ -4,6 +4,7 @@ local FT = require("ffi/freetype")
 local HB = require("ffi/harfbuzz")
 local Persist = require("persist")
 local util = require("util")
+local lfs = require("libs/libkoreader-lfs")
 local logger = require("logger")
 local dbg = require("dbg")
 
@@ -90,11 +91,11 @@ local kindle_fonts_blacklist = {
 
 local function isInFontsBlacklist(f)
     -- write test for this
-    return CanvasContext.isKindle() and kindle_fonts_blacklist[f]
+    return CanvasContext:isKindle() and kindle_fonts_blacklist[f]
 end
 
 local function getExternalFontDir()
-    if CanvasContext.hasSystemFonts() then
+    if CanvasContext:hasSystemFonts() then
         return require("frontend/ui/elements/font_settings"):getPath()
     else
         return os.getenv("EXT_FONT_DIR")
@@ -140,7 +141,7 @@ local font_exts = {
 function FontList:_readList(dir, mark)
     util.findFiles(dir, function(path, file, attr)
         -- See if we're interested
-        if file:sub(1,1) == "." then return end
+        if file:sub(1, 1) == "." then return end
         local file_type = file:lower():match(".+%.([^.]+)") or ""
         if not font_exts[file_type] then return end
 
@@ -163,15 +164,16 @@ function FontList:_readList(dir, mark)
 end
 
 function FontList:getFontList()
-    if #self.fontlist > 0 then return self.fontlist end
+    if self.fontlist[1] then return self.fontlist end
 
     local cache = Persist:new{
-        path = self.cachedir .. "/fontinfo.dat"
+        path = self.cachedir .. "/fontinfo.dat",
+        codec = dbg.is_verbose and "dump" or "zstd",
     }
 
     local t, err = cache:load()
     if not t then
-        logger.info(cache.path, err, "initializing it")
+        logger.info(cache.path, err, "-> initializing it")
 
         -- Create new subdirectory
         lfs.mkdir(self.cachedir)
@@ -183,7 +185,7 @@ function FontList:getFontList()
 
     self:_readList(self.fontdir, mark)
     -- multiple paths should be joined with semicolon
-    for dir in string.gmatch(getExternalFontDir() or "", "([^;]+)") do
+    for dir in string.gmatch(getExternalFontDir() or "", "[^;]+") do
         self:_readList(dir, mark)
     end
 
@@ -200,12 +202,13 @@ function FontList:getFontList()
         cache:save(self.fontinfo)
     elseif mark.cache_dirty then
         -- otherwise dump the db in binary (more compact), and only if something has changed
+        -- NOTE: The luajit/zstd codecs *ignore* the as_bytecode argument, as they *only* support bytecode ;).
         cache:save(self.fontinfo, true)
     end
 
     local names = self.fontnames
-    for _,coll in pairs(self.fontinfo) do
-        for _,v in ipairs(coll) do
+    for _, coll in pairs(self.fontinfo) do
+        for _, v in ipairs(coll) do
             local nlist = names[v.name] or {}
             assert(v.name)
             if #nlist == 0 then
@@ -237,7 +240,7 @@ function FontList:getFontArgFunc()
     require("document/credocument"):engineInit()
     local toggle = {}
     local face_list = cre.getFontFaces()
-    for k,v in ipairs(face_list) do
+    for _, v in ipairs(face_list) do
         table.insert(toggle, FontList:getLocalizedFontName(cre.getFontFaceFilenameAndFaceIndex(v)) or v)
     end
     return face_list, toggle

frontend/ui/data/onetime_migration.lua

@@ -7,7 +7,7 @@ local lfs = require("libs/libkoreader-lfs")
 local logger = require("logger")
 
 -- Date at which the last migration snippet was added
-local CURRENT_MIGRATION_DATE = 20220819
+local CURRENT_MIGRATION_DATE = 20220913
 
 -- Retrieve the date of the previous migration, if any
 local last_migration_date = G_reader_settings:readSetting("last_migration_date", 0)
@@ -435,5 +435,16 @@ if last_migration_date < 20220819 then
     end
 end
 
+-- Fontlist, cache format change (#9513)
+if last_migration_date < 20220913 then
+    logger.info("Performing one-time migration for 20220913")
+
+    local cache_path = DataStorage:getDataDir() .. "/cache/fontlist"
+    local ok, err = os.remove(cache_path .. "/fontinfo.dat")
+    if not ok then
+       logger.warn("os.remove:", err)
+    end
+end
+
 -- We're done, store the current migration date
 G_reader_settings:saveSetting("last_migration_date", CURRENT_MIGRATION_DATE)

frontend/ui/trapper.lua

@@ -13,8 +13,8 @@ local ConfirmBox = require("ui/widget/confirmbox")
 local InfoMessage = require("ui/widget/infomessage")
 local TrapWidget = require("ui/widget/trapwidget")
 local UIManager = require("ui/uimanager")
-local ffiutil = require("ffi/util")
 local dump = require("dump")
+local ffiutil = require("ffi/util")
 local logger = require("logger")
 local _ = require("gettext")