Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(86)

Issue 11348083: Revert 168095 - Merge 167537 - Use message passing for BeginFrameAndCommitState and clean up forced… (Closed)

Created:
8 years, 1 month ago by jamesr
Modified:
8 years, 1 month ago
Reviewers:
jamesr
CC:
chromium-reviews, cc-bugs_chromium.org
Visibility:
Public.

Description

Revert 168095 - Merge 167537 - Use message passing for BeginFrameAndCommitState and clean up forced commit logic This passes the BeginFrameAndCommitState struct along with messages instead of relying on (racy) shared memory and slightly simplifies the forced commit logic. This logic is for compositeAndReadback() and is trying to go through the normal commit flow while the main thread is blocked. The normal commit flow is this: 1.) Main thread posts task to compositor thread requesting commit 2.) Compositor thread receives notification, after some delay posts task to main to run the beginFrame logic 3.) Main thread runs the beginFrameTask, posts a blocking task to compositor thread 4.) With the main thread is blocked, compositor thread runs the beginFrameComplete task and signals the main thread to get on with life 5.) Compositor thread waits for an opportunity to draw (vsync) then goes back to the initial state. compositeAndReadback() requires that we run through the whole flow up through 5 without yielding the main thread at all. However, while it's doing this there may very well be another beginFrame task pending on the main thread's message queue. When this task does eventually run it needs to have a valid BFAC state and the reply has to be expected on the compositor side. This patch always passes the BFAC state on the beginFrame task and uses a null BFAC state for compositeAndReadback-forced beginFrame()s. This means when we do a compositeAndReadback we won't "see" state that has updated only on the compositor thread, but that's fine since we'll apply that state once we process the beginFrame task later on and BFAC state addition is additive. The tricky bit from the compositor side is keeping track of whether there's a pending beginFrame message after going through the forced commit flow. To keep track of this I've added SchedulerStateMachine::m_expectImmediateBeginFrame that keeps the scheduler in a state that expects a beginFrameComplete after finishing the readback. Typical flow: 1.) Main thread enters compositeAndReadback(), posts a blocking forceBeginFrame task to compositor thread 2.) Compositor thread posts beginFrame task to main thread 3.) Compositor thread processes forceBeginFrame task, sets scheduler bits, signals main thread to continue 4.) Main thread runs beginFrame(NULL), posts beginFrameComplete to compositor thread 5.) Compositor thread processes beginFrameComplete, runs through state machine up to draw and then goes back to waiting for a reply to the beginFrame it posted in step (2) 6.) Main thread issues sync readback, finishes compositeAndReadback() 7.) Main thread processes beginFrame message posted in (2), posts reply 8.) Profit! Note that steps (2) and (3) can happen in any order. BUG=158747 Review URL: https://chromiumcodereview.appspot.com/11362054 TBR=jamesr@chromium.org Review URL: https://codereview.chromium.org/11419027 TBR=jamesr@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=168101

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -211 lines) Patch
M cc/layer_tree_host.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M cc/layer_tree_host_unittest.cc View 1 chunk +0 lines, -42 lines 0 comments Download
M cc/scheduler.cc View 1 chunk +0 lines, -1 line 0 comments Download
M cc/scheduler_state_machine.h View 2 chunks +0 lines, -3 lines 0 comments Download
M cc/scheduler_state_machine.cc View 4 chunks +5 lines, -18 lines 0 comments Download
M cc/scheduler_state_machine_unittest.cc View 23 chunks +33 lines, -117 lines 0 comments Download
M cc/thread_proxy.h View 4 chunks +5 lines, -2 lines 0 comments Download
M cc/thread_proxy.cc View 12 chunks +36 lines, -26 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
jamesr
8 years, 1 month ago (2012-11-16 01:35:23 UTC) #1

          

Powered by Google App Engine
This is Rietveld 408576698