From b23af97914326fde91c27c2bb0cef46b9fa72caa Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sun, 14 Jun 2020 02:21:41 +0200 Subject: [PATCH] Fix partial HW dithered refreshes sometimes appearing to shift refreshed content (#6267) * Fix HW dithered partial refreshes sometimes behaving as if the refreshed content had moved a few pixels to the side... Probably a kernel issue with the alignment fixup in the EPDC? * Get rid of the legacy coordinates fixup It shouldn't be necessary anymore. And I'd rather fix the root cause, anyway. * Bump base (https://github.com/koreader/koreader-base/pull/1116) * Missed a few DIVs in #6224 --- base | 2 +- .../apps/reader/modules/readercropping.lua | 2 +- frontend/apps/reader/modules/readerfont.lua | 2 +- .../apps/reader/modules/readerhighlight.lua | 2 +- .../apps/reader/modules/readerrolling.lua | 6 +-- .../apps/reader/modules/readerstyletweak.lua | 2 +- .../apps/reader/modules/readertypography.lua | 2 +- frontend/ui/translator.lua | 2 +- frontend/ui/uimanager.lua | 47 +++++++++++++++++-- frontend/ui/widget/configdialog.lua | 2 +- frontend/ui/widget/confirmbox.lua | 2 +- frontend/ui/widget/infomessage.lua | 2 +- frontend/ui/widget/multiconfirmbox.lua | 2 +- frontend/ui/widget/notification.lua | 2 +- frontend/ui/widget/textboxwidget.lua | 2 +- 15 files changed, 58 insertions(+), 21 deletions(-) diff --git a/base b/base index 3c3b7ca24..21ac242ab 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit 3c3b7ca24bb7ac56a0b03d61a1e791edd377b088 +Subproject commit 21ac242ab2d536c9923dcb263830fd204b100960 diff --git a/frontend/apps/reader/modules/readercropping.lua b/frontend/apps/reader/modules/readercropping.lua index c29636ecd..878703984 100644 --- a/frontend/apps/reader/modules/readercropping.lua +++ b/frontend/apps/reader/modules/readercropping.lua @@ -112,7 +112,7 @@ function ReaderCropping:onPageCrop(mode) self.ui:handleEvent(Event:new("SetZoomMode", "page", "cropping")) end self.ui:handleEvent(Event:new("SetDimensions", - Geom:new{w = Screen:getWidth(), h = Screen:getHeight()*11/12}) + Geom:new{w = Screen:getWidth(), h = math.floor(Screen:getHeight()*11/12)}) ) self.bbox_widget = BBoxWidget:new{ crop = self, diff --git a/frontend/apps/reader/modules/readerfont.lua b/frontend/apps/reader/modules/readerfont.lua index 9479b125d..0509311e3 100644 --- a/frontend/apps/reader/modules/readerfont.lua +++ b/frontend/apps/reader/modules/readerfont.lua @@ -154,7 +154,7 @@ function ReaderFont:onShowFontMenu() title = self.font_menu_title, item_table = self.face_table, width = Screen:getWidth() - 100, - height = Screen:getHeight() / 2, + height = math.floor(Screen:getHeight() * 0.5), single_line = true, perpage_custom = 8, } diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua index cbad29e60..ba8438448 100644 --- a/frontend/apps/reader/modules/readerhighlight.lua +++ b/frontend/apps/reader/modules/readerhighlight.lua @@ -707,7 +707,7 @@ function ReaderHighlight:onHoldPan(_, ges) -- Also, we are not able to move hold_pos.x out of screen, -- so if we started on the right page, ignore top left corner, -- and if we started on the left page, ignore bottom right corner. - local screen_half_width = math.floor(Screen:getWidth() * 1/2) + local screen_half_width = math.floor(Screen:getWidth() * 0.5) if self.hold_pos.x >= screen_half_width and is_in_prev_page_corner then return true elseif self.hold_pos.x <= screen_half_width and is_in_next_page_corner then diff --git a/frontend/apps/reader/modules/readerrolling.lua b/frontend/apps/reader/modules/readerrolling.lua index 4d6402243..4feecb1ff 100644 --- a/frontend/apps/reader/modules/readerrolling.lua +++ b/frontend/apps/reader/modules/readerrolling.lua @@ -634,7 +634,7 @@ function ReaderRolling:onGotoXPointer(xp, marker_xp) if BD.mirroredUILayout() then -- In the middle margin, on the right of text -- Same trick as below, assuming page2_x is equal to page 1 right x - screen_x = Screen:getWidth() / 2 + screen_x = math.floor(Screen:getWidth() * 0.5) local page2_x = self.ui.document:getPageOffsetX(self.ui.document:getCurrentPage()+1) marker_w = page2_x + marker_w - screen_x screen_x = screen_x - marker_w @@ -648,7 +648,7 @@ function ReaderRolling:onGotoXPointer(xp, marker_xp) -- In the middle margin, on the left of text -- This is a bit tricky with how the middle margin is sized -- by crengine (see LVDocView::updateLayout() in lvdocview.cpp) - screen_x = Screen:getWidth() / 2 + screen_x = math.floor(Screen:getWidth() * 0.5) local page2_x = self.ui.document:getPageOffsetX(self.ui.document:getCurrentPage()+1) marker_w = page2_x + marker_w - screen_x end @@ -1086,7 +1086,7 @@ function ReaderRolling:showEngineProgress(percent) -- so it does not override the footer or a bookmark dogear local x = 0 local y = Size.margin.small - local w = Screen:getWidth() / 3 + local w = math.floor(Screen:getWidth() / 3) local h = Size.line.progress if self.engine_progress_widget then self.engine_progress_widget:setPercentage(percent) diff --git a/frontend/apps/reader/modules/readerstyletweak.lua b/frontend/apps/reader/modules/readerstyletweak.lua index 28dee9e49..678cd09c8 100644 --- a/frontend/apps/reader/modules/readerstyletweak.lua +++ b/frontend/apps/reader/modules/readerstyletweak.lua @@ -32,7 +32,7 @@ local TweakInfoWidget = InputContainer:new{ is_global_default = nil, toggle_global_default_callback = function() end, modal = true, - width = Screen:getWidth()*3/4, + width = math.floor(Screen:getWidth() * 0.75), } function TweakInfoWidget:init() diff --git a/frontend/apps/reader/modules/readertypography.lua b/frontend/apps/reader/modules/readertypography.lua index f45395380..7d1312bce 100644 --- a/frontend/apps/reader/modules/readertypography.lua +++ b/frontend/apps/reader/modules/readertypography.lua @@ -231,7 +231,7 @@ When the book's language tag is not among our presets, no specific features will title = _("Language tags (and hyphenation dictionaries) used since start up"), text = status_text, text_face = Font:getFace("smallinfont"), - height = Screen:getHeight() * 4/5, + height = math.floor(Screen:getHeight() * 0.8), }) end, keep_menu_open = true, diff --git a/frontend/ui/translator.lua b/frontend/ui/translator.lua index a05cfcf36..0d08c68f1 100644 --- a/frontend/ui/translator.lua +++ b/frontend/ui/translator.lua @@ -499,7 +499,7 @@ function Translator:_showTranslation(text, target_lang, source_lang) -- Showing the translation target language in this title may make -- it quite long and wrapped, taking valuable vertical spacing text = table.concat(output, "\n"), - height = Screen:getHeight() * 4/5, + height = math.floor(Screen:getHeight() * 0.8), justified = G_reader_settings:nilOrTrue("dict_justify"), }) end diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index b14c91109..555472b0a 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -962,6 +962,21 @@ function UIManager:_refresh(mode, region, dither) table.insert(self._refresh_stack, {mode = mode, region = region, dither = dither}) end + +-- A couple helper functions to compute aligned values... +-- c.f., & ffi/framebuffer_linux.lua +local function ALIGN_DOWN(x, a) + -- x & ~(a-1) + local mask = a - 1 + return bit.band(x, bit.bnot(mask)) +end + +local function ALIGN_UP(x, a) + -- (x + (a-1)) & ~(a-1) + local mask = a - 1 + return bit.band(x + mask, bit.bnot(mask)) +end + --- Repaints dirty widgets. function UIManager:_repaint() -- flag in which we will record if we did any repaints at all @@ -1031,12 +1046,34 @@ function UIManager:_repaint() refresh.dither = nil end dbg:v("triggering refresh", refresh) - -- NOTE: We overshoot by 1px to account for potential off-by-ones. - -- This may not strictly be needed anymore, and is blatantly unneeded for full-screen updates, - -- but checkBounds & getPhysicalRect will sanitize that in mxc_update @ ffi/framebuffer_mxcfb ;). + -- NOTE: If we're requesting hardware dithering on a partial update, make sure the rectangle is using + -- coordinates aligned to the previous multiple of 8, and dimensions aligned to the next multiple of 8. + -- Otherwise, some unlucky coordinates will play badly with the PxP's own alignment constraints, + -- leading to a refresh where content appears to have moved a few pixels to the side... + -- (Sidebar: this is probably a kernel issue, the EPDC driver is responsible for the alignment fixup, + -- c.f., epdc_process_update @ drivers/video/fbdev/mxc/mxc_epdc_v2_fb.c on a Kobo Mk. 7 kernel...). + if refresh.dither then + -- NOTE: Make sure the coordinates are positive, first! Otherwise, we'd gladly align further down below 0, + -- which would skew the rectangle's position/dimension after checkBounds... + local x_fixup = 0 + if refresh.region.x > 0 then + local x_orig = refresh.region.x + refresh.region.x = ALIGN_DOWN(x_orig, 8) + x_fixup = x_orig - refresh.region.x + end + local y_fixup = 0 + if refresh.region.y > 0 then + local y_orig = refresh.region.y + refresh.region.y = ALIGN_DOWN(y_orig, 8) + y_fixup = y_orig - refresh.region.y + end + -- And also make sure we won't be inadvertently cropping our rectangle in case of severe alignment fixups... + refresh.region.w = ALIGN_UP(refresh.region.w + (x_fixup * 2), 8) + refresh.region.h = ALIGN_UP(refresh.region.h + (y_fixup * 2), 8) + end Screen[refresh_methods[refresh.mode]](Screen, - refresh.region.x - 1, refresh.region.y - 1, - refresh.region.w + 2, refresh.region.h + 2, + refresh.region.x, refresh.region.y, + refresh.region.w, refresh.region.h, refresh.dither) end self._refresh_stack = {} diff --git a/frontend/ui/widget/configdialog.lua b/frontend/ui/widget/configdialog.lua index b31855095..47fbafe0b 100644 --- a/frontend/ui/widget/configdialog.lua +++ b/frontend/ui/widget/configdialog.lua @@ -220,7 +220,7 @@ function ConfigOption:init() -- They will carry the left default_option_hpadding, but the in-between -- one (and the right one) will be carried by the option items. -- (Both these variables are between 0 and 1 and represent a % of screen width) - local default_name_align_right = (max_option_name_width + default_option_hpadding + 2*padding_small) / Screen:getWidth() + local default_name_align_right = math.floor((max_option_name_width + default_option_hpadding + 2*padding_small) / Screen:getWidth()) default_name_align_right = math.max(default_name_align_right, 0.25) default_name_align_right = math.min(default_name_align_right, 0.5) local default_item_align_center = 1 - default_name_align_right diff --git a/frontend/ui/widget/confirmbox.lua b/frontend/ui/widget/confirmbox.lua index ddcfd92ad..628ae9d3b 100644 --- a/frontend/ui/widget/confirmbox.lua +++ b/frontend/ui/widget/confirmbox.lua @@ -76,7 +76,7 @@ function ConfirmBox:init() local text_widget = TextBoxWidget:new{ text = self.text, face = self.face, - width = Screen:getWidth()*2/3, + width = math.floor(Screen:getWidth() * 2/3), } local content = HorizontalGroup:new{ align = "center", diff --git a/frontend/ui/widget/infomessage.lua b/frontend/ui/widget/infomessage.lua index a613e1900..1bcb9a62a 100644 --- a/frontend/ui/widget/infomessage.lua +++ b/frontend/ui/widget/infomessage.lua @@ -107,7 +107,7 @@ function InfoMessage:init() local text_width if self.width == nil then - text_width = Screen:getWidth() * 2 / 3 + text_width = math.floor(Screen:getWidth() * 2/3) else text_width = self.width - image_widget:getSize().w if text_width < 0 then diff --git a/frontend/ui/widget/multiconfirmbox.lua b/frontend/ui/widget/multiconfirmbox.lua index 145bdfc5a..d25a060d8 100644 --- a/frontend/ui/widget/multiconfirmbox.lua +++ b/frontend/ui/widget/multiconfirmbox.lua @@ -87,7 +87,7 @@ function MultiConfirmBox:init() TextBoxWidget:new{ text = self.text, face = self.face, - width = Screen:getWidth()*2/3, + width = math.floor(Screen:getWidth() * 2/3), } } diff --git a/frontend/ui/widget/notification.lua b/frontend/ui/widget/notification.lua index 2a7da993a..a5a9b5193 100644 --- a/frontend/ui/widget/notification.lua +++ b/frontend/ui/widget/notification.lua @@ -53,7 +53,7 @@ function Notification:init() self[1] = CenterContainer:new{ dimen = Geom:new{ w = Screen:getWidth(), - h = Screen:getHeight()/10, + h = math.floor(Screen:getHeight() / 10), }, FrameContainer:new{ background = Blitbuffer.COLOR_WHITE, diff --git a/frontend/ui/widget/textboxwidget.lua b/frontend/ui/widget/textboxwidget.lua index 1682becc4..c05fd5f73 100644 --- a/frontend/ui/widget/textboxwidget.lua +++ b/frontend/ui/widget/textboxwidget.lua @@ -6,7 +6,7 @@ Example: local Foo = TextBoxWidget:new{ face = Font:getFace("cfont", 25), text = 'We can show multiple lines.\nFoo.\nBar.', - -- width = Screen:getWidth()*2/3, + -- width = math.floor(Screen:getWidth() * 2/3), } UIManager:show(Foo)