SF: Make BufferData mockable
Expose GraphicBuffer properties through the BufferData
class so we can inject fake GraphicBuffers in transactions.
This is required to recreate layer state from transaction
traces without actually allocating buffers.
Test: compiles
Bug: 200284593
Change-Id: I74036cba1f544cbd045489fa5337d59ae4bdebcb
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index acd9ac5..27d86bb 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -153,7 +153,8 @@
SAFE_PARCEL(output.writeBool, isTrustedOverlay);
SAFE_PARCEL(output.writeUint32, static_cast<uint32_t>(dropInputMode));
- SAFE_PARCEL(bufferData.write, output);
+ SAFE_PARCEL(output.writeNullableParcelable,
+ bufferData ? std::make_optional<BufferData>(*bufferData) : std::nullopt);
return NO_ERROR;
}
@@ -263,7 +264,9 @@
uint32_t mode;
SAFE_PARCEL(input.readUint32, &mode);
dropInputMode = static_cast<gui::DropInputMode>(mode);
- SAFE_PARCEL(bufferData.read, input);
+ std::optional<BufferData> tmpBufferData;
+ SAFE_PARCEL(input.readParcelable, &tmpBufferData);
+ bufferData = tmpBufferData ? std::make_shared<BufferData>(*tmpBufferData) : nullptr;
return NO_ERROR;
}
@@ -518,7 +521,7 @@
}
bool layer_state_t::hasValidBuffer() const {
- return bufferData.buffer || bufferData.cachedBuffer.isValid();
+ return bufferData && (bufferData->buffer || bufferData->cachedBuffer.isValid());
}
status_t layer_state_t::matrix22_t::write(Parcel& output) const {
@@ -681,64 +684,64 @@
return {buffer->getId(), frameNumber};
}
-status_t BufferData::write(Parcel& output) const {
- SAFE_PARCEL(output.writeInt32, flags.get());
+status_t BufferData::writeToParcel(Parcel* output) const {
+ SAFE_PARCEL(output->writeInt32, flags.get());
if (buffer) {
- SAFE_PARCEL(output.writeBool, true);
- SAFE_PARCEL(output.write, *buffer);
+ SAFE_PARCEL(output->writeBool, true);
+ SAFE_PARCEL(output->write, *buffer);
} else {
- SAFE_PARCEL(output.writeBool, false);
+ SAFE_PARCEL(output->writeBool, false);
}
if (acquireFence) {
- SAFE_PARCEL(output.writeBool, true);
- SAFE_PARCEL(output.write, *acquireFence);
+ SAFE_PARCEL(output->writeBool, true);
+ SAFE_PARCEL(output->write, *acquireFence);
} else {
- SAFE_PARCEL(output.writeBool, false);
+ SAFE_PARCEL(output->writeBool, false);
}
- SAFE_PARCEL(output.writeUint64, frameNumber);
- SAFE_PARCEL(output.writeStrongBinder, IInterface::asBinder(releaseBufferListener));
- SAFE_PARCEL(output.writeStrongBinder, releaseBufferEndpoint);
+ SAFE_PARCEL(output->writeUint64, frameNumber);
+ SAFE_PARCEL(output->writeStrongBinder, IInterface::asBinder(releaseBufferListener));
+ SAFE_PARCEL(output->writeStrongBinder, releaseBufferEndpoint);
- SAFE_PARCEL(output.writeStrongBinder, cachedBuffer.token.promote());
- SAFE_PARCEL(output.writeUint64, cachedBuffer.id);
+ SAFE_PARCEL(output->writeStrongBinder, cachedBuffer.token.promote());
+ SAFE_PARCEL(output->writeUint64, cachedBuffer.id);
return NO_ERROR;
}
-status_t BufferData::read(const Parcel& input) {
+status_t BufferData::readFromParcel(const Parcel* input) {
int32_t tmpInt32;
- SAFE_PARCEL(input.readInt32, &tmpInt32);
+ SAFE_PARCEL(input->readInt32, &tmpInt32);
flags = Flags<BufferDataChange>(tmpInt32);
bool tmpBool = false;
- SAFE_PARCEL(input.readBool, &tmpBool);
+ SAFE_PARCEL(input->readBool, &tmpBool);
if (tmpBool) {
buffer = new GraphicBuffer();
- SAFE_PARCEL(input.read, *buffer);
+ SAFE_PARCEL(input->read, *buffer);
}
- SAFE_PARCEL(input.readBool, &tmpBool);
+ SAFE_PARCEL(input->readBool, &tmpBool);
if (tmpBool) {
acquireFence = new Fence();
- SAFE_PARCEL(input.read, *acquireFence);
+ SAFE_PARCEL(input->read, *acquireFence);
}
- SAFE_PARCEL(input.readUint64, &frameNumber);
+ SAFE_PARCEL(input->readUint64, &frameNumber);
sp<IBinder> tmpBinder = nullptr;
- SAFE_PARCEL(input.readNullableStrongBinder, &tmpBinder);
+ SAFE_PARCEL(input->readNullableStrongBinder, &tmpBinder);
if (tmpBinder) {
releaseBufferListener = checked_interface_cast<ITransactionCompletedListener>(tmpBinder);
}
- SAFE_PARCEL(input.readNullableStrongBinder, &releaseBufferEndpoint);
+ SAFE_PARCEL(input->readNullableStrongBinder, &releaseBufferEndpoint);
tmpBinder = nullptr;
- SAFE_PARCEL(input.readNullableStrongBinder, &tmpBinder);
+ SAFE_PARCEL(input->readNullableStrongBinder, &tmpBinder);
cachedBuffer.token = tmpBinder;
- SAFE_PARCEL(input.readUint64, &cachedBuffer.id);
+ SAFE_PARCEL(input->readUint64, &cachedBuffer.id);
return NO_ERROR;
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index b10c384..6a4ddae 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -728,18 +728,18 @@
return;
}
- auto listener = state.bufferData.releaseBufferListener;
+ auto listener = state.bufferData->releaseBufferListener;
sp<Fence> fence =
- state.bufferData.acquireFence ? state.bufferData.acquireFence : Fence::NO_FENCE;
- if (state.bufferData.releaseBufferEndpoint ==
+ state.bufferData->acquireFence ? state.bufferData->acquireFence : Fence::NO_FENCE;
+ if (state.bufferData->releaseBufferEndpoint ==
IInterface::asBinder(TransactionCompletedListener::getIInstance())) {
// if the callback is in process, run on a different thread to avoid any lock contigency
// issues in the client.
SurfaceComposerClient::getDefault()
->mReleaseCallbackThread
- .addReleaseCallback(state.bufferData.generateReleaseCallbackId(), fence);
+ .addReleaseCallback(state.bufferData->generateReleaseCallbackId(), fence);
} else {
- listener->onReleaseBuffer(state.bufferData.generateReleaseCallbackId(), fence, UINT_MAX);
+ listener->onReleaseBuffer(state.bufferData->generateReleaseCallbackId(), fence, UINT_MAX);
}
}
@@ -839,7 +839,8 @@
layer_state_t* s = &(mComposerStates[handle].state);
if (!(s->what & layer_state_t::eBufferChanged)) {
continue;
- } else if (s->bufferData.flags.test(BufferData::BufferDataChange::cachedBufferChanged)) {
+ } else if (s->bufferData &&
+ s->bufferData->flags.test(BufferData::BufferDataChange::cachedBufferChanged)) {
// If eBufferChanged and eCachedBufferChanged are both trued then that means
// we already cached the buffer in a previous call to cacheBuffers, perhaps
// from writeToParcel on a Transaction that was merged in to this one.
@@ -848,22 +849,22 @@
// Don't try to cache a null buffer. Sending null buffers is cheap so we shouldn't waste
// time trying to cache them.
- if (!s->bufferData.buffer) {
+ if (!s->bufferData || !s->bufferData->buffer) {
continue;
}
uint64_t cacheId = 0;
- status_t ret = BufferCache::getInstance().getCacheId(s->bufferData.buffer, &cacheId);
+ status_t ret = BufferCache::getInstance().getCacheId(s->bufferData->buffer, &cacheId);
if (ret == NO_ERROR) {
// Cache-hit. Strip the buffer and send only the id.
- s->bufferData.buffer = nullptr;
+ s->bufferData->buffer = nullptr;
} else {
// Cache-miss. Include the buffer and send the new cacheId.
- cacheId = BufferCache::getInstance().cache(s->bufferData.buffer);
+ cacheId = BufferCache::getInstance().cache(s->bufferData->buffer);
}
- s->bufferData.flags |= BufferData::BufferDataChange::cachedBufferChanged;
- s->bufferData.cachedBuffer.token = BufferCache::getInstance().getToken();
- s->bufferData.cachedBuffer.id = cacheId;
+ s->bufferData->flags |= BufferData::BufferDataChange::cachedBufferChanged;
+ s->bufferData->cachedBuffer.token = BufferCache::getInstance().getToken();
+ s->bufferData->cachedBuffer.id = cacheId;
// If we have more buffers than the size of the cache, we should stop caching so we don't
// evict other buffers in this transaction
@@ -1322,23 +1323,22 @@
return *this;
}
-std::optional<BufferData> SurfaceComposerClient::Transaction::getAndClearBuffer(
+std::shared_ptr<BufferData> SurfaceComposerClient::Transaction::getAndClearBuffer(
const sp<SurfaceControl>& sc) {
layer_state_t* s = getLayerState(sc);
if (!s) {
- return std::nullopt;
+ return nullptr;
}
if (!(s->what & layer_state_t::eBufferChanged)) {
- return std::nullopt;
+ return nullptr;
}
- BufferData bufferData = s->bufferData;
+ std::shared_ptr<BufferData> bufferData = std::move(s->bufferData);
TransactionCompletedListener::getInstance()->removeReleaseBufferCallback(
- bufferData.generateReleaseCallbackId());
- BufferData emptyBufferData;
+ bufferData->generateReleaseCallbackId());
s->what &= ~layer_state_t::eBufferChanged;
- s->bufferData = emptyBufferData;
+ s->bufferData = nullptr;
mContainsBuffer = false;
return bufferData;
@@ -1356,24 +1356,24 @@
releaseBufferIfOverwriting(*s);
- BufferData bufferData;
- bufferData.buffer = buffer;
+ std::shared_ptr<BufferData> bufferData = std::make_shared<BufferData>();
+ bufferData->buffer = buffer;
if (frameNumber) {
- bufferData.frameNumber = *frameNumber;
- bufferData.flags |= BufferData::BufferDataChange::frameNumberChanged;
+ bufferData->frameNumber = *frameNumber;
+ bufferData->flags |= BufferData::BufferDataChange::frameNumberChanged;
}
if (fence) {
- bufferData.acquireFence = *fence;
- bufferData.flags |= BufferData::BufferDataChange::fenceChanged;
+ bufferData->acquireFence = *fence;
+ bufferData->flags |= BufferData::BufferDataChange::fenceChanged;
}
- bufferData.releaseBufferEndpoint =
+ bufferData->releaseBufferEndpoint =
IInterface::asBinder(TransactionCompletedListener::getIInstance());
if (mIsAutoTimestamp) {
mDesiredPresentTime = systemTime();
}
- setReleaseBufferCallback(&bufferData, callback);
+ setReleaseBufferCallback(bufferData.get(), callback);
s->what |= layer_state_t::eBufferChanged;
- s->bufferData = bufferData;
+ s->bufferData = std::move(bufferData);
registerSurfaceControlForCallback(sc);
mContainsBuffer = true;
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index e48f52d..968ace9 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -58,7 +58,22 @@
bool isValid() const { return token != nullptr; }
};
-struct BufferData {
+class BufferData : public Parcelable {
+public:
+ virtual ~BufferData() = default;
+ virtual bool hasBuffer() const { return buffer != nullptr; }
+ virtual bool hasSameBuffer(const BufferData& other) const {
+ return buffer == other.buffer && frameNumber == other.frameNumber;
+ }
+ virtual uint32_t getWidth() const { return buffer->getWidth(); }
+ virtual uint32_t getHeight() const { return buffer->getHeight(); }
+ Rect getBounds() const {
+ return {0, 0, static_cast<int32_t>(getWidth()), static_cast<int32_t>(getHeight())};
+ }
+ virtual uint64_t getId() const { return buffer->getId(); }
+ virtual PixelFormat getPixelFormat() const { return buffer->getPixelFormat(); }
+ virtual uint64_t getUsage() const { return buffer->getUsage(); }
+
enum class BufferDataChange : uint32_t {
fenceChanged = 0x01,
frameNumberChanged = 0x02,
@@ -89,8 +104,9 @@
// Generates the release callback id based on the buffer id and frame number.
// This is used as an identifier when release callbacks are invoked.
ReleaseCallbackId generateReleaseCallbackId() const;
- status_t write(Parcel& output) const;
- status_t read(const Parcel& input);
+
+ status_t writeToParcel(Parcel* parcel) const override;
+ status_t readFromParcel(const Parcel* parcel) override;
};
/*
@@ -204,7 +220,7 @@
uint32_t transform;
bool transformToDisplayInverse;
Rect crop;
- BufferData bufferData;
+ std::shared_ptr<BufferData> bufferData = nullptr;
ui::Dataspace dataspace;
HdrMetadata hdrMetadata;
Region surfaceDamageRegion;
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index b739b3c..62758af 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -501,7 +501,7 @@
const std::optional<sp<Fence>>& fence = std::nullopt,
const std::optional<uint64_t>& frameNumber = std::nullopt,
ReleaseBufferCallback callback = nullptr);
- std::optional<BufferData> getAndClearBuffer(const sp<SurfaceControl>& sc);
+ std::shared_ptr<BufferData> getAndClearBuffer(const sp<SurfaceControl>& sc);
Transaction& setDataspace(const sp<SurfaceControl>& sc, ui::Dataspace dataspace);
Transaction& setHdrMetadata(const sp<SurfaceControl>& sc, const HdrMetadata& hdrMetadata);
Transaction& setSurfaceDamageRegion(const sp<SurfaceControl>& sc,
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 8db974e..bd13c41 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3719,9 +3719,9 @@
if (transaction.states.size() == 1) {
const auto& state = transaction.states.begin()->state;
if ((state.flags & ~layer_state_t::eBufferChanged) == 0 &&
- state.bufferData.flags.test(BufferData::BufferDataChange::fenceChanged) &&
- state.bufferData.acquireFence &&
- state.bufferData.acquireFence->getStatus() == Fence::Status::Unsignaled) {
+ state.bufferData->flags.test(BufferData::BufferDataChange::fenceChanged) &&
+ state.bufferData->acquireFence &&
+ state.bufferData->acquireFence->getStatus() == Fence::Status::Unsignaled) {
ATRACE_NAME("transactionCanLatchUnsignaled");
return true;
}
@@ -3786,10 +3786,10 @@
for (const ComposerState& state : states) {
const layer_state_t& s = state.state;
- const bool acquireFenceChanged =
- s.bufferData.flags.test(BufferData::BufferDataChange::fenceChanged);
- if (acquireFenceChanged && s.bufferData.acquireFence && !allowLatchUnsignaled &&
- s.bufferData.acquireFence->getStatus() == Fence::Status::Unsignaled) {
+ const bool acquireFenceChanged = s.bufferData &&
+ s.bufferData->flags.test(BufferData::BufferDataChange::fenceChanged);
+ if (acquireFenceChanged && s.bufferData->acquireFence && !allowLatchUnsignaled &&
+ s.bufferData->acquireFence->getStatus() == Fence::Status::Unsignaled) {
ATRACE_NAME("fence unsignaled");
return false;
}
@@ -4420,7 +4420,7 @@
}
if (what & layer_state_t::eBufferChanged &&
- layer->setBuffer(s.bufferData, postTime, desiredPresentTime, isAutoTimestamp,
+ layer->setBuffer(*s.bufferData, postTime, desiredPresentTime, isAutoTimestamp,
dequeueBufferTimestamp, frameTimelineInfo)) {
flags |= eTraversalNeeded;
} else if (frameTimelineInfo.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) {
diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
index 378deb0..849de22 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
@@ -132,14 +132,14 @@
}
if (layer.what & layer_state_t::eBufferChanged) {
proto::LayerState_BufferData* bufferProto = proto.mutable_buffer_data();
- if (layer.bufferData.buffer) {
- bufferProto->set_buffer_id(layer.bufferData.buffer->getId());
- bufferProto->set_width(layer.bufferData.buffer->getWidth());
- bufferProto->set_height(layer.bufferData.buffer->getHeight());
+ if (layer.bufferData->buffer) {
+ bufferProto->set_buffer_id(layer.bufferData->getId());
+ bufferProto->set_width(layer.bufferData->getWidth());
+ bufferProto->set_height(layer.bufferData->getHeight());
}
- bufferProto->set_frame_number(layer.bufferData.frameNumber);
- bufferProto->set_flags(layer.bufferData.flags.get());
- bufferProto->set_cached_buffer_id(layer.bufferData.cachedBuffer.id);
+ bufferProto->set_frame_number(layer.bufferData->frameNumber);
+ bufferProto->set_flags(layer.bufferData->flags.get());
+ bufferProto->set_cached_buffer_id(layer.bufferData->cachedBuffer.id);
}
if (layer.what & layer_state_t::eSidebandStreamChanged) {
proto.set_has_sideband_stream(layer.sidebandStream != nullptr);
@@ -405,10 +405,13 @@
LayerProtoHelper::readFromProto(proto.crop(), layer.crop);
}
if (proto.what() & layer_state_t::eBufferChanged) {
+ if (!layer.bufferData) {
+ layer.bufferData = std::make_shared<BufferData>();
+ }
const proto::LayerState_BufferData& bufferProto = proto.buffer_data();
- layer.bufferData.frameNumber = bufferProto.frame_number();
- layer.bufferData.flags = Flags<BufferData::BufferDataChange>(bufferProto.flags());
- layer.bufferData.cachedBuffer.id = bufferProto.cached_buffer_id();
+ layer.bufferData->frameNumber = bufferProto.frame_number();
+ layer.bufferData->flags = Flags<BufferData::BufferDataChange>(bufferProto.flags());
+ layer.bufferData->cachedBuffer.id = bufferProto.cached_buffer_id();
}
if (proto.what() & layer_state_t::eApiChanged) {
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index 16d4b59..1ce0309 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -434,9 +434,10 @@
static ComposerState createComposerState(int layerId, sp<Fence> fence,
uint32_t stateFlags = layer_state_t::eBufferChanged) {
ComposerState composer_state;
- composer_state.state.bufferData.acquireFence = std::move(fence);
+ composer_state.state.bufferData = std::make_shared<BufferData>();
+ composer_state.state.bufferData->acquireFence = std::move(fence);
composer_state.state.layerId = layerId;
- composer_state.state.bufferData.flags = BufferData::BufferDataChange::fenceChanged;
+ composer_state.state.bufferData->flags = BufferData::BufferDataChange::fenceChanged;
composer_state.state.flags = stateFlags;
return composer_state;
}
diff --git a/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp b/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
index 6e00748..271b1c0 100644
--- a/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
@@ -39,6 +39,7 @@
layer_state_t layer;
layer.layerId = 6;
layer.what = std::numeric_limits<uint64_t>::max();
+ layer.what &= ~static_cast<uint64_t>(layer_state_t::eBufferChanged);
layer.x = 7;
layer.matrix.dsdx = 15;