SF: Introduce DisplayDeviceCreationArgs
Introduce a structure to hold the arguments used by the DisplayDevice
constructor. This simplifies the injector code used by the test, and
makes it easier to change the arguments without requiring a change to
the test or to the upcoming factory.
Test: atest libsurfaceflinger_unittest
Test: Marlin boots and appears usable
Bug: None
Change-Id: I4c806bf40f8f3c2c00f5115b83c6ab926317d628
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 4becfab..341dfd5 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -206,54 +206,46 @@
} // anonymous namespace
-// clang-format off
-DisplayDevice::DisplayDevice(
- const sp<SurfaceFlinger>& flinger,
- DisplayType type,
- int32_t id,
- bool isSecure,
- const wp<IBinder>& displayToken,
- const sp<ANativeWindow>& nativeWindow,
- const sp<DisplaySurface>& displaySurface,
- std::unique_ptr<renderengine::Surface> renderSurface,
- int displayWidth,
- int displayHeight,
- int displayInstallOrientation,
- bool hasWideColorGamut,
- const HdrCapabilities& hdrCapabilities,
- const int32_t supportedPerFrameMetadata,
- const std::unordered_map<ColorMode, std::vector<RenderIntent>>& hwcColorModes,
- int initialPowerMode)
- : lastCompositionHadVisibleLayers(false),
- mFlinger(flinger),
- mType(type),
- mId(id),
- mDisplayToken(displayToken),
- mNativeWindow(nativeWindow),
- mDisplaySurface(displaySurface),
- mSurface{std::move(renderSurface)},
- mDisplayWidth(displayWidth),
- mDisplayHeight(displayHeight),
- mDisplayInstallOrientation(displayInstallOrientation),
- mPageFlipCount(0),
- mIsSecure(isSecure),
- mLayerStack(NO_LAYER_STACK),
- mOrientation(),
- mViewport(Rect::INVALID_RECT),
- mFrame(Rect::INVALID_RECT),
- mPowerMode(initialPowerMode),
- mActiveConfig(0),
- mColorTransform(HAL_COLOR_TRANSFORM_IDENTITY),
- mHasWideColorGamut(hasWideColorGamut),
- mHasHdr10(false),
- mHasHLG(false),
- mHasDolbyVision(false),
- mSupportedPerFrameMetadata(supportedPerFrameMetadata)
-{
- // clang-format on
- populateColorModes(hwcColorModes);
+DisplayDeviceCreationArgs::DisplayDeviceCreationArgs(const sp<SurfaceFlinger>& flinger,
+ const wp<IBinder>& displayToken,
+ DisplayDevice::DisplayType type, int32_t id)
+ : flinger(flinger), displayToken(displayToken), type(type), id(id) {}
- std::vector<Hdr> types = hdrCapabilities.getSupportedHdrTypes();
+DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs&& args)
+ : lastCompositionHadVisibleLayers(false),
+ mFlinger(args.flinger),
+ mType(args.type),
+ mId(args.id),
+ mDisplayToken(args.displayToken),
+ mNativeWindow(args.nativeWindow),
+ mDisplaySurface(args.displaySurface),
+ mSurface{std::move(args.renderSurface)},
+ mDisplayWidth(args.displayWidth),
+ mDisplayHeight(args.displayHeight),
+ mDisplayInstallOrientation(args.displayInstallOrientation),
+ mPageFlipCount(0),
+ mIsSecure(args.isSecure),
+ mLayerStack(NO_LAYER_STACK),
+ mOrientation(),
+ mViewport(Rect::INVALID_RECT),
+ mFrame(Rect::INVALID_RECT),
+ mPowerMode(args.initialPowerMode),
+ mActiveConfig(0),
+ mColorTransform(HAL_COLOR_TRANSFORM_IDENTITY),
+ mHasWideColorGamut(args.hasWideColorGamut),
+ mHasHdr10(false),
+ mHasHLG(false),
+ mHasDolbyVision(false),
+ mSupportedPerFrameMetadata(args.supportedPerFrameMetadata) {
+ populateColorModes(args.hwcColorModes);
+
+ ALOGE_IF(!mNativeWindow, "No native window was set for display");
+ ALOGE_IF(!mDisplaySurface, "No display surface was set for display");
+ ALOGE_IF(!mSurface, "No render surface was set for display");
+ ALOGE_IF(mDisplayWidth <= 0 || mDisplayHeight <= 0,
+ "Invalid dimensions of %d x %d were set for display", mDisplayWidth, mDisplayHeight);
+
+ std::vector<Hdr> types = args.hdrCapabilities.getSupportedHdrTypes();
for (Hdr hdrType : types) {
switch (hdrType) {
case Hdr::HDR10:
@@ -270,9 +262,9 @@
}
}
- float minLuminance = hdrCapabilities.getDesiredMinLuminance();
- float maxLuminance = hdrCapabilities.getDesiredMaxLuminance();
- float maxAverageLuminance = hdrCapabilities.getDesiredMaxAverageLuminance();
+ float minLuminance = args.hdrCapabilities.getDesiredMinLuminance();
+ float maxLuminance = args.hdrCapabilities.getDesiredMaxLuminance();
+ float maxAverageLuminance = args.hdrCapabilities.getDesiredMaxAverageLuminance();
minLuminance = minLuminance <= 0.0 ? sDefaultMinLumiance : minLuminance;
maxLuminance = maxLuminance <= 0.0 ? sDefaultMaxLumiance : maxLuminance;
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index ed73671..38482c9 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -43,14 +43,16 @@
namespace android {
-struct DisplayInfo;
class DisplaySurface;
class Fence;
+class HWComposer;
class IGraphicBufferProducer;
class Layer;
class SurfaceFlinger;
-class HWComposer;
+
struct CompositionInfo;
+struct DisplayDeviceCreationArgs;
+struct DisplayInfo;
class DisplayDevice : public LightRefBase<DisplayDevice>
{
@@ -76,25 +78,7 @@
NO_LAYER_STACK = 0xFFFFFFFF,
};
- // clang-format off
- DisplayDevice(
- const sp<SurfaceFlinger>& flinger,
- DisplayType type,
- int32_t id,
- bool isSecure,
- const wp<IBinder>& displayToken,
- const sp<ANativeWindow>& nativeWindow,
- const sp<DisplaySurface>& displaySurface,
- std::unique_ptr<renderengine::Surface> renderSurface,
- int displayWidth,
- int displayHeight,
- int displayInstallOrientation,
- bool hasWideColorGamut,
- const HdrCapabilities& hdrCapabilities,
- const int32_t supportedPerFrameMetadata,
- const std::unordered_map<ui::ColorMode, std::vector<ui::RenderIntent>>& hwcColorModes,
- int initialPowerMode);
- // clang-format on
+ explicit DisplayDevice(DisplayDeviceCreationArgs&& args);
~DisplayDevice();
@@ -335,6 +319,31 @@
static std::atomic<int32_t> sNextSequenceId;
};
+struct DisplayDeviceCreationArgs {
+ // We use a constructor to ensure some of the values are set, without
+ // assuming a default value.
+ DisplayDeviceCreationArgs(const sp<SurfaceFlinger>& flinger, const wp<IBinder>& displayToken,
+ DisplayDevice::DisplayType type, int32_t id);
+
+ const sp<SurfaceFlinger> flinger;
+ const wp<IBinder> displayToken;
+ const DisplayDevice::DisplayType type;
+ const int32_t id;
+
+ bool isSecure{false};
+ sp<ANativeWindow> nativeWindow;
+ sp<DisplaySurface> displaySurface;
+ std::unique_ptr<renderengine::Surface> renderSurface;
+ int displayWidth{0};
+ int displayHeight{0};
+ int displayInstallOrientation{DisplayState::eOrientationDefault};
+ bool hasWideColorGamut{false};
+ HdrCapabilities hdrCapabilities;
+ int32_t supportedPerFrameMetadata{0};
+ std::unordered_map<ui::ColorMode, std::vector<ui::RenderIntent>> hwcColorModes;
+ int initialPowerMode{HWC_POWER_MODE_NORMAL};
+};
+
class DisplayRenderArea : public RenderArea {
public:
DisplayRenderArea(const sp<const DisplayDevice> device,
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 1e8958f..38af8a7 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2427,31 +2427,34 @@
sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
const wp<IBinder>& displayToken, int32_t displayId, const DisplayDeviceState& state,
const sp<DisplaySurface>& dispSurface, const sp<IGraphicBufferProducer>& producer) {
- bool hasWideColorGamut = false;
- std::unordered_map<ColorMode, std::vector<RenderIntent>> hwcColorModes;
- HdrCapabilities hdrCapabilities;
- int32_t supportedPerFrameMetadata = 0;
+ DisplayDeviceCreationArgs creationArgs(this, displayToken, state.type, displayId);
+ creationArgs.isSecure = state.isSecure;
+ creationArgs.displaySurface = dispSurface;
+ creationArgs.hasWideColorGamut = false;
+ creationArgs.supportedPerFrameMetadata = 0;
if (useColorManagement && displayId >= 0) {
std::vector<ColorMode> modes = getHwComposer().getColorModes(displayId);
for (ColorMode colorMode : modes) {
if (isWideColorMode(colorMode)) {
- hasWideColorGamut = true;
+ creationArgs.hasWideColorGamut = true;
}
std::vector<RenderIntent> renderIntents =
getHwComposer().getRenderIntents(displayId, colorMode);
- hwcColorModes.emplace(colorMode, renderIntents);
+ creationArgs.hwcColorModes.emplace(colorMode, renderIntents);
}
}
if (displayId >= 0) {
- getHwComposer().getHdrCapabilities(displayId, &hdrCapabilities);
- supportedPerFrameMetadata = getHwComposer().getSupportedPerFrameMetadata(displayId);
+ getHwComposer().getHdrCapabilities(displayId, &creationArgs.hdrCapabilities);
+ creationArgs.supportedPerFrameMetadata =
+ getHwComposer().getSupportedPerFrameMetadata(displayId);
}
auto nativeWindowSurface = mCreateNativeWindowSurface(producer);
auto nativeWindow = nativeWindowSurface->getNativeWindow();
+ creationArgs.nativeWindow = nativeWindow;
/*
* Create our display's surface
@@ -2460,8 +2463,9 @@
renderSurface->setCritical(state.type == DisplayDevice::DISPLAY_PRIMARY);
renderSurface->setAsync(state.isVirtual());
renderSurface->setNativeWindow(nativeWindow.get());
- const int displayWidth = renderSurface->getWidth();
- const int displayHeight = renderSurface->getHeight();
+ creationArgs.displayWidth = renderSurface->getWidth();
+ creationArgs.displayHeight = renderSurface->getHeight();
+ creationArgs.renderSurface = std::move(renderSurface);
// Make sure that composition can never be stalled by a virtual display
// consumer that isn't processing buffers fast enough. We have to do this
@@ -2474,18 +2478,14 @@
nativeWindow->setSwapInterval(nativeWindow.get(), 0);
}
- const int displayInstallOrientation = state.type == DisplayDevice::DISPLAY_PRIMARY ?
- primaryDisplayOrientation : DisplayState::eOrientationDefault;
+ creationArgs.displayInstallOrientation = state.type == DisplayDevice::DISPLAY_PRIMARY
+ ? primaryDisplayOrientation
+ : DisplayState::eOrientationDefault;
// virtual displays are always considered enabled
- auto initialPowerMode = state.isVirtual() ? HWC_POWER_MODE_NORMAL : HWC_POWER_MODE_OFF;
+ creationArgs.initialPowerMode = state.isVirtual() ? HWC_POWER_MODE_NORMAL : HWC_POWER_MODE_OFF;
- sp<DisplayDevice> display =
- new DisplayDevice(this, state.type, displayId, state.isSecure, displayToken,
- nativeWindow, dispSurface, std::move(renderSurface), displayWidth,
- displayHeight, displayInstallOrientation, hasWideColorGamut,
- hdrCapabilities, supportedPerFrameMetadata, hwcColorModes,
- initialPowerMode);
+ sp<DisplayDevice> display = new DisplayDevice(std::move(creationArgs));
if (maxFrameBufferAcquiredBuffers >= 3) {
nativeWindowSurface->preallocateBuffers();
@@ -2493,7 +2493,7 @@
ColorMode defaultColorMode = ColorMode::NATIVE;
Dataspace defaultDataSpace = Dataspace::UNKNOWN;
- if (hasWideColorGamut) {
+ if (display->hasWideColorGamut()) {
defaultColorMode = ColorMode::SRGB;
defaultDataSpace = Dataspace::SRGB;
}
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 341734c..33a0242 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -193,7 +193,7 @@
public:
FakePowerAdvisor() = default;
~FakePowerAdvisor() override = default;
- void setExpensiveRenderingExpected(hwc2_display_t, bool) override { }
+ void setExpensiveRenderingExpected(hwc2_display_t, bool) override {}
};
struct HWC2Display : public HWC2::Display {
@@ -316,7 +316,8 @@
public:
FakeDisplayDeviceInjector(TestableSurfaceFlinger& flinger, DisplayDevice::DisplayType type,
int32_t displayId)
- : mFlinger(flinger), mType(type), mDisplayId(displayId) {}
+ : mFlinger(flinger),
+ mCreationArgs(flinger.mFlinger.get(), mDisplayToken, type, displayId) {}
sp<IBinder> token() const { return mDisplayToken; }
@@ -339,54 +340,49 @@
auto& mutableDisplayDevice() { return mFlinger.mutableDisplays()[mDisplayToken]; }
auto& setNativeWindow(const sp<ANativeWindow>& nativeWindow) {
- mNativeWindow = nativeWindow;
+ mCreationArgs.nativeWindow = nativeWindow;
return *this;
}
auto& setDisplaySurface(const sp<DisplaySurface>& displaySurface) {
- mDisplaySurface = displaySurface;
+ mCreationArgs.displaySurface = displaySurface;
return *this;
}
auto& setRenderSurface(std::unique_ptr<renderengine::Surface> renderSurface) {
- mRenderSurface = std::move(renderSurface);
+ mCreationArgs.renderSurface = std::move(renderSurface);
return *this;
}
auto& setSecure(bool secure) {
- mSecure = secure;
+ mCreationArgs.isSecure = secure;
return *this;
}
auto& setDisplaySize(int width, int height) {
- mWidth = width;
- mHeight = height;
+ mCreationArgs.displayWidth = width;
+ mCreationArgs.displayHeight = height;
return *this;
}
auto& setPowerMode(int mode) {
- mPowerMode = mode;
+ mCreationArgs.initialPowerMode = mode;
return *this;
}
sp<DisplayDevice> inject() {
- std::unordered_map<ui::ColorMode, std::vector<ui::RenderIntent>> hdrAndRenderIntents;
- sp<DisplayDevice> device =
- new DisplayDevice(mFlinger.mFlinger.get(), mType, mDisplayId, mSecure,
- mDisplayToken, mNativeWindow, mDisplaySurface,
- std::move(mRenderSurface), mWidth, mHeight,
- DisplayState::eOrientationDefault, false, HdrCapabilities(),
- 0, hdrAndRenderIntents, mPowerMode);
- mFlinger.mutableDisplays().emplace(mDisplayToken, device);
-
DisplayDeviceState state;
- state.type = mType;
- state.isSecure = mSecure;
+ state.type = mCreationArgs.type;
+ state.isSecure = mCreationArgs.isSecure;
+
+ sp<DisplayDevice> device = new DisplayDevice(std::move(mCreationArgs));
+ mFlinger.mutableDisplays().emplace(mDisplayToken, device);
mFlinger.mutableCurrentState().displays.add(mDisplayToken, state);
mFlinger.mutableDrawingState().displays.add(mDisplayToken, state);
- if (mType >= DisplayDevice::DISPLAY_PRIMARY && mType < DisplayDevice::DISPLAY_VIRTUAL) {
- mFlinger.mutableDisplayTokens()[mType] = mDisplayToken;
+ if (state.type >= DisplayDevice::DISPLAY_PRIMARY &&
+ state.type < DisplayDevice::DISPLAY_VIRTUAL) {
+ mFlinger.mutableDisplayTokens()[state.type] = mDisplayToken;
}
return device;
@@ -395,15 +391,7 @@
private:
TestableSurfaceFlinger& mFlinger;
sp<BBinder> mDisplayToken = new BBinder();
- DisplayDevice::DisplayType mType;
- const int32_t mDisplayId;
- sp<ANativeWindow> mNativeWindow;
- sp<DisplaySurface> mDisplaySurface;
- std::unique_ptr<renderengine::Surface> mRenderSurface;
- bool mSecure = false;
- int mWidth = 0;
- int mHeight = 0;
- int mPowerMode = HWC_POWER_MODE_NORMAL;
+ DisplayDeviceCreationArgs mCreationArgs;
};
sp<SurfaceFlinger> mFlinger = new SurfaceFlinger(SurfaceFlinger::SkipInitialization);