Bug 158933 – [iOS][WK2] When an animation frame is missed, the UI process should immediately notify the Web process once a frame is committed

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Said Abou-Hallawa]

When a frame is committed, the UI process schedules a CADisplayLink and waits until the next didRefreshDisplay event is fired. This causes more delay in the scenario where there are frames are dropped. didRefreshDisplay() should be called immediately when commitLayerTree message is received and the last frame was dropped.

The fix is to have the CADisplayLink active all the times. It should be paused only when we detect at least one frame is dropped. In this case we should not send a message to the UI process since it has not sent the last requested LayerTree. Also we should not waste the CPU time by scheduling a new CADisplayLink since we have not processed the last event. We should resume CADisplayLink timer once a commitLayerTree message is received.

We are going to introduce the state variable m_didUpdateMessageState to RemoteLayerTreeDrawingAreaProxy whose value is one of the following: { NotSent, Sent, MissedCommit }. Here is its state table. The only events that affect its value are commitLayerTree() and didRefreshDisplay()


CurrentValue           commitLayerTree                            didRefreshDisplay
  NotSent                  NotSent                                  Sent          (message is sent to the WebProcess)
  Sent                     NotSent                                  MissedCommit  (no message is sent and the CADisplayLink is paused)
  MissedCommit             NotSent (call didRefreshDisplay())       MissedCommit  (no message)

[Said Abou-Hallawa]

Created attachment 281637 [details]
Patch

[Tim Horton]

Comment on attachment 281637 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=281637&action=review

> Source/WebKit2/ChangeLog:30
> +        * UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:
> +        * UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:
> +        (-[WKOneShotDisplayLinkHandler pause]):
> +        (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
> +        (WebKit::RemoteLayerTreeDrawingAreaProxy::didRefreshDisplay):
> +        (-[WKOneShotDisplayLinkHandler displayLinkFired:]): Deleted.
> +        * WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
> +        (WebKit::RemoteLayerTreeDrawingArea::BackingStoreFlusher::flush):
> +        We should clear the graphics contexts when the drawing layers are flushed
> +        in the commitQueue thread instead of delaying this to be done in the main
> +        thread when the old m_pendingBackingStoreFlusher is released.

This should probably happen in a different patch so we can separate the performance impact of the two unrelated changes.

[Said Abou-Hallawa]

<rdar://problem/23490249>

[Said Abou-Hallawa]

Created attachment 281768 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 281637 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=281637&action=review

>> Source/WebKit2/ChangeLog:30
>> +        thread when the old m_pendingBackingStoreFlusher is released.
> 
> This should probably happen in a different patch so we can separate the performance impact of the two unrelated changes.

This part was removed but I did not notice much difference in MotionMark score between adding it or removing it. So I am mot sure if it is worth adding it a separate patch even.

[Said Abou-Hallawa]

This patch shows 8% progression in MotionMark score on iPad Air.

[WebKit Commit Bot]

Comment on attachment 281768 [details]
Patch

Clearing flags on attachment: 281768

Committed r202291: <http://trac.webkit.org/changeset/202291>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.