Merge "Strengthen dataspace guarantees in SurfaceFlinger." into udc-dev
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 9e66d8a..3406e92 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -172,8 +172,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;
@@ -209,7 +208,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();
@@ -3144,7 +3142,6 @@
}
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 @@
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 @@
}
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 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 f11128f..70b4e9b 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;
@@ -333,7 +332,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 593c4ff..57d927b 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 f596061..156007b 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;
});