Add output dataspace to CachedSet rendering
This ensures that:
* The proper color conversion is applied to the rendered output.
* Color information is correctly provided when passing the cached buffer
over to HWC
Also rename dataspace to outputDataspace whenever the dataspace is used
for output, so that it's less confusing than just "dataspace"
Bug: 181192086
Bug: 181192080
Test: libcompositionengine_test
Change-Id: I024ab471a9139209d2a51add145e6b14d9ed3745
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
index 5f834be..a3e84e2 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h
@@ -89,6 +89,7 @@
sp<GraphicBuffer> buffer = nullptr;
sp<Fence> acquireFence = nullptr;
Rect displayFrame = {};
+ ui::Dataspace dataspace{ui::Dataspace::UNKNOWN};
} overrideInfo;
/*
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
index fa87fb8..b0e42b7 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
@@ -62,6 +62,7 @@
size_t getAge() const { return mAge; }
const sp<GraphicBuffer>& getBuffer() const { return mTexture.getBuffer(); }
const sp<Fence>& getDrawFence() const { return mDrawFence; }
+ ui::Dataspace getOutputDataspace() const { return mOutputDataspace; }
NonBufferHash getNonBufferHash() const;
@@ -80,6 +81,7 @@
void setLastUpdate(std::chrono::steady_clock::time_point now) { mLastUpdate = now; }
void append(const CachedSet& other) {
mTexture.setBuffer(nullptr, nullptr);
+ mOutputDataspace = ui::Dataspace::UNKNOWN;
mDrawFence = nullptr;
mLayers.insert(mLayers.end(), other.mLayers.cbegin(), other.mLayers.cend());
@@ -90,7 +92,8 @@
}
void incrementAge() { ++mAge; }
- void render(renderengine::RenderEngine&);
+ // Renders the cached set with the supplied output dataspace.
+ void render(renderengine::RenderEngine&, ui::Dataspace outputDataspace);
void dump(std::string& result) const;
@@ -129,6 +132,7 @@
Texture mTexture;
sp<Fence> mDrawFence;
+ ui::Dataspace mOutputDataspace;
static const bool sDebugHighlighLayers;
};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h
index 582723d..5b9a9f0 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h
@@ -42,7 +42,8 @@
NonBufferHash flattenLayers(const std::vector<const LayerState*>& layers, NonBufferHash,
std::chrono::steady_clock::time_point now);
- void renderCachedSets(renderengine::RenderEngine&);
+ // Renders the newest cached sets with the supplied output dataspace
+ void renderCachedSets(renderengine::RenderEngine&, ui::Dataspace outputDataspace);
void reset();
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
index 7f8cb4e..a3beadc 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
@@ -278,8 +278,8 @@
}};
using DataspaceState = OutputLayerState<ui::Dataspace, LayerStateField::Dataspace>;
- DataspaceState mDataspace{[](auto layer) { return layer->getState().dataspace; },
- DataspaceState::getHalToStrings()};
+ DataspaceState mOutputDataspace{[](auto layer) { return layer->getState().dataspace; },
+ DataspaceState::getHalToStrings()};
// TODO(b/180638831): Buffer format
@@ -341,7 +341,7 @@
std::array<const StateInterface*, 13> getNonUniqueFields() const {
return {
&mDisplayFrame, &mSourceCrop, &mZOrder, &mBufferTransform,
- &mBlendMode, &mAlpha, &mVisibleRegion, &mDataspace,
+ &mBlendMode, &mAlpha, &mVisibleRegion, &mOutputDataspace,
&mColorTransform, &mCompositionType, &mSidebandStream, &mBuffer,
&mSolidColor,
};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h
index e96abb7..89de34d 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h
@@ -59,7 +59,7 @@
compositionengine::Output::OutputLayersEnumerator<compositionengine::Output>&& layers);
// The planner will call to the Flattener to render any pending cached set
- void renderCachedSets(renderengine::RenderEngine&);
+ void renderCachedSets(renderengine::RenderEngine&, ui::Dataspace outputDataspace);
void dump(const Vector<String16>& args, std::string&);
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index dc1aacc..ded2dcc 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -1252,7 +1252,7 @@
void Output::renderCachedSets() {
if (mPlanner) {
- mPlanner->renderCachedSets(getCompositionEngine().getRenderEngine());
+ mPlanner->renderCachedSets(getCompositionEngine().getRenderEngine(), getState().dataspace);
}
}
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 54784a2..b364649 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -681,6 +681,7 @@
.buffer = getState().overrideInfo.buffer,
.fence = getState().overrideInfo.acquireFence,
}};
+ settings.sourceDataspace = getState().overrideInfo.dataspace;
settings.alpha = 1.0f;
return {static_cast<LayerFE::LayerSettings>(settings)};
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
index ba03655..137697b 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
@@ -79,10 +79,11 @@
return mFingerprint;
}
- // TODO(b/181192080): Add all fields which contribute to geometry of override layer (e.g.,
- // dataspace)
+ // TODO(b/182614524): We sometimes match this with LayerState hashes. Determine if that is
+ // necessary (and therefore we need to match implementations).
size_t hash = 0;
android::hashCombineSingle(hash, mBounds);
+ android::hashCombineSingle(hash, mOutputDataspace);
return hash;
}
@@ -148,10 +149,11 @@
}
}
-void CachedSet::render(renderengine::RenderEngine& renderEngine) {
+void CachedSet::render(renderengine::RenderEngine& renderEngine, ui::Dataspace outputDataspace) {
renderengine::DisplaySettings displaySettings{
.physicalDisplay = Rect(0, 0, mBounds.getWidth(), mBounds.getHeight()),
.clip = mBounds,
+ .outputDataspace = outputDataspace,
};
Region clearRegion = Region::INVALID_REGION;
@@ -163,8 +165,7 @@
.supportsProtectedContent = false,
.clearRegion = clearRegion,
.viewport = viewport,
- // TODO(181192086): Propagate the Output's dataspace instead of using UNKNOWN
- .dataspace = ui::Dataspace::UNKNOWN,
+ .dataspace = outputDataspace,
.realContentIsVisible = true,
.clearContent = false,
.disableBlurs = false,
@@ -216,6 +217,7 @@
if (result == NO_ERROR) {
mTexture.setBuffer(buffer, &renderEngine);
mDrawFence = new Fence(drawFence.release());
+ mOutputDataspace = outputDataspace;
}
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
index d304c9f..30b5761 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
@@ -51,12 +51,13 @@
return hash;
}
-void Flattener::renderCachedSets(renderengine::RenderEngine& renderEngine) {
+void Flattener::renderCachedSets(renderengine::RenderEngine& renderEngine,
+ ui::Dataspace outputDataspace) {
if (!mNewCachedSet) {
return;
}
- mNewCachedSet->render(renderEngine);
+ mNewCachedSet->render(renderEngine, outputDataspace);
}
void Flattener::reset() {
@@ -223,6 +224,7 @@
.buffer = mNewCachedSet->getBuffer(),
.acquireFence = mNewCachedSet->getDrawFence(),
.displayFrame = mNewCachedSet->getBounds(),
+ .dataspace = mNewCachedSet->getOutputDataspace(),
};
++incomingLayerIter;
}
@@ -253,6 +255,7 @@
.buffer = currentLayerIter->getBuffer(),
.acquireFence = currentLayerIter->getDrawFence(),
.displayFrame = currentLayerIter->getBounds(),
+ .dataspace = currentLayerIter->getOutputDataspace(),
};
++incomingLayerIter;
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp b/services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp
index f3746de..222b2be 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp
@@ -158,7 +158,8 @@
lhs.mSourceCrop == rhs.mSourceCrop && lhs.mZOrder == rhs.mZOrder &&
lhs.mBufferTransform == rhs.mBufferTransform && lhs.mBlendMode == rhs.mBlendMode &&
lhs.mAlpha == rhs.mAlpha && lhs.mVisibleRegion == rhs.mVisibleRegion &&
- lhs.mDataspace == rhs.mDataspace && lhs.mColorTransform == rhs.mColorTransform &&
+ lhs.mOutputDataspace == rhs.mOutputDataspace &&
+ lhs.mColorTransform == rhs.mColorTransform &&
lhs.mCompositionType == rhs.mCompositionType &&
lhs.mSidebandStream == rhs.mSidebandStream && lhs.mBuffer == rhs.mBuffer &&
(lhs.mCompositionType.get() != hal::Composition::SOLID_COLOR ||
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp
index 4570253..87721c7 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp
@@ -133,8 +133,9 @@
finalPlan);
}
-void Planner::renderCachedSets(renderengine::RenderEngine& renderEngine) {
- mFlattener.renderCachedSets(renderEngine);
+void Planner::renderCachedSets(renderengine::RenderEngine& renderEngine,
+ ui::Dataspace outputDataspace) {
+ mFlattener.renderCachedSets(renderEngine, outputDataspace);
}
void Planner::dump(const Vector<String16>& args, std::string& result) {
diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
index c33828f..377f817 100644
--- a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
@@ -303,6 +303,7 @@
EXPECT_EQ(Rect(0, 0, 2, 2), displaySettings.clip);
EXPECT_EQ(0.5f, layers[0]->alpha);
EXPECT_EQ(0.75f, layers[1]->alpha);
+ EXPECT_EQ(ui::Dataspace::SRGB, displaySettings.outputDataspace);
return NO_ERROR;
};
@@ -311,7 +312,7 @@
EXPECT_CALL(*layerFE2, prepareClientCompositionList(_)).WillOnce(Return(clientCompList2));
EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Invoke(drawLayers));
EXPECT_CALL(mRenderEngine, cacheExternalTextureBuffer(_));
- cachedSet.render(mRenderEngine);
+ cachedSet.render(mRenderEngine, ui::Dataspace::SRGB);
expectReadyBuffer(cachedSet);
// Now check that appending a new cached set properly cleans up RenderEngine resources.
diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
index c4bd5b3..bd77559 100644
--- a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
@@ -134,13 +134,13 @@
initializeOverrideBuffer(layers);
EXPECT_EQ(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
// same geometry, update the internal layer stack
initializeOverrideBuffer(layers);
EXPECT_EQ(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
}
void FlattenerTest::expectAllLayersFlattened(const std::vector<const LayerState*>& layers) {
@@ -150,7 +150,7 @@
initializeOverrideBuffer(layers);
EXPECT_EQ(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
for (const auto layer : layers) {
EXPECT_EQ(nullptr, layer->getOutputLayer()->getState().overrideInfo.buffer);
@@ -160,7 +160,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
const auto buffer = layers[0]->getOutputLayer()->getState().overrideInfo.buffer;
EXPECT_NE(nullptr, buffer);
@@ -195,7 +195,7 @@
initializeOverrideBuffer(layers);
EXPECT_EQ(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
}
TEST_F(FlattenerTest, flattenLayers_basicFlatten) {
@@ -241,7 +241,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_NE(nullptr, overrideBuffer1);
EXPECT_EQ(overrideBuffer1, overrideBuffer2);
@@ -276,7 +276,7 @@
initializeOverrideBuffer(layers);
EXPECT_EQ(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_EQ(nullptr, overrideBuffer1);
EXPECT_EQ(nullptr, overrideBuffer2);
@@ -313,7 +313,7 @@
initializeOverrideBuffer(layers);
EXPECT_EQ(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_EQ(nullptr, overrideBuffer1);
EXPECT_EQ(nullptr, overrideBuffer2);
@@ -322,7 +322,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_EQ(nullptr, overrideBuffer1);
EXPECT_NE(nullptr, overrideBuffer2);
@@ -335,7 +335,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_EQ(nullptr, overrideBuffer1);
EXPECT_NE(nullptr, overrideBuffer2);
@@ -344,7 +344,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_NE(nullptr, overrideBuffer1);
EXPECT_EQ(overrideBuffer1, overrideBuffer2);
@@ -386,7 +386,7 @@
initializeOverrideBuffer(layers);
EXPECT_EQ(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_EQ(nullptr, overrideBuffer1);
EXPECT_EQ(nullptr, overrideBuffer2);
@@ -399,7 +399,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_NE(nullptr, overrideBuffer1);
EXPECT_EQ(overrideBuffer1, overrideBuffer2);
@@ -411,7 +411,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_NE(nullptr, overrideBuffer1);
EXPECT_EQ(overrideBuffer1, overrideBuffer2);
@@ -426,7 +426,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_NE(nullptr, overrideBuffer1);
EXPECT_EQ(overrideBuffer1, overrideBuffer2);
@@ -437,7 +437,7 @@
initializeOverrideBuffer(layers);
EXPECT_NE(getNonBufferHash(layers),
mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
- mFlattener->renderCachedSets(mRenderEngine);
+ mFlattener->renderCachedSets(mRenderEngine, ui::Dataspace::SRGB);
EXPECT_NE(nullptr, overrideBuffer1);
EXPECT_EQ(overrideBuffer1, overrideBuffer2);