From 00b3594d442f2c76475ab1c434f12f560848ae02 Mon Sep 17 00:00:00 2001 From: zwim <36999612+zwim@users.noreply.github.com> Date: Sun, 30 Apr 2023 23:28:30 +0200 Subject: [PATCH] Kobo: Refactor frontlight toggle ramp behavior (#10305) * Rewrite the loop mechanism to use scheduled tasks instead of a single blocking-ish subprocess. * Change the actual logic to be more pleasing to the eye, especially on newer devices, as those *may* natively ramp on set; and fix a bad interaction with that behavior that could lead to no ramp at all on ramp down. * Simplify Generic's Suspend handling to deal with the refresh ordering in a saner manner. The screensaver might be visible a tad longer than before this change before the frontlight actually ramps off. --- frontend/device/devicelistener.lua | 12 ++- frontend/device/generic/device.lua | 40 ++++---- frontend/device/generic/powerd.lua | 22 +++-- frontend/device/kobo/device.lua | 7 +- frontend/device/kobo/powerd.lua | 147 +++++++++++++++++++--------- frontend/ui/widget/notification.lua | 2 +- 6 files changed, 145 insertions(+), 85 deletions(-) diff --git a/frontend/device/devicelistener.lua b/frontend/device/devicelistener.lua index 0752d9141..2351003f3 100644 --- a/frontend/device/devicelistener.lua +++ b/frontend/device/devicelistener.lua @@ -186,14 +186,18 @@ if Device:hasFrontlight() then function DeviceListener:onToggleFrontlight() local powerd = Device:getPowerDevice() - powerd:toggleFrontlight() local new_text if powerd.is_fl_on then - new_text = _("Frontlight enabled.") - else new_text = _("Frontlight disabled.") + else + new_text = _("Frontlight enabled.") end - Notification:notify(new_text) + -- We defer displaying the Notification to PowerD, as the toggle may be a ramp, and we both want to make sure the refresh fencing won't affect it, and that we only display the Notification at the end... + local notif_source = Notification.notify_source + local notif_cb = function() + Notification:notify(new_text, notif_source) + end + powerd:toggleFrontlight(notif_cb) end function DeviceListener:onShowFlDialog() diff --git a/frontend/device/generic/device.lua b/frontend/device/generic/device.lua index 3a85e5321..d305d74cf 100644 --- a/frontend/device/generic/device.lua +++ b/frontend/device/generic/device.lua @@ -308,7 +308,6 @@ function Device:onPowerEvent(ev) end -- else we were not in screensaver mode elseif ev == "Power" or ev == "Suspend" then - self.powerd:beforeSuspend() local UIManager = require("ui/uimanager") logger.dbg("Suspending...") -- Add the current state of the SleepCover flag... @@ -325,28 +324,25 @@ function Device:onPowerEvent(ev) if self:needsScreenRefreshAfterResume() then self.screen:refreshFull() end - -- NOTE: In the same vein as above, this is delayed to make sure we update the screen first. - -- (This, unfortunately, means we can't just move this to Device:_beforeSuspend :/). - UIManager:scheduleIn(0.1, function() - -- NOTE: This side of the check needs to be laxer, some platforms can handle Wi-Fi without WifiManager ;). - if self:hasWifiToggle() then - local network_manager = require("ui/network/manager") - -- NOTE: wifi_was_on does not necessarily mean that Wi-Fi is *currently* on! It means *we* enabled it. - -- This is critical on Kobos (c.f., #3936), where it might still be on from KSM or Nickel, - -- without us being aware of it (i.e., wifi_was_on still unset or false), - -- because suspend will at best fail, and at worst deadlock the system if Wi-Fi is on, - -- regardless of who enabled it! - if network_manager:isWifiOn() then - network_manager:releaseIP() - network_manager:turnOffWifi() - end - end - -- Only actually schedule suspension if we're still supposed to go to sleep, - -- because the Wi-Fi stuff above may have blocked for a significant amount of time... - if self.screen_saver_mode then - self:rescheduleSuspend() + -- NOTE: In the same vein as above, make sure we update the screen *now*, before dealing with Wi-Fi. + UIManager:forceRePaint() + -- NOTE: This side of the check needs to be laxer, some platforms can handle Wi-Fi without WifiManager ;). + if self:hasWifiToggle() then + local network_manager = require("ui/network/manager") + -- NOTE: wifi_was_on does not necessarily mean that Wi-Fi is *currently* on! It means *we* enabled it. + -- This is critical on Kobos (c.f., #3936), where it might still be on from KSM or Nickel, + -- without us being aware of it (i.e., wifi_was_on still unset or false), + -- because suspend will at best fail, and at worst deadlock the system if Wi-Fi is on, + -- regardless of who enabled it! + if network_manager:isWifiOn() then + network_manager:releaseIP() + network_manager:turnOffWifi() end - end) + end + -- Only turn off the frontlight *after* we've displayed the screensaver and dealt with Wi-Fi, + -- to prevent that from affecting the smoothness of the frontlight ramp down. + self.powerd:beforeSuspend() + self:rescheduleSuspend() end end diff --git a/frontend/device/generic/powerd.lua b/frontend/device/generic/powerd.lua index 5d52d1a7a..95cf31eef 100644 --- a/frontend/device/generic/powerd.lua +++ b/frontend/device/generic/powerd.lua @@ -1,4 +1,4 @@ -local UIManager -- will be updated when available +local UIManager = nil -- will be updated when available local Math = require("optmath") local logger = require("logger") local time = require("ui/time") @@ -43,6 +43,7 @@ end function BasePowerD:readyUI() UIManager = require("ui/uimanager") + self:readyUIHW(UIManager) end function BasePowerD:init() end @@ -62,9 +63,10 @@ function BasePowerD:isAuxChargingHW() return false end function BasePowerD:isAuxChargedHW() return false end function BasePowerD:frontlightIntensityHW() return 0 end function BasePowerD:isFrontlightOnHW() return self.fl_intensity > self.fl_min end -function BasePowerD:turnOffFrontlightHW() self:setIntensityHW(self.fl_min) end -function BasePowerD:turnOnFrontlightHW() self:setIntensityHW(self.fl_intensity) end --- @fixme: what if fl_intensity == fl_min (c.f., kindle)? +function BasePowerD:turnOffFrontlightHW(done_callback) self:setIntensityHW(self.fl_min) end +function BasePowerD:turnOnFrontlightHW(done_callback) self:setIntensityHW(self.fl_intensity) end --- @fixme: what if fl_intensity == fl_min (c.f., kindle)? function BasePowerD:frontlightWarmthHW() return 0 end +function BasePowerD:readyUIHW(uimgr) end -- Anything that needs to be done before doing a real hardware suspend. -- (Such as turning the front light off). function BasePowerD:beforeSuspend() end @@ -94,29 +96,29 @@ function BasePowerD:frontlightIntensity() return self.fl_intensity end -function BasePowerD:toggleFrontlight() +function BasePowerD:toggleFrontlight(done_callback) if not self.device:hasFrontlight() then return false end if self:isFrontlightOn() then - return self:turnOffFrontlight() + return self:turnOffFrontlight(done_callback) else - return self:turnOnFrontlight() + return self:turnOnFrontlight(done_callback) end end -function BasePowerD:turnOffFrontlight() +function BasePowerD:turnOffFrontlight(done_callback) if not self.device:hasFrontlight() then return end if self:isFrontlightOff() then return false end - self:turnOffFrontlightHW() + self:turnOffFrontlightHW(done_callback) self.is_fl_on = false self:stateChanged() return true end -function BasePowerD:turnOnFrontlight() +function BasePowerD:turnOnFrontlight(done_callback) if not self.device:hasFrontlight() then return end if self:isFrontlightOn() then return false end if self.fl_intensity == self.fl_min then return false end --- @fixme what the hell? - self:turnOnFrontlightHW() + self:turnOnFrontlightHW(done_callback) self.is_fl_on = true self:stateChanged() return true diff --git a/frontend/device/kobo/device.lua b/frontend/device/kobo/device.lua index 856c1bf44..f307a5e69 100644 --- a/frontend/device/kobo/device.lua +++ b/frontend/device/kobo/device.lua @@ -166,7 +166,7 @@ local Kobo = Generic:extend{ -- Device ships with various hardware revisions under the same device code, requirign automatic hardware detection... automagic_sysfs = false, - unexpected_wakeup_count = 0 + unexpected_wakeup_count = 0, } local KoboTrilogyA = Kobo:extend{ @@ -449,6 +449,11 @@ local KoboCadmus = Kobo:extend{ nl_min = 0, nl_max = 10, nl_inverted = false, + --- @note: The Sage natively ramps when setting the frontlight intensity. + --- A side-effect of this behavior is that if you queue a series of intensity changes ending at 0, + --- it won't ramp *at all*, and jump straight to zero. + --- So we delay the final ramp off step to prevent (both) the native and our ramping from being optimized out + ramp_off_delay = 0.5, }, boot_rota = C.FB_ROTATE_CW, battery_sysfs = "/sys/class/power_supply/battery", diff --git a/frontend/device/kobo/powerd.lua b/frontend/device/kobo/powerd.lua index 1dd8f337c..9a87ddf2f 100644 --- a/frontend/device/kobo/powerd.lua +++ b/frontend/device/kobo/powerd.lua @@ -1,8 +1,8 @@ +local UIManager = nil -- will be updated when available local BasePowerD = require("device/generic/powerd") local Math = require("optmath") local NickelConf = require("device/kobo/nickel_conf") local SysfsLight = require ("device/sysfs_light") -local ffiUtil = require("ffi/util") local RTC = require("ffi/rtc") -- Here, we only deal with the real hw intensity. @@ -133,6 +133,12 @@ function KoboPowerD:init() self.initial_is_fl_on = true if self.device:hasFrontlight() then + self.device.frontlight_settings = self.device.frontlight_settings or {} + -- Does this device require non-standard ramping behavior? + self.device.frontlight_settings.ramp_off_delay = self.device.frontlight_settings.ramp_off_delay or 0.0 + --- @note: Newer devices appear to block slightly longer on FL ioctls/sysfs, so we only really need a delay on older devices. + self.device.frontlight_settings.ramp_delay = self.device.frontlight_settings.ramp_delay or (self.device:hasNaturalLight() and 0.0 or 0.025) + -- If this device has natural light (currently only KA1 & Forma) -- Use the SysFS interface, and ioctl otherwise. -- NOTE: On the Forma, nickel still appears to prefer using ntx_io to handle the FL, @@ -241,10 +247,10 @@ function KoboPowerD:isFrontlightOnHW() self.initial_is_fl_on = nil return ret end - return self.hw_intensity > 0 + return self.hw_intensity > 0 and not self.fl_ramp_down_running end -function KoboPowerD:setIntensityHW(intensity) +function KoboPowerD:_setIntensityHW(intensity) if self.fl == nil then return end if self.fl_warmth == nil or self.device:hasNaturalLightMixer() then -- We either don't have NL, or we have a mixer: we only want to set the intensity (c.f., #5429) @@ -260,6 +266,11 @@ function KoboPowerD:setIntensityHW(intensity) self:_decideFrontlightState() end +function KoboPowerD:setIntensityHW(intensity) + self:_stopFrontlightRamp() + self:_setIntensityHW(intensity) +end + -- NOTE: We *can* actually read this from the system (as well as frontlight level, since Mk. 7), -- but this is already a huge mess, so, keep ignoring it... function KoboPowerD:frontlightWarmthHW() @@ -300,38 +311,84 @@ function KoboPowerD:isChargedHW() return false end -function KoboPowerD:turnOffFrontlightHW() +function KoboPowerD:_postponedSetIntensityHW(end_intensity, done_callback) + self:_setIntensityHW(end_intensity) + self.fl_ramp_down_running = false + + if done_callback then + done_callback() + end +end + +function KoboPowerD:_stopFrontlightRamp() + if self.fl_ramp_up_running or self.fl_ramp_down_running then + -- Make sure we have no other ramp running. + UIManager:unschedule(self.turnOffFrontlightRamp) + UIManager:unschedule(self.turnOnFrontlightRamp) + UIManager:unschedule(self._postponedSetIntensityHW) + self.fl_ramp_up_running = false + self.fl_ramp_down_running = false + end +end + +-- This will ramp down faster at high intensity values (i.e., start), and slower at lower intensity values (i.e., end). +-- That's an attempt at making the *perceived* effect appear as a more linear brightness change. +-- The whole function gets called at most log(100)/log(0.75) = 17 times, +-- leading to a 0.025*17 + 0.5 = 0.925s ramp down time (non blocking); can be aborted. +function KoboPowerD:turnOffFrontlightRamp(curr_ramp_intensity, end_intensity, done_callback) + curr_ramp_intensity = math.floor(math.max(curr_ramp_intensity * .75, self.fl_min)) + + if curr_ramp_intensity > end_intensity then + self:_setIntensityHW(curr_ramp_intensity) + UIManager:scheduleIn(self.device.frontlight_settings.ramp_delay, self.turnOffFrontlightRamp, self, curr_ramp_intensity, end_intensity, done_callback) + else + -- Some devices require delaying the final step, to prevent them from jumping straight to zero and messing up the ramp. + UIManager:scheduleIn(self.device.frontlight_settings.ramp_off_delay, self._postponedSetIntensityHW, self, end_intensity, done_callback) + -- no reschedule here, as we are done + end +end + +function KoboPowerD:turnOffFrontlightHW(done_callback) if not self:isFrontlightOnHW() then return end - ffiUtil.runInSubProcess(function() - for i = 1, 5 do - -- NOTE: Do *not* switch to (self.fl_intensity * (1/5) * i) here, it may lead to rounding errors, - -- which is problematic paired w/ math.floor because it doesn't round towards zero, - -- which means we may end up passing -1 to setIntensityHW, which will fail, - -- because we're bypassing the clamping usually done by setIntensity... - self:setIntensityHW(math.floor(self.fl_intensity - (self.fl_intensity / 5 * i))) - --- @note: Newer devices appear to block slightly longer on FL ioctls/sysfs, so only sleep on older devices, - --- otherwise we get a jump and not a ramp ;). - if not self.device:hasNaturalLight() then - if (i < 5) then - ffiUtil.usleep(35 * 1000) - end - end + + if UIManager then + -- We've got nothing to do if we're already ramping down + if not self.fl_ramp_down_running then + self:_stopFrontlightRamp() + self:turnOffFrontlightRamp(self.fl_intensity, self.fl_min, done_callback) + self.fl_ramp_down_running = true end - end, false, true) - -- NOTE: This is essentially what setIntensityHW does, except we don't actually touch the FL, - -- we only sync the state of the main process with the final state of what we're doing in the forks. - -- And update hw_intensity in our actual process ;). - self.hw_intensity = self.fl_min - -- NOTE: And don't forget to update sysfs_light, too, as a real setIntensityHW would via setBrightness - if self.fl then - self.fl.current_brightness = self.fl_min + else + -- If UIManager is not initialized yet, just turn it off immediately + self:setIntensityHW(self.fl_min) end - self:_decideFrontlightState() end -function KoboPowerD:turnOnFrontlightHW() +-- Similar functionality as `Kobo:turnOnFrontlightHW`, but the other way around ;). +function KoboPowerD:turnOnFrontlightRamp(curr_ramp_intensity, end_intensity, done_callback) + if curr_ramp_intensity == 0 then + curr_ramp_intensity = 1 + else + curr_ramp_intensity = math.ceil(math.min(curr_ramp_intensity * 1.5, self.fl_max)) + end + + if curr_ramp_intensity < end_intensity then + self:_setIntensityHW(curr_ramp_intensity) + UIManager:scheduleIn(self.device.frontlight_settings.ramp_delay, self.turnOnFrontlightRamp, self, curr_ramp_intensity, end_intensity, done_callback) + else + self:_setIntensityHW(end_intensity) + self.fl_ramp_up_running = false + + if done_callback then + done_callback() + end + -- no reschedule here, as we are done + end +end + +function KoboPowerD:turnOnFrontlightHW(done_callback) -- NOTE: Insane workaround for the first toggle after a startup with the FL off. -- The light is actually off, but hw_intensity couldn't have been set to a sane value because of a number of interactions. -- So, fix it now, so we pass the isFrontlightOnHW check (which checks if hw_intensity > fl_min). @@ -341,27 +398,19 @@ function KoboPowerD:turnOnFrontlightHW() if self:isFrontlightOnHW() then return end - ffiUtil.runInSubProcess(function() - for i = 1, 5 do - self:setIntensityHW(math.ceil(self.fl_min + (self.fl_intensity / 5 * i))) - --- @note: Newer devices appear to block slightly longer on FL ioctls/sysfs, so only sleep on older devices, - --- otherwise we get a jump and not a ramp ;). - if not self.device:hasNaturalLight() then - if (i < 5) then - ffiUtil.usleep(35 * 1000) - end - end + + if UIManager then + -- We've got nothing to do if we're already ramping up + if not self.fl_ramp_up_running then + self:_stopFrontlightRamp() + self.fl_ramp_up_running = true + + self:turnOnFrontlightRamp(self.fl_min, self.fl_intensity, done_callback) end - end, false, true) - -- NOTE: This is essentially what setIntensityHW does, except we don't actually touch the FL, - -- we only sync the state of the main process with the final state of what we're doing in the forks. - -- And update hw_intensity in our actual process ;). - self.hw_intensity = self.fl_intensity - -- NOTE: And don't forget to update sysfs_light, too, as a real setIntensityHW would via setBrightness - if self.fl then - self.fl.current_brightness = self.fl_intensity + else + -- If UIManager is not initialized yet, just turn it on immediately + self:setIntensityHW(self.fl_intensity) end - self:_decideFrontlightState() end -- Turn off front light before suspend. @@ -385,4 +434,8 @@ function KoboPowerD:afterResume() RTC:HCToSys() end +function KoboPowerD:readyUIHW(uimgr) + UIManager = uimgr +end + return KoboPowerD diff --git a/frontend/ui/widget/notification.lua b/frontend/ui/widget/notification.lua index fdd42c0ad..375ee23b4 100644 --- a/frontend/ui/widget/notification.lua +++ b/frontend/ui/widget/notification.lua @@ -156,7 +156,7 @@ end function Notification:notify(arg, source, refresh_after) source = source or self.notify_source local mask = G_reader_settings:readSetting("notification_sources_to_show_mask") or self.SOURCE_DEFAULT - if source and band(mask, self.notify_source) ~= 0 then + if source and band(mask, source) ~= 0 then UIManager:show(Notification:new{ text = arg, })