fix(collections): page the KOReader Shelves list instead of squeezing rows

Fit every library, collection and SmartScope on one screen shrank the rows with no lower bound, so a Kobo Libra with 45 shelves drew titles on top of each other. Keep rows at a readable minimum and run the shelves over further pages, repeating a section heading where it continues. Wide screens keep their two columns and turn the page once both are full, and only the shelves on the current page fetch thumbnails.

Closes #2042

koreader-plugin/bookorbit.koplugin/bookorbit_catalog_lists.lua

@@ -401,10 +401,10 @@ function CatalogLists:listPaging(context)
         end
         return windowPaging(self:listStart(context), context.page_size, context.total, context.has_next)
     end
-    if kind == "seriespage" or kind == "authorpage" then
+    if kind == "seriespage" or kind == "authorpage" or kind == "shelves" then
         return { page = context.local_page or 1, pages = context.local_pages or 1 }
     end
-    if kind == "search" or kind == "shelves" then return { page = 1, pages = 1 } end
+    if kind == "search" then return { page = 1, pages = 1 } end
     if (kind == "books" or kind == "reading") and context.page_size and context.total then
         return windowPaging(self:listStart(context), context.page_size, context.total)
     end
@@ -424,7 +424,7 @@ function CatalogLists:listGoPage(page)
     local p = self:listPaging(context)
     if page < 1 or (p.pages and page > p.pages) then return true end
     if page == p.page then return true end
-    if kind == "seriespage" or kind == "authorpage" then
+    if kind == "seriespage" or kind == "authorpage" or kind == "shelves" then
         context.local_page = page
         self:updateItems(nil, true)
         return true
@@ -1270,7 +1270,18 @@ function CatalogLists:listBody(kind, context)
             return list or {}
         end
         local m = { libraries = tag(shelves.libraries, "libraries"), collections = tag(shelves.collections, "collections"), scopes = tag(shelves["smart-scopes"], "smart-scopes") }
-        return function(f) return ListPages.shelvesBody(m, f) end, nil
+        local f = ListView.frame(sw, sh, { toolbar = false, offline = context.list_offline })
+        local layout = ListPages.shelvesLayout(m, f)
+        context.local_pages = #layout.pages
+        context.local_page = math.min(context.local_page or 1, #layout.pages)
+        m.local_page = context.local_page
+        context.visible_shelves = {}
+        for _ci, chunks in ipairs(layout.pages[context.local_page]) do
+            for _i, chunk in ipairs(chunks) do
+                for i = chunk.from, chunk.to do table.insert(context.visible_shelves, layout.groups[chunk.section][3][i]) end
+            end
+        end
+        return function(body_f) return ListPages.shelvesBody(m, body_f) end, m
     end
     if kind == "search" then
         local m = { results = context.results, query = context.query, scope = context.scope or "all", screen_w = sw, screen_h = sh,
@@ -1427,8 +1438,7 @@ function CatalogLists:listThumbnailBooks(kind, context)
     for _i, b in ipairs(context.books or {}) do addBook(b) end
     local entries = context.entries or {}
     if kind == "shelves" then
-        entries = {}
-        for _s, list in pairs(context.shelves or {}) do for _i, e in ipairs(list) do table.insert(entries, e) end end
+        entries = context.visible_shelves or {}
     elseif kind == "search" then
         entries = {}
         for _i, e in ipairs(context.results.authors or {}) do table.insert(entries, e) end

koreader-plugin/bookorbit.koplugin/bookorbit_list_data.lua

@@ -320,6 +320,63 @@ function ListData.fillRows(h, min_row)
     return { n = n, rh = math.floor(h / n) }
 end
 
+-- Libraries, collections and scopes over as many pages as their rows need.
+-- sizes: the rows each section holds. o: content_h, head_h, gap, row_h (the
+-- natural row), min_row_h, columns (the section numbers each column of a
+-- single page holds) and cols (how many columns a paged page runs).
+-- Everything stays on one page while its rows can keep min_row_h; past that
+-- the sections flow through the columns at min_row_h, a section that runs on
+-- repeating its heading. { pages = { [page] = { [col] = { { section, from,
+-- to } } } }, row_h }. An empty section keeps its heading and has to = 0.
+function ListData.shelfPages(sizes, o)
+    local rh = o.row_h
+    for _i, col in ipairs(o.columns) do
+        local rows = 0
+        for _j, s in ipairs(col) do rows = rows + sizes[s] end
+        local avail = o.content_h - #col * o.head_h - (#col - 1) * o.gap
+        rh = math.min(rh, math.floor(avail / math.max(1, rows)))
+    end
+    if rh >= o.min_row_h then
+        local page = {}
+        for ci, col in ipairs(o.columns) do
+            page[ci] = {}
+            for _j, s in ipairs(col) do table.insert(page[ci], { section = s, from = 1, to = sizes[s] }) end
+        end
+        return { pages = { page }, row_h = rh }
+    end
+
+    rh = o.min_row_h
+    local function blank()
+        local page = {}
+        for ci = 1, o.cols do page[ci] = {} end
+        return page
+    end
+    local pages = { blank() }
+    local p, c, used = 1, 1, 0
+    for s, n in ipairs(sizes) do
+        local from = 1
+        repeat
+            local gap = used > 0 and o.gap or 0
+            local room = math.floor((o.content_h - used - gap - o.head_h) / rh)
+            if room < 1 and used > 0 then
+                if c < o.cols then
+                    c = c + 1
+                else
+                    c, p = 1, p + 1
+                    pages[p] = blank()
+                end
+                used, gap = 0, 0
+                room = math.floor((o.content_h - o.head_h) / rh)
+            end
+            local take = math.min(n - from + 1, math.max(1, room))
+            table.insert(pages[p][c], { section = s, from = from, to = from + take - 1 })
+            used = used + gap + o.head_h + take * rh
+            from = from + take
+        until from > n
+    end
+    return { pages = pages, row_h = rh }
+end
+
 -- The page holding a 0-based index, 1-based.
 function ListData.pageOf(index, per)
     if not index or not per or per < 1 then return 1 end

koreader-plugin/bookorbit.koplugin/bookorbit_list_pages.lua

@@ -82,36 +82,45 @@ function Pages.listBody(m, f)
 end
 
 -- --------------------------------------------------------------- shelves
--- Libraries, collections and scopes on one page; wide screens set the
--- sections in two columns.
-function Pages.shelvesBody(m, f)
+-- Libraries, collections and scopes. Wide screens set the sections in two
+-- columns; more shelves than a screen holds at a readable row run on to
+-- further pages, a section that continues repeating its heading.
+function Pages.shelvesLayout(m, f)
     local groups = {
         { _("Libraries"), "library", m.libraries or {} },
         { _("Collections"), "folder-open", m.collections or {} },
         { _("SmartScopes"), "aperture", m.scopes or {} },
     }
     local wide = ListView.screenW() >= 760
-    local gap_s = px(14)
-    local columns = wide and { { groups[1] }, { groups[2], groups[3] } } or { groups }
-    local col_w = wide and math.floor((f.w - px(28)) / 2) or f.w
-    local rh = px(72)
-    for _i, cg in ipairs(columns) do
-        local heads, rows = #cg, 0
-        for _j, g in ipairs(cg) do rows = rows + #g[3] end
-        rh = math.min(rh, math.floor((f.content_h - heads * px(36) - (heads - 1) * gap_s) / math.max(1, rows)))
-    end
+    local layout = ListData.shelfPages({ #groups[1][3], #groups[2][3], #groups[3][3] }, {
+        content_h = f.content_h, head_h = px(36), gap = px(14), row_h = px(72), min_row_h = px(56),
+        columns = wide and { { 1 }, { 2, 3 } } or { { 1, 2, 3 } }, cols = wide and 2 or 1,
+    })
+    layout.groups, layout.wide = groups, wide
+    return layout
+end
+
+function Pages.shelvesBody(m, f)
+    local layout = Pages.shelvesLayout(m, f)
+    local page = math.max(1, math.min(#layout.pages, m.local_page or 1))
+    local col_w = layout.wide and math.floor((f.w - px(28)) / 2) or f.w
+    local rh = layout.row_h
     local col_els = {}
-    for _i, cg in ipairs(columns) do
-        local kids = {}
-        for gi, g in ipairs(cg) do
-            if gi > 1 then table.insert(kids, space(nil, gap_s)) end
-            table.insert(kids, ListView.sectionHead(col_w, { icon = g[2], label = g[1], count = #g[3] }))
-            for i, s in ipairs(g[3]) do
-                local inner = vstack({ w = col_w, h = rh }, { i > 1 and ListView.rule(col_w) or false, flex(0), ListView.shelfRow(col_w, rh, s), flex(0) })
-                table.insert(kids, region(inner, { kind = "open-shelf", shelf = s }, "shelf-" .. g[1] .. "-" .. i, "shelf-" .. s.section .. "-" .. tostring(s.id)))
+    for _ci, chunks in ipairs(layout.pages[page]) do
+        if #chunks > 0 then
+            local kids = {}
+            for ki, chunk in ipairs(chunks) do
+                local g = layout.groups[chunk.section]
+                if ki > 1 then table.insert(kids, space(nil, px(14))) end
+                table.insert(kids, ListView.sectionHead(col_w, { icon = g[2], label = g[1], count = #g[3] }))
+                for i = chunk.from, chunk.to do
+                    local s = g[3][i]
+                    local inner = vstack({ w = col_w, h = rh }, { i > chunk.from and ListView.rule(col_w) or false, flex(0), ListView.shelfRow(col_w, rh, s), flex(0) })
+                    table.insert(kids, region(inner, { kind = "open-shelf", shelf = s }, "shelf-" .. g[1] .. "-" .. i, "shelf-" .. s.section .. "-" .. tostring(s.id)))
+                end
             end
+            table.insert(col_els, vstack({ w = col_w }, kids))
         end
-        table.insert(col_els, vstack({ w = col_w }, kids))
     end
     return hstack({ w = f.w, valign = "top", gap = px(28) }, col_els)
 end

koreader-plugin/spec/bookorbit_shelves_paging_test.lua

@@ -0,0 +1,211 @@
+-- Proves the Shelves page never squeezes a row below its own text: shelves a
+-- screen cannot hold at a readable row run on to further pages, a section that
+-- continues repeats its heading, and every shelf is shown exactly once.
+
+package.loaded["gettext"] = function(text)
+    return text
+end
+package.loaded["ffi/util"] = {
+    template = function(text, ...)
+        local values = { ... }
+        return (text:gsub("%%(%d+)", function(index)
+            return tostring(values[tonumber(index)])
+        end))
+    end,
+}
+package.loaded["document/documentregistry"] = { hasProvider = function() return true end }
+package.loaded["util"] = {
+    fixUtf8 = function(value) return value end,
+    diskUsage = function() return { total = 1, available = 1 } end,
+}
+package.loaded["datetime"] = {
+    shortDayOfWeekToLongTranslation = {},
+    shortDayOfWeekTranslation = {},
+    longMonthTranslation = {},
+    shortMonthTranslation = {},
+    secondsToHour = function() return "12:00" end,
+    stringISO8601ToSeconds = function() return nil end,
+}
+package.loaded["ui/network/manager"] = { isConnected = function() return true end }
+package.loaded["bookorbit_stats_reader"] = {
+    getDailySeconds = function() return {} end,
+    getBookPace = function() return nil end,
+    getHourMinutes = function() return nil end,
+    getSessionRecords = function() return nil end,
+}
+package.loaded["lua-ljsqlite3/init"] = false
+package.loaded["datastorage"] = { getSettingsDir = function() return "/nonexistent" end }
+package.loaded["libs/libkoreader-lfs"] = { attributes = function() return nil end }
+package.loaded["ffi/blitbuffer"] = setmetatable({}, { __index = function() return 0 end })
+package.loaded["ui/font"] = { getFace = function() return {} end }
+
+-- A Kobo Libra: 1264 x 1680 at 300 dpi.
+local screen = { w = 1264, h = 1680, dpi = 300 }
+package.loaded["device"] = {
+    hasFrontlight = function() return false end,
+    screen = {
+        getWidth = function() return screen.w end,
+        getHeight = function() return screen.h end,
+        getDPI = function() return screen.dpi end,
+        scaleBySize = function(_, value)
+            return math.ceil(value * math.min(screen.w, screen.h) / 600)
+        end,
+    },
+}
+
+G_reader_settings = { isTrue = function() return false end }
+
+package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path
+
+local ListData = require("bookorbit_list_data")
+local BoardMetrics = require("bookorbit_board_metrics")
+
+local function assertEqual(actual, expected, label)
+    if actual ~= expected then
+        error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual)))
+    end
+end
+
+local function assertTrue(value, label)
+    if not value then error(label) end
+end
+
+-- Every shelf, in the order the pages show them, as "section:row".
+local function shown(layout)
+    local out = {}
+    for _p, page in ipairs(layout.pages) do
+        for _c, chunks in ipairs(page) do
+            for _k, chunk in ipairs(chunks) do
+                for i = chunk.from, chunk.to do table.insert(out, chunk.section .. ":" .. i) end
+            end
+        end
+    end
+    return out
+end
+
+local function expected(sizes)
+    local out = {}
+    for s, n in ipairs(sizes) do
+        for i = 1, n do table.insert(out, s .. ":" .. i) end
+    end
+    return out
+end
+
+local function assertAllShownOnce(layout, sizes, label)
+    assertEqual(table.concat(shown(layout), ","), table.concat(expected(sizes), ","), label .. ": every shelf shows once, in order")
+end
+
+local function assertColumnsFit(layout, o, label)
+    for p, page in ipairs(layout.pages) do
+        for c, chunks in ipairs(page) do
+            local used = 0
+            for k, chunk in ipairs(chunks) do
+                used = used + (k > 1 and o.gap or 0) + o.head_h + (chunk.to - chunk.from + 1) * layout.row_h
+            end
+            assertTrue(used <= o.content_h, string.format("%s: page %d column %d is %d tall in %d", label, p, c, used, o.content_h))
+        end
+    end
+end
+
+local function opts(extra)
+    local o = { content_h = 1000, head_h = 36, gap = 14, row_h = 72, min_row_h = 56, columns = { { 1, 2, 3 } }, cols = 1 }
+    for k, v in pairs(extra or {}) do o[k] = v end
+    return o
+end
+
+-- A few shelves keep their natural row on one page.
+local sizes = { 1, 2, 1 }
+local layout = ListData.shelfPages(sizes, opts())
+assertEqual(#layout.pages, 1, "a few shelves stay on one page")
+assertEqual(layout.row_h, 72, "at the natural row height")
+assertAllShownOnce(layout, sizes, "few shelves")
+
+-- Rows shrink toward the minimum before anything pages.
+sizes = { 3, 8, 3 }
+layout = ListData.shelfPages(sizes, opts())
+assertEqual(#layout.pages, 1, "fourteen shelves still fit one page")
+assertEqual(layout.row_h, math.floor((1000 - 3 * 36 - 2 * 14) / 14), "their rows share the room")
+assertTrue(layout.row_h >= 56 and layout.row_h < 72, "between the minimum and the natural row")
+
+-- One row more than the minimum allows pages instead of squeezing.
+local fits = math.floor((1000 - 3 * 36 - 2 * 14) / 56)
+sizes = { 3, fits - 3, 0 }
+layout = ListData.shelfPages(sizes, opts())
+assertEqual(#layout.pages, 1, "the most rows a page holds at the minimum stay on one page")
+assertTrue(layout.row_h >= 56 and layout.row_h < 58, "at about the minimum")
+sizes = { 3, fits - 2, 0 }
+layout = ListData.shelfPages(sizes, opts())
+assertTrue(#layout.pages > 1, "one more row pages")
+assertEqual(layout.row_h, 56, "and never goes below the minimum")
+assertAllShownOnce(layout, sizes, "boundary")
+assertColumnsFit(layout, opts(), "boundary")
+
+-- The reported library: 3 libraries, 27 collections, 15 scopes.
+sizes = { 3, 27, 15 }
+layout = ListData.shelfPages(sizes, opts())
+assertTrue(#layout.pages > 1, "45 shelves run over several pages")
+assertEqual(layout.row_h, 56, "at the minimum row")
+assertAllShownOnce(layout, sizes, "reported library")
+assertColumnsFit(layout, opts(), "reported library")
+local first = layout.pages[1][1][1]
+assertEqual(first.section .. ":" .. first.from, "1:1", "Libraries open the first page")
+local second = layout.pages[2][1][1]
+assertTrue(second.from > 1 or second.section > 1, "a later page continues where the last stopped")
+for p = 2, #layout.pages do
+    local chunk = layout.pages[p][1][1]
+    assertTrue(chunk.to >= chunk.from, "a continuing section always has a row under its heading (page " .. p .. ")")
+end
+
+-- Two columns flow left to right, then on to the next page.
+local wide = opts({ columns = { { 1 }, { 2, 3 } }, cols = 2 })
+layout = ListData.shelfPages(sizes, wide)
+assertTrue(#layout.pages > 1, "45 shelves page on a wide screen too")
+assertTrue(#layout.pages < #ListData.shelfPages(sizes, opts()).pages, "two columns need fewer pages than one")
+assertAllShownOnce(layout, sizes, "wide")
+assertColumnsFit(layout, wide, "wide")
+assertEqual(#layout.pages[1][2] > 0, true, "the second column fills before a page turns")
+
+-- A wide page whose columns fit keeps the sections where they were.
+sizes = { 3, 6, 4 }
+layout = ListData.shelfPages(sizes, wide)
+assertEqual(#layout.pages, 1, "a few shelves fit one wide page")
+assertEqual(#layout.pages[1][1], 1, "Libraries have the left column")
+assertEqual(#layout.pages[1][2], 2, "Collections and SmartScopes share the right")
+assertAllShownOnce(layout, sizes, "wide fit")
+
+-- Empty sections keep their heading and never break the flow.
+layout = ListData.shelfPages({ 0, 0, 0 }, opts())
+assertEqual(#layout.pages, 1, "no shelves at all is one page")
+assertEqual(layout.pages[1][1][2].to, 0, "an empty section has no rows")
+layout = ListData.shelfPages({ 0, 40, 0 }, opts())
+assertAllShownOnce(layout, { 0, 40, 0 }, "only collections")
+assertColumnsFit(layout, opts(), "only collections")
+
+-- No section is left with a heading and no rows, however the page breaks.
+for total = 1, 80 do
+    local s = { 3, total, 15 }
+    local l = ListData.shelfPages(s, opts())
+    assertAllShownOnce(l, s, "sweep " .. total)
+    assertColumnsFit(l, opts(), "sweep " .. total)
+    for p, page in ipairs(l.pages) do
+        for _c, chunks in ipairs(page) do
+            for _k, chunk in ipairs(chunks) do
+                assertTrue(chunk.to >= chunk.from or s[chunk.section] == 0, "sweep " .. total .. ": page " .. p .. " has a heading without rows")
+            end
+        end
+    end
+end
+
+-- On the Libra itself the minimum row holds the two lines of a shelf's text.
+local px, lh1 = BoardMetrics.px, BoardMetrics.lh1
+local text_h = lh1(15) + px(2) + lh1(13)
+assertTrue(px(56) >= text_h, string.format("a minimum row (%d) holds a title and its count (%d)", px(56), text_h))
+local content_h = screen.h - px(6) - px(8) - px(64) - px(8) - px(40)
+local libra = ListData.shelfPages({ 3, 27, 15 }, {
+    content_h = content_h, head_h = px(36), gap = px(14), row_h = px(72), min_row_h = px(56),
+    columns = { { 1, 2, 3 } }, cols = 1,
+})
+assertTrue(libra.row_h >= text_h, string.format("the reported library's rows (%d) are never shorter than their text (%d)", libra.row_h, text_h))
+assertTrue(#libra.pages >= 2, "the reported library pages on a Libra")
+
+print("bookorbit_shelves_paging_test.lua: ok")