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
diff --git a/libs/renderengine/ExternalTexture.cpp b/libs/renderengine/ExternalTexture.cpp
index 210dca5..9eb42cd 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 @@
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 621a209..82e5d83 100644
--- a/libs/renderengine/include/renderengine/ExternalTexture.h
+++ b/libs/renderengine/include/renderengine/ExternalTexture.h
@@ -46,6 +46,8 @@
// 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 c0e24f0..d30262d 100644
--- a/libs/renderengine/include/renderengine/impl/ExternalTexture.h
+++ b/libs/renderengine/include/renderengine/impl/ExternalTexture.h
@@ -51,10 +51,12 @@
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 b95f011..474e2e7 100644
--- a/libs/renderengine/include/renderengine/mock/FakeExternalTexture.h
+++ b/libs/renderengine/include/renderengine/mock/FakeExternalTexture.h
@@ -45,6 +45,7 @@
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 c1920ba..7d16aad 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -171,8 +171,7 @@
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 @@
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 @@
}
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 @@
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 @@
}
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 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 552ea37..429df37 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -137,7 +137,6 @@
wp<Layer> touchableRegionCrop;
ui::Dataspace dataspace;
- bool dataspaceRequested;
uint64_t frameNumber;
ui::Transform transform;
@@ -332,7 +331,6 @@
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 7642122..a64a9af 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
@@ -47,6 +47,7 @@
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 19a93e1..6391fec 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -605,7 +605,7 @@
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 @@
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 @@
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;
});