Summary: | [GTK] Timers might never be fired during animations | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | gustavo, pnormand, svillar, zan | ||||
Priority: | P2 | Keywords: | Gtk | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Carlos Garcia Campos
2014-11-26 06:26:11 PST
Created attachment 242226 [details]
Patch
Comment on attachment 242226 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242226&action=review Looks good to me. I have a couple readability suggestions, but I think the general approach is just fine. Though please double-check that this doesn't harm animations while resizing on faster machines. Thanks! > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:66 > +// Use a higher priority than WebCore timers. > +static const int layerFlushTimerPriority = GDK_PRIORITY_REDRAW - 1; > + Maybe just move this down to right before it is used? > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:264 > + static const double targetDelay = 1 / 60.0; I would call this targetFramerate or something like that. > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:270 > + static const double maxDurationImmediateFlush = 0.100; Maybe call this time maxDurationOfImmediateFlushes. > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:272 > + double nextFlush = std::max(targetDelay - (current - fireTime), 0.0); I would call this timeToNextFlush. > Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:281 > + if (m_lastImmediateFlushTime && m_lastImmediateFlushTime + maxDurationImmediateFlush < current) { > + nextFlush = targetDelay; > + m_lastImmediateFlushTime = 0; > } Would it be possible to move this to a helper called shouldSkipAFrameBecauseOfContinousImmediateFlushes? I know the name is super silly, but I think it would help the readability of the code here. Committed r177144: <http://trac.webkit.org/changeset/177144> |