Strengthen dataspace guarantees in SurfaceFlinger.

Correctly converting from yuv to rgb in RenderEngine requires that the
buffer dataspace is accurate. So, if the dataspace on a layer differs
from the buffer then update the dataspace in the buffer's metadata to be
consistent.

Moreover, some GPU drivers do not perform yuv2rgb in a reasonable way
when the dataspace is UNKNOWN. In that case, reauthor the dataspace to
be sRGB so that there is consistent behavior for an UNKNOWN dataspace.

Finally, some GPU drivers cache gralloc metadata on creation of GPU
resources, which is not compliant with gralloc expectations. So that
vendors have some time to fix the drivers and so that GSI tests pass,
recreate GPU resources when the vendor partition is old and the buffer
is a YCbCr format.

Bug: 247826480
Test: SurfaceControlTest
Change-Id: Iee2641acce3926c826e96c56ececb431868d8598

8 files changed, 61 insertions, 17 deletions

diff --git a/libs/renderengine/ExternalTexture.cpp b/libs/renderengine/ExternalTexture.cpp
index 210dca5429..9eb42cd8e1 100644
--- a/libs/renderengine/ExternalTexture.cpp
+++ b/libs/renderengine/ExternalTexture.cpp
@@ -14,17 +14,17 @@
  * limitations under the License.
  */
 
+#include <log/log.h>
 #include <renderengine/RenderEngine.h>
 #include <renderengine/impl/ExternalTexture.h>
 #include <ui/GraphicBuffer.h>
-
-#include "log/log_main.h"
+#include <utils/Trace.h>
 
 namespace android::renderengine::impl {
 
 ExternalTexture::ExternalTexture(const sp<GraphicBuffer>& buffer,
                                  renderengine::RenderEngine& renderEngine, uint32_t usage)
-      : mBuffer(buffer), mRenderEngine(renderEngine) {
+      : mBuffer(buffer), mRenderEngine(renderEngine), mWritable(usage & WRITEABLE) {
     LOG_ALWAYS_FATAL_IF(buffer == nullptr,
                         "Attempted to bind a null buffer to an external texture!");
     // GLESRenderEngine has a separate texture cache for output buffers,
@@ -35,11 +35,20 @@ ExternalTexture::ExternalTexture(const sp<GraphicBuffer>& buffer,
                  renderengine::RenderEngine::RenderEngineType::THREADED)) {
         return;
     }
-    mRenderEngine.mapExternalTextureBuffer(mBuffer, usage & WRITEABLE);
+    mRenderEngine.mapExternalTextureBuffer(mBuffer, mWritable);
 }
 
 ExternalTexture::~ExternalTexture() {
     mRenderEngine.unmapExternalTextureBuffer(std::move(mBuffer));
 }
 
+void ExternalTexture::remapBuffer() {
+    ATRACE_CALL();
+    {
+        auto buf = mBuffer;
+        mRenderEngine.unmapExternalTextureBuffer(std::move(buf));
+    }
+    mRenderEngine.mapExternalTextureBuffer(mBuffer, mWritable);
+}
+
 } // namespace android::renderengine::impl
diff --git a/libs/renderengine/include/renderengine/ExternalTexture.h b/libs/renderengine/include/renderengine/ExternalTexture.h
index 621a209afa..82e5d83c30 100644
--- a/libs/renderengine/include/renderengine/ExternalTexture.h
+++ b/libs/renderengine/include/renderengine/ExternalTexture.h
@@ -46,6 +46,8 @@ public:
     // Retrieves the buffer that is bound to this texture.
     virtual const sp<GraphicBuffer>& getBuffer() const = 0;
 
+    virtual void remapBuffer() = 0;
+
     Rect getBounds() const {
         return {0, 0, static_cast<int32_t>(getWidth()), static_cast<int32_t>(getHeight())};
     }
diff --git a/libs/renderengine/include/renderengine/impl/ExternalTexture.h b/libs/renderengine/include/renderengine/impl/ExternalTexture.h
index c0e24f0c10..d30262d985 100644
--- a/libs/renderengine/include/renderengine/impl/ExternalTexture.h
+++ b/libs/renderengine/include/renderengine/impl/ExternalTexture.h
@@ -51,10 +51,12 @@ public:
     bool hasSameBuffer(const renderengine::ExternalTexture& other) const override {
         return getBuffer() == other.getBuffer();
     }
+    void remapBuffer() override;
 
 private:
     sp<GraphicBuffer> mBuffer;
     android::renderengine::RenderEngine& mRenderEngine;
+    const bool mWritable;
 };
 
 } // namespace android::renderengine::impl
diff --git a/libs/renderengine/include/renderengine/mock/FakeExternalTexture.h b/libs/renderengine/include/renderengine/mock/FakeExternalTexture.h
index b95f011753..474e2e72c8 100644
--- a/libs/renderengine/include/renderengine/mock/FakeExternalTexture.h
+++ b/libs/renderengine/include/renderengine/mock/FakeExternalTexture.h
@@ -45,6 +45,7 @@ public:
     uint64_t getId() const override { return mId; }
     PixelFormat getPixelFormat() const override { return mPixelFormat; }
     uint64_t getUsage() const override { return mUsage; }
+    void remapBuffer() override {}
     ~FakeExternalTexture() = default;
 };
 
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index c1920bae50..7d16aad9bf 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -171,8 +171,7 @@ Layer::Layer(const LayerCreationArgs& args)
     mDrawingState.crop.makeInvalid();
     mDrawingState.acquireFence = sp<Fence>::make(-1);
     mDrawingState.acquireFenceTime = std::make_shared<FenceTime>(mDrawingState.acquireFence);
-    mDrawingState.dataspace = ui::Dataspace::UNKNOWN;
-    mDrawingState.dataspaceRequested = false;
+    mDrawingState.dataspace = ui::Dataspace::V0_SRGB;
     mDrawingState.hdrMetadata.validTypes = 0;
     mDrawingState.surfaceDamageRegion = Region::INVALID_REGION;
     mDrawingState.cornerRadius = 0.0f;
@@ -208,7 +207,6 @@ Layer::Layer(const LayerCreationArgs& args)
     mPremultipliedAlpha = !(args.flags & ISurfaceComposerClient::eNonPremultiplied);
     mPotentialCursor = args.flags & ISurfaceComposerClient::eCursorWindow;
     mProtectedByApp = args.flags & ISurfaceComposerClient::eProtectedByApp;
-    mDrawingState.dataspace = ui::Dataspace::V0_SRGB;
 
     mSnapshot->sequence = sequence;
     mSnapshot->name = getDebugName();
@@ -3136,7 +3134,6 @@ void Layer::recordLayerHistoryAnimationTx(const scheduler::LayerProps& layerProp
 }
 
 bool Layer::setDataspace(ui::Dataspace dataspace) {
-    mDrawingState.dataspaceRequested = true;
     if (mDrawingState.dataspace == dataspace) return false;
     mDrawingState.dataspace = dataspace;
     mDrawingState.modified = true;
@@ -3401,6 +3398,42 @@ void Layer::gatherBufferInfo() {
     mBufferInfo.mTransform = mDrawingState.bufferTransform;
     auto lastDataspace = mBufferInfo.mDataspace;
     mBufferInfo.mDataspace = translateDataspace(mDrawingState.dataspace);
+    if (mBufferInfo.mBuffer != nullptr) {
+        auto& mapper = GraphicBufferMapper::get();
+        // TODO: We should measure if it's faster to do a blind write if we're on newer api levels
+        // and don't need to possibly remaps buffers.
+        ui::Dataspace dataspace = ui::Dataspace::UNKNOWN;
+        status_t err = OK;
+        {
+            ATRACE_NAME("getDataspace");
+            err = mapper.getDataspace(mBufferInfo.mBuffer->getBuffer()->handle, &dataspace);
+        }
+        if (err != OK || dataspace != mBufferInfo.mDataspace) {
+            {
+                ATRACE_NAME("setDataspace");
+                err = mapper.setDataspace(mBufferInfo.mBuffer->getBuffer()->handle,
+                                          static_cast<ui::Dataspace>(mBufferInfo.mDataspace));
+            }
+
+            // Some GPU drivers may cache gralloc metadata which means before we composite we need
+            // to upsert RenderEngine's caches. Put in a special workaround to be backwards
+            // compatible with old vendors, with a ticking clock.
+            static const int32_t kVendorVersion =
+                    base::GetIntProperty("ro.vndk.version", __ANDROID_API_FUTURE__);
+            if (const auto format =
+                        static_cast<aidl::android::hardware::graphics::common::PixelFormat>(
+                                mBufferInfo.mBuffer->getPixelFormat());
+                err == OK && kVendorVersion < __ANDROID_API_U__ &&
+                (format ==
+                         aidl::android::hardware::graphics::common::PixelFormat::
+                                 IMPLEMENTATION_DEFINED ||
+                 format == aidl::android::hardware::graphics::common::PixelFormat::YCBCR_420_888 ||
+                 format == aidl::android::hardware::graphics::common::PixelFormat::YV12 ||
+                 format == aidl::android::hardware::graphics::common::PixelFormat::YCBCR_P010)) {
+                mBufferInfo.mBuffer->remapBuffer();
+            }
+        }
+    }
     if (lastDataspace != mBufferInfo.mDataspace) {
         mFlinger->mHdrLayerInfoChanged = true;
     }
@@ -3991,10 +4024,6 @@ uint32_t Layer::getBufferTransform() const {
 }
 
 ui::Dataspace Layer::getDataSpace() const {
-    return mDrawingState.dataspaceRequested ? getRequestedDataSpace() : ui::Dataspace::UNKNOWN;
-}
-
-ui::Dataspace Layer::getRequestedDataSpace() const {
     return hasBufferOrSidebandStream() ? mBufferInfo.mDataspace : mDrawingState.dataspace;
 }
 
@@ -4002,6 +4031,8 @@ ui::Dataspace Layer::translateDataspace(ui::Dataspace dataspace) {
     ui::Dataspace updatedDataspace = dataspace;
     // translate legacy dataspaces to modern dataspaces
     switch (dataspace) {
+        // Treat unknown dataspaces as V0_sRGB
+        case ui::Dataspace::UNKNOWN:
         case ui::Dataspace::SRGB:
             updatedDataspace = ui::Dataspace::V0_SRGB;
             break;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 552ea3701c..429df37f14 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -137,7 +137,6 @@ public:
         wp<Layer> touchableRegionCrop;
 
         ui::Dataspace dataspace;
-        bool dataspaceRequested;
 
         uint64_t frameNumber;
         ui::Transform transform;
@@ -332,7 +331,6 @@ public:
     virtual FrameRateCompatibility getDefaultFrameRateCompatibility() const;
     //
     ui::Dataspace getDataSpace() const;
-    ui::Dataspace getRequestedDataSpace() const;
 
     virtual sp<LayerFE> getCompositionEngineLayerFE() const;
     virtual sp<LayerFE> copyCompositionEngineLayerFE() const;
diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
index 7642122adc..a64a9af310 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
@@ -47,6 +47,7 @@ public:
     uint64_t getId() const override { return mId; }
     PixelFormat getPixelFormat() const override { return mPixelFormat; }
     uint64_t getUsage() const override { return mUsage; }
+    void remapBuffer() override {}
     ~FakeExternalTexture() = default;
 };
 
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 19a93e1820..6391fec577 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -605,7 +605,7 @@ struct BaseLayerProperties {
                     EXPECT_EQ(false, layer.source.buffer.isOpaque);
                     EXPECT_EQ(0.0, layer.geometry.roundedCornersRadius.x);
                     EXPECT_EQ(0.0, layer.geometry.roundedCornersRadius.y);
-                    EXPECT_EQ(ui::Dataspace::UNKNOWN, layer.sourceDataspace);
+                    EXPECT_EQ(ui::Dataspace::V0_SRGB, layer.sourceDataspace);
                     EXPECT_EQ(LayerProperties::COLOR[3], layer.alpha);
                     return resultFuture;
                 });
@@ -654,7 +654,7 @@ struct BaseLayerProperties {
                               layer.source.solidColor);
                     EXPECT_EQ(0.0, layer.geometry.roundedCornersRadius.x);
                     EXPECT_EQ(0.0, layer.geometry.roundedCornersRadius.y);
-                    EXPECT_EQ(ui::Dataspace::UNKNOWN, layer.sourceDataspace);
+                    EXPECT_EQ(ui::Dataspace::V0_SRGB, layer.sourceDataspace);
                     EXPECT_EQ(LayerProperties::COLOR[3], layer.alpha);
                     return resultFuture;
                 });
@@ -731,7 +731,7 @@ struct CommonSecureLayerProperties : public BaseLayerProperties<LayerProperties>
                     EXPECT_EQ(half3(0.0f, 0.0f, 0.0f), layer.source.solidColor);
                     EXPECT_EQ(0.0, layer.geometry.roundedCornersRadius.x);
                     EXPECT_EQ(0.0, layer.geometry.roundedCornersRadius.y);
-                    EXPECT_EQ(ui::Dataspace::UNKNOWN, layer.sourceDataspace);
+                    EXPECT_EQ(ui::Dataspace::V0_SRGB, layer.sourceDataspace);
                     EXPECT_EQ(1.0f, layer.alpha);
                     return resultFuture;
                 });