ReaderZooming: Deal with some more fallout of the new zoom modes (#7728)

* Namely, ensure zoom_mode is consistent with genus & type *both ways*. (I only dealt with the "no zoom_mode" case in my original fixup).
Because documents with settings dating back from before the new zoom modes had "old" zoom_mode settings mixed with "new" genus/type defaults that didn't agree with each other.
It lead to super-confusing ConfigDialog behavior, because ConfigDialog was in fact not reflecting the reality.
(As the source of truth is actually `zoom_mode`).

* There was a snafu in manual mode, because of the extremely weird way prefixes are handled by Configurable/ReaderConfig/DocSettings/ConfigDialog.
So, make sure we only have a *single* zoom_factor, and that it's updated and saved properly under the right name everywhere.

Fixes inconsistencies between first swapping to manual mode, and what the ConfigDialog said/did (because again: possibly a lie), vs., re-opening the same document, which would magically use *different* settings, closer to what was expected (but still broken because of the prefix mismatch and a disagreement on defaults between the two variants).

Fallout from #6885

frontend/apps/reader/modules/readerzooming.lua

@@ -46,9 +46,9 @@ local ReaderZooming = InputContainer:new{
     -- for pan mode: fit to width/zoom_factor,
     -- with overlap of zoom_overlap_h % (horizontally)
     -- and zoom_overlap_v % (vertically).
-    zoom_factor = 2,
+    kopt_zoom_factor = 1.5,
     zoom_pan_settings = {
-        "zoom_factor",
+        "kopt_zoom_factor",
         "zoom_overlap_h",
         "zoom_overlap_v",
         "zoom_bottom_to_top",
@@ -122,11 +122,55 @@ function ReaderZooming:init()
 end
 
 function ReaderZooming:onReadSettings(config)
+    -- We may need to poke at the Configurable directly, because ReaderConfig is instantiated before us,
+    -- so simply updating the DocSetting doesn't cut it...
+    -- Conditional because this is an optional engine feature (only if self.document.info.configurable is true).
+    local configurable = self.document and self.document.configurable
+
     -- If we have a composite zoom_mode stored, use that
     local zoom_mode = config:readSetting("zoom_mode")
                    or G_reader_settings:readSetting("zoom_mode")
-    -- Otherwise, build it from the split genus & type settings
-    if not zoom_mode then
+    if zoom_mode then
+        -- Validate it first
+        zoom_mode = util.arrayContains(self.available_zoom_modes, zoom_mode)
+                and zoom_mode
+                 or self.DEFAULT_ZOOM_MODE
+
+        -- Make sure the split genus & type match, to have an up-to-date ConfigDialog...
+        local mode_to_genus = {}
+        for k, v in pairs(self.zoom_mode_genus_map) do
+            mode_to_genus[v] = k
+        end
+        local mode_to_type = {}
+        for k, v in pairs(self.zoom_mode_type_map) do
+            mode_to_type[v] = k
+        end
+
+        -- Quick'n dirty zoom_mode to genus/type conversion...
+        local zgenus, ztype = zoom_mode:match("^(page)(%l*)$")
+        if not zgenus then
+            zgenus, ztype = zoom_mode:match("^(content)(%l*)$")
+        end
+        if not zgenus then
+            zgenus = zoom_mode
+        end
+        if not ztype then
+            ztype = ""
+        end
+
+        local zoom_mode_genus = mode_to_genus[zgenus]
+        config:saveSetting("kopt_zoom_mode_genus", zoom_mode_genus)
+        if configurable then
+            -- Configurable keys aren't prefixed, unlike the actual settings...
+            configurable.zoom_mode_genus = zoom_mode_genus
+        end
+        local zoom_mode_type = mode_to_type[ztype]
+        config:saveSetting("kopt_zoom_mode_type", zoom_mode_type)
+        if configurable then
+            configurable.zoom_mode_type = zoom_mode_type
+        end
+    else
+        -- Otherwise, build it from the split genus & type settings
         local zoom_mode_genus = config:readSetting("kopt_zoom_mode_genus")
                              or G_reader_settings:readSetting("kopt_zoom_mode_genus")
         local zoom_mode_type = config:readSetting("kopt_zoom_mode_type")
@@ -139,10 +183,23 @@ function ReaderZooming:onReadSettings(config)
             zoom_mode_type = self.zoom_mode_type_map[zoom_mode_type]
             zoom_mode = zoom_mode_genus .. zoom_mode_type
         end
+
+        -- Validate it
+        zoom_mode = util.arrayContains(self.available_zoom_modes, zoom_mode)
+                and zoom_mode
+                 or self.DEFAULT_ZOOM_MODE
+    end
+
+    -- Import legacy zoom_factor settings
+    if config:has("zoom_factor") and config:hasNot("kopt_zoom_factor") then
+        config:saveSetting("kopt_zoom_factor", config:readSetting("zoom_factor"))
+        if configurable then
+            configurable.zoom_factor = config:readSetting("kopt_zoom_factor")
+        end
+        config:delSetting("zoom_factor")
+    elseif config:has("zoom_factor") and config:has("kopt_zoom_factor") then
+        config:delSetting("zoom_factor")
     end
-    zoom_mode = util.arrayContains(self.available_zoom_modes, zoom_mode)
-            and zoom_mode
-             or self.DEFAULT_ZOOM_MODE
 
     -- Don't stomp on normal_zoom_mode in ReaderKoptListener if we're reflowed...
     local is_reflowed = config:has("kopt_text_wrap") and config:readSetting("kopt_text_wrap") == 1
@@ -281,7 +338,7 @@ function ReaderZooming:onDefineZoom(btn, when_applied_callback)
     if zoom_mode == "columns" or zoom_mode == "rows" then
         if btn ~= "columns" and btn ~= "rows" then
             self.ui:handleEvent(Event:new("SetZoomPan", settings, true))
-            settings.zoom_factor = self:setNumberOf(
+            settings.kopt_zoom_factor = self:setNumberOf(
                 zoom_mode,
                 zoom_range_number,
                 zoom_mode == "columns" and settings.zoom_overlap_h or settings.zoom_overlap_v
@@ -290,10 +347,14 @@ function ReaderZooming:onDefineZoom(btn, when_applied_callback)
     elseif zoom_mode == "manual" then
         if btn == "manual" then
             config.zoom_factor = self:getNumberOf("columns")
+            settings.kopt_zoom_factor = config.zoom_factor
+            -- We *want* a redraw the first time we swap to manual mode (like any other mode swap)
+            self.ui:handleEvent(Event:new("SetZoomPan", settings))
         else
             self:setNumberOf("columns", zoom_factor)
+            -- No redraw here, because setNumberOf already took care of it
+            self.ui:handleEvent(Event:new("SetZoomPan", settings, true))
         end
-        self.ui:handleEvent(Event:new("SetZoomPan", settings, true))
     end
     self.ui:handleEvent(Event:new("SetZoomMode", zoom_mode))
     if btn == "columns" or btn == "rows" then
@@ -319,7 +380,7 @@ function ReaderZooming:onDefineZoom(btn, when_applied_callback)
                 settings.zoom_overlap_h,
                 ("%.2f"):format(self:getNumberOf("rows", settings.zoom_overlap_v)),
                 settings.zoom_overlap_v,
-                ("%.2f"):format(self:getNumberOf("columns"))),
+                ("%.2f"):format(config.zoom_factor)),
             dismiss_callback = when_applied_callback,
         })
     end
@@ -453,9 +514,9 @@ function ReaderZooming:getZoom(pageno)
     elseif self.zoom_mode == "free" then
         zoom = self.zoom
     else
-        local zoom_factor = self.ui.doc_settings:readSetting("zoom_factor")
-                         or G_reader_settings:readSetting("zoom_factor")
-                         or self.zoom_factor
+        local zoom_factor = self.ui.doc_settings:readSetting("kopt_zoom_factor")
+                         or G_reader_settings:readSetting("kopt_zoom_factor")
+                         or self.kopt_zoom_factor
         zoom = zoom_w * zoom_factor
     end
     if zoom and zoom > 10 and not DocCache:willAccept(zoom * (self.dimen.w * self.dimen.h + 512)) then
@@ -562,7 +623,7 @@ function ReaderZooming:setNumberOf(what, num, overlap)
     if what == "rows" then
         zoom_factor = zoom_factor * zoom_h / zoom_w
     end
-    self.ui:handleEvent(Event:new("SetZoomPan", {zoom_factor = zoom_factor}))
+    self.ui:handleEvent(Event:new("SetZoomPan", {kopt_zoom_factor = zoom_factor}))
     self.ui:handleEvent(Event:new("RedrawCurrentPage"))
 end
 

frontend/ui/data/creoptions.lua

@@ -46,7 +46,7 @@ local function prettifyCreWeights(t)
 end
 
 local CreOptions = {
-    prefix = 'copt',
+    prefix = "copt",
     {
         icon = "appbar.rotation",
         options = {

frontend/ui/data/koptoptions.lua

@@ -21,7 +21,7 @@ local tableOfNumbersToTableOfStrings = function(numbers)
 end
 
 local KoptOptions = {
-    prefix = 'kopt',
+    prefix = "kopt",
     needs_redraw_on_change = true,
     {
         icon = "appbar.rotation",

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 = 20210518
+local CURRENT_MIGRATION_DATE = 20210521
 
 -- Retrieve the date of the previous migration, if any
 local last_migration_date = G_reader_settings:readSetting("last_migration_date", 0)
@@ -234,5 +234,18 @@ if last_migration_date < 20210518 then
     G_reader_settings:saveSetting("footer", settings)
 end
 
+-- 20210521, ReaderZooming, zoom_factor -> kopt_zoom_factor, https://github.com/koreader/koreader/pull/7728
+if last_migration_date < 20210521 then
+    logger.info("Performing one-time migration for 20210521")
+
+    -- ReaderZooming:init has the same logic for individual DocSettings in onReadSettings
+    if G_reader_settings:has("zoom_factor") and G_reader_settings:hasNot("kopt_zoom_factor") then
+        G_reader_settings:saveSetting("kopt_zoom_factor", G_reader_settings:readSetting("zoom_factor"))
+        G_reader_settings:delSetting("zoom_factor")
+    elseif G_reader_settings:has("zoom_factor") and G_reader_settings:has("kopt_zoom_factor") then
+        G_reader_settings:delSetting("zoom_factor")
+    end
+end
+
 -- We're done, store the current migration date
 G_reader_settings:saveSetting("last_migration_date", CURRENT_MIGRATION_DATE)