Request help on a checkerboard issue

[Chong]

Background & Issue:

I'm working on VSync Aligned Input on Compositor (https://crbug.com/625689, behind a flag) but enabling it will cause persistent checkerboard on MacBook. 

Basically the feature is:

1. Queue gesture scroll/fling/pinch events in |InputHandlerProxy::HandleInputEventWithLatencyInfo()|
   
2. Batch delivery in |LayerTreeHostImpl::WillBeginImplFrame()|

However I can't think of why this would cause checkerboard and hope I could get some hints on where I should look at & add some debug logs.

Thank you for your time!

-- Some More Info --

Major CL/File:

https://codereview.chromium.org/2429953002/diff/320001/ui/events/blink/input_handler_proxy.cc#newcode297

Telemetry Story:

"smoothness.tough_scrolling_cases --story-filter=text.constant.full.page.raster.30000.pixels.per.second"

On MacBook Pro 15-inch:

• "...30000.pixels.per.second" => Half-page checkerboard
  
• "...50000.pixels.per.second" => Almost full-page checkerboard
  
• "...60000.pixels.per.second" => Full-page checkerboard

On Linux Desktop (Probably not related?):

• No checkerboard with or without flag at any speed (Two weeks ago)
  
• Has the same checkerboard with or without flag (Latest Tree)

Good Trace:

https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_0-2017-04-20_10-34-06-60568.html

Bad Trace:

https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_0-2017-04-20_10-28-50-50534.html

Notes (From ajuma@):

• Comment out |SchedulerStateMachine::ShouldPrepareTiles()::prepare_tiles_funnel_ > 0| logic => Checkerboard reduced (e.g. -= half-page)
  

[Sunny Sachanandani]

In the bad trace I see input events arriving after begin frame (see InputEventFilter::ForwardToHandler traces). Most of the begin frames cause a draw immediately and run prepare tiles after drawing. The input events arriving after begin frame's deadline are not picked up in the prepare tiles. Since we limit to one prepare tiles per frame ("funnel" logic) those input events don't affect tile priorities until next frame. If we're missing visible tiles we'll checkerboard until those are rasterized. Therefore those checkerboarded tiles will be drawn at least two frames later.

If the input events arrive at/before begin frame and tile priority changes get picked up in prepare tiles, then we would schedule raster tasks and stop checkerboarding at least one frame later (next draw). I suspect removing the prepare tiles funnel has the effect.

[Chong]

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

The only explanation I could think of is:

1. We always have checkerboard issue if the event arrives at a bad timing (e.g. right after begin frame);
2. The telemetry tests used to dispatch events right before begin frame, but my patch delayed events to be always right after begin frame.

If 1. is the reality then it would just be a telemetry test issue, otherwise there must be something went out-of-sync from receiving to dispatching input events.

On Thursday, April 20, 2017 at 6:24:00 PM UTC-4, Sunny Sachanandani wrote:

In the bad trace I see input events arriving after begin frame (see InputEventFilter::ForwardToHandler traces). Most of the begin frames cause a draw immediately and run prepare tiles after drawing. The input events arriving after begin frame's deadline are not picked up in the prepare tiles. Since we limit to one prepare tiles per frame ("funnel" logic) those input events don't affect tile priorities until next frame. If we're missing visible tiles we'll checkerboard until those are rasterized. Therefore those checkerboarded tiles will be drawn at least two frames later.

Do you mean the events arriving after begin frame's deadline will affect scrolling offset immediately, but won't get picked up by prepare tiles until next frame, thus tiles and scrolling offset will get out-of-sync by 1 frame and cause checkerboard? Sorry if I misunderstood as I'm not so familiar with the compositing process...

 

If the input events arrive at/before begin frame and tile priority changes get picked up in prepare tiles, then we would schedule raster tasks and stop checkerboarding at least one frame later (next draw). I suspect removing the prepare tiles funnel has the effect.

Yes, removing funnel logic will reduce checkerboard by ~half-page, but I haven't figured out why it won't remove checkerboard entirely.

[Sunny Sachanandani]

On Fri, Apr 21, 2017 at 2:50 PM Chong <cho...@chromium.org> wrote:

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

LayerTreeHostImpl::ScrollBegin is in the InputEventFilter::ForwardToHandler stack frame. That means the input event is being processed immediately rather than being queued.

 

The only explanation I could think of is:

1. We always have checkerboard issue if the event arrives at a bad timing (e.g. right after begin frame);
2. The telemetry tests used to dispatch events right before begin frame, but my patch delayed events to be always right after begin frame.

If 1. is the reality then it would just be a telemetry test issue, otherwise there must be something went out-of-sync from receiving to dispatching input events.

On Thursday, April 20, 2017 at 6:24:00 PM UTC-4, Sunny Sachanandani wrote:

In the bad trace I see input events arriving after begin frame (see InputEventFilter::ForwardToHandler traces). Most of the begin frames cause a draw immediately and run prepare tiles after drawing. The input events arriving after begin frame's deadline are not picked up in the prepare tiles. Since we limit to one prepare tiles per frame ("funnel" logic) those input events don't affect tile priorities until next frame. If we're missing visible tiles we'll checkerboard until those are rasterized. Therefore those checkerboarded tiles will be drawn at least two frames later.

Do you mean the events arriving after begin frame's deadline will affect scrolling offset immediately, but won't get picked up by prepare tiles until next frame, thus tiles and scrolling offset will get out-of-sync by 1 frame and cause checkerboard? Sorry if I misunderstood as I'm not so familiar with the compositing process...

We do prepare tiles at two places: inside a commit or otherwise after draw. We do this because prepare tiles and update draw properties that's needed for it is expensive. So even if the input event arrived before a begin frame (but after previous draw) and changes tile priorities, the tiles may not be scheduled for raster until the next draw. The draw might submit incomplete tiles to the browser.

We'd like to improve the scheduling of prepare tiles. Here's what we want in addition to vsync aligned input:

1. In low latency mode (begin main frame and commit every frame) do UDP and prepare tiles after commit. Wait for activation until the deadline. This gives time for any active tree tiles that would otherwise checkerboard to be rasterized.

2. In high latency mode (begin main frame and commit are late) do UDP and prepare tiles at the beginning of the frame after applying input and impl-side animations. Wait for tile manager's ready to draw signal (this tells when visible active tree tiles are done) until the deadline. 

3. Compositor only mode (no begin main frame) is same as 2.

There'd be extra overhead of one UDP and prepare tiles when we switch from low latency to high latency mode. We can experiment with different heuristics for that.

One bug we have today is that we do UDP and prepare tiles before the begin frame to handle 2. Another is that we don't wait for a deadline after prepare tiles for active tree tiles.

[dan...@chromium.org]

On Fri, Apr 21, 2017 at 9:12 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 2:50 PM Chong <cho...@chromium.org> wrote:

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

LayerTreeHostImpl::ScrollBegin is in the InputEventFilter::ForwardToHandler stack frame. That means the input event is being processed immediately rather than being queued.

I'm trying to understand the bad trace and I'm seeing Scheduler::BeginImplFrame followed almost immediately by draw, is that surprising to you Sunny? Because that would prevent a PrepareTiles in layerTreeHostImpl::WillBeginImplFrame from being useful.

When we are batching input, the goal is that the compositor's state is not changing at all between LayerTreeHostImpl::WillBeginImplFrame. Outside of that range, events can be delivered immediately, they only needs be batched during the compositor frame. I'm not sure if that would help in landing this as an incremental step (and make the work done after the commits more accurate since PrepareTiles is happening in there).

Are the commits happening outside of the impl frame because of high latency mode?

[dan...@chromium.org]

On Tue, Apr 25, 2017 at 12:01 PM, <dan...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 9:12 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 2:50 PM Chong <cho...@chromium.org> wrote:

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

LayerTreeHostImpl::ScrollBegin is in the InputEventFilter::ForwardToHandler stack frame. That means the input event is being processed immediately rather than being queued.

I'm trying to understand the bad trace and I'm seeing Scheduler::BeginImplFrame followed almost immediately by draw, is that surprising to you Sunny? Because that would prevent a PrepareTiles in layerTreeHostImpl::WillBeginImplFrame from being useful.

When we are batching input, the goal is that the compositor's state is not changing at all between LayerTreeHostImpl::WillBeginImplFrame.

Oops! I mean between LayerTreeHostImpl::WillBeginImplFrame and LayerTreeHostImpl::DrawLayers (ie the deadline).

[Sunny Sachanandani]

On Tue, Apr 25, 2017 at 9:12 AM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 12:01 PM, <dan...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 9:12 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 2:50 PM Chong <cho...@chromium.org> wrote:

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

LayerTreeHostImpl::ScrollBegin is in the InputEventFilter::ForwardToHandler stack frame. That means the input event is being processed immediately rather than being queued.

I'm trying to understand the bad trace and I'm seeing Scheduler::BeginImplFrame followed almost immediately by draw, is that surprising to you Sunny? Because that would prevent a PrepareTiles in layerTreeHostImpl::WillBeginImplFrame from being useful.

If we do PrepareTiles in WillBeginImplFrame we have to use a regular deadline and wait for a bit for NotifyReadyToDraw (signals visible active tree tiles done) before drawing. Today we draw immediately if main thread is in high latency mode or if commit isn't needed.

 

When we are batching input, the goal is that the compositor's state is not changing at all between LayerTreeHostImpl::WillBeginImplFrame.

Oops! I mean between LayerTreeHostImpl::WillBeginImplFrame and LayerTreeHostImpl::DrawLayers (ie the deadline).

 

Outside of that range, events can be delivered immediately, they only needs be batched during the compositor frame. I'm not sure if that would help in landing this as an incremental step (and make the work done after the commits more accurate since PrepareTiles is happening in there).

I think the easiest implementation would be to be batch in WillBeginImplFrame (as chongz@ has implemented). Are you suggesting an alternative?

 

Are the commits happening outside of the impl frame because of high latency mode?

Commits may happen outside of impl frame in high latency mode.

[dan...@chromium.org]

On Tue, Apr 25, 2017 at 1:29 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 9:12 AM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 12:01 PM, <dan...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 9:12 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 2:50 PM Chong <cho...@chromium.org> wrote:

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

LayerTreeHostImpl::ScrollBegin is in the InputEventFilter::ForwardToHandler stack frame. That means the input event is being processed immediately rather than being queued.

I'm trying to understand the bad trace and I'm seeing Scheduler::BeginImplFrame followed almost immediately by draw, is that surprising to you Sunny? Because that would prevent a PrepareTiles in layerTreeHostImpl::WillBeginImplFrame from being useful.

If we do PrepareTiles in WillBeginImplFrame we have to use a regular deadline and wait for a bit for NotifyReadyToDraw (signals visible active tree tiles done) before drawing. Today we draw immediately if main thread is in high latency mode or if commit isn't needed.

 

When we are batching input, the goal is that the compositor's state is not changing at all between LayerTreeHostImpl::WillBeginImplFrame.

Oops! I mean between LayerTreeHostImpl::WillBeginImplFrame and LayerTreeHostImpl::DrawLayers (ie the deadline).

 

Outside of that range, events can be delivered immediately, they only needs be batched during the compositor frame. I'm not sure if that would help in landing this as an incremental step (and make the work done after the commits more accurate since PrepareTiles is happening in there).

I think the easiest implementation would be to be batch in WillBeginImplFrame (as chongz@ has implemented). Are you suggesting an alternative?

It seems like that is causing problems for high latency mode since we do PrepareTiles outside of WillBeginImplFrame and we don't do one inside. As I see it solutions could be:

1) Do PrepareTiles in WillBeginImplFrame. This is what we want but I think it requires removing the one after submitting a frame also, and the change is behind a flag right now. I've been planning to wait on compositor changes until the input batching isn't behind a flag but we can revisit that if we need to.

or

2) Get input events sent to the compositor after a commit. What if LayerTreeHostImpl::CommitComplete did a DeliverInputForBeginFrame() before UpdateDrawProps/PrepareTiles if it's not inside an ImplFrame?

or

3) It seems like the problem is CommitComplete not seeing events, so an alternative could be to batch input during the ImplFrame but not outside, to let CommitComplete get events sooner.

or

4) We don't UpdateDrawProps/PrepareTiles in CommitComplete (when batching is on)?

[Sunny Sachanandani]

On Tue, Apr 25, 2017 at 10:37 AM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 1:29 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 9:12 AM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 12:01 PM, <dan...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 9:12 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 2:50 PM Chong <cho...@chromium.org> wrote:

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

LayerTreeHostImpl::ScrollBegin is in the InputEventFilter::ForwardToHandler stack frame. That means the input event is being processed immediately rather than being queued.

I'm trying to understand the bad trace and I'm seeing Scheduler::BeginImplFrame followed almost immediately by draw, is that surprising to you Sunny? Because that would prevent a PrepareTiles in layerTreeHostImpl::WillBeginImplFrame from being useful.

If we do PrepareTiles in WillBeginImplFrame we have to use a regular deadline and wait for a bit for NotifyReadyToDraw (signals visible active tree tiles done) before drawing. Today we draw immediately if main thread is in high latency mode or if commit isn't needed.

 

When we are batching input, the goal is that the compositor's state is not changing at all between LayerTreeHostImpl::WillBeginImplFrame.

Oops! I mean between LayerTreeHostImpl::WillBeginImplFrame and LayerTreeHostImpl::DrawLayers (ie the deadline).

 

Outside of that range, events can be delivered immediately, they only needs be batched during the compositor frame. I'm not sure if that would help in landing this as an incremental step (and make the work done after the commits more accurate since PrepareTiles is happening in there).

I think the easiest implementation would be to be batch in WillBeginImplFrame (as chongz@ has implemented). Are you suggesting an alternative?

It seems like that is causing problems for high latency mode since we do PrepareTiles outside of WillBeginImplFrame and we don't do one inside. As I see it solutions could be:

1) Do PrepareTiles in WillBeginImplFrame. This is what we want but I think it requires removing the one after submitting a frame also, and the change is behind a flag right now. I've been planning to wait on compositor changes until the input batching isn't behind a flag but we can revisit that if we need to.

I'd strongly prefer this. Two concerns though:

1. Will we also do a PrepareTiles in commit? Would doing two PrepareTiles hurt performance?

2. If a commit and activation happens in the same frame, will we waste any time spent rasterizing active tree tiles that aren't there any more?

Maybe we should do PrepareTiles at the beginning of the frame in high latency mode only?

 

or

2) Get input events sent to the compositor after a commit. What if LayerTreeHostImpl::CommitComplete did a DeliverInputForBeginFrame() before UpdateDrawProps/PrepareTiles if it's not inside an ImplFrame?

or

3) It seems like the problem is CommitComplete not seeing events, so an alternative could be to batch input during the ImplFrame but not outside, to let CommitComplete get events sooner. 

or

4) We don't UpdateDrawProps/PrepareTiles in CommitComplete (when batching is on)?

This means no low latency mode, right?

[dan...@chromium.org]

On Tue, Apr 25, 2017 at 4:48 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 10:37 AM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 1:29 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 9:12 AM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 12:01 PM, <dan...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 9:12 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 2:50 PM Chong <cho...@chromium.org> wrote:

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

LayerTreeHostImpl::ScrollBegin is in the InputEventFilter::ForwardToHandler stack frame. That means the input event is being processed immediately rather than being queued.

I'm trying to understand the bad trace and I'm seeing Scheduler::BeginImplFrame followed almost immediately by draw, is that surprising to you Sunny? Because that would prevent a PrepareTiles in layerTreeHostImpl::WillBeginImplFrame from being useful.

If we do PrepareTiles in WillBeginImplFrame we have to use a regular deadline and wait for a bit for NotifyReadyToDraw (signals visible active tree tiles done) before drawing. Today we draw immediately if main thread is in high latency mode or if commit isn't needed.

 

When we are batching input, the goal is that the compositor's state is not changing at all between LayerTreeHostImpl::WillBeginImplFrame.

Oops! I mean between LayerTreeHostImpl::WillBeginImplFrame and LayerTreeHostImpl::DrawLayers (ie the deadline).

 

Outside of that range, events can be delivered immediately, they only needs be batched during the compositor frame. I'm not sure if that would help in landing this as an incremental step (and make the work done after the commits more accurate since PrepareTiles is happening in there).

I think the easiest implementation would be to be batch in WillBeginImplFrame (as chongz@ has implemented). Are you suggesting an alternative?

It seems like that is causing problems for high latency mode since we do PrepareTiles outside of WillBeginImplFrame and we don't do one inside. As I see it solutions could be:

1) Do PrepareTiles in WillBeginImplFrame. This is what we want but I think it requires removing the one after submitting a frame also, and the change is behind a flag right now. I've been planning to wait on compositor changes until the input batching isn't behind a flag but we can revisit that if we need to.

I'd strongly prefer this. Two concerns though:

1. Will we also do a PrepareTiles in commit?

In high latency mode (commit is outside of BeginFrame), we could, it depends I guess. That was roughly my suggestion 3 vs 4. In my mind we should do "Get input events. UpdateDrawProps. PrepareTiles." together always. There's little point in doing part of that sequence without the rest, and at worst you waste work or produce stale results. So I guess my answer is related to if we'd get input events in CommitComplete.

 

Would doing two PrepareTiles hurt performance?

If we receive input in the meantime then we also have to UpdateDrawProps which is the more heavy thing. This is the long term thing but doing 3 per frame would be definitely a power regression.

 

2. If a commit and activation happens in the same frame, will we waste any time spent rasterizing active tree tiles that aren't there any more?

We can prioritize required for activation tiles. Do we do that now? This seems like tile scheduling problem to be solved there, rather than by avoiding doing PrepareTiles at all. We should get the right math data available as early as possible in the frame, and make decisions based on it IMO.

 

Maybe we should do PrepareTiles at the beginning of the frame in high latency mode only?

I think this would produce less optimal results, where you're preparing tiles from the last frame's scroll offset etc, like we have now.

 

 

or

2) Get input events sent to the compositor after a commit. What if LayerTreeHostImpl::CommitComplete did a DeliverInputForBeginFrame() before UpdateDrawProps/PrepareTiles if it's not inside an ImplFrame?

or

3) It seems like the problem is CommitComplete not seeing events, so an alternative could be to batch input during the ImplFrame but not outside, to let CommitComplete get events sooner. 

or

4) We don't UpdateDrawProps/PrepareTiles in CommitComplete (when batching is on)?

This means no low latency mode, right?

I don't think so, we'd just wait until the BeginFrame to prepare the tiles in high latency mode instead of doing it at CommitComplete. But I might be misunderstanding low latency mode.

[Sunny Sachanandani]

On Tue, Apr 25, 2017 at 1:56 PM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 4:48 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 10:37 AM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 1:29 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 9:12 AM <dan...@chromium.org> wrote:

On Tue, Apr 25, 2017 at 12:01 PM, <dan...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 9:12 PM, Sunny Sachanandani <sun...@chromium.org> wrote:

On Fri, Apr 21, 2017 at 2:50 PM Chong <cho...@chromium.org> wrote:

Thank you for the comment! That's really helpful on understanding the scheduling order.

I can see how the event was delayed and missed the frame, but I'm still not clear why this would cause checkerboard. IIUC the delayed event should be handled normally next frame as if it just arrived, and it shouldn't affect any status before it was popped from the queue and processed.

LayerTreeHostImpl::ScrollBegin is in the InputEventFilter::ForwardToHandler stack frame. That means the input event is being processed immediately rather than being queued.

I'm trying to understand the bad trace and I'm seeing Scheduler::BeginImplFrame followed almost immediately by draw, is that surprising to you Sunny? Because that would prevent a PrepareTiles in layerTreeHostImpl::WillBeginImplFrame from being useful.

If we do PrepareTiles in WillBeginImplFrame we have to use a regular deadline and wait for a bit for NotifyReadyToDraw (signals visible active tree tiles done) before drawing. Today we draw immediately if main thread is in high latency mode or if commit isn't needed.

 

When we are batching input, the goal is that the compositor's state is not changing at all between LayerTreeHostImpl::WillBeginImplFrame.

Oops! I mean between LayerTreeHostImpl::WillBeginImplFrame and LayerTreeHostImpl::DrawLayers (ie the deadline).

 

Outside of that range, events can be delivered immediately, they only needs be batched during the compositor frame. I'm not sure if that would help in landing this as an incremental step (and make the work done after the commits more accurate since PrepareTiles is happening in there).

I think the easiest implementation would be to be batch in WillBeginImplFrame (as chongz@ has implemented). Are you suggesting an alternative?

It seems like that is causing problems for high latency mode since we do PrepareTiles outside of WillBeginImplFrame and we don't do one inside. As I see it solutions could be:

1) Do PrepareTiles in WillBeginImplFrame. This is what we want but I think it requires removing the one after submitting a frame also, and the change is behind a flag right now. I've been planning to wait on compositor changes until the input batching isn't behind a flag but we can revisit that if we need to.

I'd strongly prefer this. Two concerns though:

1. Will we also do a PrepareTiles in commit?

In high latency mode (commit is outside of BeginFrame), we could, it depends I guess. That was roughly my suggestion 3 vs 4. In my mind we should do "Get input events. UpdateDrawProps. PrepareTiles." together always. There's little point in doing part of that sequence without the rest, and at worst you waste work or produce stale results. So I guess my answer is related to if we'd get input events in CommitComplete.

Also add Animate to the input-UDP-PrepareTiles list. At commit (assuming input batching in WillBeginImplFrame):

1. In low latency mode, input events were sent before BeginMainFrame was sent so the commit already includes all input events that's been seen by compositor thread.

2. In high latency mode, main thread hasn't seen input events but some things like scroll offsets are synced between active and pending trees. So again, there's no need to batch input events in commit.

 

 

Would doing two PrepareTiles hurt performance?

If we receive input in the meantime then we also have to UpdateDrawProps which is the more heavy thing. This is the long term thing but doing 3 per frame would be definitely a power regression.

3 per frame? I don't think we ever need more than 2 per frame?

 

 

2. If a commit and activation happens in the same frame, will we waste any time spent rasterizing active tree tiles that aren't there any more?

We can prioritize required for activation tiles. Do we do that now? This seems like tile scheduling problem to be solved there, rather than by avoiding doing PrepareTiles at all. We should get the right math data available as early as possible in the frame, and make decisions based on it IMO.

In low latency mode, if we do a PrepareTiles in WillBeginImplFrame, we'll schedule some active tree tiles for raster (assuming no pending tree). Some of these raster tasks could run quickly and queue up work in the GPU process. Then on commit we'll get a pending tree and schedule more tiles at a higher priority than active tree tiles (depends on tree priority). Now if we're able to activate in the same frame it's possible the already rasterized active tree tiles aren't needed any more.

 

 

Maybe we should do PrepareTiles at the beginning of the frame in high latency mode only?

I think this would produce less optimal results, where you're preparing tiles from the last frame's scroll offset etc, like we have now.

You mean the PrepareTiles inside CommitComplete? The scroll offsets should be synced between active and pending tree.

 

 

 

or

2) Get input events sent to the compositor after a commit. What if LayerTreeHostImpl::CommitComplete did a DeliverInputForBeginFrame() before UpdateDrawProps/PrepareTiles if it's not inside an ImplFrame?

or

3) It seems like the problem is CommitComplete not seeing events, so an alternative could be to batch input during the ImplFrame but not outside, to let CommitComplete get events sooner. 

or

4) We don't UpdateDrawProps/PrepareTiles in CommitComplete (when batching is on)?

This means no low latency mode, right?

I don't think so, we'd just wait until the BeginFrame to prepare the tiles in high latency mode instead of doing it at CommitComplete. But I might be misunderstanding low latency mode.

We need PrepareTiles to rasterize pending tree tiles and activate before the deadline. Otherwise, we'll be drawing active tree tiles only.

low latency mode == commit in the same begin frame that begin main frame was sent

[Chong]

IIUC the preferred long-term solution is to do PrepareTiles in WillBeginImplFrame and eliminate the ones elsewhere.

A possible incremental plan could be:

1. Change my feature to only queue input events during ImplFrame (e.g. Between LayerTreeHostImpl::WillBeginImplFrame and LayerTreeHostImpl::DidFinishImplFrame?)

  * This way my feature will at least not exacerbating checkerboard.

2. Ship the feature and remove the flag to unblock other works

3. Work towards the long-term solution

4. Change to queue all input events if necessary

 

Does it sound reasonable?

 

Would doing two PrepareTiles hurt performance?

If we receive input in the meantime then we also have to UpdateDrawProps which is the more heavy thing. This is the long term thing but doing 3 per frame would be definitely a power regression.

3 per frame? I don't think we ever need more than 2 per frame?

I have the same question as well...

 

 

 

2. If a commit and activation happens in the same frame, will we waste any time spent rasterizing active tree tiles that aren't there any more?

We can prioritize required for activation tiles. Do we do that now? This seems like tile scheduling problem to be solved there, rather than by avoiding doing PrepareTiles at all. We should get the right math data available as early as possible in the frame, and make decisions based on it IMO.

In low latency mode, if we do a PrepareTiles in WillBeginImplFrame, we'll schedule some active tree tiles for raster (assuming no pending tree). Some of these raster tasks could run quickly and queue up work in the GPU process. Then on commit we'll get a pending tree and schedule more tiles at a higher priority than active tree tiles (depends on tree priority). Now if we're able to activate in the same frame it's possible the already rasterized active tree tiles aren't needed any more.

IIUC the situation is we scheduled and finished some raster tasks quickly after WillBeginImplFrame but before commit or activation. Then we activated a new pending tree which doesn't need those rasterized tasks, which means we have done some redundant work.

Maybe we should do something different for low/high latency modes? But anyway I guess this shouldn't block the incremental plan above.

[dan...@chromium.org]

I think what I'm trying to get at is that commit means we will PrepareTiles etc when the commit is done, which means we want to have all the input events flushed before that, or we're going to potentially waste work. Consider

- High latency mode

- A main frame is prepared, currently at scroll offset A on the compositor thread

- Some scrolls happen that would move us to offset B on compositor thread, but they are queued

- Commit completes, we PrepareTiles with scroll offset A

- We end up rastering tiles for offset A on the pending tree

- Raster takes some time, and we enter the next begin frame, we flush the event queue and move to scroll offset B, we no longer need the work we did for scroll offset A

 

 

 

Would doing two PrepareTiles hurt performance?

If we receive input in the meantime then we also have to UpdateDrawProps which is the more heavy thing. This is the long term thing but doing 3 per frame would be definitely a power regression.

3 per frame? I don't think we ever need more than 2 per frame?

Right, I was concerned about the PrepareTiles after draw, it would need to be removed if we add one in WillBeginImplFrame, but I guess it would actually kinda do that for free. It'd become dead if we always PrepareTile inside the impl frame, cuz it would never need to run.

 

 

 

2. If a commit and activation happens in the same frame, will we waste any time spent rasterizing active tree tiles that aren't there any more?

We can prioritize required for activation tiles. Do we do that now? This seems like tile scheduling problem to be solved there, rather than by avoiding doing PrepareTiles at all. We should get the right math data available as early as possible in the frame, and make decisions based on it IMO.

In low latency mode, if we do a PrepareTiles in WillBeginImplFrame, we'll schedule some active tree tiles for raster (assuming no pending tree). Some of these raster tasks could run quickly and queue up work in the GPU process. Then on commit we'll get a pending tree and schedule more tiles at a higher priority than active tree tiles (depends on tree priority). Now if we're able to activate in the same frame it's possible the already rasterized active tree tiles aren't needed any more.

I see. We want to wait for the commit to finish before running PrepareTiles in low latency mode, but do it immediately in high latency mode.

 

 

 

Maybe we should do PrepareTiles at the beginning of the frame in high latency mode only?

I think this would produce less optimal results, where you're preparing tiles from the last frame's scroll offset etc, like we have now.

You mean the PrepareTiles inside CommitComplete? The scroll offsets should be synced between active and pending tree.

I get what you mean now, wait for the commit in low latency so we don't PrepareTiles twice in the same frame.

 

or

2) Get input events sent to the compositor after a commit. What if LayerTreeHostImpl::CommitComplete did a DeliverInputForBeginFrame() before UpdateDrawProps/PrepareTiles if it's not inside an ImplFrame?

or

3) It seems like the problem is CommitComplete not seeing events, so an alternative could be to batch input during the ImplFrame but not outside, to let CommitComplete get events sooner. 

or

4) We don't UpdateDrawProps/PrepareTiles in CommitComplete (when batching is on)?

This means no low latency mode, right?

I don't think so, we'd just wait until the BeginFrame to prepare the tiles in high latency mode instead of doing it at CommitComplete. But I might be misunderstanding low latency mode.

We need PrepareTiles to rasterize pending tree tiles and activate before the deadline. Otherwise, we'll be drawing active tree tiles only.

I guess I was assuming the next ImplFrame would be coming very shortly after CommitComplete, but CommitComplete can happen inside the frame, I wasn't considering that.

[dan...@chromium.org]

My one concern here is that we would be doing input event work at very uncontrolled times still. Would this work better than doing DeliverInputForBeginFrame() in LayerTreeHostImpl::CommitComplete() if we're not inside an ImplFrame? The latter seems like something we wouldn't need to iterate on some more, but the former does. Otherwise, this seems okay to me.

[Chong]

Thanks for the comments! I assume you mean we should do DeliverInputForBeginFrame() in both WillBeginImplFrame() and CommitComplete(), otherwise we will ended up flushing input once per, for example, two frames (high latency mode)?

And in CommitComplete() we should flush input before UpdateSyncTreeAfterCommitOrImplSideInvalidation() so the input could be picked up by PrepareTiles().

Anyway I've updated the CL: https://codereview.chromium.org/2841263002/

PTAL if that make sense to you, thanks!

[Sunny Sachanandani]

Just thought of this:

- Extend impl side invalidation to UDP on active tree in addition to UDP on pending tree and PrepareTiles (see next point)

- Impl side invalidation only creates pending tree if needed for image checkering / other use cases

- Move impl side invalidation to beginning of frame

- Pending begin main frame or commit blocks impl side invalidation (same as today)

- Wait for ready to draw / deadline before drawing (no immediate deadline)

- No prepare tiles at end of frame (ScheduledActionPrepareTiles goes away completely)

Note that UDP/PrepareTiles are nops if not needed.

Drawbacks:

- If SetNeedsBeginMainFrame happens in the middle of the frame we might do udp / prepare tiles twice? This shouldn't be common.

- anything else?