SF: Move pickColorMode and getBestDataspace to CompositionEngine
Test: atest libsurfaceflinger_unittest libcompositionengine_test
Bug: 121291683
Change-Id: I37c39f49bf0e1c851d0d5e4040dc3aa3e433e9f8
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
index b329f76..1001cf7 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -21,6 +21,7 @@
#include <compositionengine/Display.h>
#include <compositionengine/Layer.h>
+#include <compositionengine/OutputColorSetting.h>
namespace android::compositionengine {
@@ -42,6 +43,15 @@
// If true, forces the entire display to be considered dirty and repainted
bool repaintEverything{false};
+ // Controls how the color mode is chosen for an output
+ OutputColorSetting outputColorSetting{OutputColorSetting::kEnhanced};
+
+ // If not Dataspace::UNKNOWN, overrides the dataspace on each output
+ ui::Dataspace colorSpaceAgnosticDataspace{ui::Dataspace::UNKNOWN};
+
+ // Forces a color mode on the outputs being refreshed
+ ui::ColorMode forceOutputColorMode{ui::ColorMode::NATIVE};
+
// If set, causes the dirty regions to flash with the delay
std::optional<std::chrono::microseconds> devOptFlashDirtyRegionsDelay;
};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
index fa2bb46..82cde4b 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h
@@ -63,6 +63,13 @@
std::unordered_map<HWC2::Layer*, sp<Fence>> layerFences;
};
+ struct ColorProfile {
+ ui::ColorMode mode{ui::ColorMode::NATIVE};
+ ui::Dataspace dataspace{ui::Dataspace::UNKNOWN};
+ ui::RenderIntent renderIntent{ui::RenderIntent::COLORIMETRIC};
+ ui::Dataspace colorSpaceAgnosticDataspace{ui::Dataspace::UNKNOWN};
+ };
+
virtual ~Output();
// Returns true if the output is valid. This is meant to be checked post-
@@ -87,8 +94,7 @@
virtual void setColorTransform(const mat4&) = 0;
// Sets the output color mode
- virtual void setColorMode(ui::ColorMode, ui::Dataspace, ui::RenderIntent,
- ui::Dataspace colorSpaceAgnosticDataspace) = 0;
+ virtual void setColorProfile(const ColorProfile&) = 0;
// Outputs a string with a state dump
virtual void dump(std::string&) const = 0;
@@ -153,6 +159,9 @@
// Takes (moves) the set of layers being released this frame.
virtual ReleasedLayers takeReleasedLayers() = 0;
+ // Updates the color mode used on this output
+ virtual void updateColorProfile(const CompositionRefreshArgs&) = 0;
+
// Signals that a frame is beginning on the output
virtual void beginFrame() = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputColorSetting.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputColorSetting.h
new file mode 100644
index 0000000..6e798ce
--- /dev/null
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputColorSetting.h
@@ -0,0 +1,27 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+namespace android::compositionengine {
+
+enum class OutputColorSetting : int32_t {
+ kManaged = 0,
+ kUnmanaged = 1,
+ kEnhanced = 2,
+};
+
+} // namespace android::compositionengine
\ No newline at end of file
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
index bdc1291..dba112eb 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h
@@ -41,7 +41,7 @@
// compositionengine::Output overrides
void dump(std::string&) const override;
void setColorTransform(const mat4&) override;
- void setColorMode(ui::ColorMode, ui::Dataspace, ui::RenderIntent, ui::Dataspace) override;
+ void setColorProfile(const ColorProfile&) override;
void chooseCompositionStrategy() override;
bool getSkipColorTransform() const override;
compositionengine::Output::FrameFences presentAndGetFrameFences() override;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
index b854314..904edc7 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h
@@ -45,7 +45,7 @@
void setLayerStackFilter(uint32_t layerStackId, bool isInternal) override;
void setColorTransform(const mat4&) override;
- void setColorMode(ui::ColorMode, ui::Dataspace, ui::RenderIntent, ui::Dataspace) override;
+ void setColorProfile(const ColorProfile&) override;
void dump(std::string&) const override;
@@ -75,6 +75,8 @@
void setReleasedLayers(ReleasedLayers&&) override;
ReleasedLayers takeReleasedLayers() override;
+ void updateColorProfile(const compositionengine::CompositionRefreshArgs&) override;
+
void beginFrame() override;
void prepareFrame() override;
void devOptRepaintFlash(const compositionengine::CompositionRefreshArgs&) override;
@@ -100,6 +102,9 @@
private:
void dirtyEntireOutput();
+ ui::Dataspace getBestDataspace(ui::Dataspace*, bool*) const;
+ compositionengine::Output::ColorProfile pickColorProfile(
+ const compositionengine::CompositionRefreshArgs&) const;
const CompositionEngine& mCompositionEngine;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
index ade745c..44cd5e9 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h
@@ -42,7 +42,7 @@
MOCK_METHOD2(setLayerStackFilter, void(uint32_t, bool));
MOCK_METHOD1(setColorTransform, void(const mat4&));
- MOCK_METHOD4(setColorMode, void(ui::ColorMode, ui::Dataspace, ui::RenderIntent, ui::Dataspace));
+ MOCK_METHOD1(setColorProfile, void(const ColorProfile&));
MOCK_CONST_METHOD1(dump, void(std::string&));
MOCK_CONST_METHOD0(getName, const std::string&());
@@ -74,6 +74,8 @@
MOCK_METHOD1(setReleasedLayers, void(ReleasedLayers&&));
MOCK_METHOD0(takeReleasedLayers, ReleasedLayers());
+ MOCK_METHOD1(updateColorProfile, void(const compositionengine::CompositionRefreshArgs&));
+
MOCK_METHOD0(beginFrame, void());
MOCK_METHOD0(prepareFrame, void());
MOCK_METHOD0(chooseCompositionStrategy, void());
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index cabbc08..108720a 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -78,15 +78,15 @@
mId ? to_string(*mId).c_str() : "", result);
}
-void Display::setColorMode(ui::ColorMode mode, ui::Dataspace dataspace,
- ui::RenderIntent renderIntent,
- ui::Dataspace colorSpaceAgnosticDataspace) {
- ui::Dataspace targetDataspace =
- getDisplayColorProfile()->getTargetDataspace(mode, dataspace,
- colorSpaceAgnosticDataspace);
+void Display::setColorProfile(const ColorProfile& colorProfile) {
+ const ui::Dataspace targetDataspace =
+ getDisplayColorProfile()->getTargetDataspace(colorProfile.mode, colorProfile.dataspace,
+ colorProfile.colorSpaceAgnosticDataspace);
- if (mode == getState().colorMode && dataspace == getState().dataspace &&
- renderIntent == getState().renderIntent && targetDataspace == getState().targetDataspace) {
+ if (colorProfile.mode == getState().colorMode &&
+ colorProfile.dataspace == getState().dataspace &&
+ colorProfile.renderIntent == getState().renderIntent &&
+ targetDataspace == getState().targetDataspace) {
return;
}
@@ -95,10 +95,10 @@
return;
}
- Output::setColorMode(mode, dataspace, renderIntent, colorSpaceAgnosticDataspace);
+ Output::setColorProfile(colorProfile);
auto& hwc = getCompositionEngine().getHwComposer();
- hwc.setActiveColorMode(*mId, mode, renderIntent);
+ hwc.setActiveColorMode(*mId, colorProfile.mode, colorProfile.renderIntent);
}
void Display::dump(std::string& out) const {
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 8b156f8..cc7e453 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -114,28 +114,27 @@
dirtyEntireOutput();
}
-void Output::setColorMode(ui::ColorMode mode, ui::Dataspace dataspace,
- ui::RenderIntent renderIntent,
- ui::Dataspace colorSpaceAgnosticDataspace) {
- ui::Dataspace targetDataspace =
- getDisplayColorProfile()->getTargetDataspace(mode, dataspace,
- colorSpaceAgnosticDataspace);
+void Output::setColorProfile(const ColorProfile& colorProfile) {
+ const ui::Dataspace targetDataspace =
+ getDisplayColorProfile()->getTargetDataspace(colorProfile.mode, colorProfile.dataspace,
+ colorProfile.colorSpaceAgnosticDataspace);
- if (mState.colorMode == mode && mState.dataspace == dataspace &&
- mState.renderIntent == renderIntent && mState.targetDataspace == targetDataspace) {
+ if (mState.colorMode == colorProfile.mode && mState.dataspace == colorProfile.dataspace &&
+ mState.renderIntent == colorProfile.renderIntent &&
+ mState.targetDataspace == targetDataspace) {
return;
}
- mState.colorMode = mode;
- mState.dataspace = dataspace;
- mState.renderIntent = renderIntent;
+ mState.colorMode = colorProfile.mode;
+ mState.dataspace = colorProfile.dataspace;
+ mState.renderIntent = colorProfile.renderIntent;
mState.targetDataspace = targetDataspace;
- mRenderSurface->setBufferDataspace(dataspace);
+ mRenderSurface->setBufferDataspace(colorProfile.dataspace);
ALOGV("Set active color mode: %s (%d), active render intent: %s (%d)",
- decodeColorMode(mode).c_str(), mode, decodeRenderIntent(renderIntent).c_str(),
- renderIntent);
+ decodeColorMode(colorProfile.mode).c_str(), colorProfile.mode,
+ decodeRenderIntent(colorProfile.renderIntent).c_str(), colorProfile.renderIntent);
dirtyEntireOutput();
}
@@ -261,6 +260,116 @@
return std::move(mReleasedLayers);
}
+void Output::updateColorProfile(const compositionengine::CompositionRefreshArgs& refreshArgs) {
+ setColorProfile(pickColorProfile(refreshArgs));
+}
+
+// Returns a data space that fits all visible layers. The returned data space
+// can only be one of
+// - Dataspace::SRGB (use legacy dataspace and let HWC saturate when colors are enhanced)
+// - Dataspace::DISPLAY_P3
+// - Dataspace::DISPLAY_BT2020
+// The returned HDR data space is one of
+// - Dataspace::UNKNOWN
+// - Dataspace::BT2020_HLG
+// - Dataspace::BT2020_PQ
+ui::Dataspace Output::getBestDataspace(ui::Dataspace* outHdrDataSpace,
+ bool* outIsHdrClientComposition) const {
+ ui::Dataspace bestDataSpace = ui::Dataspace::V0_SRGB;
+ *outHdrDataSpace = ui::Dataspace::UNKNOWN;
+
+ for (const auto& layer : mOutputLayersOrderedByZ) {
+ switch (layer->getLayer().getState().frontEnd.dataspace) {
+ case ui::Dataspace::V0_SCRGB:
+ case ui::Dataspace::V0_SCRGB_LINEAR:
+ case ui::Dataspace::BT2020:
+ case ui::Dataspace::BT2020_ITU:
+ case ui::Dataspace::BT2020_LINEAR:
+ case ui::Dataspace::DISPLAY_BT2020:
+ bestDataSpace = ui::Dataspace::DISPLAY_BT2020;
+ break;
+ case ui::Dataspace::DISPLAY_P3:
+ bestDataSpace = ui::Dataspace::DISPLAY_P3;
+ break;
+ case ui::Dataspace::BT2020_PQ:
+ case ui::Dataspace::BT2020_ITU_PQ:
+ bestDataSpace = ui::Dataspace::DISPLAY_P3;
+ *outHdrDataSpace = ui::Dataspace::BT2020_PQ;
+ *outIsHdrClientComposition =
+ layer->getLayer().getState().frontEnd.forceClientComposition;
+ break;
+ case ui::Dataspace::BT2020_HLG:
+ case ui::Dataspace::BT2020_ITU_HLG:
+ bestDataSpace = ui::Dataspace::DISPLAY_P3;
+ // When there's mixed PQ content and HLG content, we set the HDR
+ // data space to be BT2020_PQ and convert HLG to PQ.
+ if (*outHdrDataSpace == ui::Dataspace::UNKNOWN) {
+ *outHdrDataSpace = ui::Dataspace::BT2020_HLG;
+ }
+ break;
+ default:
+ break;
+ }
+ }
+
+ return bestDataSpace;
+}
+
+compositionengine::Output::ColorProfile Output::pickColorProfile(
+ const compositionengine::CompositionRefreshArgs& refreshArgs) const {
+ if (refreshArgs.outputColorSetting == OutputColorSetting::kUnmanaged) {
+ return ColorProfile{ui::ColorMode::NATIVE, ui::Dataspace::UNKNOWN,
+ ui::RenderIntent::COLORIMETRIC,
+ refreshArgs.colorSpaceAgnosticDataspace};
+ }
+
+ ui::Dataspace hdrDataSpace;
+ bool isHdrClientComposition = false;
+ ui::Dataspace bestDataSpace = getBestDataspace(&hdrDataSpace, &isHdrClientComposition);
+
+ switch (refreshArgs.forceOutputColorMode) {
+ case ui::ColorMode::SRGB:
+ bestDataSpace = ui::Dataspace::V0_SRGB;
+ break;
+ case ui::ColorMode::DISPLAY_P3:
+ bestDataSpace = ui::Dataspace::DISPLAY_P3;
+ break;
+ default:
+ break;
+ }
+
+ // respect hdrDataSpace only when there is no legacy HDR support
+ const bool isHdr = hdrDataSpace != ui::Dataspace::UNKNOWN &&
+ !mDisplayColorProfile->hasLegacyHdrSupport(hdrDataSpace) && !isHdrClientComposition;
+ if (isHdr) {
+ bestDataSpace = hdrDataSpace;
+ }
+
+ ui::RenderIntent intent;
+ switch (refreshArgs.outputColorSetting) {
+ case OutputColorSetting::kManaged:
+ case OutputColorSetting::kUnmanaged:
+ intent = isHdr ? ui::RenderIntent::TONE_MAP_COLORIMETRIC
+ : ui::RenderIntent::COLORIMETRIC;
+ break;
+ case OutputColorSetting::kEnhanced:
+ intent = isHdr ? ui::RenderIntent::TONE_MAP_ENHANCE : ui::RenderIntent::ENHANCE;
+ break;
+ default: // vendor display color setting
+ intent = static_cast<ui::RenderIntent>(refreshArgs.outputColorSetting);
+ break;
+ }
+
+ ui::ColorMode outMode;
+ ui::Dataspace outDataSpace;
+ ui::RenderIntent outRenderIntent;
+ mDisplayColorProfile->getBestColorMode(bestDataSpace, intent, &outDataSpace, &outMode,
+ &outRenderIntent);
+
+ return ColorProfile{outMode, outDataSpace, outRenderIntent,
+ refreshArgs.colorSpaceAgnosticDataspace};
+}
+
void Output::beginFrame() {
const bool dirty = !getDirtyRegion(false).isEmpty();
const bool empty = mOutputLayersOrderedByZ.empty();
diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
index e975260..06e3a70 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -162,6 +162,8 @@
*/
TEST_F(DisplayTest, setColorModeSetsModeUnlessNoChange) {
+ using ColorProfile = Output::ColorProfile;
+
mock::RenderSurface* renderSurface = new StrictMock<mock::RenderSurface>();
mDisplay.setRenderSurfaceForTest(std::unique_ptr<RenderSurface>(renderSurface));
mock::DisplayColorProfile* colorProfile = new StrictMock<mock::DisplayColorProfile>();
@@ -177,8 +179,8 @@
ASSERT_EQ(ui::Dataspace::UNKNOWN, mDisplay.getState().targetDataspace);
// If the set values are unchanged, nothing happens
- mDisplay.setColorMode(ui::ColorMode::NATIVE, ui::Dataspace::UNKNOWN,
- ui::RenderIntent::COLORIMETRIC, ui::Dataspace::UNKNOWN);
+ mDisplay.setColorProfile(ColorProfile{ui::ColorMode::NATIVE, ui::Dataspace::UNKNOWN,
+ ui::RenderIntent::COLORIMETRIC, ui::Dataspace::UNKNOWN});
EXPECT_EQ(ui::ColorMode::NATIVE, mDisplay.getState().colorMode);
EXPECT_EQ(ui::Dataspace::UNKNOWN, mDisplay.getState().dataspace);
@@ -192,8 +194,9 @@
ui::RenderIntent::TONE_MAP_COLORIMETRIC))
.Times(1);
- mDisplay.setColorMode(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
- ui::RenderIntent::TONE_MAP_COLORIMETRIC, ui::Dataspace::UNKNOWN);
+ mDisplay.setColorProfile(ColorProfile{ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
+ ui::RenderIntent::TONE_MAP_COLORIMETRIC,
+ ui::Dataspace::UNKNOWN});
EXPECT_EQ(ui::ColorMode::DISPLAY_P3, mDisplay.getState().colorMode);
EXPECT_EQ(ui::Dataspace::DISPLAY_P3, mDisplay.getState().dataspace);
@@ -202,6 +205,8 @@
}
TEST_F(DisplayTest, setColorModeDoesNothingForVirtualDisplay) {
+ using ColorProfile = Output::ColorProfile;
+
impl::Display virtualDisplay{mCompositionEngine,
DisplayCreationArgs{false, true, DEFAULT_DISPLAY_ID}};
@@ -214,8 +219,9 @@
ui::Dataspace::UNKNOWN))
.WillOnce(Return(ui::Dataspace::UNKNOWN));
- virtualDisplay.setColorMode(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
- ui::RenderIntent::TONE_MAP_COLORIMETRIC, ui::Dataspace::UNKNOWN);
+ virtualDisplay.setColorProfile(
+ ColorProfile{ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
+ ui::RenderIntent::TONE_MAP_COLORIMETRIC, ui::Dataspace::UNKNOWN});
EXPECT_EQ(ui::ColorMode::NATIVE, virtualDisplay.getState().colorMode);
EXPECT_EQ(ui::Dataspace::UNKNOWN, virtualDisplay.getState().dataspace);
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index eddb67f..dccad58 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -211,14 +211,17 @@
*/
TEST_F(OutputTest, setColorModeSetsStateAndDirtiesOutputIfChanged) {
+ using ColorProfile = Output::ColorProfile;
+
EXPECT_CALL(*mDisplayColorProfile,
getTargetDataspace(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
ui::Dataspace::UNKNOWN))
.WillOnce(Return(ui::Dataspace::UNKNOWN));
EXPECT_CALL(*mRenderSurface, setBufferDataspace(ui::Dataspace::DISPLAY_P3)).Times(1);
- mOutput.setColorMode(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
- ui::RenderIntent::TONE_MAP_COLORIMETRIC, ui::Dataspace::UNKNOWN);
+ mOutput.setColorProfile(ColorProfile{ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
+ ui::RenderIntent::TONE_MAP_COLORIMETRIC,
+ ui::Dataspace::UNKNOWN});
EXPECT_EQ(ui::ColorMode::DISPLAY_P3, mOutput.getState().colorMode);
EXPECT_EQ(ui::Dataspace::DISPLAY_P3, mOutput.getState().dataspace);
@@ -229,6 +232,8 @@
}
TEST_F(OutputTest, setColorModeDoesNothingIfNoChange) {
+ using ColorProfile = Output::ColorProfile;
+
EXPECT_CALL(*mDisplayColorProfile,
getTargetDataspace(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
ui::Dataspace::UNKNOWN))
@@ -239,8 +244,9 @@
mOutput.editState().renderIntent = ui::RenderIntent::TONE_MAP_COLORIMETRIC;
mOutput.editState().targetDataspace = ui::Dataspace::UNKNOWN;
- mOutput.setColorMode(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
- ui::RenderIntent::TONE_MAP_COLORIMETRIC, ui::Dataspace::UNKNOWN);
+ mOutput.setColorProfile(ColorProfile{ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3,
+ ui::RenderIntent::TONE_MAP_COLORIMETRIC,
+ ui::Dataspace::UNKNOWN});
EXPECT_THAT(mOutput.getState().dirtyRegion, RegionEq(Region()));
}