Page Overlap: Fix rectangle computation and arrow mode (#7269)

* In ReaderPaging, the panning step pre-PanningUpdate can be wildly overshot near page edges, so, use the corrected value instead by recomputing it after the panning has been effectively computed by ReaderView.
This fixes slight inaccuracies, as well as glaring mistakes when going backwards, or when near page edges.
This is in line with how ReaderRolling computes the value, which I only realized later because I'm an idiot. 
* Minor cleanups around the handling of the dim_area Geom object in general.

* Fix the "Arrow" page overlap mode to be painted in the right coordinates when going backward. Issue might not have been terribly clear because of the previous issue ;).
* Center the arrow's point, while we're here.
* Don't use AlphaContainer to make it translucent, because AlphaContainer is horribly broken, and has weird quirks and behavior that make no sense to me unless some very specific and unlikely constraints are met, and they definitely aren't here.
This fixes the arrow copying an arrow-sized square of the original page the book was opened on on the top-left corner of *every* page with an arrow. (lol).
* Do real proper alpha-blending via Icon/ImageWidget from the original icon, instead of faking it via addBlitFrom, in order to avoid the dimming *around* the triangle's shape.

frontend/apps/reader/modules/readerpaging.lua

@@ -233,7 +233,7 @@ function ReaderPaging:addToMainMenu(menu_items)
             callback = function()
                 self.show_overlap_enable = not self.show_overlap_enable
                 if not self.show_overlap_enable then
-                    self.view:resetDimArea()
+                    self.view.dim_area:clear()
                 end
             end,
             separator = true,
@@ -916,7 +916,7 @@ function ReaderPaging:onGotoPageRel(diff)
         end
     end
 
-    -- Move the view area towerds line end
+    -- Move the view area towards line end
     new_va[x] = old_va[x] + x_pan_off
     new_va[y] = old_va[y]
 
@@ -937,31 +937,26 @@ function ReaderPaging:onGotoPageRel(diff)
     end
 
     -- signal panning update
-    local panned_x, panned_y = (new_va.x - old_va.x), (new_va.y - old_va.y)
-    -- adjust for crazy floating point overflow...
-    if math.abs(panned_x) < 1 then
-        panned_x = 0
-    end
-    if math.abs(panned_y) < 1 then
-        panned_y = 0
-    end
+    local panned_x, panned_y = math.floor(new_va.x - old_va.x), math.floor(new_va.y - old_va.y)
     self.view:PanningUpdate(panned_x, panned_y)
 
-    -- update dim area in ReaderView
+    -- Update dim area in ReaderView
     if self.show_overlap_enable then
         if self.current_page ~= old_page then
-            self.view.dim_area.x = 0
-            self.view.dim_area.y = 0
+            self.view.dim_area:clear()
         else
-            self.view.dim_area.h = new_va.h - math.abs(panned_y)
-            self.view.dim_area.w = new_va.w - math.abs(panned_x)
+            -- We're post PanningUpdate, recompute via self.visible_area instead of new_va for accuracy, it'll have been updated via ViewRecalculate
+            panned_x, panned_y = math.floor(self.visible_area.x - old_va.x), math.floor(self.visible_area.y - old_va.y)
+
+            self.view.dim_area.h = self.visible_area.h - math.abs(panned_y)
+            self.view.dim_area.w = self.visible_area.w - math.abs(panned_x)
             if panned_y < 0 then
-                self.view.dim_area.y = new_va.y - panned_y
+                self.view.dim_area.y = self.visible_area.h - self.view.dim_area.h
             else
                 self.view.dim_area.y = 0
             end
             if panned_x < 0 then
-                self.view.dim_area.x = new_va.x - panned_x
+                self.view.dim_area.x = self.visible_area.w - self.view.dim_area.w
             else
                 self.view.dim_area.x = 0
             end

frontend/apps/reader/modules/readerrolling.lua

@@ -423,7 +423,7 @@ You can set how many lines are shown.]])
             callback = function()
                 self.show_overlap_enable = not self.show_overlap_enable
                 if not self.show_overlap_enable then
-                    self.view:resetDimArea()
+                    self.view.dim_area:clear()
                 end
             end
         },
@@ -950,10 +950,10 @@ function ReaderRolling:_gotoPos(new_pos, do_dim_area)
         if self.current_pos > max_pos - self.ui.dimen.h/2 then
             -- Avoid a fully dimmed page when reaching end of document
             -- (the scroll would bump and not be a full page long)
-            self.view:resetDimArea()
+            self.view.dim_area:clear()
         end
     else
-        self.view:resetDimArea()
+        self.view.dim_area:clear()
     end
     self.ui.document:gotoPos(new_pos)
     -- The current page we get in scroll mode may be a bit innacurate,

frontend/apps/reader/modules/readerview.lua

@@ -2,7 +2,6 @@
 ReaderView module handles all the screen painting for document browsing.
 ]]
 
-local AlphaContainer = require("ui/widget/container/alphacontainer")
 local Blitbuffer = require("ffi/blitbuffer")
 local ConfirmBox = require("ui/widget/confirmbox")
 local Device = require("device")
@@ -80,17 +79,13 @@ function ReaderView:init()
     -- fix recalculate from close document pageno
     self.state.page = nil
     -- fix inherited dim_area for following opened documents
-    self:resetDimArea()
+    self.dim_area = Geom:new{w = 0, h = 0}
     self:addWidgets()
     self.emitHintPageEvent = function()
         self.ui:handleEvent(Event:new("HintPage", self.hinting))
     end
 end
 
-function ReaderView:resetDimArea()
-    self.dim_area = Geom:new{w = 0, h = 0}
-end
-
 function ReaderView:addWidgets()
     self.dogear = ReaderDogear:new{
         view = self,
@@ -105,14 +100,13 @@ function ReaderView:addWidgets()
         ui = self.ui,
     }
     local arrow_size = Screen:scaleBySize(16)
-    self.arrow = AlphaContainer:new{
-        alpha = 0.6,
-        IconWidget:new{
-            icon = "control.expand",
-            width = arrow_size,
-            height = arrow_size,
-        }
+    self.arrow = IconWidget:new{
+        icon = "control.expand.alpha",
+        width = arrow_size,
+        height = arrow_size,
+        alpha = true, -- Keep the alpha layer intact, the fill opacity is set at 75%
     }
+
     self[1] = self.dogear
     self[2] = self.footer
     self[3] = self.flipping
@@ -175,14 +169,16 @@ function ReaderView:paintTo(bb, x, y)
     end
 
     -- dim last read area
-    if self.dim_area.w ~= 0 and self.dim_area.h ~= 0 then
+    if not self.dim_area:isEmpty() then
         if self.page_overlap_style == "dim" then
             bb:dimRect(
                 self.dim_area.x, self.dim_area.y,
                 self.dim_area.w, self.dim_area.h
             )
         elseif self.page_overlap_style == "arrow" then
-            self.arrow:paintTo(bb, 0, self.dim_area.h)
+            local center_offset = bit.rshift(self.arrow.height, 1)
+            -- Paint at the proper y origin depending on wheter we paged forward (dim_area.y == 0) or backward
+            self.arrow:paintTo(bb, 0, self.dim_area.y == 0 and self.dim_area.h - center_offset or self.dim_area.y - center_offset)
         end
     end
     -- draw saved highlight
@@ -594,8 +590,7 @@ function ReaderView:recalculate()
             self.visible_area:offsetWithin(self.page_area, 0, 0)
         end
         -- clear dim area
-        self.dim_area.w = 0
-        self.dim_area.h = 0
+        self.dim_area:clear()
         self.ui:handleEvent(
             Event:new("ViewRecalculate", self.visible_area, self.page_area))
     else

frontend/ui/geometry.lua

@@ -404,4 +404,27 @@ function Geom:center()
     }
 end
 
+--[[--
+Resets an existing Geom object to zero.
+@treturn Geom
+]]
+function Geom:clear()
+    self.x = 0
+    self.y = 0
+    self.w = 0
+    self.h = 0
+    return self
+end
+
+--[[--
+Checks if a dimension or rectangle is empty.
+@return bool
+]]
+function Geom:isEmpty()
+    if self.w == 0 or self.h == 0 then
+        return true
+    end
+    return false
+end
+
 return Geom

frontend/ui/widget/container/alphacontainer.lua

@@ -35,9 +35,20 @@ function AlphaContainer:paintTo(bb, x, y)
     local private_bb = self.private_bb
 
     if self.background_bb then
-        -- we have a saved copy of what was below our paint area
-        -- we restore this first
-        bb:blitFrom(self.background_bb, self.background_bb_x, self.background_bb_y)
+        -- NOTE: Best as I can tell, this was an attempt at avoiding alpha layering issues when an AlphaContainer is repainted *at the same coordinates* AND *over the same background*.
+        --       Unfortunately, those are hard constraints to respect, and, while we can take care of the first by invalidating the cache if coordinates have changed,
+        --       we can't do anything about the second (and that's exactly what happens in ReaderUI when paging around, for example: that'll obviously have changed what's below AlphaContainer ;)).
+        --       FWIW, MovableContainer's alpha handling rely on callers using setDirty("all") to force a repaint of the whole stack to avoid layering issues.
+        --       A better approach would probably involve letting UIManager handle it: if it finds a dirty translucent widget, mark all the widgets below it dirty, too...
+        if self.background_bb_x == x and self.background_bb_y == y then
+            bb:blitFrom(self.background_bb, self.background_bb_x, self.background_bb_y)
+        else
+            -- We moved, invalidate the bg cache.
+            self.background_bb:free()
+            self.background_bb = nil
+            self.background_bb_x = nil
+            self.background_bb_y = nil
+        end
     end
 
     if not private_bb
@@ -62,9 +73,11 @@ function AlphaContainer:paintTo(bb, x, y)
             self.background_bb = Blitbuffer.new(contentSize.w, contentSize.h, bb:getType())
         end
         self.background_bb:blitFrom(bb, 0, 0, x, y)
+        self.background_bb_x = x
+        self.background_bb_y = y
     end
 
-    -- now have our childs paint to the private blitbuffer
+    -- now have our child widget paint to the private blitbuffer
     private_bb:fill(Blitbuffer.COLOR_WHITE)
     self[1]:paintTo(private_bb, 0, 0)
 

frontend/ui/widget/iconwidget.lua

@@ -35,7 +35,9 @@ local IconWidget = ImageWidget:extend{
     -- be overriden by callers.
     width = Screen:scaleBySize(DGENERIC_ICON_SIZE), -- our icons are square
     height = Screen:scaleBySize(DGENERIC_ICON_SIZE),
-    alpha = false, --- @note: our icons have a transparent background, but we flatten them at caching time, and this flag is only checked at blitting time.
+    alpha = false, --- @note: Our icons have a transparent background, but, by default, we flatten them at caching time.
+                   ---        Our caller may choose to override that by setting this to true, in which case,
+                   ---        the alpha layer will be kept intact, and we'll do alpha-blending at blitting time.
     is_icon = true, -- avoid dithering in ImageWidget:paintTo()
 }
 

frontend/ui/widget/imagewidget.lua

@@ -179,11 +179,11 @@ function ImageWidget:_loadfile()
                 end
             end
 
-            -- Now, if that was *also* one of our icons, and it has an alpha channel,
-            -- compose it against a background-colored BB now, and cache *that*.
+            -- Now, if that was *also* one of our icons, we haven't explicitly requested to keep the alpha channel intact,
+            -- and it actually has an alpha channel, compose it against a background-colored BB now, and cache *that*.
             -- This helps us avoid repeating alpha-blending steps down the line,
             -- and also ensures icon highlights/unhighlights behave sensibly.
-            if self.is_icon then
+            if self.is_icon and not self.alpha then
                 local bbtype = self._bb:getType()
                 if bbtype == Blitbuffer.TYPE_BB8A or bbtype == Blitbuffer.TYPE_BBRGB32 then
                     local icon_bb = Blitbuffer.new(self._bb.w, self._bb.h, Screen.bb:getType())

resources/icons/mdlight/control.expand.alpha.svg

@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg
+   xmlns:dc="http://purl.org/dc/elements/1.1/"
+   xmlns:cc="http://creativecommons.org/ns#"
+   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
+   xmlns:svg="http://www.w3.org/2000/svg"
+   xmlns="http://www.w3.org/2000/svg"
+   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
+   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
+   version="1.1"
+   width="16"
+   height="16"
+   viewBox="0 0 16 16"
+   enable-background="new 0 0 76.00 76.00"
+   xml:space="preserve"
+   id="svg2"
+   inkscape:version="1.0.2 (e86c870879, 2021-01-15)"
+   sodipodi:docname="control.expand.alpha.svg"
+   inkscape:export-xdpi="90"
+   inkscape:export-ydpi="90"><metadata
+     id="metadata10"><rdf:RDF><cc:Work
+         rdf:about=""><dc:format>image/svg+xml</dc:format><dc:type
+           rdf:resource="http://purl.org/dc/dcmitype/StillImage" /><dc:title /></cc:Work></rdf:RDF></metadata><defs
+     id="defs8" /><sodipodi:namedview
+     pagecolor="#ffffff"
+     bordercolor="#666666"
+     borderopacity="1"
+     objecttolerance="10"
+     gridtolerance="10"
+     guidetolerance="10"
+     inkscape:pageopacity="0"
+     inkscape:pageshadow="2"
+     inkscape:window-width="2560"
+     inkscape:window-height="1381"
+     id="namedview6"
+     showgrid="false"
+     inkscape:zoom="12.421053"
+     inkscape:cx="30.041478"
+     inkscape:cy="18.178404"
+     inkscape:window-x="0"
+     inkscape:window-y="0"
+     inkscape:window-maximized="1"
+     inkscape:current-layer="svg2"
+     inkscape:document-rotation="0" /><path
+     d="M 3,0 13,8 13,8 3,16 z"
+     id="path4"
+     inkscape:connector-curvature="0"
+     style="fill:#000000;fill-opacity:0.75;stroke-width:0.2;stroke-linejoin:round;opacity:1"
+     sodipodi:nodetypes="ccccc" /></svg>