Merge "Strengthen dataspace guarantees in SurfaceFlinger." into udc-dev

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 9e66d8a1ad..3406e924ab 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -172,8 +172,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;
@@ -209,7 +208,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();
@@ -3144,7 +3142,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;
@@ -3409,6 +3406,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;
     }
@@ -3999,10 +4032,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;
 }
 
@@ -4010,6 +4039,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 f11128f520..70b4e9b249 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;
@@ -333,7 +332,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 593c4ffc03..57d927b6bb 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 f5960614b0..156007b862 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;
                 });