diff options
author | 2023-07-21 01:25:14 +0000 | |
---|---|---|
committer | 2023-07-25 17:22:06 +0000 | |
commit | 88790f34d48e84b349014fc10b275385d5037d8e (patch) | |
tree | 5a91925400a24e3b0fbfce891172d274a0cdfe43 | |
parent | 227a7f8fd9cbc8e4105bfb8d0688aa5f37c0d9a7 (diff) |
Remove the concept of target dataspace
This is only needed for configuring colorspace agnostic dataspaces, but
no device does that. For colorspace agnostic layers, it's sufficient to
just use the colormode dataspace, with the possible exception of HDR
where we can preserve some bespoke logic, so we can just get rid of
target dataspaces and any associated plumbing.
Bug: 292162273
Test: builds
Test: libsurfaceflinger_unittest
Test: libcompositionengine_test
Change-Id: I319bb354e80e3ad1eaaacd896b897e6696f96588
20 files changed, 36 insertions, 192 deletions
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h index 09bc46747a..1a8644e9fe 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h @@ -67,9 +67,6 @@ struct CompositionRefreshArgs { // 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}; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/DisplayColorProfile.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/DisplayColorProfile.h index df44e75625..9c80cacf6f 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/DisplayColorProfile.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/DisplayColorProfile.h @@ -87,10 +87,6 @@ public: // Returns true if HWC for this profile supports the dataspace virtual bool isDataspaceSupported(ui::Dataspace) const = 0; - // Returns the target dataspace for picked color mode and dataspace - virtual ui::Dataspace getTargetDataspace(ui::ColorMode, ui::Dataspace, - ui::Dataspace colorSpaceAgnosticDataspace) const = 0; - // Debugging virtual void dump(std::string&) const = 0; }; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h index a3d86398cf..370c7cf223 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h @@ -136,7 +136,6 @@ public: ui::ColorMode mode{ui::ColorMode::NATIVE}; ui::Dataspace dataspace{ui::Dataspace::UNKNOWN}; ui::RenderIntent renderIntent{ui::RenderIntent::COLORIMETRIC}; - ui::Dataspace colorSpaceAgnosticDataspace{ui::Dataspace::UNKNOWN}; }; // Use internally to incrementally compute visibility/coverage diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/DisplayColorProfile.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/DisplayColorProfile.h index 9bc0e681c7..3aad04c1a3 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/DisplayColorProfile.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/DisplayColorProfile.h @@ -55,7 +55,6 @@ public: const HdrCapabilities& getHdrCapabilities() const override; bool isDataspaceSupported(ui::Dataspace) const override; - ui::Dataspace getTargetDataspace(ui::ColorMode, ui::Dataspace, ui::Dataspace) const override; void dump(std::string&) const override; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h index 28c6e92b06..6cb1e7e4c2 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h @@ -116,9 +116,6 @@ struct OutputCompositionState { // Current active dataspace ui::Dataspace dataspace{ui::Dataspace::UNKNOWN}; - // Current target dataspace - ui::Dataspace targetDataspace{ui::Dataspace::UNKNOWN}; - std::optional<android::HWComposer::DeviceRequestedChanges> previousDeviceRequestedChanges{}; bool previousDeviceRequestedSuccess = false; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/DisplayColorProfile.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/DisplayColorProfile.h index 1aaebea295..a9045211af 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/DisplayColorProfile.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/DisplayColorProfile.h @@ -43,8 +43,6 @@ public: MOCK_CONST_METHOD0(getHdrCapabilities, const HdrCapabilities&()); MOCK_CONST_METHOD1(isDataspaceSupported, bool(ui::Dataspace)); - MOCK_CONST_METHOD3(getTargetDataspace, - ui::Dataspace(ui::ColorMode, ui::Dataspace, ui::Dataspace)); MOCK_CONST_METHOD1(dump, void(std::string&)); }; diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp index 85fc09549b..f2acfc9e6e 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -107,14 +107,9 @@ void Display::setColorTransform(const compositionengine::CompositionRefreshArgs& } void Display::setColorProfile(const ColorProfile& colorProfile) { - const ui::Dataspace targetDataspace = - getDisplayColorProfile()->getTargetDataspace(colorProfile.mode, colorProfile.dataspace, - colorProfile.colorSpaceAgnosticDataspace); - if (colorProfile.mode == getState().colorMode && colorProfile.dataspace == getState().dataspace && - colorProfile.renderIntent == getState().renderIntent && - targetDataspace == getState().targetDataspace) { + colorProfile.renderIntent == getState().renderIntent) { return; } diff --git a/services/surfaceflinger/CompositionEngine/src/DisplayColorProfile.cpp b/services/surfaceflinger/CompositionEngine/src/DisplayColorProfile.cpp index a7c45129b1..8f67f3683a 100644 --- a/services/surfaceflinger/CompositionEngine/src/DisplayColorProfile.cpp +++ b/services/surfaceflinger/CompositionEngine/src/DisplayColorProfile.cpp @@ -391,17 +391,6 @@ bool DisplayColorProfile::isDataspaceSupported(Dataspace dataspace) const { } } -ui::Dataspace DisplayColorProfile::getTargetDataspace(ColorMode mode, Dataspace dataspace, - Dataspace colorSpaceAgnosticDataspace) const { - if (isHdrColorMode(mode)) { - return Dataspace::UNKNOWN; - } - if (colorSpaceAgnosticDataspace != ui::Dataspace::UNKNOWN) { - return colorSpaceAgnosticDataspace; - } - return dataspace; -} - void DisplayColorProfile::dump(std::string& out) const { out.append(" Composition Display Color State:"); diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 1205a2ce71..e42e27dfbb 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -261,22 +261,16 @@ void Output::setColorTransform(const compositionengine::CompositionRefreshArgs& } void Output::setColorProfile(const ColorProfile& colorProfile) { - ui::Dataspace targetDataspace = - getDisplayColorProfile()->getTargetDataspace(colorProfile.mode, colorProfile.dataspace, - colorProfile.colorSpaceAgnosticDataspace); - auto& outputState = editState(); if (outputState.colorMode == colorProfile.mode && outputState.dataspace == colorProfile.dataspace && - outputState.renderIntent == colorProfile.renderIntent && - outputState.targetDataspace == targetDataspace) { + outputState.renderIntent == colorProfile.renderIntent) { return; } outputState.colorMode = colorProfile.mode; outputState.dataspace = colorProfile.dataspace; outputState.renderIntent = colorProfile.renderIntent; - outputState.targetDataspace = targetDataspace; mRenderSurface->setBufferDataspace(colorProfile.dataspace); @@ -985,8 +979,7 @@ 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::RenderIntent::COLORIMETRIC}; } ui::Dataspace hdrDataSpace; @@ -1032,8 +1025,7 @@ compositionengine::Output::ColorProfile Output::pickColorProfile( mDisplayColorProfile->getBestColorMode(bestDataSpace, intent, &outDataSpace, &outMode, &outRenderIntent); - return ColorProfile{outMode, outDataSpace, outRenderIntent, - refreshArgs.colorSpaceAgnosticDataspace}; + return ColorProfile{outMode, outDataSpace, outRenderIntent}; } void Output::beginFrame() { diff --git a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp index 9713e79fe3..39cf67165a 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp @@ -52,7 +52,6 @@ void OutputCompositionState::dump(std::string& out) const { dumpVal(out, "colorMode", toString(colorMode), colorMode); dumpVal(out, "renderIntent", toString(renderIntent), renderIntent); dumpVal(out, "dataspace", toString(dataspace), dataspace); - dumpVal(out, "targetDataspace", toString(targetDataspace), targetDataspace); out.append("\n "); dumpVal(out, "colorTransformMatrix", colorTransformMatrix); diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp index 0ac0ecb727..b492b6a79e 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -315,9 +315,10 @@ void OutputLayer::updateCompositionState( // Determine the output dependent dataspace for this layer. If it is // colorspace agnostic, it just uses the dataspace chosen for the output to // avoid the need for color conversion. - state.dataspace = layerFEState->isColorspaceAgnostic && - outputState.targetDataspace != ui::Dataspace::UNKNOWN - ? outputState.targetDataspace + // For now, also respect the colorspace agnostic flag if we're drawing to HDR, to avoid drastic + // luminance shift. TODO(b/292162273): we should check if that's true though. + state.dataspace = layerFEState->isColorspaceAgnostic && !isHdrDataspace(outputState.dataspace) + ? outputState.dataspace : layerFEState->dataspace; // Override the dataspace transfer from 170M to sRGB if the device configuration requests this. diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayColorProfileTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayColorProfileTest.cpp index 21b9aa93ea..b3ff2ec6a0 100644 --- a/services/surfaceflinger/CompositionEngine/tests/DisplayColorProfileTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/DisplayColorProfileTest.cpp @@ -646,29 +646,5 @@ TEST_F(DisplayColorProfileTest, isDataspaceSupportedWorksForProfileWithHlgSuppor EXPECT_TRUE(profile.isDataspaceSupported(Dataspace::BT2020_ITU_HLG)); } -/* - * RenderSurface::getTargetDataspace() - */ - -TEST_F(DisplayColorProfileTest, getTargetDataspaceWorks) { - auto profile = ProfileFactory::createProfileWithNoColorModeSupport(); - - // For a non-HDR colorspace with no colorSpaceAgnosticDataspace override, - // the input dataspace should be returned. - EXPECT_EQ(Dataspace::DISPLAY_P3, - profile.getTargetDataspace(ColorMode::DISPLAY_P3, Dataspace::DISPLAY_P3, - Dataspace::UNKNOWN)); - - // If colorSpaceAgnosticDataspace is set, its value should be returned - EXPECT_EQ(Dataspace::V0_SRGB, - profile.getTargetDataspace(ColorMode::DISPLAY_P3, Dataspace::DISPLAY_P3, - Dataspace::V0_SRGB)); - - // For an HDR colorspace, Dataspace::UNKNOWN should be returned. - EXPECT_EQ(Dataspace::UNKNOWN, - profile.getTargetDataspace(ColorMode::BT2100_PQ, Dataspace::BT2020_PQ, - Dataspace::UNKNOWN)); -} - } // namespace } // namespace android::compositionengine diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp index 9be6bc2075..027004acf6 100644 --- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp @@ -403,23 +403,18 @@ TEST_F(DisplaySetColorModeTest, setsModeUnlessNoChange) { mock::DisplayColorProfile* colorProfile = new StrictMock<mock::DisplayColorProfile>(); mDisplay->setDisplayColorProfileForTest(std::unique_ptr<DisplayColorProfile>(colorProfile)); - EXPECT_CALL(*colorProfile, getTargetDataspace(_, _, _)) - .WillRepeatedly(Return(ui::Dataspace::UNKNOWN)); - // These values are expected to be the initial state. ASSERT_EQ(ui::ColorMode::NATIVE, mDisplay->getState().colorMode); ASSERT_EQ(ui::Dataspace::UNKNOWN, mDisplay->getState().dataspace); ASSERT_EQ(ui::RenderIntent::COLORIMETRIC, mDisplay->getState().renderIntent); - ASSERT_EQ(ui::Dataspace::UNKNOWN, mDisplay->getState().targetDataspace); // Otherwise if the values are unchanged, nothing happens mDisplay->setColorProfile(ColorProfile{ui::ColorMode::NATIVE, ui::Dataspace::UNKNOWN, - ui::RenderIntent::COLORIMETRIC, ui::Dataspace::UNKNOWN}); + ui::RenderIntent::COLORIMETRIC}); EXPECT_EQ(ui::ColorMode::NATIVE, mDisplay->getState().colorMode); EXPECT_EQ(ui::Dataspace::UNKNOWN, mDisplay->getState().dataspace); EXPECT_EQ(ui::RenderIntent::COLORIMETRIC, mDisplay->getState().renderIntent); - EXPECT_EQ(ui::Dataspace::UNKNOWN, mDisplay->getState().targetDataspace); // Otherwise if the values are different, updates happen EXPECT_CALL(*renderSurface, setBufferDataspace(ui::Dataspace::DISPLAY_P3)).Times(1); @@ -429,13 +424,11 @@ TEST_F(DisplaySetColorModeTest, setsModeUnlessNoChange) { .Times(1); mDisplay->setColorProfile(ColorProfile{ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3, - ui::RenderIntent::TONE_MAP_COLORIMETRIC, - ui::Dataspace::UNKNOWN}); + ui::RenderIntent::TONE_MAP_COLORIMETRIC}); EXPECT_EQ(ui::ColorMode::DISPLAY_P3, mDisplay->getState().colorMode); EXPECT_EQ(ui::Dataspace::DISPLAY_P3, mDisplay->getState().dataspace); EXPECT_EQ(ui::RenderIntent::TONE_MAP_COLORIMETRIC, mDisplay->getState().renderIntent); - EXPECT_EQ(ui::Dataspace::UNKNOWN, mDisplay->getState().targetDataspace); } TEST_F(DisplaySetColorModeTest, doesNothingForVirtualDisplay) { @@ -448,19 +441,13 @@ TEST_F(DisplaySetColorModeTest, doesNothingForVirtualDisplay) { virtualDisplay->setDisplayColorProfileForTest( std::unique_ptr<DisplayColorProfile>(colorProfile)); - EXPECT_CALL(*colorProfile, - getTargetDataspace(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3, - ui::Dataspace::UNKNOWN)) - .WillOnce(Return(ui::Dataspace::UNKNOWN)); - - virtualDisplay->setColorProfile( - ColorProfile{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}); EXPECT_EQ(ui::ColorMode::NATIVE, virtualDisplay->getState().colorMode); EXPECT_EQ(ui::Dataspace::UNKNOWN, virtualDisplay->getState().dataspace); EXPECT_EQ(ui::RenderIntent::COLORIMETRIC, virtualDisplay->getState().renderIntent); - EXPECT_EQ(ui::Dataspace::UNKNOWN, virtualDisplay->getState().targetDataspace); } /* diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp index aa83883e95..9039d16aeb 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp @@ -642,7 +642,7 @@ TEST_F(OutputLayerUpdateCompositionStateTest, TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceCorrectly) { mLayerFEState.dataspace = ui::Dataspace::DISPLAY_P3; - mOutputState.targetDataspace = ui::Dataspace::V0_SCRGB; + mOutputState.dataspace = ui::Dataspace::V0_SCRGB; // If the layer is not colorspace agnostic, the output layer dataspace // should use the layers requested colorspace. @@ -659,11 +659,18 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceCorrectly mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0); EXPECT_EQ(ui::Dataspace::V0_SCRGB, mOutputLayer.getState().dataspace); + + // If the output is HDR, then don't blind the user with a colorspace agnostic dataspace + // drawing all white + mOutputState.dataspace = ui::Dataspace::BT2020_PQ; + + mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0); + + EXPECT_EQ(ui::Dataspace::DISPLAY_P3, mOutputLayer.getState().dataspace); } TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceWith170mReplacement) { mLayerFEState.dataspace = ui::Dataspace::TRANSFER_SMPTE_170M; - mOutputState.targetDataspace = ui::Dataspace::V0_SCRGB; mOutputState.treat170mAsSrgb = false; mLayerFEState.isColorspaceAgnostic = false; diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index 9e0e7b5a53..ebf9a2b2e2 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -175,12 +175,10 @@ const Rect OutputTest::kDefaultDisplaySize{100, 200}; using ColorProfile = compositionengine::Output::ColorProfile; void dumpColorProfile(ColorProfile profile, std::string& result, const char* name) { - android::base::StringAppendF(&result, "%s (%s[%d] %s[%d] %s[%d] %s[%d]) ", name, + android::base::StringAppendF(&result, "%s (%s[%d] %s[%d] %s[%d]) ", name, toString(profile.mode).c_str(), profile.mode, toString(profile.dataspace).c_str(), profile.dataspace, - toString(profile.renderIntent).c_str(), profile.renderIntent, - toString(profile.colorSpaceAgnosticDataspace).c_str(), - profile.colorSpaceAgnosticDataspace); + toString(profile.renderIntent).c_str(), profile.renderIntent); } // Checks for a ColorProfile match @@ -192,8 +190,7 @@ MATCHER_P(ColorProfileEq, expected, "") { *result_listener << buf; return (expected.mode == arg.mode) && (expected.dataspace == arg.dataspace) && - (expected.renderIntent == arg.renderIntent) && - (expected.colorSpaceAgnosticDataspace == arg.colorSpaceAgnosticDataspace); + (expected.renderIntent == arg.renderIntent); } /* @@ -540,20 +537,14 @@ using OutputSetColorProfileTest = OutputTest; TEST_F(OutputSetColorProfileTest, setsStateAndDirtiesOutputIfChanged) { 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->setColorProfile(ColorProfile{ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3, - ui::RenderIntent::TONE_MAP_COLORIMETRIC, - ui::Dataspace::UNKNOWN}); + ui::RenderIntent::TONE_MAP_COLORIMETRIC}); EXPECT_EQ(ui::ColorMode::DISPLAY_P3, mOutput->getState().colorMode); EXPECT_EQ(ui::Dataspace::DISPLAY_P3, mOutput->getState().dataspace); EXPECT_EQ(ui::RenderIntent::TONE_MAP_COLORIMETRIC, mOutput->getState().renderIntent); - EXPECT_EQ(ui::Dataspace::UNKNOWN, mOutput->getState().targetDataspace); EXPECT_THAT(mOutput->getState().dirtyRegion, RegionEq(Region(kDefaultDisplaySize))); } @@ -561,19 +552,12 @@ TEST_F(OutputSetColorProfileTest, setsStateAndDirtiesOutputIfChanged) { TEST_F(OutputSetColorProfileTest, doesNothingIfNoChange) { using ColorProfile = Output::ColorProfile; - EXPECT_CALL(*mDisplayColorProfile, - getTargetDataspace(ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3, - ui::Dataspace::UNKNOWN)) - .WillOnce(Return(ui::Dataspace::UNKNOWN)); - mOutput->editState().colorMode = ui::ColorMode::DISPLAY_P3; mOutput->editState().dataspace = ui::Dataspace::DISPLAY_P3; mOutput->editState().renderIntent = ui::RenderIntent::TONE_MAP_COLORIMETRIC; - mOutput->editState().targetDataspace = ui::Dataspace::UNKNOWN; mOutput->setColorProfile(ColorProfile{ui::ColorMode::DISPLAY_P3, ui::Dataspace::DISPLAY_P3, - ui::RenderIntent::TONE_MAP_COLORIMETRIC, - ui::Dataspace::UNKNOWN}); + ui::RenderIntent::TONE_MAP_COLORIMETRIC}); EXPECT_THAT(mOutput->getState().dirtyRegion, RegionEq(Region())); } @@ -2133,12 +2117,11 @@ TEST_F(OutputUpdateColorProfileTest, setsAColorProfileWhenUnmanaged) { EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(3u)); EXPECT_CALL(mOutput, - setColorProfile(ColorProfileEq( - ColorProfile{ui::ColorMode::NATIVE, ui::Dataspace::UNKNOWN, - ui::RenderIntent::COLORIMETRIC, ui::Dataspace::UNKNOWN}))); + setColorProfile( + ColorProfileEq(ColorProfile{ui::ColorMode::NATIVE, ui::Dataspace::UNKNOWN, + ui::RenderIntent::COLORIMETRIC}))); mRefreshArgs.outputColorSetting = OutputColorSetting::kUnmanaged; - mRefreshArgs.colorSpaceAgnosticDataspace = ui::Dataspace::UNKNOWN; mOutput.updateColorProfile(mRefreshArgs); } @@ -2148,7 +2131,6 @@ struct OutputUpdateColorProfileTest_GetBestColorModeResultBecomesSetProfile OutputUpdateColorProfileTest_GetBestColorModeResultBecomesSetProfile() { EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(0u)); mRefreshArgs.outputColorSetting = OutputColorSetting::kEnhanced; - mRefreshArgs.colorSpaceAgnosticDataspace = ui::Dataspace::UNKNOWN; } struct ExpectBestColorModeCallResultUsedToSetColorProfileState @@ -2163,8 +2145,7 @@ struct OutputUpdateColorProfileTest_GetBestColorModeResultBecomesSetProfile SetArgPointee<4>(renderIntent))); EXPECT_CALL(getInstance()->mOutput, setColorProfile( - ColorProfileEq(ColorProfile{colorMode, dataspace, renderIntent, - ui::Dataspace::UNKNOWN}))); + ColorProfileEq(ColorProfile{colorMode, dataspace, renderIntent}))); return nextState<ExecuteState>(); } }; @@ -2191,55 +2172,6 @@ TEST_F(OutputUpdateColorProfileTest_GetBestColorModeResultBecomesSetProfile, .execute(); } -struct OutputUpdateColorProfileTest_ColorSpaceAgnosticeDataspaceAffectsSetColorProfile - : public OutputUpdateColorProfileTest { - OutputUpdateColorProfileTest_ColorSpaceAgnosticeDataspaceAffectsSetColorProfile() { - EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(0u)); - EXPECT_CALL(*mDisplayColorProfile, - getBestColorMode(ui::Dataspace::V0_SRGB, ui::RenderIntent::ENHANCE, _, _, _)) - .WillRepeatedly(DoAll(SetArgPointee<2>(ui::Dataspace::UNKNOWN), - SetArgPointee<3>(ui::ColorMode::NATIVE), - SetArgPointee<4>(ui::RenderIntent::COLORIMETRIC))); - mRefreshArgs.outputColorSetting = OutputColorSetting::kEnhanced; - } - - struct IfColorSpaceAgnosticDataspaceSetToState - : public CallOrderStateMachineHelper<TestType, IfColorSpaceAgnosticDataspaceSetToState> { - [[nodiscard]] auto ifColorSpaceAgnosticDataspaceSetTo(ui::Dataspace dataspace) { - getInstance()->mRefreshArgs.colorSpaceAgnosticDataspace = dataspace; - return nextState<ThenExpectSetColorProfileCallUsesColorSpaceAgnosticDataspaceState>(); - } - }; - - struct ThenExpectSetColorProfileCallUsesColorSpaceAgnosticDataspaceState - : public CallOrderStateMachineHelper< - TestType, ThenExpectSetColorProfileCallUsesColorSpaceAgnosticDataspaceState> { - [[nodiscard]] auto thenExpectSetColorProfileCallUsesColorSpaceAgnosticDataspace( - ui::Dataspace dataspace) { - EXPECT_CALL(getInstance()->mOutput, - setColorProfile(ColorProfileEq( - ColorProfile{ui::ColorMode::NATIVE, ui::Dataspace::UNKNOWN, - ui::RenderIntent::COLORIMETRIC, dataspace}))); - return nextState<ExecuteState>(); - } - }; - - // Call this member function to start using the mini-DSL defined above. - [[nodiscard]] auto verify() { return IfColorSpaceAgnosticDataspaceSetToState::make(this); } -}; - -TEST_F(OutputUpdateColorProfileTest_ColorSpaceAgnosticeDataspaceAffectsSetColorProfile, DisplayP3) { - verify().ifColorSpaceAgnosticDataspaceSetTo(ui::Dataspace::DISPLAY_P3) - .thenExpectSetColorProfileCallUsesColorSpaceAgnosticDataspace(ui::Dataspace::DISPLAY_P3) - .execute(); -} - -TEST_F(OutputUpdateColorProfileTest_ColorSpaceAgnosticeDataspaceAffectsSetColorProfile, V0_SRGB) { - verify().ifColorSpaceAgnosticDataspaceSetTo(ui::Dataspace::V0_SRGB) - .thenExpectSetColorProfileCallUsesColorSpaceAgnosticDataspace(ui::Dataspace::V0_SRGB) - .execute(); -} - struct OutputUpdateColorProfileTest_TopmostLayerPreferenceSetsOutputPreference : public OutputUpdateColorProfileTest { // Internally the implementation looks through the dataspaces of all the @@ -2248,7 +2180,6 @@ struct OutputUpdateColorProfileTest_TopmostLayerPreferenceSetsOutputPreference OutputUpdateColorProfileTest_TopmostLayerPreferenceSetsOutputPreference() { mRefreshArgs.outputColorSetting = OutputColorSetting::kEnhanced; - mRefreshArgs.colorSpaceAgnosticDataspace = ui::Dataspace::UNKNOWN; EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(3u)); EXPECT_CALL(mOutput, setColorProfile(_)).WillRepeatedly(Return()); @@ -2368,7 +2299,6 @@ struct OutputUpdateColorProfileTest_ForceOutputColorOverrides OutputUpdateColorProfileTest_ForceOutputColorOverrides() { mRefreshArgs.outputColorSetting = OutputColorSetting::kEnhanced; - mRefreshArgs.colorSpaceAgnosticDataspace = ui::Dataspace::UNKNOWN; mLayer1.mLayerFEState.dataspace = ui::Dataspace::DISPLAY_BT2020; @@ -2424,7 +2354,6 @@ TEST_F(OutputUpdateColorProfileTest_ForceOutputColorOverrides, DisplayP3_Overrid struct OutputUpdateColorProfileTest_Hdr : public OutputUpdateColorProfileTest { OutputUpdateColorProfileTest_Hdr() { mRefreshArgs.outputColorSetting = OutputColorSetting::kEnhanced; - mRefreshArgs.colorSpaceAgnosticDataspace = ui::Dataspace::UNKNOWN; EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(2u)); EXPECT_CALL(mOutput, setColorProfile(_)).WillRepeatedly(Return()); } @@ -2703,7 +2632,6 @@ struct OutputUpdateColorProfile_AffectsChosenRenderIntentTest OutputUpdateColorProfile_AffectsChosenRenderIntentTest() { mRefreshArgs.outputColorSetting = OutputColorSetting::kEnhanced; - mRefreshArgs.colorSpaceAgnosticDataspace = ui::Dataspace::UNKNOWN; mLayer1.mLayerFEState.dataspace = ui::Dataspace::BT2020_PQ; EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(1u)); EXPECT_CALL(mOutput, setColorProfile(_)).WillRepeatedly(Return()); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 198e92abaf..b1c6ccc689 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -406,9 +406,6 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI wideColorGamutCompositionPixelFormat = static_cast<ui::PixelFormat>(wcg_composition_pixel_format(ui::PixelFormat::RGBA_8888)); - mColorSpaceAgnosticDataspace = - static_cast<ui::Dataspace>(color_space_agnostic_dataspace(Dataspace::UNKNOWN)); - mLayerCachingEnabled = [] { const bool enable = android::sysprop::SurfaceFlingerProperties::enable_layer_caching().value_or(false); @@ -1526,7 +1523,7 @@ status_t SurfaceFlinger::setActiveColorMode(const sp<IBinder>& displayToken, ui: } display->getCompositionDisplay()->setColorProfile( - {mode, Dataspace::UNKNOWN, RenderIntent::COLORIMETRIC, Dataspace::UNKNOWN}); + {mode, Dataspace::UNKNOWN, RenderIntent::COLORIMETRIC}); return NO_ERROR; }); @@ -2609,7 +2606,6 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( refreshArgs.outputColorSetting = useColorManagement ? mDisplayColorSetting : compositionengine::OutputColorSetting::kUnmanaged; - refreshArgs.colorSpaceAgnosticDataspace = mColorSpaceAgnosticDataspace; refreshArgs.forceOutputColorMode = mForceColorMode; refreshArgs.updatingOutputGeometryThisFrame = mVisibleRegionsDirty; @@ -3419,8 +3415,7 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal( } display->getCompositionDisplay()->setColorProfile( compositionengine::Output::ColorProfile{defaultColorMode, defaultDataSpace, - RenderIntent::COLORIMETRIC, - Dataspace::UNKNOWN}); + RenderIntent::COLORIMETRIC}); if (const auto& physical = state.physical) { mPhysicalDisplays.get(physical->id) diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 3d46239759..85b1422faa 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1302,7 +1302,6 @@ private: ui::Dataspace mDefaultCompositionDataspace; ui::Dataspace mWideColorGamutCompositionDataspace; - ui::Dataspace mColorSpaceAgnosticDataspace; std::unique_ptr<renderengine::RenderEngine> mRenderEngine; std::atomic<int> mNumTrustedPresentationListeners = 0; diff --git a/services/surfaceflinger/SurfaceFlingerProperties.cpp b/services/surfaceflinger/SurfaceFlingerProperties.cpp index 96c8b54005..66c8f33fe5 100644 --- a/services/surfaceflinger/SurfaceFlingerProperties.cpp +++ b/services/surfaceflinger/SurfaceFlingerProperties.cpp @@ -227,14 +227,6 @@ int32_t wcg_composition_pixel_format(PixelFormat defaultValue) { return static_cast<int32_t>(defaultValue); } -int64_t color_space_agnostic_dataspace(Dataspace defaultValue) { - auto temp = SurfaceFlingerProperties::color_space_agnostic_dataspace(); - if (temp.has_value()) { - return *temp; - } - return static_cast<int64_t>(defaultValue); -} - bool refresh_rate_switching(bool defaultValue) { #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" diff --git a/services/surfaceflinger/SurfaceFlingerProperties.h b/services/surfaceflinger/SurfaceFlingerProperties.h index 951f8f8cb3..a08042009b 100644 --- a/services/surfaceflinger/SurfaceFlingerProperties.h +++ b/services/surfaceflinger/SurfaceFlingerProperties.h @@ -71,9 +71,6 @@ int64_t wcg_composition_dataspace( int32_t wcg_composition_pixel_format( android::hardware::graphics::common::V1_2::PixelFormat defaultValue); -int64_t color_space_agnostic_dataspace( - android::hardware::graphics::common::V1_2::Dataspace defaultValue); - bool refresh_rate_switching(bool defaultValue); int32_t set_idle_timer_ms(int32_t defaultValue); diff --git a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop index 5cac086c21..be29be4ef4 100644 --- a/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop +++ b/services/surfaceflinger/sysprop/SurfaceFlingerProperties.sysprop @@ -199,6 +199,7 @@ prop { # useColorManagement indicates whether SurfaceFlinger should manage color # by switching to appropriate color mode automatically depending on the # Dataspace of the surfaces on screen. +# DEPRECATED: SurfaceFlinger is always color managed. prop { api_name: "use_color_management" type: Boolean @@ -207,7 +208,7 @@ prop { prop_name: "ro.surface_flinger.use_color_management" } -# The following four propertiess define: +# The following four properties define: # Returns the default data space and pixel format that SurfaceFlinger # expects to receive and output as well as the wide color gamut data space # and pixel format for wide color gamut surfaces. @@ -276,7 +277,7 @@ prop { # The variable works only when useColorManagement is specified. If # unspecified, the data space follows what SurfaceFlinger expects for # surfaces when useColorManagement is specified. - +# DEPRECATED: do not use prop { api_name: "color_space_agnostic_dataspace" type: Long |