Fixed bug with caching buffers
SCC::cacheBuffers checks if the Transaction contains a buffer before
looking through the states. However, the flag mContainsBuffer could be
cleared if a single layer clears the buffer. This isn't correct since
there could be other SC in the Transaction that have a buffer and we
would end up not caching them.
Rename mContainsBuffer to mMayContainBuffer since it's more of a hint
that the Transaction may have a buffer. It's not really worth storing a
count when we're going to check each layer anyway. mMayContainBuffer
just helps optimize in the scenario when we know for sure that no buffer
has even been set before apply was called.
Additionally, remove the code that clears mMayContainBuffer when the
buffer has been removed since there may be a different buffer set for a
different SC.
Because mMayContainBuffer is only used to determine whether to cache,
there's no need to parcel it. When writeToParcel is called, the data is
cached so the other process doesn't need to know about it.
Test: Builds
Bug: 246986005
Change-Id: Id642cba7f13112084d842b1222b2bc7256d2e577
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 751721e..a9d6b0e 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -632,7 +632,7 @@
mAnimation(other.mAnimation),
mEarlyWakeupStart(other.mEarlyWakeupStart),
mEarlyWakeupEnd(other.mEarlyWakeupEnd),
- mContainsBuffer(other.mContainsBuffer),
+ mMayContainBuffer(other.mMayContainBuffer),
mDesiredPresentTime(other.mDesiredPresentTime),
mIsAutoTimestamp(other.mIsAutoTimestamp),
mFrameTimelineInfo(other.mFrameTimelineInfo),
@@ -667,7 +667,6 @@
const bool animation = parcel->readBool();
const bool earlyWakeupStart = parcel->readBool();
const bool earlyWakeupEnd = parcel->readBool();
- const bool containsBuffer = parcel->readBool();
const int64_t desiredPresentTime = parcel->readInt64();
const bool isAutoTimestamp = parcel->readBool();
FrameTimelineInfo frameTimelineInfo;
@@ -745,7 +744,6 @@
mAnimation = animation;
mEarlyWakeupStart = earlyWakeupStart;
mEarlyWakeupEnd = earlyWakeupEnd;
- mContainsBuffer = containsBuffer;
mDesiredPresentTime = desiredPresentTime;
mIsAutoTimestamp = isAutoTimestamp;
mFrameTimelineInfo = frameTimelineInfo;
@@ -777,7 +775,6 @@
parcel->writeBool(mAnimation);
parcel->writeBool(mEarlyWakeupStart);
parcel->writeBool(mEarlyWakeupEnd);
- parcel->writeBool(mContainsBuffer);
parcel->writeInt64(mDesiredPresentTime);
parcel->writeBool(mIsAutoTimestamp);
mFrameTimelineInfo.writeToParcel(parcel);
@@ -876,7 +873,7 @@
mInputWindowCommands.merge(other.mInputWindowCommands);
- mContainsBuffer |= other.mContainsBuffer;
+ mMayContainBuffer |= other.mMayContainBuffer;
mEarlyWakeupStart = mEarlyWakeupStart || other.mEarlyWakeupStart;
mEarlyWakeupEnd = mEarlyWakeupEnd || other.mEarlyWakeupEnd;
mApplyToken = other.mApplyToken;
@@ -892,7 +889,7 @@
mDisplayStates.clear();
mListenerCallbacks.clear();
mInputWindowCommands.clear();
- mContainsBuffer = false;
+ mMayContainBuffer = false;
mForceSynchronous = 0;
mTransactionNestCount = 0;
mAnimation = false;
@@ -920,7 +917,7 @@
}
void SurfaceComposerClient::Transaction::cacheBuffers() {
- if (!mContainsBuffer) {
+ if (!mMayContainBuffer) {
return;
}
@@ -1464,7 +1461,6 @@
s->what &= ~layer_state_t::eBufferChanged;
s->bufferData = nullptr;
- mContainsBuffer = false;
return bufferData;
}
@@ -1495,7 +1491,6 @@
if (buffer == nullptr) {
s->what &= ~layer_state_t::eBufferChanged;
s->bufferData = nullptr;
- mContainsBuffer = false;
return *this;
}
@@ -1530,7 +1525,7 @@
const std::vector<SurfaceControlStats>&) {},
nullptr);
- mContainsBuffer = true;
+ mMayContainBuffer = true;
return *this;
}
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 8c47ebc..963cc09 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -409,8 +409,10 @@
bool mEarlyWakeupStart = false;
bool mEarlyWakeupEnd = false;
- // Indicates that the Transaction contains a buffer that should be cached
- bool mContainsBuffer = false;
+ // Indicates that the Transaction may contain buffers that should be cached. The reason this
+ // is only a guess is that buffers can be removed before cache is called. This is only a
+ // hint that at some point a buffer was added to this transaction before apply was called.
+ bool mMayContainBuffer = false;
// mDesiredPresentTime is the time in nanoseconds that the client would like the transaction
// to be presented. When it is not possible to present at exactly that time, it will be