Summary: | Only display refresh monitors having requested display refresh callback should get notified on screen updates. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Per Arne Vollan <pvollan> | ||||||||
Component: | WebCore Misc. | Assignee: | Per Arne Vollan <pvollan> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ahmad.saleem792, bfulgham, commit-queue, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Per Arne Vollan
2018-06-07 09:20:16 PDT
Created attachment 342180 [details]
Patch
Created attachment 342300 [details]
Patch
Comment on attachment 342300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=342300&action=review I think this seems like a very reasonable approach. We just need to be sure there aren't cases where we were relying on an active monitor (that has not sent a message recently) getting updates. We might find that we need to be more explicit about specifically requesting refresh callback if we encounter animation or other rendering issues. > Source/WebCore/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=186397 Radar? > Source/WebCore/ChangeLog:9 > + we should make sure that only the monitors having requested callback, are getting notified on screen No comma after "callback" please. > Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp:131 > + if (monitor->hasRequestedRefreshCallback()) How frequently do we have active monitors that have not requested refresh callbacks? I'm curious how much of a benefit this might be... (In reply to Brent Fulgham from comment #4) > Comment on attachment 342300 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=342300&action=review > > I think this seems like a very reasonable approach. We just need to be sure > there aren't cases where we were relying on an active monitor (that has not > sent a message recently) getting updates. > > We might find that we need to be more explicit about specifically requesting > refresh callback if we encounter animation or other rendering issues. > > > Source/WebCore/ChangeLog:4 > > + https://bugs.webkit.org/show_bug.cgi?id=186397 > > Radar? > > > Source/WebCore/ChangeLog:9 > > + we should make sure that only the monitors having requested callback, are getting notified on screen > > No comma after "callback" please. > > > Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp:131 > > + if (monitor->hasRequestedRefreshCallback()) > > How frequently do we have active monitors that have not requested refresh > callbacks? I'm curious how much of a benefit this might be... I think this is rare. While inspecting the code, I only found that this is only possible if the window is changing screen while a display refresh monitor is active. Thanks for reviewing! Created attachment 342327 [details]
Patch
Comment on attachment 342327 [details] Patch Clearing flags on attachment: 342327 Committed r232652: <https://trac.webkit.org/changeset/232652> Checked via GitHub using BugID and it landed in following: https://github.com/WebKit/WebKit/commit/86295446fc35891e6e5c57909e64023fbc77380f and didn't backed out. Marking this as "RESOLVED FIXED". Thanks! |