Merge "Async BLAST Sync[4/N]: Replace draw handlers with seqId" into tm-dev
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index 451e777..74537be 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -2513,7 +2513,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 76ff4c0..3200291 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);
@@ -6011,7 +5969,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.
@@ -6039,7 +5997,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;
@@ -6096,7 +6054,7 @@
*/
@Override
boolean useBLASTSync() {
- return super.useBLASTSync() || (mPendingDrawHandlers.size() != 0);
+ return super.useBLASTSync() || (mDrawHandlers.size() != 0);
}
/**
@@ -6108,11 +6066,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,
@@ -6120,21 +6079,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) {
@@ -6142,13 +6090,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 bb0c7f7..c81e7a25 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());