Merge "SF: Clean up HWC2::Layer ownership" into sc-dev am: e9f6e108e8
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/native/+/14471627
Change-Id: Ibc051d373f10147e6b8750a7ce9c0f2ef9db9ff2
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index 8c8331c..953eb76 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -192,16 +192,7 @@
if (const auto halDisplayId = HalDisplayId::tryCast(mId);
outputLayer && !mIsDisconnected && halDisplayId) {
auto& hwc = getCompositionEngine().getHwComposer();
- // Note: For the moment we ensure it is safe to take a reference to the
- // HWComposer implementation by destroying all the OutputLayers (and
- // hence the HWC2::Layers they own) before setting a new HWComposer. See
- // for example SurfaceFlinger::updateVrFlinger().
- // TODO(b/121291683): Make this safer.
- auto hwcLayer =
- std::shared_ptr<HWC2::Layer>(hwc.createLayer(*halDisplayId),
- [&hwc, id = *halDisplayId](HWC2::Layer* layer) {
- hwc.destroyLayer(id, layer);
- });
+ auto hwcLayer = hwc.createLayer(*halDisplayId);
ALOGE_IF(!hwcLayer, "Failed to create a HWC layer for a HWC supported display %s",
getName().c_str());
outputLayer->setHwcLayer(std::move(hwcLayer));
diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
index e63d09a..e12cb57 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -533,16 +533,15 @@
TEST_F(DisplayCreateOutputLayerTest, setsHwcLayer) {
sp<mock::LayerFE> layerFE = new StrictMock<mock::LayerFE>();
- StrictMock<HWC2::mock::Layer> hwcLayer;
+ auto hwcLayer = std::make_shared<StrictMock<HWC2::mock::Layer>>();
EXPECT_CALL(mHwComposer, createLayer(HalDisplayId(DEFAULT_DISPLAY_ID)))
- .WillOnce(Return(&hwcLayer));
+ .WillOnce(Return(hwcLayer));
auto outputLayer = mDisplay->createOutputLayer(layerFE);
- EXPECT_EQ(&hwcLayer, outputLayer->getHwcLayer());
+ EXPECT_EQ(hwcLayer.get(), outputLayer->getHwcLayer());
- EXPECT_CALL(mHwComposer, destroyLayer(HalDisplayId(DEFAULT_DISPLAY_ID), &hwcLayer));
outputLayer.reset();
}
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 74d4b92..cd2d09e 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -48,8 +48,7 @@
MOCK_METHOD3(allocateVirtualDisplay,
std::optional<DisplayId>(uint32_t, uint32_t, ui::PixelFormat*));
MOCK_METHOD2(allocatePhysicalDisplay, void(hal::HWDisplayId, PhysicalDisplayId));
- MOCK_METHOD1(createLayer, HWC2::Layer*(HalDisplayId));
- MOCK_METHOD2(destroyLayer, void(HalDisplayId, HWC2::Layer*));
+ MOCK_METHOD1(createLayer, std::shared_ptr<HWC2::Layer>(HalDisplayId));
MOCK_METHOD4(getDeviceCompositionChanges,
status_t(HalDisplayId, bool, std::chrono::steady_clock::time_point,
std::optional<android::HWComposer::DeviceRequestedChanges>*));
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index d04b5f7..27146ab 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -78,7 +78,19 @@
}
Display::~Display() {
- mLayers.clear();
+ // Note: The calls to onOwningDisplayDestroyed() are allowed (and expected)
+ // to call Display::onLayerDestroyed(). As that call removes entries from
+ // mLayers, we do not want to have a for loop directly over it here. Since
+ // the end goal is an empty mLayers anyway, we just go ahead and swap an
+ // initially empty local container with mLayers, and then enumerate
+ // the contents of the local container.
+ Layers destroyingLayers;
+ std::swap(mLayers, destroyingLayers);
+ for (const auto& [_, weakLayer] : destroyingLayers) {
+ if (std::shared_ptr layer = weakLayer.lock()) {
+ layer->onOwningDisplayDestroyed();
+ }
+ }
Error error = Error::NONE;
const char* msg;
@@ -110,29 +122,21 @@
return static_cast<Error>(intError);
}
-Error Display::createLayer(HWC2::Layer** outLayer) {
- if (!outLayer) {
- return Error::BAD_PARAMETER;
- }
+base::expected<std::shared_ptr<HWC2::Layer>, hal::Error> Display::createLayer() {
HWLayerId layerId = 0;
auto intError = mComposer.createLayer(mId, &layerId);
auto error = static_cast<Error>(intError);
if (error != Error::NONE) {
- return error;
+ return base::unexpected(error);
}
- auto layer = std::make_unique<impl::Layer>(mComposer, mCapabilities, mId, layerId);
- *outLayer = layer.get();
- mLayers.emplace(layerId, std::move(layer));
- return Error::NONE;
+ auto layer = std::make_shared<impl::Layer>(mComposer, mCapabilities, *this, layerId);
+ mLayers.emplace(layerId, layer);
+ return layer;
}
-Error Display::destroyLayer(HWC2::Layer* layer) {
- if (!layer) {
- return Error::BAD_PARAMETER;
- }
- mLayers.erase(layer->getId());
- return Error::NONE;
+void Display::onLayerDestroyed(hal::HWLayerId layerId) {
+ mLayers.erase(layerId);
}
bool Display::isVsyncPeriodSwitchSupported() const {
@@ -161,7 +165,7 @@
auto type = types[element];
ALOGV("getChangedCompositionTypes: adding %" PRIu64 " %s",
layer->getId(), to_string(type).c_str());
- outTypes->emplace(layer, type);
+ outTypes->emplace(layer.get(), type);
} else {
ALOGE("getChangedCompositionTypes: invalid layer %" PRIu64 " found"
" on display %" PRIu64, layerIds[element], mId);
@@ -254,7 +258,7 @@
if (layer) {
auto layerRequest =
static_cast<LayerRequest>(layerRequests[element]);
- outLayerRequests->emplace(layer, layerRequest);
+ outLayerRequests->emplace(layer.get(), layerRequest);
} else {
ALOGE("getRequests: invalid layer %" PRIu64 " found on display %"
PRIu64, layerIds[element], mId);
@@ -340,7 +344,7 @@
auto layer = getLayerById(layerIds[element]);
if (layer) {
sp<Fence> fence(new Fence(fenceFds[element]));
- releaseFences.emplace(layer, fence);
+ releaseFences.emplace(layer.get(), fence);
} else {
ALOGE("getReleaseFences: invalid layer %" PRIu64
" found on display %" PRIu64, layerIds[element], mId);
@@ -550,12 +554,9 @@
// Other Display methods
-HWC2::Layer* Display::getLayerById(HWLayerId id) const {
- if (mLayers.count(id) == 0) {
- return nullptr;
- }
-
- return mLayers.at(id).get();
+std::shared_ptr<HWC2::Layer> Display::getLayerById(HWLayerId id) const {
+ auto it = mLayers.find(id);
+ return it != mLayers.end() ? it->second.lock() : nullptr;
}
} // namespace impl
@@ -566,47 +567,78 @@
namespace impl {
Layer::Layer(android::Hwc2::Composer& composer, const std::unordered_set<Capability>& capabilities,
- HWDisplayId displayId, HWLayerId layerId)
+ HWC2::Display& display, HWLayerId layerId)
: mComposer(composer),
mCapabilities(capabilities),
- mDisplayId(displayId),
+ mDisplay(&display),
mId(layerId),
mColorMatrix(android::mat4()) {
- ALOGV("Created layer %" PRIu64 " on display %" PRIu64, layerId, displayId);
+ ALOGV("Created layer %" PRIu64 " on display %" PRIu64, layerId, display.getId());
}
Layer::~Layer()
{
- auto intError = mComposer.destroyLayer(mDisplayId, mId);
+ onOwningDisplayDestroyed();
+}
+
+void Layer::onOwningDisplayDestroyed() {
+ // Note: onOwningDisplayDestroyed() may be called to perform cleanup by
+ // either the Layer dtor or by the Display dtor and must be safe to call
+ // from either path. In particular, the call to Display::onLayerDestroyed()
+ // is expected to be safe to do,
+
+ if (CC_UNLIKELY(!mDisplay)) {
+ return;
+ }
+
+ mDisplay->onLayerDestroyed(mId);
+
+ // Note: If the HWC display was actually disconnected, these calls are will
+ // return an error. We always make them as there may be other reasons for
+ // the HWC2::Display to be destroyed.
+ auto intError = mComposer.destroyLayer(mDisplay->getId(), mId);
auto error = static_cast<Error>(intError);
ALOGE_IF(error != Error::NONE,
"destroyLayer(%" PRIu64 ", %" PRIu64 ")"
" failed: %s (%d)",
- mDisplayId, mId, to_string(error).c_str(), intError);
+ mDisplay->getId(), mId, to_string(error).c_str(), intError);
+
+ mDisplay = nullptr;
}
Error Layer::setCursorPosition(int32_t x, int32_t y)
{
- auto intError = mComposer.setCursorPosition(mDisplayId, mId, x, y);
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
+ auto intError = mComposer.setCursorPosition(mDisplay->getId(), mId, x, y);
return static_cast<Error>(intError);
}
Error Layer::setBuffer(uint32_t slot, const sp<GraphicBuffer>& buffer,
const sp<Fence>& acquireFence)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
if (buffer == nullptr && mBufferSlot == slot) {
return Error::NONE;
}
mBufferSlot = slot;
int32_t fenceFd = acquireFence->dup();
- auto intError = mComposer.setLayerBuffer(mDisplayId, mId, slot, buffer,
- fenceFd);
+ auto intError = mComposer.setLayerBuffer(mDisplay->getId(), mId, slot, buffer, fenceFd);
return static_cast<Error>(intError);
}
Error Layer::setSurfaceDamage(const Region& damage)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
if (damage.isRect() && mDamageRegion.isRect() &&
(damage.getBounds() == mDamageRegion.getBounds())) {
return Error::NONE;
@@ -617,8 +649,8 @@
// rects for HWC
Hwc2::Error intError = Hwc2::Error::NONE;
if (damage.isRect() && damage.getBounds() == Rect::INVALID_RECT) {
- intError = mComposer.setLayerSurfaceDamage(mDisplayId,
- mId, std::vector<Hwc2::IComposerClient::Rect>());
+ intError = mComposer.setLayerSurfaceDamage(mDisplay->getId(), mId,
+ std::vector<Hwc2::IComposerClient::Rect>());
} else {
size_t rectCount = 0;
auto rectArray = damage.getArray(&rectCount);
@@ -629,7 +661,7 @@
rectArray[rect].right, rectArray[rect].bottom});
}
- intError = mComposer.setLayerSurfaceDamage(mDisplayId, mId, hwcRects);
+ intError = mComposer.setLayerSurfaceDamage(mDisplay->getId(), mId, hwcRects);
}
return static_cast<Error>(intError);
@@ -637,34 +669,54 @@
Error Layer::setBlendMode(BlendMode mode)
{
- auto intError = mComposer.setLayerBlendMode(mDisplayId, mId, mode);
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
+ auto intError = mComposer.setLayerBlendMode(mDisplay->getId(), mId, mode);
return static_cast<Error>(intError);
}
Error Layer::setColor(Color color) {
- auto intError = mComposer.setLayerColor(mDisplayId, mId, color);
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
+ auto intError = mComposer.setLayerColor(mDisplay->getId(), mId, color);
return static_cast<Error>(intError);
}
Error Layer::setCompositionType(Composition type)
{
- auto intError = mComposer.setLayerCompositionType(mDisplayId, mId, type);
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
+ auto intError = mComposer.setLayerCompositionType(mDisplay->getId(), mId, type);
return static_cast<Error>(intError);
}
Error Layer::setDataspace(Dataspace dataspace)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
if (dataspace == mDataSpace) {
return Error::NONE;
}
mDataSpace = dataspace;
- auto intError = mComposer.setLayerDataspace(mDisplayId, mId, mDataSpace);
+ auto intError = mComposer.setLayerDataspace(mDisplay->getId(), mId, mDataSpace);
return static_cast<Error>(intError);
}
Error Layer::setPerFrameMetadata(const int32_t supportedPerFrameMetadata,
const android::HdrMetadata& metadata)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
if (metadata == mHdrMetadata) {
return Error::NONE;
}
@@ -705,7 +757,7 @@
}
Error error = static_cast<Error>(
- mComposer.setLayerPerFrameMetadata(mDisplayId, mId, perFrameMetadatas));
+ mComposer.setLayerPerFrameMetadata(mDisplay->getId(), mId, perFrameMetadatas));
if (validTypes & HdrMetadata::HDR10PLUS) {
if (CC_UNLIKELY(mHdrMetadata.hdr10plus.size() == 0)) {
@@ -715,8 +767,9 @@
std::vector<Hwc2::PerFrameMetadataBlob> perFrameMetadataBlobs;
perFrameMetadataBlobs.push_back(
{Hwc2::PerFrameMetadataKey::HDR10_PLUS_SEI, mHdrMetadata.hdr10plus});
- Error setMetadataBlobsError = static_cast<Error>(
- mComposer.setLayerPerFrameMetadataBlobs(mDisplayId, mId, perFrameMetadataBlobs));
+ Error setMetadataBlobsError =
+ static_cast<Error>(mComposer.setLayerPerFrameMetadataBlobs(mDisplay->getId(), mId,
+ perFrameMetadataBlobs));
if (error == Error::NONE) {
return setMetadataBlobsError;
}
@@ -726,46 +779,70 @@
Error Layer::setDisplayFrame(const Rect& frame)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
Hwc2::IComposerClient::Rect hwcRect{frame.left, frame.top,
frame.right, frame.bottom};
- auto intError = mComposer.setLayerDisplayFrame(mDisplayId, mId, hwcRect);
+ auto intError = mComposer.setLayerDisplayFrame(mDisplay->getId(), mId, hwcRect);
return static_cast<Error>(intError);
}
Error Layer::setPlaneAlpha(float alpha)
{
- auto intError = mComposer.setLayerPlaneAlpha(mDisplayId, mId, alpha);
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
+ auto intError = mComposer.setLayerPlaneAlpha(mDisplay->getId(), mId, alpha);
return static_cast<Error>(intError);
}
Error Layer::setSidebandStream(const native_handle_t* stream)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
if (mCapabilities.count(Capability::SIDEBAND_STREAM) == 0) {
ALOGE("Attempted to call setSidebandStream without checking that the "
"device supports sideband streams");
return Error::UNSUPPORTED;
}
- auto intError = mComposer.setLayerSidebandStream(mDisplayId, mId, stream);
+ auto intError = mComposer.setLayerSidebandStream(mDisplay->getId(), mId, stream);
return static_cast<Error>(intError);
}
Error Layer::setSourceCrop(const FloatRect& crop)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
Hwc2::IComposerClient::FRect hwcRect{
crop.left, crop.top, crop.right, crop.bottom};
- auto intError = mComposer.setLayerSourceCrop(mDisplayId, mId, hwcRect);
+ auto intError = mComposer.setLayerSourceCrop(mDisplay->getId(), mId, hwcRect);
return static_cast<Error>(intError);
}
Error Layer::setTransform(Transform transform)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
auto intTransform = static_cast<Hwc2::Transform>(transform);
- auto intError = mComposer.setLayerTransform(mDisplayId, mId, intTransform);
+ auto intError = mComposer.setLayerTransform(mDisplay->getId(), mId, intTransform);
return static_cast<Error>(intError);
}
Error Layer::setVisibleRegion(const Region& region)
{
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
if (region.isRect() && mVisibleRegion.isRect() &&
(region.getBounds() == mVisibleRegion.getBounds())) {
return Error::NONE;
@@ -781,22 +858,30 @@
rectArray[rect].right, rectArray[rect].bottom});
}
- auto intError = mComposer.setLayerVisibleRegion(mDisplayId, mId, hwcRects);
+ auto intError = mComposer.setLayerVisibleRegion(mDisplay->getId(), mId, hwcRects);
return static_cast<Error>(intError);
}
Error Layer::setZOrder(uint32_t z)
{
- auto intError = mComposer.setLayerZOrder(mDisplayId, mId, z);
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
+ auto intError = mComposer.setLayerZOrder(mDisplay->getId(), mId, z);
return static_cast<Error>(intError);
}
// Composer HAL 2.3
Error Layer::setColorTransform(const android::mat4& matrix) {
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
if (matrix == mColorMatrix) {
return Error::NONE;
}
- auto intError = mComposer.setLayerColorTransform(mDisplayId, mId, matrix.asArray());
+ auto intError = mComposer.setLayerColorTransform(mDisplay->getId(), mId, matrix.asArray());
Error error = static_cast<Error>(intError);
if (error != Error::NONE) {
return error;
@@ -808,7 +893,12 @@
// Composer HAL 2.4
Error Layer::setLayerGenericMetadata(const std::string& name, bool mandatory,
const std::vector<uint8_t>& value) {
- auto intError = mComposer.setLayerGenericMetadata(mDisplayId, mId, name, mandatory, value);
+ if (CC_UNLIKELY(!mDisplay)) {
+ return Error::BAD_DISPLAY;
+ }
+
+ auto intError =
+ mComposer.setLayerGenericMetadata(mDisplay->getId(), mId, name, mandatory, value);
return static_cast<Error>(intError);
}
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.h b/services/surfaceflinger/DisplayHardware/HWC2.h
index e7bf286..fae95e7 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2.h
@@ -16,6 +16,7 @@
#pragma once
+#include <android-base/expected.h>
#include <gui/HdrMetadata.h>
#include <math/mat4.h>
#include <ui/HdrCapabilities.h>
@@ -84,10 +85,11 @@
virtual void setConnected(bool connected) = 0; // For use by Device only
virtual const std::unordered_set<hal::DisplayCapability>& getCapabilities() const = 0;
virtual bool isVsyncPeriodSwitchSupported() const = 0;
+ virtual void onLayerDestroyed(hal::HWLayerId layerId) = 0;
[[clang::warn_unused_result]] virtual hal::Error acceptChanges() = 0;
- [[clang::warn_unused_result]] virtual hal::Error createLayer(Layer** outLayer) = 0;
- [[clang::warn_unused_result]] virtual hal::Error destroyLayer(Layer* layer) = 0;
+ [[clang::warn_unused_result]] virtual base::expected<std::shared_ptr<HWC2::Layer>, hal::Error>
+ createLayer() = 0;
[[clang::warn_unused_result]] virtual hal::Error getChangedCompositionTypes(
std::unordered_map<Layer*, hal::Composition>* outTypes) = 0;
[[clang::warn_unused_result]] virtual hal::Error getColorModes(
@@ -152,6 +154,8 @@
namespace impl {
+class Layer;
+
class Display : public HWC2::Display {
public:
Display(android::Hwc2::Composer&, const std::unordered_set<hal::Capability>&, hal::HWDisplayId,
@@ -160,10 +164,9 @@
// Required by HWC2
hal::Error acceptChanges() override;
- hal::Error createLayer(Layer** outLayer) override;
- hal::Error destroyLayer(Layer*) override;
+ base::expected<std::shared_ptr<HWC2::Layer>, hal::Error> createLayer() override;
hal::Error getChangedCompositionTypes(
- std::unordered_map<Layer*, hal::Composition>* outTypes) override;
+ std::unordered_map<HWC2::Layer*, hal::Composition>* outTypes) override;
hal::Error getColorModes(std::vector<hal::ColorMode>* outModes) const override;
// Returns a bitmask which contains HdrMetadata::Type::*.
int32_t getSupportedPerFrameMetadata() const override;
@@ -174,7 +177,7 @@
hal::Error getName(std::string* outName) const override;
hal::Error getRequests(
hal::DisplayRequest* outDisplayRequests,
- std::unordered_map<Layer*, hal::LayerRequest>* outLayerRequests) override;
+ std::unordered_map<HWC2::Layer*, hal::LayerRequest>* outLayerRequests) override;
hal::Error getConnectionType(ui::DisplayConnectionType*) const override;
hal::Error supportsDoze(bool* outSupport) const override;
hal::Error getHdrCapabilities(android::HdrCapabilities* outCapabilities) const override;
@@ -185,8 +188,8 @@
uint64_t maxFrames) const override;
hal::Error getDisplayedContentSample(uint64_t maxFrames, uint64_t timestamp,
android::DisplayedFrameStats* outStats) const override;
- hal::Error getReleaseFences(
- std::unordered_map<Layer*, android::sp<android::Fence>>* outFences) const override;
+ hal::Error getReleaseFences(std::unordered_map<HWC2::Layer*, android::sp<android::Fence>>*
+ outFences) const override;
hal::Error present(android::sp<android::Fence>* outPresentFence) override;
hal::Error setClientTarget(uint32_t slot, const android::sp<android::GraphicBuffer>& target,
const android::sp<android::Fence>& acquireFence,
@@ -218,13 +221,14 @@
const std::unordered_set<hal::DisplayCapability>& getCapabilities() const override {
return mDisplayCapabilities;
};
- virtual bool isVsyncPeriodSwitchSupported() const override;
+ bool isVsyncPeriodSwitchSupported() const override;
+ void onLayerDestroyed(hal::HWLayerId layerId) override;
private:
// This may fail (and return a null pointer) if no layer with this ID exists
// on this display
- Layer* getLayerById(hal::HWLayerId) const;
+ std::shared_ptr<HWC2::Layer> getLayerById(hal::HWLayerId id) const;
friend android::TestableSurfaceFlinger;
@@ -240,7 +244,8 @@
hal::DisplayType mType;
bool mIsConnected = false;
- std::unordered_map<hal::HWLayerId, std::unique_ptr<Layer>> mLayers;
+ using Layers = std::unordered_map<hal::HWLayerId, std::weak_ptr<HWC2::impl::Layer>>;
+ Layers mLayers;
std::once_flag mDisplayCapabilityQueryFlag;
std::unordered_set<hal::DisplayCapability> mDisplayCapabilities;
@@ -295,10 +300,12 @@
class Layer : public HWC2::Layer {
public:
Layer(android::Hwc2::Composer& composer,
- const std::unordered_set<hal::Capability>& capabilities, hal::HWDisplayId displayId,
+ const std::unordered_set<hal::Capability>& capabilities, HWC2::Display& display,
hal::HWLayerId layerId);
~Layer() override;
+ void onOwningDisplayDestroyed();
+
hal::HWLayerId getId() const override { return mId; }
hal::Error setCursorPosition(int32_t x, int32_t y) override;
@@ -334,7 +341,7 @@
android::Hwc2::Composer& mComposer;
const std::unordered_set<hal::Capability>& mCapabilities;
- hal::HWDisplayId mDisplayId;
+ HWC2::Display* mDisplay;
hal::HWLayerId mId;
// Cached HWC2 data, to ensure the same commands aren't sent to the HWC
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index dc4839e..36876dc 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -305,20 +305,15 @@
return value;
}
-HWC2::Layer* HWComposer::createLayer(HalDisplayId displayId) {
+std::shared_ptr<HWC2::Layer> HWComposer::createLayer(HalDisplayId displayId) {
RETURN_IF_INVALID_DISPLAY(displayId, nullptr);
- HWC2::Layer* layer;
- auto error = mDisplayData[displayId].hwcDisplay->createLayer(&layer);
- RETURN_IF_HWC_ERROR(error, displayId, nullptr);
- return layer;
-}
-
-void HWComposer::destroyLayer(HalDisplayId displayId, HWC2::Layer* layer) {
- RETURN_IF_INVALID_DISPLAY(displayId);
-
- auto error = mDisplayData[displayId].hwcDisplay->destroyLayer(layer);
- RETURN_IF_HWC_ERROR(error, displayId);
+ auto expected = mDisplayData[displayId].hwcDisplay->createLayer();
+ if (!expected.has_value()) {
+ auto error = std::move(expected).error();
+ RETURN_IF_HWC_ERROR(error, displayId, nullptr);
+ }
+ return std::move(expected).value();
}
bool HWComposer::isConnected(PhysicalDisplayId displayId) const {
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 5bad529..d0c0c11 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -116,9 +116,7 @@
virtual void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId) = 0;
// Attempts to create a new layer on this display
- virtual HWC2::Layer* createLayer(HalDisplayId) = 0;
- // Destroy a previously created layer
- virtual void destroyLayer(HalDisplayId, HWC2::Layer*) = 0;
+ virtual std::shared_ptr<HWC2::Layer> createLayer(HalDisplayId) = 0;
// Gets any required composition change requests from the HWC device.
//
@@ -264,9 +262,7 @@
void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId) override;
// Attempts to create a new layer on this display
- HWC2::Layer* createLayer(HalDisplayId) override;
- // Destroy a previously created layer
- void destroyLayer(HalDisplayId, HWC2::Layer*) override;
+ std::shared_ptr<HWC2::Layer> createLayer(HalDisplayId) override;
status_t getDeviceCompositionChanges(
HalDisplayId, bool frameUsesClientComposition,
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 88fb811..b33434e 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -94,6 +94,7 @@
"VSyncReactorTest.cpp",
"VsyncConfigurationTest.cpp",
"mock/DisplayHardware/MockComposer.cpp",
+ "mock/DisplayHardware/MockHWC2.cpp",
"mock/DisplayHardware/MockPowerAdvisor.cpp",
"mock/MockEventThread.cpp",
"mock/MockFrameTimeline.cpp",
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index 6b82170..cbf8cc2 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -37,6 +37,7 @@
#include "DisplayHardware/HWComposer.h"
#include "DisplayHardware/Hal.h"
#include "mock/DisplayHardware/MockComposer.h"
+#include "mock/DisplayHardware/MockHWC2.h"
// TODO(b/129481165): remove the #pragma below and fix conversion issues
#pragma clang diagnostic pop // ignored "-Wconversion"
@@ -120,13 +121,19 @@
static constexpr hal::HWLayerId kLayerId = static_cast<hal::HWLayerId>(1002);
HWComposerLayerTest(const std::unordered_set<hal::Capability>& capabilities)
- : mCapabilies(capabilities) {}
+ : mCapabilies(capabilities) {
+ EXPECT_CALL(mDisplay, getId()).WillRepeatedly(Return(kDisplayId));
+ }
- ~HWComposerLayerTest() override { EXPECT_CALL(*mHal, destroyLayer(kDisplayId, kLayerId)); }
+ ~HWComposerLayerTest() override {
+ EXPECT_CALL(mDisplay, onLayerDestroyed(kLayerId));
+ EXPECT_CALL(*mHal, destroyLayer(kDisplayId, kLayerId));
+ }
std::unique_ptr<Hwc2::mock::Composer> mHal{new StrictMock<Hwc2::mock::Composer>()};
const std::unordered_set<hal::Capability> mCapabilies;
- HWC2::impl::Layer mLayer{*mHal, mCapabilies, kDisplayId, kLayerId};
+ StrictMock<HWC2::mock::Display> mDisplay;
+ HWC2::impl::Layer mLayer{*mHal, mCapabilies, mDisplay, kLayerId};
};
struct HWComposerLayerGenericMetadataTest : public HWComposerLayerTest {
@@ -176,4 +183,4 @@
}
} // namespace
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.cpp b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.cpp
new file mode 100644
index 0000000..2647bf4
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.cpp
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2021 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 "MockHWC2"
+
+#include "mock/DisplayHardware/MockHWC2.h"
+
+namespace android::HWC2::mock {
+
+// Explicit default instantiation is recommended.
+Display::Display() = default;
+Display::~Display() = default;
+
+Layer::Layer() = default;
+Layer::~Layer() = default;
+
+} // namespace android::HWC2::mock
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
new file mode 100644
index 0000000..c3919d9
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
@@ -0,0 +1,122 @@
+/*
+ * Copyright 2021 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 <gmock/gmock.h>
+
+#include "DisplayHardware/HWC2.h"
+
+namespace android::HWC2::mock {
+
+class Display : public HWC2::Display {
+public:
+ Display();
+ ~Display() override;
+
+ MOCK_METHOD(hal::HWDisplayId, getId, (), (const, override));
+ MOCK_METHOD(bool, isConnected, (), (const, override));
+ MOCK_METHOD(void, setConnected, (bool), (override));
+ MOCK_METHOD(const std::unordered_set<hal::DisplayCapability> &, getCapabilities, (),
+ (const, override));
+ MOCK_METHOD(bool, isVsyncPeriodSwitchSupported, (), (const, override));
+ MOCK_METHOD(void, onLayerDestroyed, (hal::HWLayerId), (override));
+
+ MOCK_METHOD(hal::Error, acceptChanges, (), (override));
+ MOCK_METHOD((base::expected<std::shared_ptr<HWC2::Layer>, hal::Error>), createLayer, (),
+ (override));
+ MOCK_METHOD(hal::Error, getChangedCompositionTypes,
+ ((std::unordered_map<Layer *, hal::Composition> *)), (override));
+ MOCK_METHOD(hal::Error, getColorModes, (std::vector<hal::ColorMode> *), (const, override));
+ MOCK_METHOD(int32_t, getSupportedPerFrameMetadata, (), (const, override));
+ MOCK_METHOD(hal::Error, getRenderIntents, (hal::ColorMode, std::vector<hal::RenderIntent> *),
+ (const, override));
+ MOCK_METHOD(hal::Error, getDataspaceSaturationMatrix, (hal::Dataspace, android::mat4 *),
+ (override));
+ MOCK_METHOD(hal::Error, getName, (std::string *), (const, override));
+ MOCK_METHOD(hal::Error, getRequests,
+ (hal::DisplayRequest *, (std::unordered_map<Layer *, hal::LayerRequest> *)),
+ (override));
+ MOCK_METHOD(hal::Error, getConnectionType, (ui::DisplayConnectionType *), (const, override));
+ MOCK_METHOD(hal::Error, supportsDoze, (bool *), (const, override));
+ MOCK_METHOD(hal::Error, getHdrCapabilities, (android::HdrCapabilities *), (const, override));
+ MOCK_METHOD(hal::Error, getDisplayedContentSamplingAttributes,
+ (hal::PixelFormat *, hal::Dataspace *, uint8_t *), (const, override));
+ MOCK_METHOD(hal::Error, setDisplayContentSamplingEnabled, (bool, uint8_t, uint64_t),
+ (const, override));
+ MOCK_METHOD(hal::Error, getDisplayedContentSample,
+ (uint64_t, uint64_t, android::DisplayedFrameStats *), (const, override));
+ MOCK_METHOD(hal::Error, getReleaseFences,
+ ((std::unordered_map<Layer *, android::sp<android::Fence>> *)), (const, override));
+ MOCK_METHOD(hal::Error, present, (android::sp<android::Fence> *), (override));
+ MOCK_METHOD(hal::Error, setClientTarget,
+ (uint32_t, const android::sp<android::GraphicBuffer> &,
+ const android::sp<android::Fence> &, hal::Dataspace),
+ (override));
+ MOCK_METHOD(hal::Error, setColorMode, (hal::ColorMode, hal::RenderIntent), (override));
+ MOCK_METHOD(hal::Error, setColorTransform, (const android::mat4 &, hal::ColorTransform),
+ (override));
+ MOCK_METHOD(hal::Error, setOutputBuffer,
+ (const android::sp<android::GraphicBuffer> &, const android::sp<android::Fence> &),
+ (override));
+ MOCK_METHOD(hal::Error, setPowerMode, (hal::PowerMode), (override));
+ MOCK_METHOD(hal::Error, setVsyncEnabled, (hal::Vsync), (override));
+ MOCK_METHOD(hal::Error, validate, (uint32_t *, uint32_t *), (override));
+ MOCK_METHOD(hal::Error, presentOrValidate,
+ (uint32_t *, uint32_t *, android::sp<android::Fence> *, uint32_t *), (override));
+ MOCK_METHOD(std::future<hal::Error>, setDisplayBrightness, (float), (override));
+ MOCK_METHOD(hal::Error, setActiveConfigWithConstraints,
+ (hal::HWConfigId, const hal::VsyncPeriodChangeConstraints &,
+ hal::VsyncPeriodChangeTimeline *),
+ (override));
+ MOCK_METHOD(hal::Error, setAutoLowLatencyMode, (bool), (override));
+ MOCK_METHOD(hal::Error, getSupportedContentTypes, (std::vector<hal::ContentType> *),
+ (const, override));
+ MOCK_METHOD(hal::Error, setContentType, (hal::ContentType), (override));
+ MOCK_METHOD(hal::Error, getClientTargetProperty, (hal::ClientTargetProperty *), (override));
+};
+
+class Layer : public HWC2::Layer {
+public:
+ Layer();
+ ~Layer() override;
+
+ MOCK_METHOD(hal::HWLayerId, getId, (), (const, override));
+ MOCK_METHOD(hal::Error, setCursorPosition, (int32_t, int32_t), (override));
+ MOCK_METHOD(hal::Error, setBuffer,
+ (uint32_t, const android::sp<android::GraphicBuffer> &,
+ const android::sp<android::Fence> &),
+ (override));
+ MOCK_METHOD(hal::Error, setSurfaceDamage, (const android::Region &), (override));
+ MOCK_METHOD(hal::Error, setBlendMode, (hal::BlendMode), (override));
+ MOCK_METHOD(hal::Error, setColor, (hal::Color), (override));
+ MOCK_METHOD(hal::Error, setCompositionType, (hal::Composition), (override));
+ MOCK_METHOD(hal::Error, setDataspace, (android::ui::Dataspace), (override));
+ MOCK_METHOD(hal::Error, setPerFrameMetadata, (const int32_t, const android::HdrMetadata &),
+ (override));
+ MOCK_METHOD(hal::Error, setDisplayFrame, (const android::Rect &), (override));
+ MOCK_METHOD(hal::Error, setPlaneAlpha, (float), (override));
+ MOCK_METHOD(hal::Error, setSidebandStream, (const native_handle_t *), (override));
+ MOCK_METHOD(hal::Error, setSourceCrop, (const android::FloatRect &), (override));
+ MOCK_METHOD(hal::Error, setTransform, (hal::Transform), (override));
+ MOCK_METHOD(hal::Error, setVisibleRegion, (const android::Region &), (override));
+ MOCK_METHOD(hal::Error, setZOrder, (uint32_t), (override));
+ MOCK_METHOD(hal::Error, setColorTransform, (const android::mat4 &), (override));
+ MOCK_METHOD(hal::Error, setLayerGenericMetadata,
+ (const std::string &, bool, const std::vector<uint8_t> &), (override));
+};
+
+} // namespace android::HWC2::mock