surfaceflinger: layer dataspace is not a state

Like current crop or transform, layer dataspace is latched with the
buffer.  It is not a layer state, and should take effect
immediately.

Bug: 80139629
Test: manual
Change-Id: I52342ca345f174e647550b45defa977484db95a1
Merged-In: I52342ca345f174e647550b45defa977484db95a1

5 files changed, 23 insertions, 28 deletions

diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index 6feec53257..17ed9aa00c 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -553,7 +553,7 @@ Region BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime
         default:
             break;
     }
-    setDataSpace(dataSpace);
+    mCurrentDataSpace = dataSpace;
 
     Rect crop(mConsumer->getCurrentCrop());
     const uint32_t transform(mConsumer->getCurrentTransform());
@@ -660,10 +660,10 @@ void BufferLayer::setPerFrameData(const sp<const DisplayDevice>& displayDevice)
         setCompositionType(hwcId, HWC2::Composition::Device);
     }
 
-    ALOGV("setPerFrameData: dataspace = %d", mDrawingState.dataSpace);
-    error = hwcLayer->setDataspace(mDrawingState.dataSpace);
+    ALOGV("setPerFrameData: dataspace = %d", mCurrentDataSpace);
+    error = hwcLayer->setDataspace(mCurrentDataSpace);
     if (error != HWC2::Error::None) {
-        ALOGE("[%s] Failed to set dataspace %d: %s (%d)", mName.string(), mDrawingState.dataSpace,
+        ALOGE("[%s] Failed to set dataspace %d: %s (%d)", mName.string(), mCurrentDataSpace,
               to_string(error).c_str(), static_cast<int32_t>(error));
     }
 
@@ -866,9 +866,9 @@ void BufferLayer::drawWithOpenGL(const RenderArea& renderArea, bool useIdentityT
     auto& engine(mFlinger->getRenderEngine());
     engine.setupLayerBlending(mPremultipliedAlpha, isOpaque(s), false /* disableTexture */,
                               getColor());
-    engine.setSourceDataSpace(mCurrentState.dataSpace);
+    engine.setSourceDataSpace(mCurrentDataSpace);
 
-    if (mCurrentState.dataSpace == ui::Dataspace::BT2020_ITU_PQ &&
+    if (mCurrentDataSpace == ui::Dataspace::BT2020_ITU_PQ &&
         mConsumer->getCurrentApi() == NATIVE_WINDOW_API_MEDIA &&
         getBE().compositionInfo.mBuffer->getPixelFormat() == HAL_PIXEL_FORMAT_RGBA_1010102) {
         engine.setSourceY410BT2020(true);
diff --git a/services/surfaceflinger/ColorLayer.cpp b/services/surfaceflinger/ColorLayer.cpp
index c87b66981d..512564c2db 100644
--- a/services/surfaceflinger/ColorLayer.cpp
+++ b/services/surfaceflinger/ColorLayer.cpp
@@ -77,9 +77,9 @@ void ColorLayer::setPerFrameData(const sp<const DisplayDevice>& displayDevice) {
 
     setCompositionType(hwcId, HWC2::Composition::SolidColor);
 
-    error = hwcLayer->setDataspace(mDrawingState.dataSpace);
+    error = hwcLayer->setDataspace(mCurrentDataSpace);
     if (error != HWC2::Error::None) {
-        ALOGE("[%s] Failed to set dataspace %d: %s (%d)", mName.string(), mDrawingState.dataSpace,
+        ALOGE("[%s] Failed to set dataspace %d: %s (%d)", mName.string(), mCurrentDataSpace,
               to_string(error).c_str(), static_cast<int32_t>(error));
     }
 
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 2077598371..b94af77967 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -123,7 +123,6 @@ Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client, const String8& n
     mCurrentState.layerStack = 0;
     mCurrentState.sequence = 0;
     mCurrentState.requested = mCurrentState.active;
-    mCurrentState.dataSpace = ui::Dataspace::UNKNOWN;
     mCurrentState.appId = 0;
     mCurrentState.type = 0;
 
@@ -1328,15 +1327,6 @@ bool Layer::setLayerStack(uint32_t layerStack) {
     return true;
 }
 
-bool Layer::setDataSpace(ui::Dataspace dataSpace) {
-    if (mCurrentState.dataSpace == dataSpace) return false;
-    mCurrentState.sequence++;
-    mCurrentState.dataSpace = dataSpace;
-    mCurrentState.modified = true;
-    setTransactionFlags(eTransactionNeeded);
-    return true;
-}
-
 uint32_t Layer::getLayerStack() const {
     auto p = mDrawingParent.promote();
     if (p == nullptr) {
@@ -1429,7 +1419,7 @@ LayerDebugInfo Layer::getLayerDebugInfo() const {
     info.mColor = ds.color;
     info.mFlags = ds.flags;
     info.mPixelFormat = getPixelFormat();
-    info.mDataSpace = static_cast<android_dataspace>(ds.dataSpace);
+    info.mDataSpace = static_cast<android_dataspace>(mCurrentDataSpace);
     info.mMatrix[0][0] = ds.active.transform[0][0];
     info.mMatrix[0][1] = ds.active.transform[0][1];
     info.mMatrix[1][0] = ds.active.transform[1][0];
@@ -1648,7 +1638,7 @@ bool Layer::detachChildren() {
 
 bool Layer::isLegacyDataSpace() const {
     // return true when no higher bits are set
-    return !(mDrawingState.dataSpace & (ui::Dataspace::STANDARD_MASK |
+    return !(mCurrentDataSpace & (ui::Dataspace::STANDARD_MASK |
                 ui::Dataspace::TRANSFER_MASK | ui::Dataspace::RANGE_MASK));
 }
 
@@ -1956,7 +1946,10 @@ void Layer::writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet)
 
     layerInfo->set_is_opaque(isOpaque(state));
     layerInfo->set_invalidate(contentDirty);
-    layerInfo->set_dataspace(dataspaceDetails(static_cast<android_dataspace>(state.dataSpace)));
+
+    // XXX (b/79210409) mCurrentDataSpace is not protected
+    layerInfo->set_dataspace(dataspaceDetails(static_cast<android_dataspace>(mCurrentDataSpace)));
+
     layerInfo->set_pixel_format(decodePixelFormat(getPixelFormat()));
     LayerProtoHelper::writeToProto(getColor(), layerInfo->mutable_color());
     LayerProtoHelper::writeToProto(state.color, layerInfo->mutable_requested_color());
@@ -1975,6 +1968,7 @@ void Layer::writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet)
         layerInfo->set_z_order_relative_of(zOrderRelativeOf->sequence);
     }
 
+    // XXX getBE().compositionInfo.mBuffer is not protected
     auto buffer = getBE().compositionInfo.mBuffer;
     if (buffer != nullptr) {
         LayerProtoHelper::writeToProto(buffer, layerInfo->mutable_active_buffer());
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 91eb15ae4a..7342c8b987 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -209,7 +209,6 @@ public:
         // dependent.
         Region activeTransparentRegion;
         Region requestedTransparentRegion;
-        ui::Dataspace dataSpace;
 
         int32_t appId;
         int32_t type;
@@ -287,7 +286,6 @@ public:
     bool setTransparentRegionHint(const Region& transparent);
     bool setFlags(uint8_t flags, uint8_t mask);
     bool setLayerStack(uint32_t layerStack);
-    bool setDataSpace(ui::Dataspace dataSpace);
     uint32_t getLayerStack() const;
     void deferTransactionUntil(const sp<IBinder>& barrierHandle, uint64_t frameNumber);
     void deferTransactionUntil(const sp<Layer>& barrierLayer, uint64_t frameNumber);
@@ -298,6 +296,8 @@ public:
     bool reparent(const sp<IBinder>& newParentHandle);
     bool detachChildren();
 
+    ui::Dataspace getDataSpace() const { return mCurrentDataSpace; }
+
     // Before color management is introduced, contents on Android have to be
     // desaturated in order to match what they appears like visually.
     // With color management, these contents will appear desaturated, thus
@@ -743,6 +743,7 @@ protected:
     int mActiveBufferSlot;
     sp<GraphicBuffer> mActiveBuffer;
     sp<NativeHandle> mSidebandStream;
+    ui::Dataspace mCurrentDataSpace = ui::Dataspace::UNKNOWN;
     Rect mCurrentCrop;
     uint32_t mCurrentTransform;
     // We encode unset as -1.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 9117207544..e7892ef109 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1879,7 +1879,7 @@ Dataspace SurfaceFlinger::getBestDataspace(
     *outHdrDataSpace = Dataspace::UNKNOWN;
 
     for (const auto& layer : displayDevice->getVisibleLayersSortedByZ()) {
-        switch (layer->getDrawingState().dataSpace) {
+        switch (layer->getDataSpace()) {
             case Dataspace::V0_SCRGB:
             case Dataspace::V0_SCRGB_LINEAR:
                 bestDataSpace = Dataspace::V0_SCRGB_LINEAR;
@@ -2098,13 +2098,13 @@ void SurfaceFlinger::setUpHWComposer() {
                     "display %zd: %d", displayId, result);
         }
         for (auto& layer : displayDevice->getVisibleLayersSortedByZ()) {
-            if ((layer->getDrawingState().dataSpace == Dataspace::BT2020_PQ ||
-                 layer->getDrawingState().dataSpace == Dataspace::BT2020_ITU_PQ) &&
+            if ((layer->getDataSpace() == Dataspace::BT2020_PQ ||
+                 layer->getDataSpace() == Dataspace::BT2020_ITU_PQ) &&
                     !displayDevice->hasHDR10Support()) {
                 layer->forceClientComposition(hwcId);
             }
-            if ((layer->getDrawingState().dataSpace == Dataspace::BT2020_HLG ||
-                 layer->getDrawingState().dataSpace == Dataspace::BT2020_ITU_HLG) &&
+            if ((layer->getDataSpace() == Dataspace::BT2020_HLG ||
+                 layer->getDataSpace() == Dataspace::BT2020_ITU_HLG) &&
                     !displayDevice->hasHLGSupport()) {
                 layer->forceClientComposition(hwcId);
             }