SurfaceView: Synchronize destframe updates with SurfaceView size changes

This CL fixes one of the issues with SurfaceView parent frame and
content syncing.

With BLAST, we have two surface controls each setting a scale. The
parent surface control sets a scale based on the requested surface
size and the SurfaceView layout size. The BlastBufferQueue surface
control scales the buffer to the requested buffer size if the buffer
has the appropriate scale mode.

The destination frame controls the second scaling and it must be
applied with the parent surface scale changes. This cl fixes flickers
where the requested fixed surface size changes without any view size
changes. This cl allows the caller to pass in a transaction to
BLASTBufferQueue#update which is updated with the destination frame
changes. This transaction can then be applied with the parent
surface changes.

This also fixes an issue where destination Frame was being set on
every buffer update and when we updated the BlastBufferQueue size.
Since buffer transactions can be queued up on the server side, a
stale value maybe applied for a few frames causing flickers.

Fixes: 194458377
Test: bug repro steps
Test: atest SurfaceViewSyncTest#testSurfaceViewSetFixedSize

Change-Id: I216586842ff45bfd398659b5cc0c89eaf51394ff

2 files changed, 19 insertions, 9 deletions

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 7d57d8b02c..56a9683773 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -166,9 +166,10 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp<SurfaceCont
     mTransformHint = mSurfaceControl->getTransformHint();
     mBufferItemConsumer->setTransformHint(mTransformHint);
     SurfaceComposerClient::Transaction()
-          .setFlags(surface, layer_state_t::eEnableBackpressure,
-                    layer_state_t::eEnableBackpressure)
-          .apply();
+            .setFlags(surface, layer_state_t::eEnableBackpressure,
+                      layer_state_t::eEnableBackpressure)
+            .setApplyToken(mApplyToken)
+            .apply();
     mNumAcquired = 0;
     mNumFrameAvailable = 0;
     BQA_LOGV("BLASTBufferQueue created width=%d height=%d format=%d mTransformHint=%d", width,
@@ -190,7 +191,7 @@ BLASTBufferQueue::~BLASTBufferQueue() {
 }
 
 void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height,
-                              int32_t format) {
+                              int32_t format, SurfaceComposerClient::Transaction* outTransaction) {
     std::unique_lock _lock{mMutex};
     if (mFormat != format) {
         mFormat = format;
@@ -227,15 +228,18 @@ void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width,
             // We only need to update the scale if we've received at least one buffer. The reason
             // for this is the scale is calculated based on the requested size and buffer size.
             // If there's no buffer, the scale will always be 1.
+            SurfaceComposerClient::Transaction* destFrameTransaction =
+                    (outTransaction) ? outTransaction : &t;
             if (mSurfaceControl != nullptr && mLastBufferInfo.hasBuffer) {
-                t.setDestinationFrame(mSurfaceControl,
-                                      Rect(0, 0, newSize.getWidth(), newSize.getHeight()));
+                destFrameTransaction->setDestinationFrame(mSurfaceControl,
+                                                          Rect(0, 0, newSize.getWidth(),
+                                                               newSize.getHeight()));
             }
             applyTransaction = true;
         }
     }
     if (applyTransaction) {
-        t.apply();
+        t.setApplyToken(mApplyToken).apply();
     }
 }
 
@@ -453,6 +457,9 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) {
     incStrong((void*)transactionCallbackThunk);
 
     Rect crop = computeCrop(bufferItem);
+    const bool updateDestinationFrame =
+            bufferItem.mScalingMode == NATIVE_WINDOW_SCALING_MODE_FREEZE ||
+            !mLastBufferInfo.hasBuffer;
     mLastBufferInfo.update(true /* hasBuffer */, bufferItem.mGraphicBuffer->getWidth(),
                            bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform,
                            bufferItem.mScalingMode, crop);
@@ -470,7 +477,9 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) {
     t->addTransactionCompletedCallback(transactionCallbackThunk, static_cast<void*>(this));
     mSurfaceControlsWithPendingCallback.push(mSurfaceControl);
 
-    t->setDestinationFrame(mSurfaceControl, Rect(0, 0, mSize.getWidth(), mSize.getHeight()));
+    if (updateDestinationFrame) {
+        t->setDestinationFrame(mSurfaceControl, Rect(0, 0, mSize.getWidth(), mSize.getHeight()));
+    }
     t->setBufferCrop(mSurfaceControl, crop);
     t->setTransform(mSurfaceControl, bufferItem.mTransform);
     t->setTransformToDisplayInverse(mSurfaceControl, bufferItem.mTransformToDisplayInverse);
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 0d6a673b02..ea9b1c68af 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -96,7 +96,8 @@ public:
     void setTransactionCompleteCallback(uint64_t frameNumber,
                                         std::function<void(int64_t)>&& transactionCompleteCallback);
 
-    void update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height, int32_t format);
+    void update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height, int32_t format,
+                SurfaceComposerClient::Transaction* outTransaction = nullptr);
     void flushShadowQueue() {}
 
     status_t setFrameRate(float frameRate, int8_t compatibility, bool shouldBeSeamless);