Summary: | Allow non-60fps display updates to be driven by DisplayRefreshMonitor | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||
Component: | WebCore Misc. | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cdumez, esprehn+autocc, ews-watchlist, graouts, kangil.han, sabouhallawa, sam, simon.fraser, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2021-03-29 22:08:33 PDT
Created attachment 424616 [details]
Patch
Comment on attachment 424616 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424616&action=review Love the tests! > Source/WebCore/ChangeLog:21 > + Also add preferredFramesPerSecond() to use here, and have both it and I'm unclear what "here" is in this context. > Source/WebCore/page/Page.cpp:1201 > renderingUpdateScheduler().windowScreenDidChange(displayID); > + renderingUpdateScheduler().adjustRenderingUpdateFrequency(); I would either pull renderingUpdateScheduler into a local (since the getter does work) or just have windowScreenDidChange() call adjustRenderingUpdateFrequency(), > Source/WebCore/page/Page.h:429 > + // This can return nullopt if throttling reasons result in a frequency less than one, in which case > + // preferredRenderingUpdateInterval provides the frequency. > + Optional<FramesPerSecond> preferredRenderingUpdateFramesPerSecond() const; This seems a little silly that two calls are needed. In the future, we should we consider making FramesPerSecond be a struct containing a ratio that can represent frequencies less than one, or make this return a Variant<FramesPerSecond, Seconds>. > Source/WebCore/platform/Logging.cpp:95 > + LogRequestAnimationFrame.state = WTFLogChannelState::On; > + LogDisplayLink.state = WTFLogChannelState::On; Did you mean to leave these in? > Source/WebCore/platform/graphics/AnimationFrameRate.cpp:33 > +static constexpr OptionSet<ThrottlingReason> halfSpeedThrottlingReasons = { ThrottlingReason::LowPowerMode, ThrottlingReason::NonInteractedCrossOriginFrame, ThrottlingReason::VisuallyIdle }; I don't think the `=` is needed here. |