[Webkit-unassigned] [Bug 139062] [GTK] Timers might never be fired during animations

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=139062

Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #242226|review?                     |review+
              Flags|                            |

--- Comment #2 from Martin Robinson <mrobinson at webkit.org> ---
Comment on attachment 242226
  --> https://bugs.webkit.org/attachment.cgi?id=242226
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141211/11e727c4/attachment-0002.html>