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

Issue 11419273: Revert 170057 (Closed)

Created:
8 years ago by ccameron
Modified:
8 years ago
Reviewers:
dharani
CC:
chromium-reviews, cc-bugs_chromium.org
Visibility:
Public.

Description

Revert 170057 > Revert 168254 - 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/11416043 > > TBR=jamesr@chromium.org > Review URL: https://codereview.chromium.org/11411240 TBR=dharani@google.com Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170638

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+208 lines, -79 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 +42 lines, -0 lines 0 comments Download
M cc/scheduler.cc View 1 chunk +1 line, -0 lines 0 comments Download
M cc/scheduler_state_machine.h View 2 chunks +3 lines, -0 lines 0 comments Download
M cc/scheduler_state_machine.cc View 4 chunks +18 lines, -5 lines 0 comments Download
M cc/scheduler_state_machine_unittest.cc View 23 chunks +117 lines, -33 lines 0 comments Download
M cc/thread_proxy.h View 4 chunks +2 lines, -5 lines 0 comments Download
M cc/thread_proxy.cc View 13 chunks +23 lines, -34 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
ccameron
8 years ago (2012-12-01 02:52:01 UTC) #1

          

Powered by Google App Engine
This is Rietveld 408576698