SF: Fix color transform for composited virtual displays

ag/6455586 inadvertently changed the condition to never skip it.

Bug: 149968803
Test: Overlay display is not doubly sepia when Night Light is on.
Test: libcompositionengine_test
Change-Id: Ic33ba3069be8544ada1304461ab881b39b9fd1f9

6 files changed, 17 insertions, 26 deletions

diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index c3454059aa..308ec5ae13 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -268,12 +268,9 @@ void Display::chooseCompositionStrategy() {
 }
 
 bool Display::getSkipColorTransform() const {
-    if (!mId) {
-        return false;
-    }
-
-    auto& hwc = getCompositionEngine().getHwComposer();
-    return hwc.hasDisplayCapability(*mId, HWC2::DisplayCapability::SkipClientColorTransform);
+    const auto& hwc = getCompositionEngine().getHwComposer();
+    return mId ? hwc.hasDisplayCapability(*mId, HWC2::DisplayCapability::SkipClientColorTransform)
+               : hwc.hasCapability(HWC2::Capability::SkipClientColorTransform);
 }
 
 bool Display::anyLayersRequireClientComposition() const {
diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
index 88f2686aca..f73a6f749e 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -681,15 +681,17 @@ TEST_F(DisplayChooseCompositionStrategyTest, normalOperationWithChanges) {
 
 using DisplayGetSkipColorTransformTest = DisplayWithLayersTestCommon;
 
-TEST_F(DisplayGetSkipColorTransformTest, doesNothingIfNonHwcDisplay) {
+TEST_F(DisplayGetSkipColorTransformTest, checksCapabilityIfNonHwcDisplay) {
+    EXPECT_CALL(mHwComposer, hasCapability(HWC2::Capability::SkipClientColorTransform))
+            .WillOnce(Return(true));
     auto args = getDisplayCreationArgsForNonHWCVirtualDisplay();
     auto nonHwcDisplay{impl::createDisplay(mCompositionEngine, args)};
-    EXPECT_FALSE(nonHwcDisplay->getSkipColorTransform());
+    EXPECT_TRUE(nonHwcDisplay->getSkipColorTransform());
 }
 
-TEST_F(DisplayGetSkipColorTransformTest, checksHwcCapability) {
+TEST_F(DisplayGetSkipColorTransformTest, checksDisplayCapability) {
     EXPECT_CALL(mHwComposer,
-                hasDisplayCapability(std::make_optional(DEFAULT_DISPLAY_ID),
+                hasDisplayCapability(DEFAULT_DISPLAY_ID,
                                      HWC2::DisplayCapability::SkipClientColorTransform))
             .WillOnce(Return(true));
     EXPECT_TRUE(mDisplay->getSkipColorTransform());
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 02226ab13a..99d6cc05f9 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -40,8 +40,7 @@ public:
     MOCK_CONST_METHOD3(getDisplayIdentificationData,
                        bool(hwc2_display_t, uint8_t*, DisplayIdentificationData*));
     MOCK_CONST_METHOD1(hasCapability, bool(HWC2::Capability));
-    MOCK_CONST_METHOD2(hasDisplayCapability,
-                       bool(const std::optional<DisplayId>&, HWC2::DisplayCapability));
+    MOCK_CONST_METHOD2(hasDisplayCapability, bool(DisplayId, HWC2::DisplayCapability));
 
     MOCK_METHOD3(allocateVirtualDisplay,
                  std::optional<DisplayId>(uint32_t, uint32_t, ui::PixelFormat*));
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 1c1e113d24..593dbc8e29 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -190,17 +190,10 @@ bool HWComposer::hasCapability(HWC2::Capability capability) const {
     return mCapabilities.count(capability) > 0;
 }
 
-bool HWComposer::hasDisplayCapability(const std::optional<DisplayId>& displayId,
+bool HWComposer::hasDisplayCapability(DisplayId displayId,
                                       HWC2::DisplayCapability capability) const {
-    if (!displayId) {
-        // Checkout global capabilities for displays without a corresponding HWC display.
-        if (capability == HWC2::DisplayCapability::SkipClientColorTransform) {
-            return hasCapability(HWC2::Capability::SkipClientColorTransform);
-        }
-        return false;
-    }
-    RETURN_IF_INVALID_DISPLAY(*displayId, false);
-    return mDisplayData.at(*displayId).hwcDisplay->getCapabilities().count(capability) > 0;
+    RETURN_IF_INVALID_DISPLAY(displayId, false);
+    return mDisplayData.at(displayId).hwcDisplay->getCapabilities().count(capability) > 0;
 }
 
 std::optional<DisplayIdentificationInfo> HWComposer::onHotplug(hwc2_display_t hwcDisplayId,
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 41db501f0d..a8e6300935 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -70,7 +70,7 @@ public:
                                               DisplayIdentificationData* outData) const = 0;
 
     virtual bool hasCapability(HWC2::Capability capability) const = 0;
-    virtual bool hasDisplayCapability(const std::optional<DisplayId>& displayId,
+    virtual bool hasDisplayCapability(DisplayId displayId,
                                       HWC2::DisplayCapability capability) const = 0;
 
     // Attempts to allocate a virtual display and returns its ID if created on the HWC device.
@@ -231,7 +231,7 @@ public:
                                       DisplayIdentificationData* outData) const override;
 
     bool hasCapability(HWC2::Capability capability) const override;
-    bool hasDisplayCapability(const std::optional<DisplayId>& displayId,
+    bool hasDisplayCapability(DisplayId displayId,
                               HWC2::DisplayCapability capability) const override;
 
     // Attempts to allocate a virtual display and returns its ID if created on the HWC device.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2da92b3145..e419125048 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1199,7 +1199,7 @@ status_t SurfaceFlinger::getAutoLowLatencyModeSupport(const sp<IBinder>& display
               displayToken.get());
         return NAME_NOT_FOUND;
     }
-    *outSupport = getHwComposer().hasDisplayCapability(displayId,
+    *outSupport = getHwComposer().hasDisplayCapability(*displayId,
                                                        HWC2::DisplayCapability::AutoLowLatencyMode);
     return NO_ERROR;
 }
@@ -1470,7 +1470,7 @@ status_t SurfaceFlinger::getDisplayBrightnessSupport(const sp<IBinder>& displayT
         return NAME_NOT_FOUND;
     }
     *outSupport =
-            getHwComposer().hasDisplayCapability(displayId, HWC2::DisplayCapability::Brightness);
+            getHwComposer().hasDisplayCapability(*displayId, HWC2::DisplayCapability::Brightness);
     return NO_ERROR;
 }