diff options
37 files changed, 461 insertions, 469 deletions
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp index b1bd705f19..fe473597b1 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -165,7 +165,6 @@ filegroup { "FrameTracer/FrameTracer.cpp", "FrameTracker.cpp", "HdrLayerInfoReporter.cpp", - "HwcSlotGenerator.cpp", "WindowInfosListenerInvoker.cpp", "Layer.cpp", "LayerFE.cpp", diff --git a/services/surfaceflinger/ClientCache.cpp b/services/surfaceflinger/ClientCache.cpp index 2bd8f324e1..09e41ffede 100644 --- a/services/surfaceflinger/ClientCache.cpp +++ b/services/surfaceflinger/ClientCache.cpp @@ -118,7 +118,8 @@ ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer) Usage::READABLE)); } -void ClientCache::erase(const client_cache_t& cacheId) { +sp<GraphicBuffer> ClientCache::erase(const client_cache_t& cacheId) { + sp<GraphicBuffer> buffer; auto& [processToken, id] = cacheId; std::vector<sp<ErasedRecipient>> pendingErase; { @@ -126,9 +127,11 @@ void ClientCache::erase(const client_cache_t& cacheId) { ClientCacheBuffer* buf = nullptr; if (!getBuffer(cacheId, &buf)) { ALOGE("failed to erase buffer, could not retrieve buffer"); - return; + return nullptr; } + buffer = buf->buffer->getBuffer(); + for (auto& recipient : buf->recipients) { sp<ErasedRecipient> erasedRecipient = recipient.promote(); if (erasedRecipient) { @@ -142,6 +145,7 @@ void ClientCache::erase(const client_cache_t& cacheId) { for (auto& recipient : pendingErase) { recipient->bufferErased(cacheId); } + return buffer; } std::shared_ptr<renderengine::ExternalTexture> ClientCache::get(const client_cache_t& cacheId) { diff --git a/services/surfaceflinger/ClientCache.h b/services/surfaceflinger/ClientCache.h index cdeac2bbc1..b56b252d9f 100644 --- a/services/surfaceflinger/ClientCache.h +++ b/services/surfaceflinger/ClientCache.h @@ -33,6 +33,17 @@ namespace android { +// This class manages a cache of buffer handles between SurfaceFlinger clients +// and the SurfaceFlinger process which optimizes away some of the cost of +// sending buffer handles across processes. +// +// Buffers are explicitly cached and uncached by the SurfaceFlinger client. When +// a buffer is uncached, it is not only purged from this cache, but the buffer +// ID is also passed down to CompositionEngine to purge it from a similar cache +// used between SurfaceFlinger and Composer HAL. The buffer ID used to purge +// both the SurfaceFlinger side of this other cache, as well as Composer HAL's +// side of the cache. +// class ClientCache : public Singleton<ClientCache> { public: ClientCache(); @@ -41,7 +52,8 @@ public: base::expected<std::shared_ptr<renderengine::ExternalTexture>, AddError> add( const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer); - void erase(const client_cache_t& cacheId); + + sp<GraphicBuffer> erase(const client_cache_t& cacheId); std::shared_ptr<renderengine::ExternalTexture> get(const client_cache_t& cacheId); diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h index f861fc97e4..415a04113c 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h @@ -52,6 +52,9 @@ struct CompositionRefreshArgs { // All the layers that have queued updates. Layers layersWithQueuedFrames; + // All graphic buffers that will no longer be used and should be removed from caches. + std::vector<uint64_t> bufferIdsToUncache; + // Controls how the color mode is chosen for an output OutputColorSetting outputColorSetting{OutputColorSetting::kEnhanced}; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h index 974f7c6134..ad98e93232 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h @@ -163,7 +163,6 @@ struct LayerFECompositionState { // The buffer and related state sp<GraphicBuffer> buffer; - int bufferSlot{BufferQueue::INVALID_BUFFER_SLOT}; sp<Fence> acquireFence = Fence::NO_FENCE; Region surfaceDamage; uint64_t frameNumber = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h index 874b330c1c..bd43c897a0 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h @@ -23,6 +23,7 @@ #include <type_traits> #include <unordered_map> #include <utility> +#include <vector> #include <compositionengine/LayerFE.h> #include <renderengine/LayerSettings.h> @@ -272,6 +273,7 @@ protected: virtual void setDisplayColorProfile(std::unique_ptr<DisplayColorProfile>) = 0; virtual void setRenderSurface(std::unique_ptr<RenderSurface>) = 0; + virtual void uncacheBuffers(const std::vector<uint64_t>&) = 0; virtual void rebuildLayerStacks(const CompositionRefreshArgs&, LayerFESet&) = 0; virtual void collectVisibleLayers(const CompositionRefreshArgs&, CoverageState&) = 0; virtual void ensureOutputLayerIfVisible(sp<LayerFE>&, CoverageState&) = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h index 6d0c395b2f..4dbf8d2fce 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h @@ -19,6 +19,7 @@ #include <cstdint> #include <optional> #include <string> +#include <vector> #include <ui/Transform.h> #include <utils/StrongPointer.h> @@ -81,6 +82,10 @@ public: // TODO(lpique): Make this protected once it is only internally called. virtual CompositionState& editState() = 0; + // Clear the cache entries for a set of buffers that SurfaceFlinger no + // longer cares about. + virtual void uncacheBuffers(const std::vector<uint64_t>& bufferIdsToUncache) = 0; + // Recalculates the state of the output layer from the output-independent // layer. If includeGeometry is false, the geometry state can be skipped. // internalDisplayRotationFlags must be set to the rotation flags for the diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h index fd22aa3a2a..6e9ea6ffd8 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/HwcBufferCache.h @@ -17,7 +17,8 @@ #pragma once #include <cstdint> -#include <vector> +#include <stack> +#include <unordered_map> // TODO(b/129481165): remove the #pragma below and fix conversion issues #pragma clang diagnostic push @@ -37,35 +38,76 @@ class GraphicBuffer; namespace compositionengine::impl { -// With HIDLized hwcomposer HAL, the HAL can maintain a buffer cache for each -// HWC display and layer. When updating a display target or a layer buffer, -// we have the option to send the buffer handle over or to request the HAL to -// retrieve it from its cache. The latter is cheaper since it eliminates the -// overhead to transfer the handle over the trasport layer, and the overhead -// for the HAL to clone and retain the handle. +// The buffer cache returns both a slot and the buffer that should be sent to HWC. In cases +// where the buffer is already cached, the buffer is a nullptr and will not be sent to HWC as +// an optimization. +struct HwcSlotAndBuffer { + uint32_t slot; + sp<GraphicBuffer> buffer; +}; + +// +// Manages the slot assignments for a buffers stored in Composer HAL's cache. +// +// Cache slots are an optimization when communicating buffer handles to Composer +// HAL. When updating a layer's buffer, we can either send a new buffer handle +// along with it's slot assignment or request the HAL to reuse a buffer handle +// that we've already sent by using the slot assignment. The latter is cheaper +// since it eliminates the overhead to transfer the buffer handle over IPC and +// the overhead for the HAL to clone the handle. // -// To be able to find out whether a buffer is already in the HAL's cache, we -// use HWComposerBufferCache to mirror the cache in SF. class HwcBufferCache { +private: + static const constexpr size_t kMaxLayerBufferCount = BufferQueue::NUM_BUFFER_SLOTS; + public: + // public for testing + // Override buffers don't use the normal cache slots because we don't want them to evict client + // buffers from the cache. We add an extra slot at the end for the override buffers. + static const constexpr size_t kOverrideBufferSlot = kMaxLayerBufferCount; + HwcBufferCache(); - // Given a buffer, return the HWC cache slot and - // buffer to be sent to HWC. + + // + // Given a buffer, return the HWC cache slot and buffer to send to HWC. + // + // If the buffer is already in the cache, the buffer is null to optimize away sending HWC the + // buffer handle. // - // outBuffer is set to buffer when buffer is not in the HWC cache; - // otherwise, outBuffer is set to nullptr. - void getHwcBuffer(int slot, const sp<GraphicBuffer>& buffer, uint32_t* outSlot, - sp<GraphicBuffer>* outBuffer); + HwcSlotAndBuffer getHwcSlotAndBuffer(const sp<GraphicBuffer>& buffer); + // + // Given a buffer, return the HWC cache slot and buffer to send to HWC. + // + // A special slot number is used for override buffers. + // + // If the buffer is already in the cache, the buffer is null to optimize away sending HWC the + // buffer handle. + // + HwcSlotAndBuffer getHwcSlotAndBufferForOverride(const sp<GraphicBuffer>& buffer); - // Special caching slot for the layer caching feature. - static const constexpr size_t FLATTENER_CACHING_SLOT = BufferQueue::NUM_BUFFER_SLOTS; + // + // When a client process discards a buffer, it needs to be purged from the HWC cache. + // + // Returns the slot number of the buffer, or UINT32_MAX if it wasn't found in the cache. + // + uint32_t uncache(uint64_t graphicBufferId); private: - // an array where the index corresponds to a slot and the value corresponds to a (counter, - // buffer) pair. "counter" is a unique value that indicates the last time this slot was updated - // or used and allows us to keep track of the least-recently used buffer. - static const constexpr size_t kMaxLayerBufferCount = BufferQueue::NUM_BUFFER_SLOTS + 1; - wp<GraphicBuffer> mBuffers[kMaxLayerBufferCount]; + uint32_t cache(const sp<GraphicBuffer>& buffer); + uint32_t getLeastRecentlyUsedSlot(); + + struct Cache { + sp<GraphicBuffer> buffer; + uint32_t slot; + // Cache entries are evicted according to least-recently-used when more than + // kMaxLayerBufferCount unique buffers have been sent to a layer. + uint64_t lruCounter; + }; + + std::unordered_map<uint64_t, Cache> mCacheByBufferId; + sp<GraphicBuffer> mLastOverrideBuffer; + std::stack<uint32_t> mFreeSlots; + uint64_t mLeastRecentlyUsedCounter; }; } // namespace compositionengine::impl diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h index 9ca5da95bb..1393e29cc2 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h @@ -82,6 +82,7 @@ public: void prepare(const CompositionRefreshArgs&, LayerFESet&) override; void present(const CompositionRefreshArgs&) override; + void uncacheBuffers(const std::vector<uint64_t>& bufferIdsToUncache) override; void rebuildLayerStacks(const CompositionRefreshArgs&, LayerFESet&) override; void collectVisibleLayers(const CompositionRefreshArgs&, compositionengine::Output::CoverageState&) override; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h index 6d4abf9f47..f383392e55 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h @@ -20,6 +20,7 @@ #include <memory> #include <optional> #include <string> +#include <vector> #include <compositionengine/LayerFE.h> #include <compositionengine/OutputLayer.h> @@ -44,6 +45,8 @@ public: void setHwcLayer(std::shared_ptr<HWC2::Layer>) override; + void uncacheBuffers(const std::vector<uint64_t>& bufferIdsToUncache) override; + void updateCompositionState(bool includeGeometry, bool forceClientComposition, ui::Transform::RotationFlags) override; void writeStateToHWC(bool includeGeometry, bool skipLayer, uint32_t z, bool zIsOverridden, diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h index 7592cac582..18e6879bdb 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h @@ -82,6 +82,7 @@ public: MOCK_METHOD2(prepare, void(const compositionengine::CompositionRefreshArgs&, LayerFESet&)); MOCK_METHOD1(present, void(const compositionengine::CompositionRefreshArgs&)); + MOCK_METHOD1(uncacheBuffers, void(const std::vector<uint64_t>&)); MOCK_METHOD2(rebuildLayerStacks, void(const compositionengine::CompositionRefreshArgs&, LayerFESet&)); MOCK_METHOD2(collectVisibleLayers, diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h index c22f1bf260..5fef63a510 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h @@ -35,6 +35,8 @@ public: MOCK_METHOD1(setHwcLayer, void(std::shared_ptr<HWC2::Layer>)); + MOCK_METHOD1(uncacheBuffers, void(const std::vector<uint64_t>&)); + MOCK_CONST_METHOD0(getOutput, const compositionengine::Output&()); MOCK_CONST_METHOD0(getLayerFE, compositionengine::LayerFE&()); diff --git a/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp b/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp index f95382d921..d64fd5707c 100644 --- a/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp +++ b/services/surfaceflinger/CompositionEngine/src/HwcBufferCache.cpp @@ -16,43 +16,80 @@ #include <compositionengine/impl/HwcBufferCache.h> -// TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wconversion" - #include <gui/BufferQueue.h> #include <ui/GraphicBuffer.h> -// TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic pop // ignored "-Wconversion" - namespace android::compositionengine::impl { HwcBufferCache::HwcBufferCache() { - std::fill(std::begin(mBuffers), std::end(mBuffers), wp<GraphicBuffer>(nullptr)); + for (uint32_t i = 0; i < kMaxLayerBufferCount; i++) { + mFreeSlots.push(i); + } } -void HwcBufferCache::getHwcBuffer(int slot, const sp<GraphicBuffer>& buffer, uint32_t* outSlot, - sp<GraphicBuffer>* outBuffer) { - // default is 0 - if (slot == BufferQueue::INVALID_BUFFER_SLOT || slot < 0 || - slot >= static_cast<int32_t>(kMaxLayerBufferCount)) { - *outSlot = 0; - } else { - *outSlot = static_cast<uint32_t>(slot); +HwcSlotAndBuffer HwcBufferCache::getHwcSlotAndBuffer(const sp<GraphicBuffer>& buffer) { + // TODO(b/261930578): This is for unit tests which don't mock GraphicBuffers but instead send + // in nullptrs. + if (buffer == nullptr) { + return {0, nullptr}; + } + if (auto i = mCacheByBufferId.find(buffer->getId()); i != mCacheByBufferId.end()) { + Cache& cache = i->second; + // mark this cache slot as more recently used so it won't get evicted anytime soon + cache.lruCounter = mLeastRecentlyUsedCounter++; + return {cache.slot, nullptr}; } + return {cache(buffer), buffer}; +} - auto& currentBuffer = mBuffers[*outSlot]; - wp<GraphicBuffer> weakCopy(buffer); - if (currentBuffer == weakCopy) { - // already cached in HWC, skip sending the buffer - *outBuffer = nullptr; - } else { - *outBuffer = buffer; +HwcSlotAndBuffer HwcBufferCache::getHwcSlotAndBufferForOverride(const sp<GraphicBuffer>& buffer) { + if (buffer == mLastOverrideBuffer) { + return {kOverrideBufferSlot, nullptr}; + } + mLastOverrideBuffer = buffer; + return {kOverrideBufferSlot, buffer}; +} + +uint32_t HwcBufferCache::uncache(uint64_t bufferId) { + if (auto i = mCacheByBufferId.find(bufferId); i != mCacheByBufferId.end()) { + uint32_t slot = i->second.slot; + mCacheByBufferId.erase(i); + mFreeSlots.push(slot); + return slot; + } + if (mLastOverrideBuffer && bufferId == mLastOverrideBuffer->getId()) { + mLastOverrideBuffer = nullptr; + return kOverrideBufferSlot; + } + return UINT32_MAX; +} + +uint32_t HwcBufferCache::cache(const sp<GraphicBuffer>& buffer) { + Cache cache; + cache.slot = getLeastRecentlyUsedSlot(); + cache.lruCounter = mLeastRecentlyUsedCounter++; + cache.buffer = buffer; + mCacheByBufferId.emplace(buffer->getId(), cache); + return cache.slot; +} - // update cache - currentBuffer = buffer; +uint32_t HwcBufferCache::getLeastRecentlyUsedSlot() { + if (mFreeSlots.empty()) { + assert(!mCacheByBufferId.empty()); + // evict the least recently used cache entry + auto cacheToErase = mCacheByBufferId.begin(); + for (auto i = cacheToErase; i != mCacheByBufferId.end(); ++i) { + if (i->second.lruCounter < cacheToErase->second.lruCounter) { + cacheToErase = i; + } + } + uint32_t slot = cacheToErase->second.slot; + mCacheByBufferId.erase(cacheToErase); + mFreeSlots.push(slot); } + uint32_t slot = mFreeSlots.top(); + mFreeSlots.pop(); + return slot; } } // namespace android::compositionengine::impl diff --git a/services/surfaceflinger/CompositionEngine/src/LayerFECompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/LayerFECompositionState.cpp index 6631a2772c..a405c4df88 100644 --- a/services/surfaceflinger/CompositionEngine/src/LayerFECompositionState.cpp +++ b/services/surfaceflinger/CompositionEngine/src/LayerFECompositionState.cpp @@ -106,7 +106,6 @@ void LayerFECompositionState::dump(std::string& out) const { dumpVal(out, "composition type", toString(compositionType), compositionType); out.append("\n buffer: "); - dumpVal(out, "slot", bufferSlot); dumpVal(out, "buffer", buffer.get()); out.append("\n "); diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 3ee8017d39..16ef812d04 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -428,6 +428,7 @@ void Output::prepare(const compositionengine::CompositionRefreshArgs& refreshArg ALOGV(__FUNCTION__); rebuildLayerStacks(refreshArgs, geomSnapshots); + uncacheBuffers(refreshArgs.bufferIdsToUncache); } void Output::present(const compositionengine::CompositionRefreshArgs& refreshArgs) { @@ -455,6 +456,15 @@ void Output::present(const compositionengine::CompositionRefreshArgs& refreshArg renderCachedSets(refreshArgs); } +void Output::uncacheBuffers(std::vector<uint64_t> const& bufferIdsToUncache) { + if (bufferIdsToUncache.empty()) { + return; + } + for (auto outputLayer : getOutputLayersOrderedByZ()) { + outputLayer->uncacheBuffers(bufferIdsToUncache); + } +} + void Output::rebuildLayerStacks(const compositionengine::CompositionRefreshArgs& refreshArgs, LayerFESet& layerFESet) { ATRACE_CALL(); diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp index a39c527655..a7c24b628d 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -610,6 +610,19 @@ void OutputLayer::writeSidebandStateToHWC(HWC2::Layer* hwcLayer, } } +void OutputLayer::uncacheBuffers(std::vector<uint64_t> const& bufferIdsToUncache) { + auto& state = editState(); + // Skip doing this if there is no HWC interface + if (!state.hwc) { + return; + } + + for (auto bufferId : bufferIdsToUncache) { + state.hwc->hwcBufferCache.uncache(bufferId); + // TODO(b/258196272): send uncache requests to Composer HAL + } +} + void OutputLayer::writeBufferStateToHWC(HWC2::Layer* hwcLayer, const LayerFECompositionState& outputIndependentState, bool skipLayer) { @@ -622,27 +635,24 @@ void OutputLayer::writeBufferStateToHWC(HWC2::Layer* hwcLayer, to_string(error).c_str(), static_cast<int32_t>(error)); } - sp<GraphicBuffer> buffer = outputIndependentState.buffer; - sp<Fence> acquireFence = outputIndependentState.acquireFence; - int slot = outputIndependentState.bufferSlot; + HwcSlotAndBuffer hwcSlotAndBuffer; + sp<Fence> hwcFence; + // Override buffers use a special cache slot so that they don't evict client buffers. if (getState().overrideInfo.buffer != nullptr && !skipLayer) { - buffer = getState().overrideInfo.buffer->getBuffer(); - acquireFence = getState().overrideInfo.acquireFence; - slot = HwcBufferCache::FLATTENER_CACHING_SLOT; + hwcSlotAndBuffer = editState().hwc->hwcBufferCache.getHwcSlotAndBufferForOverride( + getState().overrideInfo.buffer->getBuffer()); + hwcFence = getState().overrideInfo.acquireFence; + } else { + hwcSlotAndBuffer = + editState().hwc->hwcBufferCache.getHwcSlotAndBuffer(outputIndependentState.buffer); + hwcFence = outputIndependentState.acquireFence; } - ALOGV("Writing buffer %p", buffer.get()); - - uint32_t hwcSlot = 0; - sp<GraphicBuffer> hwcBuffer; - // We need access to the output-dependent state for the buffer cache there, - // though otherwise the buffer is not output-dependent. - editState().hwc->hwcBufferCache.getHwcBuffer(slot, buffer, &hwcSlot, &hwcBuffer); - - if (auto error = hwcLayer->setBuffer(hwcSlot, hwcBuffer, acquireFence); + if (auto error = hwcLayer->setBuffer(hwcSlotAndBuffer.slot, hwcSlotAndBuffer.buffer, hwcFence); error != hal::Error::NONE) { - ALOGE("[%s] Failed to set buffer %p: %s (%d)", getLayerFE().getDebugName(), buffer->handle, - to_string(error).c_str(), static_cast<int32_t>(error)); + ALOGE("[%s] Failed to set buffer %p: %s (%d)", getLayerFE().getDebugName(), + hwcSlotAndBuffer.buffer->handle, to_string(error).c_str(), + static_cast<int32_t>(error)); } } diff --git a/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp b/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp index 7197780c17..cf72e8f8de 100644 --- a/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/HwcBufferCacheTest.cpp @@ -22,66 +22,172 @@ namespace android::compositionengine { namespace { -class TestableHwcBufferCache : public impl::HwcBufferCache { -public: - void getHwcBuffer(int slot, const sp<GraphicBuffer>& buffer, uint32_t* outSlot, - sp<GraphicBuffer>* outBuffer) { - HwcBufferCache::getHwcBuffer(slot, buffer, outSlot, outBuffer); - } -}; +using impl::HwcBufferCache; +using impl::HwcSlotAndBuffer; class HwcBufferCacheTest : public testing::Test { public: ~HwcBufferCacheTest() override = default; - void testSlot(const int inSlot, const uint32_t expectedSlot) { - uint32_t outSlot; - sp<GraphicBuffer> outBuffer; - - // The first time, the output is the same as the input - mCache.getHwcBuffer(inSlot, mBuffer1, &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(mBuffer1, outBuffer); - - // The second time with the same buffer, the outBuffer is nullptr. - mCache.getHwcBuffer(inSlot, mBuffer1, &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(nullptr, outBuffer.get()); - - // With a new buffer, the outBuffer is the input. - mCache.getHwcBuffer(inSlot, mBuffer2, &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(mBuffer2, outBuffer); - - // Again, the second request with the same buffer sets outBuffer to nullptr. - mCache.getHwcBuffer(inSlot, mBuffer2, &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(nullptr, outBuffer.get()); - - // Setting a slot to use nullptr lookslike works, but note that - // the output values make it look like no new buffer is being set.... - mCache.getHwcBuffer(inSlot, sp<GraphicBuffer>(), &outSlot, &outBuffer); - EXPECT_EQ(expectedSlot, outSlot); - EXPECT_EQ(nullptr, outBuffer.get()); - } - - impl::HwcBufferCache mCache; sp<GraphicBuffer> mBuffer1 = sp<GraphicBuffer>::make(1u, 1u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u); sp<GraphicBuffer> mBuffer2 = sp<GraphicBuffer>::make(1u, 1u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u); }; -TEST_F(HwcBufferCacheTest, cacheWorksForSlotZero) { - testSlot(0, 0); +TEST_F(HwcBufferCacheTest, getHwcSlotAndBuffer_returnsUniqueSlotNumberForEachBuffer) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + HwcSlotAndBuffer slotAndBufferFor1 = cache.getHwcSlotAndBuffer(mBuffer1); + EXPECT_NE(slotAndBufferFor1.slot, UINT32_MAX); + EXPECT_EQ(slotAndBufferFor1.buffer, mBuffer1); + + HwcSlotAndBuffer slotAndBufferFor2 = cache.getHwcSlotAndBuffer(mBuffer2); + EXPECT_NE(slotAndBufferFor2.slot, slotAndBufferFor1.slot); + EXPECT_NE(slotAndBufferFor2.slot, UINT32_MAX); + EXPECT_EQ(slotAndBufferFor2.buffer, mBuffer2); +} + +TEST_F(HwcBufferCacheTest, getHwcSlotAndBuffer_whenCached_returnsSameSlotNumberAndNullBuffer) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + HwcSlotAndBuffer originalSlotAndBuffer = cache.getHwcSlotAndBuffer(mBuffer1); + EXPECT_NE(originalSlotAndBuffer.slot, UINT32_MAX); + EXPECT_EQ(originalSlotAndBuffer.buffer, mBuffer1); + + HwcSlotAndBuffer finalSlotAndBuffer = cache.getHwcSlotAndBuffer(mBuffer1); + EXPECT_EQ(finalSlotAndBuffer.slot, originalSlotAndBuffer.slot); + EXPECT_EQ(finalSlotAndBuffer.buffer, nullptr); +} + +TEST_F(HwcBufferCacheTest, getHwcSlotAndBuffer_whenSlotsFull_evictsOldestCachedBuffer) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + sp<GraphicBuffer> graphicBuffers[100]; + HwcSlotAndBuffer slotsAndBuffers[100]; + int finalCachedBufferIndex = 0; + for (int i = 0; i < 100; ++i) { + graphicBuffers[i] = sp<GraphicBuffer>::make(1u, 1u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u); + slotsAndBuffers[i] = cache.getHwcSlotAndBuffer(graphicBuffers[i]); + // we fill up the cache when the slot number for the first buffer is reused + if (i > 0 && slotsAndBuffers[i].slot == slotsAndBuffers[0].slot) { + finalCachedBufferIndex = i; + break; + } + } + ASSERT_GT(finalCachedBufferIndex, 1); + // the final cached buffer has the same slot value as the oldest buffer + EXPECT_EQ(slotsAndBuffers[finalCachedBufferIndex].slot, slotsAndBuffers[0].slot); + // the oldest buffer is no longer in the cache because it was evicted + EXPECT_EQ(cache.uncache(graphicBuffers[0]->getId()), UINT32_MAX); +} + +TEST_F(HwcBufferCacheTest, uncache_whenCached_returnsSlotNumber) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + HwcSlotAndBuffer slotAndBufferFor1 = cache.getHwcSlotAndBuffer(mBuffer1); + ASSERT_NE(slotAndBufferFor1.slot, UINT32_MAX); + + HwcSlotAndBuffer slotAndBufferFor2 = cache.getHwcSlotAndBuffer(mBuffer2); + ASSERT_NE(slotAndBufferFor2.slot, UINT32_MAX); + + // the 1st buffer should be found in the cache with a slot number + EXPECT_EQ(cache.uncache(mBuffer1->getId()), slotAndBufferFor1.slot); + // since the 1st buffer has been previously uncached, we should no longer receive a slot number + EXPECT_EQ(cache.uncache(mBuffer1->getId()), UINT32_MAX); + // the 2nd buffer should be still found in the cache with a slot number + EXPECT_EQ(cache.uncache(mBuffer2->getId()), slotAndBufferFor2.slot); + // since the 2nd buffer has been previously uncached, we should no longer receive a slot number + EXPECT_EQ(cache.uncache(mBuffer2->getId()), UINT32_MAX); } -TEST_F(HwcBufferCacheTest, cacheWorksForMaxSlot) { - testSlot(BufferQueue::NUM_BUFFER_SLOTS - 1, BufferQueue::NUM_BUFFER_SLOTS - 1); +TEST_F(HwcBufferCacheTest, uncache_whenUncached_returnsInvalidSlotNumber) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + HwcSlotAndBuffer slotAndBufferFor1 = cache.getHwcSlotAndBuffer(mBuffer1); + ASSERT_NE(slotAndBufferFor1.slot, UINT32_MAX); + + EXPECT_EQ(cache.uncache(mBuffer2->getId()), UINT32_MAX); +} + +TEST_F(HwcBufferCacheTest, getHwcSlotAndBufferForOverride_whenCached_returnsSameSlotAndNullBuffer) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + HwcSlotAndBuffer originalSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(mBuffer1); + EXPECT_NE(originalSlotAndBuffer.slot, UINT32_MAX); + EXPECT_EQ(originalSlotAndBuffer.buffer, mBuffer1); + + HwcSlotAndBuffer finalSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(mBuffer1); + EXPECT_EQ(finalSlotAndBuffer.slot, originalSlotAndBuffer.slot); + EXPECT_EQ(finalSlotAndBuffer.buffer, nullptr); +} + +TEST_F(HwcBufferCacheTest, getHwcSlotAndBufferForOverride_whenSlotsFull_returnsIndependentSlot) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + sp<GraphicBuffer> graphicBuffers[100]; + HwcSlotAndBuffer slotsAndBuffers[100]; + int finalCachedBufferIndex = -1; + for (int i = 0; i < 100; ++i) { + graphicBuffers[i] = sp<GraphicBuffer>::make(1u, 1u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u); + slotsAndBuffers[i] = cache.getHwcSlotAndBuffer(graphicBuffers[i]); + // we fill up the cache when the slot number for the first buffer is reused + if (i > 0 && slotsAndBuffers[i].slot == slotsAndBuffers[0].slot) { + finalCachedBufferIndex = i; + break; + } + } + // expect to have cached at least a few buffers before evicting + ASSERT_GT(finalCachedBufferIndex, 1); + + sp<GraphicBuffer> overrideBuffer = + sp<GraphicBuffer>::make(1u, 1u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u); + HwcSlotAndBuffer overrideSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(overrideBuffer); + // expect us to have a slot number + EXPECT_NE(overrideSlotAndBuffer.slot, UINT32_MAX); + // expect this to be the first time we cached the buffer + EXPECT_NE(overrideSlotAndBuffer.buffer, nullptr); + + // expect the slot number to not equal any other slot number, even after the slots have been + // exhausted, indicating that the override buffer slot is independent from the slots for + // non-override buffers + for (int i = 0; i < finalCachedBufferIndex; ++i) { + EXPECT_NE(overrideSlotAndBuffer.slot, slotsAndBuffers[i].slot); + } + // the override buffer is independently uncached from the oldest cached buffer + // expect to find the override buffer still in the override buffer slot + EXPECT_EQ(cache.uncache(overrideBuffer->getId()), overrideSlotAndBuffer.slot); + // expect that the first buffer was not evicted from the cache when the override buffer was + // cached + EXPECT_EQ(cache.uncache(graphicBuffers[1]->getId()), slotsAndBuffers[1].slot); +} + +TEST_F(HwcBufferCacheTest, uncache_whenOverrideCached_returnsSlotNumber) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + HwcSlotAndBuffer hwcSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(mBuffer1); + ASSERT_NE(hwcSlotAndBuffer.slot, UINT32_MAX); + + EXPECT_EQ(cache.uncache(mBuffer1->getId()), hwcSlotAndBuffer.slot); + EXPECT_EQ(cache.uncache(mBuffer1->getId()), UINT32_MAX); } -TEST_F(HwcBufferCacheTest, cacheMapsNegativeSlotToZero) { - testSlot(-123, 0); +TEST_F(HwcBufferCacheTest, uncache_whenOverrideUncached_returnsInvalidSlotNumber) { + HwcBufferCache cache; + sp<GraphicBuffer> outBuffer; + + HwcSlotAndBuffer hwcSlotAndBuffer = cache.getHwcSlotAndBufferForOverride(mBuffer1); + ASSERT_NE(hwcSlotAndBuffer.slot, UINT32_MAX); + + EXPECT_EQ(cache.uncache(mBuffer2->getId()), UINT32_MAX); } } // namespace diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp index 0f7dce896c..03cf2927af 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp @@ -774,7 +774,7 @@ struct OutputLayerWriteStateToHWCTest : public OutputLayerTest { static constexpr ui::Dataspace kOverrideDataspace = static_cast<ui::Dataspace>(72); static constexpr int kSupportedPerFrameMetadata = 101; static constexpr int kExpectedHwcSlot = 0; - static constexpr int kOverrideHwcSlot = impl::HwcBufferCache::FLATTENER_CACHING_SLOT; + static constexpr int kOverrideHwcSlot = impl::HwcBufferCache::kOverrideBufferSlot; static constexpr bool kLayerGenericMetadata1Mandatory = true; static constexpr bool kLayerGenericMetadata2Mandatory = true; static constexpr float kWhitePointNits = 200.f; @@ -823,7 +823,6 @@ struct OutputLayerWriteStateToHWCTest : public OutputLayerTest { mLayerFEState.hdrMetadata = kHdrMetadata; mLayerFEState.sidebandStream = NativeHandle::create(kSidebandStreamHandle, false); mLayerFEState.buffer = kBuffer; - mLayerFEState.bufferSlot = BufferQueue::INVALID_BUFFER_SLOT; mLayerFEState.acquireFence = kFence; mOutputState.displayBrightnessNits = kDisplayBrightnessNits; diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index 21099876f4..bfd863b88a 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -1192,14 +1192,49 @@ struct OutputPrepareTest : public testing::Test { compositionengine::LayerFESet&)); }; + OutputPrepareTest() { + EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(2u)); + EXPECT_CALL(mOutput, getOutputLayerOrderedByZByIndex(0)) + .WillRepeatedly(Return(&mLayer1.outputLayer)); + EXPECT_CALL(mOutput, getOutputLayerOrderedByZByIndex(1)) + .WillRepeatedly(Return(&mLayer2.outputLayer)); + + mRefreshArgs.layers.push_back(mLayer1.layerFE); + mRefreshArgs.layers.push_back(mLayer2.layerFE); + } + + struct Layer { + StrictMock<mock::OutputLayer> outputLayer; + sp<StrictMock<mock::LayerFE>> layerFE = sp<StrictMock<mock::LayerFE>>::make(); + }; + StrictMock<OutputPartialMock> mOutput; CompositionRefreshArgs mRefreshArgs; LayerFESet mGeomSnapshots; + Layer mLayer1; + Layer mLayer2; }; -TEST_F(OutputPrepareTest, justInvokesRebuildLayerStacks) { +TEST_F(OutputPrepareTest, callsUncacheBuffersOnEachOutputLayerAndThenRebuildsLayerStacks) { InSequence seq; + + mRefreshArgs.bufferIdsToUncache = {1, 3, 5}; + + EXPECT_CALL(mOutput, rebuildLayerStacks(Ref(mRefreshArgs), Ref(mGeomSnapshots))); + EXPECT_CALL(mLayer1.outputLayer, uncacheBuffers(Ref(mRefreshArgs.bufferIdsToUncache))); + EXPECT_CALL(mLayer2.outputLayer, uncacheBuffers(Ref(mRefreshArgs.bufferIdsToUncache))); + + mOutput.prepare(mRefreshArgs, mGeomSnapshots); +} + +TEST_F(OutputPrepareTest, skipsUncacheBuffersIfEmptyAndThenRebuildsLayerStacks) { + InSequence seq; + + mRefreshArgs.bufferIdsToUncache = {}; + EXPECT_CALL(mOutput, rebuildLayerStacks(Ref(mRefreshArgs), Ref(mGeomSnapshots))); + EXPECT_CALL(mLayer1.outputLayer, uncacheBuffers(_)).Times(0); + EXPECT_CALL(mLayer2.outputLayer, uncacheBuffers(_)).Times(0); mOutput.prepare(mRefreshArgs, mGeomSnapshots); } diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index eb14933a61..ce602a8ad9 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -72,6 +72,10 @@ FramebufferSurface::FramebufferSurface(HWComposer& hwc, PhysicalDisplayId displa mConsumer->setDefaultBufferSize(limitedSize.width, limitedSize.height); mConsumer->setMaxAcquiredBufferCount( SurfaceFlinger::maxFrameBufferAcquiredBuffers - 1); + + for (size_t i = 0; i < sizeof(mHwcBufferIds) / sizeof(mHwcBufferIds[0]); ++i) { + mHwcBufferIds[i] = UINT64_MAX; + } } void FramebufferSurface::resizeBuffers(const ui::Size& newSize) { @@ -88,31 +92,16 @@ status_t FramebufferSurface::prepareFrame(CompositionType /*compositionType*/) { } status_t FramebufferSurface::advanceFrame() { - uint32_t slot = 0; - sp<GraphicBuffer> buf; - sp<Fence> acquireFence(Fence::NO_FENCE); - Dataspace dataspace = Dataspace::UNKNOWN; - status_t result = nextBuffer(slot, buf, acquireFence, dataspace); - mDataSpace = dataspace; - if (result != NO_ERROR) { - ALOGE("error latching next FramebufferSurface buffer: %s (%d)", - strerror(-result), result); - } - return result; -} - -status_t FramebufferSurface::nextBuffer(uint32_t& outSlot, - sp<GraphicBuffer>& outBuffer, sp<Fence>& outFence, - Dataspace& outDataspace) { Mutex::Autolock lock(mMutex); BufferItem item; status_t err = acquireBufferLocked(&item, 0); if (err == BufferQueue::NO_BUFFER_AVAILABLE) { - mHwcBufferCache.getHwcBuffer(mCurrentBufferSlot, mCurrentBuffer, &outSlot, &outBuffer); + mDataspace = Dataspace::UNKNOWN; return NO_ERROR; } else if (err != NO_ERROR) { ALOGE("error acquiring buffer: %s (%d)", strerror(-err), err); + mDataspace = Dataspace::UNKNOWN; return err; } @@ -133,13 +122,18 @@ status_t FramebufferSurface::nextBuffer(uint32_t& outSlot, mCurrentBufferSlot = item.mSlot; mCurrentBuffer = mSlots[mCurrentBufferSlot].mGraphicBuffer; mCurrentFence = item.mFence; + mDataspace = static_cast<Dataspace>(item.mDataSpace); - outFence = item.mFence; - mHwcBufferCache.getHwcBuffer(mCurrentBufferSlot, mCurrentBuffer, &outSlot, &outBuffer); - outDataspace = static_cast<Dataspace>(item.mDataSpace); - status_t result = mHwc.setClientTarget(mDisplayId, outSlot, outFence, outBuffer, outDataspace); + // assume HWC has previously seen the buffer in this slot + sp<GraphicBuffer> hwcBuffer = sp<GraphicBuffer>(nullptr); + if (mCurrentBuffer->getId() != mHwcBufferIds[mCurrentBufferSlot]) { + mHwcBufferIds[mCurrentBufferSlot] = mCurrentBuffer->getId(); + hwcBuffer = mCurrentBuffer; // HWC hasn't previously seen this buffer in this slot + } + status_t result = mHwc.setClientTarget(mDisplayId, mCurrentBufferSlot, mCurrentFence, hwcBuffer, + mDataspace); if (result != NO_ERROR) { - ALOGE("error posting framebuffer: %d", result); + ALOGE("error posting framebuffer: %s (%d)", strerror(-result), result); return result; } @@ -190,7 +184,7 @@ ui::Size FramebufferSurface::limitSizeInternal(const ui::Size& size, const ui::S limitedSize.width = maxSize.height * aspectRatio; wasLimited = true; } - ALOGI_IF(wasLimited, "framebuffer size has been limited to [%dx%d] from [%dx%d]", + ALOGI_IF(wasLimited, "Framebuffer size has been limited to [%dx%d] from [%dx%d]", limitedSize.width, limitedSize.height, size.width, size.height); return limitedSize; } @@ -198,9 +192,9 @@ ui::Size FramebufferSurface::limitSizeInternal(const ui::Size& size, const ui::S void FramebufferSurface::dumpAsString(String8& result) const { Mutex::Autolock lock(mMutex); result.append(" FramebufferSurface\n"); - result.appendFormat(" mDataSpace=%s (%d)\n", - dataspaceDetails(static_cast<android_dataspace>(mDataSpace)).c_str(), - mDataSpace); + result.appendFormat(" mDataspace=%s (%d)\n", + dataspaceDetails(static_cast<android_dataspace>(mDataspace)).c_str(), + mDataspace); ConsumerBase::dumpLocked(result, " "); } diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h index d41a856e68..0b863daf47 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h @@ -21,7 +21,7 @@ #include <sys/types.h> #include <compositionengine/DisplaySurface.h> -#include <compositionengine/impl/HwcBufferCache.h> +#include <gui/BufferQueue.h> #include <gui/ConsumerBase.h> #include <ui/DisplayId.h> #include <ui/Size.h> @@ -69,12 +69,6 @@ private: virtual void dumpLocked(String8& result, const char* prefix) const; - // nextBuffer waits for and then latches the next buffer from the - // BufferQueue and releases the previously latched buffer to the - // BufferQueue. The new buffer is returned in the 'buffer' argument. - status_t nextBuffer(uint32_t& outSlot, sp<GraphicBuffer>& outBuffer, - sp<Fence>& outFence, ui::Dataspace& outDataspace); - const PhysicalDisplayId mDisplayId; // Framebuffer size has a dimension limitation in pixels based on the graphics capabilities of @@ -91,7 +85,7 @@ private: // compositing. Otherwise it will display the dataspace of the buffer // use for compositing which can change as wide-color content is // on/off. - ui::Dataspace mDataSpace; + ui::Dataspace mDataspace; // mCurrentBuffer is the current buffer or nullptr to indicate that there is // no current buffer. @@ -103,7 +97,9 @@ private: // Hardware composer, owned by SurfaceFlinger. HWComposer& mHwc; - compositionengine::impl::HwcBufferCache mHwcBufferCache; + // Buffers that HWC has seen before, indexed by slot number. + // NOTE: The BufferQueue slot number is the same as the HWC slot number. + uint64_t mHwcBufferIds[BufferQueue::NUM_BUFFER_SLOTS]; // Previous buffer to release after getting an updated retire fence bool mHasPendingRelease; diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index 3803a78670..d62075ec65 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -103,6 +103,10 @@ VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, VirtualDisplayId d sink->setAsyncMode(true); IGraphicBufferProducer::QueueBufferOutput output; mSource[SOURCE_SCRATCH]->connect(nullptr, NATIVE_WINDOW_API_EGL, false, &output); + + for (size_t i = 0; i < sizeof(mHwcBufferIds) / sizeof(mHwcBufferIds[0]); ++i) { + mHwcBufferIds[i] = UINT64_MAX; + } } VirtualDisplaySurface::~VirtualDisplaySurface() { @@ -197,9 +201,9 @@ status_t VirtualDisplaySurface::advanceFrame() { return NO_MEMORY; } - sp<GraphicBuffer> fbBuffer = mFbProducerSlot >= 0 ? - mProducerBuffers[mFbProducerSlot] : sp<GraphicBuffer>(nullptr); - sp<GraphicBuffer> outBuffer = mProducerBuffers[mOutputProducerSlot]; + sp<GraphicBuffer> const& fbBuffer = + mFbProducerSlot >= 0 ? mProducerBuffers[mFbProducerSlot] : sp<GraphicBuffer>(nullptr); + sp<GraphicBuffer> const& outBuffer = mProducerBuffers[mOutputProducerSlot]; VDS_LOGV("%s: fb=%d(%p) out=%d(%p)", __func__, mFbProducerSlot, fbBuffer.get(), mOutputProducerSlot, outBuffer.get()); @@ -211,12 +215,14 @@ status_t VirtualDisplaySurface::advanceFrame() { status_t result = NO_ERROR; if (fbBuffer != nullptr) { - uint32_t hwcSlot = 0; - sp<GraphicBuffer> hwcBuffer; - mHwcBufferCache.getHwcBuffer(mFbProducerSlot, fbBuffer, &hwcSlot, &hwcBuffer); - + // assume that HWC has previously seen the buffer in this slot + sp<GraphicBuffer> hwcBuffer = sp<GraphicBuffer>(nullptr); + if (fbBuffer->getId() != mHwcBufferIds[mFbProducerSlot]) { + mHwcBufferIds[mFbProducerSlot] = fbBuffer->getId(); + hwcBuffer = fbBuffer; // HWC hasn't previously seen this buffer in this slot + } // TODO: Correctly propagate the dataspace from GL composition - result = mHwc.setClientTarget(*halDisplayId, hwcSlot, mFbFence, hwcBuffer, + result = mHwc.setClientTarget(*halDisplayId, mFbProducerSlot, mFbFence, hwcBuffer, ui::Dataspace::UNKNOWN); } diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index e21095aa88..be06e2bb10 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -20,7 +20,7 @@ #include <string> #include <compositionengine/DisplaySurface.h> -#include <compositionengine/impl/HwcBufferCache.h> +#include <gui/BufferQueue.h> #include <gui/ConsumerBase.h> #include <gui/IGraphicBufferProducer.h> #include <ui/DisplayId.h> @@ -164,6 +164,10 @@ private: sp<IGraphicBufferProducer> mSource[2]; // indexed by SOURCE_* uint32_t mDefaultOutputFormat; + // Buffers that HWC has seen before, indexed by HWC slot number. + // NOTE: The BufferQueue slot number is the same as the HWC slot number. + uint64_t mHwcBufferIds[BufferQueue::NUM_BUFFER_SLOTS]; + // // Inter-frame state // @@ -260,8 +264,6 @@ private: bool mMustRecompose = false; - compositionengine::impl::HwcBufferCache mHwcBufferCache; - bool mForceHwcCopy; }; diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp index 45058d900f..9a3d750670 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp @@ -190,7 +190,6 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta if (clientState.what & layer_state_t::eBufferChanged) { externalTexture = resolvedComposerState.externalTexture; - hwcBufferSlot = resolvedComposerState.hwcBufferSlot; } if (clientState.what & layer_state_t::ePositionChanged) { diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h index 0ddf5e2895..7e5a79fe00 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h @@ -86,7 +86,6 @@ struct RequestedLayerState : layer_state_t { ui::Transform requestedTransform; std::shared_ptr<FenceTime> acquireFenceTime; std::shared_ptr<renderengine::ExternalTexture> externalTexture; - int hwcBufferSlot = 0; // book keeping states bool handleAlive = true; diff --git a/services/surfaceflinger/HwcSlotGenerator.cpp b/services/surfaceflinger/HwcSlotGenerator.cpp deleted file mode 100644 index 939c35b8b3..0000000000 --- a/services/surfaceflinger/HwcSlotGenerator.cpp +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#undef LOG_TAG -#define LOG_TAG "HwcSlotGenerator" -#define ATRACE_TAG ATRACE_TAG_GRAPHICS - -#include <gui/BufferQueue.h> - -#include "HwcSlotGenerator.h" - -namespace android { - -HwcSlotGenerator::HwcSlotGenerator() { - for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - mFreeHwcCacheSlots.push(i); - } -} - -void HwcSlotGenerator::bufferErased(const client_cache_t& clientCacheId) { - std::lock_guard lock(mMutex); - if (!clientCacheId.isValid()) { - ALOGE("invalid process, failed to erase buffer"); - return; - } - eraseBufferLocked(clientCacheId); -} - -int HwcSlotGenerator::getHwcCacheSlot(const client_cache_t& clientCacheId) { - std::lock_guard<std::mutex> lock(mMutex); - auto itr = mCachedBuffers.find(clientCacheId); - if (itr == mCachedBuffers.end()) { - return addCachedBuffer(clientCacheId); - } - auto& [hwcCacheSlot, counter] = itr->second; - counter = mCounter++; - return hwcCacheSlot; -} - -int HwcSlotGenerator::addCachedBuffer(const client_cache_t& clientCacheId) REQUIRES(mMutex) { - if (!clientCacheId.isValid()) { - ALOGE("invalid process, returning invalid slot"); - return BufferQueue::INVALID_BUFFER_SLOT; - } - - ClientCache::getInstance().registerErasedRecipient(clientCacheId, - wp<ErasedRecipient>::fromExisting(this)); - - int hwcCacheSlot = getFreeHwcCacheSlot(); - mCachedBuffers[clientCacheId] = {hwcCacheSlot, mCounter++}; - return hwcCacheSlot; -} - -int HwcSlotGenerator::getFreeHwcCacheSlot() REQUIRES(mMutex) { - if (mFreeHwcCacheSlots.empty()) { - evictLeastRecentlyUsed(); - } - - int hwcCacheSlot = mFreeHwcCacheSlots.top(); - mFreeHwcCacheSlots.pop(); - return hwcCacheSlot; -} - -void HwcSlotGenerator::evictLeastRecentlyUsed() REQUIRES(mMutex) { - uint64_t minCounter = UINT_MAX; - client_cache_t minClientCacheId = {}; - for (const auto& [clientCacheId, slotCounter] : mCachedBuffers) { - const auto& [hwcCacheSlot, counter] = slotCounter; - if (counter < minCounter) { - minCounter = counter; - minClientCacheId = clientCacheId; - } - } - eraseBufferLocked(minClientCacheId); - - ClientCache::getInstance().unregisterErasedRecipient(minClientCacheId, - wp<ErasedRecipient>::fromExisting(this)); -} - -void HwcSlotGenerator::eraseBufferLocked(const client_cache_t& clientCacheId) REQUIRES(mMutex) { - auto itr = mCachedBuffers.find(clientCacheId); - if (itr == mCachedBuffers.end()) { - return; - } - auto& [hwcCacheSlot, counter] = itr->second; - - // TODO send to hwc cache and resources - - mFreeHwcCacheSlots.push(hwcCacheSlot); - mCachedBuffers.erase(clientCacheId); -} - -} // namespace android diff --git a/services/surfaceflinger/HwcSlotGenerator.h b/services/surfaceflinger/HwcSlotGenerator.h deleted file mode 100644 index 5a1b6d7d9d..0000000000 --- a/services/surfaceflinger/HwcSlotGenerator.h +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include <functional> -#include <mutex> -#include <stack> -#include <unordered_map> - -#include "ClientCache.h" - -namespace android { - -class HwcSlotGenerator : public ClientCache::ErasedRecipient { -public: - HwcSlotGenerator(); - void bufferErased(const client_cache_t& clientCacheId); - int getHwcCacheSlot(const client_cache_t& clientCacheId); - -private: - friend class SlotGenerationTest; - int addCachedBuffer(const client_cache_t& clientCacheId) REQUIRES(mMutex); - int getFreeHwcCacheSlot() REQUIRES(mMutex); - void evictLeastRecentlyUsed() REQUIRES(mMutex); - void eraseBufferLocked(const client_cache_t& clientCacheId) REQUIRES(mMutex); - - struct CachedBufferHash { - std::size_t operator()(const client_cache_t& clientCacheId) const { - return std::hash<uint64_t>{}(clientCacheId.id); - } - }; - - std::mutex mMutex; - - std::unordered_map<client_cache_t, std::pair<int /*HwcCacheSlot*/, uint64_t /*counter*/>, - CachedBufferHash> - mCachedBuffers GUARDED_BY(mMutex); - std::stack<int /*HwcCacheSlot*/> mFreeHwcCacheSlots GUARDED_BY(mMutex); - - // The cache increments this counter value when a slot is updated or used. - // Used to track the least recently-used buffer - uint64_t mCounter = 0; -}; -} // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 0017af0476..b0ee850541 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -144,7 +144,6 @@ Layer::Layer(const LayerCreationArgs& args) mLayerCreationFlags(args.flags), mBorderEnabled(false), mTextureName(args.textureName), - mHwcSlotGenerator(sp<HwcSlotGenerator>::make()), mLayerFE(args.flinger->getFactory().createLayerFE(mName)) { ALOGV("Creating Layer %s", getDebugName()); @@ -573,9 +572,6 @@ void Layer::preparePerFrameBufferCompositionState() { } snapshot->buffer = getBuffer(); - snapshot->bufferSlot = (mBufferInfo.mBufferSlot == BufferQueue::INVALID_BUFFER_SLOT) - ? 0 - : mBufferInfo.mBufferSlot; snapshot->acquireFence = mBufferInfo.mFence; snapshot->frameNumber = mBufferInfo.mFrameNumber; snapshot->sidebandStreamHasFrame = false; @@ -2840,7 +2836,7 @@ bool Layer::setPosition(float x, float y) { bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, const BufferData& bufferData, nsecs_t postTime, nsecs_t desiredPresentTime, bool isAutoTimestamp, std::optional<nsecs_t> dequeueTime, - const FrameTimelineInfo& info, int hwcBufferSlot) { + const FrameTimelineInfo& info) { ATRACE_FORMAT("setBuffer %s - hasBuffer=%s", getDebugName(), (buffer ? "true" : "false")); if (!buffer) { return false; @@ -2886,7 +2882,6 @@ bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, mDrawingState.releaseBufferListener = bufferData.releaseBufferListener; mDrawingState.buffer = std::move(buffer); mDrawingState.clientCacheId = bufferData.cachedBuffer; - mDrawingState.hwcBufferSlot = hwcBufferSlot; mDrawingState.acquireFence = bufferData.flags.test(BufferData::BufferDataChange::fenceChanged) ? bufferData.acquireFence : Fence::NO_FENCE; @@ -3185,7 +3180,6 @@ void Layer::gatherBufferInfo() { mBufferInfo.mHdrMetadata = mDrawingState.hdrMetadata; mBufferInfo.mApi = mDrawingState.api; mBufferInfo.mTransformToDisplayInverse = mDrawingState.transformToDisplayInverse; - mBufferInfo.mBufferSlot = mDrawingState.hwcBufferSlot; } Rect Layer::computeBufferCrop(const State& s) { @@ -3968,10 +3962,6 @@ void Layer::updateRelativeMetadataSnapshot(const LayerMetadata& relativeLayerMet } } -int Layer::getHwcCacheSlot(const client_cache_t& clientCacheId) { - return mHwcSlotGenerator->getHwcCacheSlot(clientCacheId); -} - LayerSnapshotGuard::LayerSnapshotGuard(Layer* layer) : mLayer(layer) { if (mLayer) { mLayer->mLayerFE->mSnapshot = std::move(mLayer->mSnapshot); diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index f743896c03..866339b3ce 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -51,7 +51,6 @@ #include "Client.h" #include "DisplayHardware/HWComposer.h" #include "FrameTracker.h" -#include "HwcSlotGenerator.h" #include "LayerFE.h" #include "LayerVector.h" #include "Scheduler/LayerInfo.h" @@ -146,7 +145,6 @@ public: bool transformToDisplayInverse; Region transparentRegionHint; std::shared_ptr<renderengine::ExternalTexture> buffer; - int hwcBufferSlot; client_cache_t clientCacheId; sp<Fence> acquireFence; std::shared_ptr<FenceTime> acquireFenceTime; @@ -298,8 +296,7 @@ public: bool setBuffer(std::shared_ptr<renderengine::ExternalTexture>& /* buffer */, const BufferData& /* bufferData */, nsecs_t /* postTime */, nsecs_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/, - std::optional<nsecs_t> /* dequeueTime */, const FrameTimelineInfo& /*info*/, - int /* hwcBufferSlot */); + std::optional<nsecs_t> /* dequeueTime */, const FrameTimelineInfo& /*info*/); bool setDataspace(ui::Dataspace /*dataspace*/); bool setHdrMetadata(const HdrMetadata& /*hdrMetadata*/); bool setSurfaceDamageRegion(const Region& /*surfaceDamage*/); @@ -500,7 +497,6 @@ public: std::shared_ptr<renderengine::ExternalTexture> mBuffer; uint64_t mFrameNumber; - int mBufferSlot{BufferQueue::INVALID_BUFFER_SLOT}; bool mFrameLatencyNeeded{false}; }; @@ -813,7 +809,6 @@ public: void updateMetadataSnapshot(const LayerMetadata& parentMetadata); void updateRelativeMetadataSnapshot(const LayerMetadata& relativeLayerMetadata, std::unordered_set<Layer*>& visited); - int getHwcCacheSlot(const client_cache_t& clientCacheId); protected: // For unit tests @@ -1124,8 +1119,6 @@ private: // not specify a destination frame. ui::Transform mRequestedTransform; - sp<HwcSlotGenerator> mHwcSlotGenerator; - sp<LayerFE> mLayerFE; std::unique_ptr<LayerSnapshot> mSnapshot = std::make_unique<LayerSnapshot>(); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index bd3c0f12cc..a7b59a16ed 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -112,6 +112,7 @@ #include <ui/DisplayIdentification.h> #include "BackgroundExecutor.h" #include "Client.h" +#include "ClientCache.h" #include "Colorizer.h" #include "Display/DisplayMap.h" #include "DisplayDevice.h" @@ -2255,6 +2256,8 @@ void SurfaceFlinger::composite(TimePoint frameTime, VsyncId vsyncId) }); } + refreshArgs.bufferIdsToUncache = std::move(mBufferIdsToUncache); + refreshArgs.layersWithQueuedFrames.reserve(mLayersWithQueuedFrames.size()); for (auto layer : mLayersWithQueuedFrames) { if (auto layerFE = layer->getCompositionEngineLayerFE()) @@ -3999,10 +4002,6 @@ status_t SurfaceFlinger::setTransactionState( getExternalTextureFromBufferData(*resolvedState.state.bufferData, layerName.c_str(), transactionId); mBufferCountTracker.increment(resolvedState.state.surface->localBinder()); - if (layer) { - resolvedState.hwcBufferSlot = - layer->getHwcCacheSlot(resolvedState.state.bufferData->cachedBuffer); - } } } @@ -4078,7 +4077,10 @@ bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin } if (uncacheBuffer.isValid()) { - ClientCache::getInstance().erase(uncacheBuffer); + sp<GraphicBuffer> buffer = ClientCache::getInstance().erase(uncacheBuffer); + if (buffer != nullptr) { + mBufferIdsToUncache.push_back(buffer->getId()); + } } // If a synchronous transaction is explicitly requested without any changes, force a transaction @@ -4469,7 +4471,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime if (what & layer_state_t::eBufferChanged) { if (layer->setBuffer(composerState.externalTexture, *s.bufferData, postTime, desiredPresentTime, isAutoTimestamp, dequeueBufferTimestamp, - frameTimelineInfo, composerState.hwcBufferSlot)) { + frameTimelineInfo)) { flags |= eTraversalNeeded; } } else if (frameTimelineInfo.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index b9903a7768..e7ecf6c757 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -96,6 +96,7 @@ #include <unordered_map> #include <unordered_set> #include <utility> +#include <vector> #include <aidl/android/hardware/graphics/common/DisplayDecorationSupport.h> #include "Client.h" @@ -1091,6 +1092,10 @@ private: std::atomic<uint32_t> mUniqueTransactionId = 1; SortedVector<sp<Layer>> mLayersPendingRemoval; + // Buffers that have been discarded by clients and need to be evicted from per-layer caches so + // the graphics memory can be immediately freed. + std::vector<uint64_t> mBufferIdsToUncache; + // global color transform states Daltonizer mDaltonizer; float mGlobalSaturationFactor = 1.0f; diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h index 7bde2c1e23..380301fc73 100644 --- a/services/surfaceflinger/TransactionState.h +++ b/services/surfaceflinger/TransactionState.h @@ -27,13 +27,12 @@ namespace android { -// Extends the client side composer state by resolving buffer cache ids. +// Extends the client side composer state by resolving buffer. class ResolvedComposerState : public ComposerState { public: ResolvedComposerState() = default; ResolvedComposerState(ComposerState&& source) { state = std::move(source.state); } std::shared_ptr<renderengine::ExternalTexture> externalTexture; - int hwcBufferSlot = 0; }; struct TransactionState { diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp index c5b3fa67ae..acfc1d4dc8 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp +++ b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp @@ -160,7 +160,7 @@ void LayerFuzzer::invokeBufferStateLayer() { layer->setBuffer(texture, {} /*bufferData*/, mFdp.ConsumeIntegral<nsecs_t>() /*postTime*/, mFdp.ConsumeIntegral<nsecs_t>() /*desiredTime*/, mFdp.ConsumeBool() /*isAutoTimestamp*/, - {mFdp.ConsumeIntegral<nsecs_t>()} /*dequeue*/, {} /*info*/, 0 /* hwcslot */); + {mFdp.ConsumeIntegral<nsecs_t>()} /*dequeue*/, {} /*info*/); LayerRenderArea layerArea(*(flinger.flinger()), layer, getFuzzedRect(), {mFdp.ConsumeIntegral<int32_t>(), diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 8b0cd78732..96db43e59b 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -70,7 +70,6 @@ cc_test { ":libsurfaceflinger_sources", "libsurfaceflinger_unittest_main.cpp", "AidlPowerHalWrapperTest.cpp", - "CachingTest.cpp", "CompositionTest.cpp", "DispSyncSourceTest.cpp", "DisplayIdGeneratorTest.cpp", diff --git a/services/surfaceflinger/tests/unittests/CachingTest.cpp b/services/surfaceflinger/tests/unittests/CachingTest.cpp deleted file mode 100644 index c1cbbfb4ef..0000000000 --- a/services/surfaceflinger/tests/unittests/CachingTest.cpp +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright 2019 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#undef LOG_TAG -#define LOG_TAG "CachingTest" - -#include <gmock/gmock.h> -#include <gtest/gtest.h> -#include <gui/BufferQueue.h> - -#include "HwcSlotGenerator.h" - -namespace android { - -class SlotGenerationTest : public testing::Test { -protected: - sp<HwcSlotGenerator> mHwcSlotGenerator = sp<HwcSlotGenerator>::make(); - sp<GraphicBuffer> mBuffer1 = - sp<GraphicBuffer>::make(1u, 1u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u); - sp<GraphicBuffer> mBuffer2 = - sp<GraphicBuffer>::make(1u, 1u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u); - sp<GraphicBuffer> mBuffer3 = - sp<GraphicBuffer>::make(10u, 10u, HAL_PIXEL_FORMAT_RGBA_8888, 1u, 0u); -}; - -TEST_F(SlotGenerationTest, getHwcCacheSlot_Invalid) { - sp<IBinder> binder = sp<BBinder>::make(); - // test getting invalid client_cache_id - client_cache_t id; - int slot = mHwcSlotGenerator->getHwcCacheSlot(id); - EXPECT_EQ(BufferQueue::INVALID_BUFFER_SLOT, slot); -} - -TEST_F(SlotGenerationTest, getHwcCacheSlot_Basic) { - sp<IBinder> binder = sp<BBinder>::make(); - client_cache_t id; - id.token = binder; - id.id = 0; - int slot = mHwcSlotGenerator->getHwcCacheSlot(id); - EXPECT_EQ(BufferQueue::NUM_BUFFER_SLOTS - 1, slot); - - client_cache_t idB; - idB.token = binder; - idB.id = 1; - slot = mHwcSlotGenerator->getHwcCacheSlot(idB); - EXPECT_EQ(BufferQueue::NUM_BUFFER_SLOTS - 2, slot); - - slot = mHwcSlotGenerator->getHwcCacheSlot(idB); - EXPECT_EQ(BufferQueue::NUM_BUFFER_SLOTS - 2, slot); - - slot = mHwcSlotGenerator->getHwcCacheSlot(id); - EXPECT_EQ(BufferQueue::NUM_BUFFER_SLOTS - 1, slot); -} - -TEST_F(SlotGenerationTest, getHwcCacheSlot_Reuse) { - sp<IBinder> binder = sp<BBinder>::make(); - std::vector<client_cache_t> ids; - uint32_t cacheId = 0; - // fill up cache - for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - client_cache_t id; - id.token = binder; - id.id = cacheId; - ids.push_back(id); - - int slot = mHwcSlotGenerator->getHwcCacheSlot(id); - EXPECT_EQ(BufferQueue::NUM_BUFFER_SLOTS - (i + 1), slot); - cacheId++; - } - for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - int slot = mHwcSlotGenerator->getHwcCacheSlot(ids[static_cast<uint32_t>(i)]); - EXPECT_EQ(BufferQueue::NUM_BUFFER_SLOTS - (i + 1), slot); - } - - for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - client_cache_t id; - id.token = binder; - id.id = cacheId; - int slot = mHwcSlotGenerator->getHwcCacheSlot(id); - EXPECT_EQ(BufferQueue::NUM_BUFFER_SLOTS - (i + 1), slot); - cacheId++; - } -} -} // namespace android diff --git a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp index 09d002f6e8..1173d1c876 100644 --- a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp @@ -126,7 +126,7 @@ public: HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); layer->setBuffer(externalTexture, bufferData, postTime, /*desiredPresentTime*/ 30, false, - dequeueTime, FrameTimelineInfo{}, 0); + dequeueTime, FrameTimelineInfo{}); commitTransaction(layer.get()); nsecs_t latchTime = 25; diff --git a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp index 7dfbcc00d4..ae03db43a7 100644 --- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp @@ -131,7 +131,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo); acquireFence->signalForTest(12); commitTransaction(layer.get()); @@ -166,7 +166,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); const auto droppedSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX; @@ -183,7 +183,7 @@ public: 2ULL /* bufferId */, HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); - layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfo); nsecs_t end = systemTime(); acquireFence2->signalForTest(12); @@ -229,7 +229,7 @@ public: 1ULL /* bufferId */, HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo); acquireFence->signalForTest(12); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); @@ -264,7 +264,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); @@ -307,7 +307,7 @@ public: FrameTimelineInfo ftInfo3; ftInfo3.vsyncId = 3; ftInfo3.inputEventId = 0; - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo3, 0); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo3); EXPECT_EQ(2u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); const auto bufferSurfaceFrameTX = layer->mDrawingState.bufferSurfaceFrameTX; @@ -352,7 +352,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); const auto droppedSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX; @@ -367,7 +367,7 @@ public: 1ULL /* bufferId */, HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); - layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfo); acquireFence2->signalForTest(12); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); @@ -404,7 +404,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture1, bufferData, 10, 20, false, std::nullopt, ftInfo); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); const auto droppedSurfaceFrame1 = layer->mDrawingState.bufferSurfaceFrameTX; @@ -424,7 +424,7 @@ public: FrameTimelineInfo ftInfoInv; ftInfoInv.vsyncId = FrameTimelineInfo::INVALID_VSYNC_ID; ftInfoInv.inputEventId = 0; - layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfoInv, 0); + layer->setBuffer(externalTexture2, bufferData, 10, 20, false, std::nullopt, ftInfoInv); auto dropEndTime1 = systemTime(); EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size()); ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX); @@ -445,7 +445,7 @@ public: FrameTimelineInfo ftInfo2; ftInfo2.vsyncId = 2; ftInfo2.inputEventId = 0; - layer->setBuffer(externalTexture3, bufferData, 10, 20, false, std::nullopt, ftInfo2, 0); + layer->setBuffer(externalTexture3, bufferData, 10, 20, false, std::nullopt, ftInfo2); auto dropEndTime2 = systemTime(); acquireFence3->signalForTest(12); @@ -494,7 +494,7 @@ public: FrameTimelineInfo ftInfo; ftInfo.vsyncId = 1; ftInfo.inputEventId = 0; - layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo, 0); + layer->setBuffer(externalTexture, bufferData, 10, 20, false, std::nullopt, ftInfo); FrameTimelineInfo ftInfo2; ftInfo2.vsyncId = 2; ftInfo2.inputEventId = 0; |