Async BLAST Sync[4/N]: Replace draw handlers with seqId
Our current model for draw handlers use relayoutWindow to correlate
calls to finishDrawing with specific states. Namely all states
that were changed by the time relayoutWindow returned, will be
included in the next finishDrawing. This is the meaning of
the mPending and mCurrent draw handlers. Any draw handlers
(and assosciate state change) added after a call to relayout
wont be observed by the client, and so we dont want to
execute the draw handler when we receive finish drawing. See
BLASTSync.md for a detailed discussion. Understanding that the previous
code simply only executes draw handlers which were added after relayout
we can see that the new code does the same thing. Draw handlers which
are executed must have a seqId less than the one returned from the
relayout immediately proceeding finishDrawing, and so it must be the
same set of draw handlers which were added before relayout. Again
see BLASTSync.md for a detailed discussion of why this is expected
to work even without relayoutWindow
Bug: 161810301
Bug: 175861051
Bug: 175861127
Bug: 200285149
Change-Id: Icd36129927da32fb175ea9071319b06d4d1475a7
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index 92864b6..9ee6e5b 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -2510,7 +2510,6 @@
if (mUseBLASTSync && win.useBLASTSync() && viewVisibility != View.GONE
&& (win.mSyncSeqId > win.mLastSeqIdSentToRelayout)) {
- win.prepareDrawHandlers();
win.markRedrawForSyncReported();
win.mLastSeqIdSentToRelayout = win.mSyncSeqId;
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index b20781f..6a722f3 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -371,6 +371,23 @@
private boolean mDragResizingChangeReported = true;
private int mResizeMode;
private boolean mRedrawForSyncReported;
+
+ /**
+ * Used to assosciate a given set of state changes sent from MSG_RESIZED
+ * with a given call to finishDrawing (does this call contain or not contain
+ * those state changes). We need to use it to handle cases like this:
+ * 1. Server changes some state, calls applyWithNextDraw
+ * 2. Client observes state change, begins drawing frame.
+ * 3. Server makes another state change, and calls applyWithNextDraw again
+ * 4. We receive finishDrawing, and it only contains the first frame
+ * but there was no way for us to know, because we no longer rely
+ * on a synchronous call to relayout before draw.
+ * We track this by storing seqIds in each draw handler, and increment
+ * this seqId every time we send MSG_RESIZED. The client sends it back
+ * with finishDrawing, and this way we can know is the client replying to
+ * the latest MSG_RESIZED or an earlier one. For a detailed discussion,
+ * examine the git commit message introducing this comment and variable.2
+ */
int mSyncSeqId = 0;
int mLastSeqIdSentToRelayout = 0;
@@ -779,75 +796,16 @@
private final WindowProcessController mWpcForDisplayAreaConfigChanges;
- /**
- * We split the draw handlers in to a "pending" and "ready" list, in order to solve
- * sequencing problems. Think of it this way, let's say I update a windows orientation
- * (in configuration), and then I call applyWithNextDraw. What I'm hoping for is to
- * apply with the draw that contains the orientation change. However, since the client
- * can call finishDrawing at any time, it could be about to call a previous call to
- * finishDrawing (or maybe its already called it, we just haven't handled it). Since this
- * frame was already completed it had no time to include the orientation change we made.
- * To solve this problem we accumulate draw handlers in mPendingDrawHandlers, and then force
- * the client to call relayout. Only the frame post relayout will contain the configuration
- * change since the window has to relayout), and so in relayout we drain mPendingDrawHandlers
- * into mReadyDrawHandlers. Finally once we get to finishDrawing we know everything in
- * mReadyDrawHandlers corresponds to state which was observed by the client and we can
- * invoke the consumers.
- *
- * To see in more detail that this works, we can look at it like this:
- *
- * The client is in one of these states:
- *
- * 1. Asleep
- * 2. Traversal scheduled
- * 3. Starting traversal
- * 4. In relayout
- * 5. Already drawing
- *
- * The property we want to implement with the draw handlers is:
- * If WM code makes a change to client observable state (e.g. configuration),
- * and registers a draw handler (without releasing the WM lock in between),
- * the FIRST frame reflecting that change, will be in the Transaction passed
- * to the draw handler.
- *
- * We describe the expected sequencing in each of the possible client states.
- * We aim to "prove" that the WM can call applyWithNextDraw() with the client
- * starting in any state, and achieve the desired result.
- *
- * 1. Asleep: The client will wake up in response to MSG_RESIZED, call relayout,
- * observe the changes. Relayout will return BLAST_SYNC, and the client will
- * send the transaction to finishDrawing. Since the client was asleep. This
- * will be the first finishDrawing reflecting the change.
- * 2, 3: traversal scheduled/starting traversal: These two states can be considered
- * together. Each has two sub-states:
- * a) Traversal will call relayout. In this case we proceed like the starting
- * from asleep case.
- * b) Traversal will not call relayout. In this case, the client produced
- * frame will not include the change. Because there is no call to relayout
- * there is no call to prepareDrawHandlers() and even if the client calls
- * finish drawing the draw handler will not be invoked. We have to wait
- * on the client to receive MSG_RESIZED, and will sync on the next frame
- * 4. In relayout. In this case we are careful to prepare draw handlers and check
- * whether to return the BLAST flag at the end of relayoutWindow. This means if you
- * add a draw handler while the client is in relayout, BLAST_SYNC will be
- * immediately returned, and the client will submit the frame corresponding
- * to what returns from layout. When we prepare the draw handlers we clear the
- * flag which would later cause us to report draw for sync. Since we reported
- * sync through relayout (by luck the client was calling relayout perhaps)
- * there is no need for a MSG_RESIZED.
- * 5. Already drawing. This works much like cases 2 and 3. If there is no call to
- * finishDrawing then of course the draw handlers will not be invoked and we just
- * wait on the next frame for sync. If there is a call to finishDrawing,
- * the draw handler will not have been prepared (since we did not call relayout)
- * and we will have to wait on the next frame.
- *
- * By this logic we can see no matter which of the client states we are in when the
- * draw handler is added, it will always execute on the expected frame.
- */
- private final List<Consumer<SurfaceControl.Transaction>> mPendingDrawHandlers
- = new ArrayList<>();
- private final List<Consumer<SurfaceControl.Transaction>> mReadyDrawHandlers
- = new ArrayList<>();
+ class DrawHandler {
+ Consumer<SurfaceControl.Transaction> mConsumer;
+ int mSeqId;
+
+ DrawHandler(int seqId, Consumer<SurfaceControl.Transaction> consumer) {
+ mSeqId = seqId;
+ mConsumer = consumer;
+ }
+ }
+ private final List<DrawHandler> mDrawHandlers = new ArrayList<>();
private final Consumer<SurfaceControl.Transaction> mSeamlessRotationFinishedConsumer = t -> {
finishSeamlessRotation(t);
@@ -6001,7 +5959,7 @@
.notifyStartingWindowDrawn(mActivityRecord);
}
- final boolean hasSyncHandlers = executeDrawHandlers(postDrawTransaction);
+ final boolean hasSyncHandlers = executeDrawHandlers(postDrawTransaction, syncSeqId);
boolean skipLayout = false;
// Control the timing to switch the appearance of window with different rotations.
@@ -6029,7 +5987,7 @@
}
void immediatelyNotifyBlastSync() {
- prepareDrawHandlers();
+ // We could be more subtle with Integer.MAX_VALUE and track a seqId in the timeout.
finishDrawing(null, Integer.MAX_VALUE);
mWmService.mH.removeMessages(WINDOW_STATE_BLAST_SYNC_TIMEOUT, this);
if (!useBLASTSync()) return;
@@ -6086,7 +6044,7 @@
*/
@Override
boolean useBLASTSync() {
- return super.useBLASTSync() || (mPendingDrawHandlers.size() != 0);
+ return super.useBLASTSync() || (mDrawHandlers.size() != 0);
}
/**
@@ -6098,11 +6056,12 @@
* 2. Call applyWithNextDraw
* 3. After finishDrawing, our consumer will be passed the Transaction
* containing the buffer, and we can merge in additional operations.
- * See {@link WindowState#mPendingDrawHandlers}
+ * See {@link WindowState#mDrawHandlers}
*/
void applyWithNextDraw(Consumer<SurfaceControl.Transaction> consumer) {
- mPendingDrawHandlers.add(consumer);
mSyncSeqId++;
+ mDrawHandlers.add(new DrawHandler(mSyncSeqId, consumer));
+
requestRedrawForSync();
mWmService.mH.sendNewMessageDelayed(WINDOW_STATE_BLAST_SYNC_TIMEOUT, this,
@@ -6110,21 +6069,10 @@
}
/**
- * Called from relayout, to indicate the next "finishDrawing" will contain
- * all changes applied by the time mPendingDrawHandlers was populated.
- *
- * See {@link WindowState#mPendingDrawHandlers}
- */
- void prepareDrawHandlers() {
- mReadyDrawHandlers.addAll(mPendingDrawHandlers);
- mPendingDrawHandlers.clear();
- }
-
- /**
* Drain the draw handlers, called from finishDrawing()
* See {@link WindowState#mPendingDrawHandlers}
*/
- boolean executeDrawHandlers(SurfaceControl.Transaction t) {
+ boolean executeDrawHandlers(SurfaceControl.Transaction t, int seqId) {
boolean hadHandlers = false;
boolean applyHere = false;
if (t == null) {
@@ -6132,13 +6080,16 @@
applyHere = true;
}
- for (int i = 0; i < mReadyDrawHandlers.size(); i++) {
- mReadyDrawHandlers.get(i).accept(t);
- hadHandlers = true;
+ for (int i = mDrawHandlers.size() - 1; i >= 0; i--) {
+ DrawHandler h = mDrawHandlers.get(i);
+ if (h.mSeqId <= seqId) {
+ h.mConsumer.accept(t);
+ mDrawHandlers.remove(h);
+ hadHandlers = true;
+ }
}
if (hadHandlers) {
- mReadyDrawHandlers.clear();
mWmService.mH.removeMessages(WINDOW_STATE_BLAST_SYNC_TIMEOUT, this);
}
diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java
index 91bde7b..635939e 100644
--- a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java
@@ -503,7 +503,6 @@
win.applyWithNextDraw(t -> handledT[0] = t);
assertTrue(win.useBLASTSync());
final SurfaceControl.Transaction drawT = new StubTransaction();
- win.prepareDrawHandlers();
assertTrue(win.finishDrawing(drawT, Integer.MAX_VALUE));
assertEquals(drawT, handledT[0]);
assertFalse(win.useBLASTSync());