[SurfaceFlinger] Implement per layer color transformation.
Previously we introduced a new composer HAL API to set color transform for per
layer and added the plumbing in SurfaceFlinger. This patch implements the
functionality and alwasy mark those layers to fall back to GPU composition
until composer 2.3 is implemented.
BUG: 111562338
Test: Build, boot, flash, tested by setting a greyscale matrix on Settings
Test: adb shell /data/nativetest/SurfaceFlinger_test/SurfaceFlinger_test
Change-Id: If8d5ed52bf920d8cc962602196fb1b0b6e2955da
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 5a8d8db..2b0a461 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -77,6 +77,9 @@
output.writeBool(false);
}
+ memcpy(output.writeInplace(16 * sizeof(float)),
+ colorTransform.asArray(), 16 * sizeof(float));
+
return NO_ERROR;
}
@@ -130,6 +133,8 @@
sidebandStream = NativeHandle::create(input.readNativeHandle(), true);
}
+ colorTransform = mat4(static_cast<const float*>(input.readInplace(16 * sizeof(float))));
+
return NO_ERROR;
}
@@ -314,6 +319,10 @@
what |= eSidebandStreamChanged;
sidebandStream = other.sidebandStream;
}
+ if (other.what & eColorTransformChanged) {
+ what |= eColorTransformChanged;
+ colorTransform = other.colorTransform;
+ }
if ((other.what & what) != other.what) {
ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating? "
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 09ea0f6..1ac9609 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -602,6 +602,18 @@
return *this;
}
+SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setColorTransform(
+ const sp<SurfaceControl>& sc, const mat3& matrix, const vec3& translation) {
+ layer_state_t* s = getLayerState(sc);
+ if (!s) {
+ mStatus = BAD_INDEX;
+ return *this;
+ }
+ s->what |= layer_state_t::eColorTransformChanged;
+ s->colorTransform = mat4(matrix, translation);
+ return *this;
+}
+
// ---------------------------------------------------------------------------
DisplayState& SurfaceComposerClient::Transaction::getDisplayState(const sp<IBinder>& token) {
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 9a9f633..e06e2b1 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -23,6 +23,7 @@
#include <utils/Errors.h>
#include <gui/IGraphicBufferProducer.h>
+#include <math/mat4.h>
#include <math/vec3.h>
#include <ui/GraphicTypes.h>
#include <ui/Rect.h>
@@ -72,6 +73,7 @@
eSurfaceDamageRegionChanged = 0x02000000,
eApiChanged = 0x04000000,
eSidebandStreamChanged = 0x08000000,
+ eColorTransformChanged = 0x10000000,
};
layer_state_t()
@@ -94,7 +96,8 @@
crop(Rect::INVALID_RECT),
dataspace(ui::Dataspace::UNKNOWN),
surfaceDamageRegion(),
- api(-1) {
+ api(-1),
+ colorTransform(mat4()) {
matrix.dsdx = matrix.dtdy = 1.0f;
matrix.dsdy = matrix.dtdx = 0.0f;
hdrMetadata.validTypes = 0;
@@ -150,6 +153,7 @@
Region surfaceDamageRegion;
int32_t api;
sp<NativeHandle> sidebandStream;
+ mat4 colorTransform;
};
struct ComposerState {
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 314b118..69a759f 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -269,6 +269,10 @@
Transaction& destroySurface(const sp<SurfaceControl>& sc);
+ // Set a color transform matrix on the given layer on the built-in display.
+ Transaction& setColorTransform(const sp<SurfaceControl>& sc, const mat3& matrix,
+ const vec3& translation);
+
status_t setDisplaySurface(const sp<IBinder>& token,
const sp<IGraphicBufferProducer>& bufferProducer);
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index 642ed2f..1a73ff0 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -258,6 +258,7 @@
getBE().compositionInfo.hwc.dataspace = mCurrentDataSpace;
getBE().compositionInfo.hwc.hdrMetadata = getDrawingHdrMetadata();
getBE().compositionInfo.hwc.supportedPerFrameMetadata = display->getSupportedPerFrameMetadata();
+ getBE().compositionInfo.hwc.colorTransform = getColorTransform();
setHwcLayerBuffer(display);
}
diff --git a/services/surfaceflinger/ColorLayer.cpp b/services/surfaceflinger/ColorLayer.cpp
index b02c16c..3a554c9 100644
--- a/services/surfaceflinger/ColorLayer.cpp
+++ b/services/surfaceflinger/ColorLayer.cpp
@@ -75,6 +75,7 @@
// Clear out the transform, because it doesn't make sense absent a source buffer
getBE().compositionInfo.hwc.transform = HWC2::Transform::None;
+ getBE().compositionInfo.hwc.colorTransform = getColorTransform();
}
// ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index f9bc1e7..8afd3b3 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1546,6 +1546,25 @@
return true;
}
+bool Layer::setColorTransform(const mat4& matrix) {
+ if (mCurrentState.colorTransform == matrix) {
+ return false;
+ }
+ ++mCurrentState.sequence;
+ mCurrentState.colorTransform = matrix;
+ setTransactionFlags(eTransactionNeeded);
+ return true;
+}
+
+const mat4& Layer::getColorTransform() const {
+ return getDrawingState().colorTransform;
+}
+
+bool Layer::hasColorTransform() const {
+ static const mat4 identityMatrix = mat4();
+ return getDrawingState().colorTransform != identityMatrix;
+}
+
bool Layer::isLegacyDataSpace() const {
// return true when no higher bits are set
return !(mCurrentDataSpace & (ui::Dataspace::STANDARD_MASK |
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 874b551..4890fa6 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -181,6 +181,7 @@
int32_t api;
sp<NativeHandle> sidebandStream;
+ mat4 colorTransform;
};
explicit Layer(const LayerCreationArgs& args);
@@ -255,6 +256,9 @@
virtual void setChildrenDrawingParent(const sp<Layer>& layer);
virtual bool reparent(const sp<IBinder>& newParentHandle);
virtual bool detachChildren();
+ virtual bool setColorTransform(const mat4& matrix);
+ virtual const mat4& getColorTransform() const;
+ virtual bool hasColorTransform() const;
// Used only to set BufferStateLayer state
virtual bool setTransform(uint32_t /*transform*/) { return false; };
diff --git a/services/surfaceflinger/LayerBE.cpp b/services/surfaceflinger/LayerBE.cpp
index ef017aa..c9b7933 100644
--- a/services/surfaceflinger/LayerBE.cpp
+++ b/services/surfaceflinger/LayerBE.cpp
@@ -112,6 +112,20 @@
hwc.surfaceDamage.dump(regionString, "surfaceDamage");
result += regionString.string();
}
+
+ result += base::StringPrintf("\tcolor transform matrix:\n"
+ "\t\t[%f, %f, %f, %f,\n"
+ "\t\t %f, %f, %f, %f,\n"
+ "\t\t %f, %f, %f, %f,\n"
+ "\t\t %f, %f, %f, %f]\n",
+ hwc.colorTransform[0][0], hwc.colorTransform[1][0],
+ hwc.colorTransform[2][0], hwc.colorTransform[3][0],
+ hwc.colorTransform[0][1], hwc.colorTransform[1][1],
+ hwc.colorTransform[2][1], hwc.colorTransform[3][1],
+ hwc.colorTransform[0][2], hwc.colorTransform[1][2],
+ hwc.colorTransform[2][2], hwc.colorTransform[3][2],
+ hwc.colorTransform[0][3], hwc.colorTransform[1][3],
+ hwc.colorTransform[2][3], hwc.colorTransform[3][3]);
}
void CompositionInfo::dumpRe(std::string& result, const char* tag) const {
diff --git a/services/surfaceflinger/LayerBE.h b/services/surfaceflinger/LayerBE.h
index 2722b01..d63d16f 100644
--- a/services/surfaceflinger/LayerBE.h
+++ b/services/surfaceflinger/LayerBE.h
@@ -60,6 +60,7 @@
bool clearClientTarget = false;
bool supportedPerFrameMetadata = false;
HdrMetadata hdrMetadata;
+ mat4 colorTransform;
} hwc;
struct {
bool blackoutLayer = false;
diff --git a/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.cpp b/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.cpp
index 813c9e6..1b0a539 100644
--- a/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.cpp
+++ b/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.cpp
@@ -743,7 +743,7 @@
mState.textureEnabled = true;
}
-void GLES20RenderEngine::setupColorTransform(const mat4& colorTransform) {
+void GLES20RenderEngine::setColorTransform(const mat4& colorTransform) {
mState.colorMatrix = colorTransform;
}
diff --git a/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.h b/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.h
index fa01410..4f03a90 100644
--- a/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.h
+++ b/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.h
@@ -86,7 +86,7 @@
void setupLayerTexturing(const Texture& texture) override;
void setupLayerBlackedOut() override;
void setupFillWithColor(float r, float g, float b, float a) override;
- void setupColorTransform(const mat4& colorTransform) override;
+ void setColorTransform(const mat4& colorTransform) override;
void disableTexturing() override;
void disableBlending() override;
diff --git a/services/surfaceflinger/RenderEngine/include/renderengine/RenderEngine.h b/services/surfaceflinger/RenderEngine/include/renderengine/RenderEngine.h
index 05668f8..122271f 100644
--- a/services/surfaceflinger/RenderEngine/include/renderengine/RenderEngine.h
+++ b/services/surfaceflinger/RenderEngine/include/renderengine/RenderEngine.h
@@ -115,7 +115,9 @@
virtual void setupLayerTexturing(const Texture& texture) = 0;
virtual void setupLayerBlackedOut() = 0;
virtual void setupFillWithColor(float r, float g, float b, float a) = 0;
- virtual void setupColorTransform(const mat4& /* colorTransform */) = 0;
+
+ // Set a color transform matrix that is applied in linear space right before OETF.
+ virtual void setColorTransform(const mat4& /* colorTransform */) = 0;
virtual void disableTexturing() = 0;
virtual void disableBlending() = 0;
@@ -163,7 +165,6 @@
bool useNativeFenceSync() const override;
bool useWaitSync() const override;
- void setupColorTransform(const mat4& /* colorTransform */) override {}
protected:
RenderEngine(uint32_t featureFlags);
diff --git a/services/surfaceflinger/RenderEngine/include/renderengine/private/Description.h b/services/surfaceflinger/RenderEngine/include/renderengine/private/Description.h
index efab8ff..911bb7a 100644
--- a/services/surfaceflinger/RenderEngine/include/renderengine/private/Description.h
+++ b/services/surfaceflinger/RenderEngine/include/renderengine/private/Description.h
@@ -68,6 +68,8 @@
// projection matrix
mat4 projectionMatrix;
+
+ // The color matrix will be applied in linear space right before OETF.
mat4 colorMatrix;
mat4 inputTransformMatrix;
mat4 outputTransformMatrix;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 9410cdb..c37b3b1 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1604,6 +1604,11 @@
layer->forceClientComposition(displayId);
}
+ // TODO(b/111562338) remove when composer 2.3 is shipped.
+ if (layer->hasColorTransform()) {
+ layer->forceClientComposition(displayId);
+ }
+
if (layer->getForceClientComposition(displayId)) {
ALOGV("[%s] Requesting Client composition", layer->getName().string());
layer->setCompositionType(displayId, HWC2::Composition::Client);
@@ -2141,6 +2146,8 @@
ALOGE_IF(error != HWC2::Error::None,
"[SF] Failed to set surface damage: %s (%d)",
to_string(error).c_str(), static_cast<int32_t>(error));
+
+ error = (compositionInfo.hwc.hwcLayer)->setColorTransform(compositionInfo.hwc.colorTransform);
}
void SurfaceFlinger::configureDeviceComposition(const CompositionInfo& compositionInfo) const
@@ -3091,6 +3098,7 @@
const bool hasClientComposition = getBE().mHwc->hasClientComposition(displayId);
ATRACE_INT("hasClientComposition", hasClientComposition);
+ mat4 colorMatrix;
bool applyColorMatrix = false;
bool needsEnhancedColorMatrix = false;
@@ -3109,7 +3117,7 @@
const bool skipClientColorTransform = getBE().mHwc->hasCapability(
HWC2::Capability::SkipClientColorTransform);
- mat4 colorMatrix;
+ // Compute the global color transform matrix.
applyColorMatrix = !hasDeviceComposition && !skipClientColorTransform;
if (applyColorMatrix) {
colorMatrix = mDrawingState.colorMatrix;
@@ -3125,8 +3133,6 @@
colorMatrix *= mEnhancedSaturationMatrix;
}
- getRenderEngine().setupColorTransform(colorMatrix);
-
if (!display->makeCurrent()) {
ALOGW("DisplayDevice::makeCurrent failed. Aborting surface composition for display %s",
display->getDisplayName().c_str());
@@ -3205,6 +3211,19 @@
break;
}
case HWC2::Composition::Client: {
+ if (layer->hasColorTransform()) {
+ mat4 tmpMatrix;
+ if (applyColorMatrix) {
+ tmpMatrix = mDrawingState.colorMatrix;
+ }
+ tmpMatrix *= layer->getColorTransform();
+ if (needsEnhancedColorMatrix) {
+ tmpMatrix *= mEnhancedSaturationMatrix;
+ }
+ getRenderEngine().setColorTransform(tmpMatrix);
+ } else {
+ getRenderEngine().setColorTransform(colorMatrix);
+ }
layer->draw(renderArea, clip);
break;
}
@@ -3217,9 +3236,8 @@
firstLayer = false;
}
- if (applyColorMatrix || needsEnhancedColorMatrix) {
- getRenderEngine().setupColorTransform(mat4());
- }
+ // Clear color transform matrix at the end of the frame.
+ getRenderEngine().setColorTransform(mat4());
// disable scissor at the end of the frame
getBE().mRenderEngine->disableScissor();
@@ -3596,6 +3614,11 @@
if (layer->setColor(s.color))
flags |= eTraversalNeeded;
}
+ if (what & layer_state_t::eColorTransformChanged) {
+ if (layer->setColorTransform(s.colorTransform)) {
+ flags |= eTraversalNeeded;
+ }
+ }
if (what & layer_state_t::eMatrixChanged) {
// TODO: b/109894387
//
@@ -5441,7 +5464,9 @@
engine.clearWithColor(0, 0, 0, alpha);
traverseLayers([&](Layer* layer) {
+ engine.setColorTransform(layer->getColorTransform());
layer->draw(renderArea, useIdentityTransform);
+ engine.setColorTransform(mat4());
});
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index b77bf48..3f3086b 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -454,7 +454,6 @@
status_t getCompositionPreference(ui::Dataspace* outDataSpace,
ui::PixelFormat* outPixelFormat) const override;
-
/* ------------------------------------------------------------------------
* DeathRecipient interface
*/
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 3af98e5..3166a8c 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -2064,6 +2064,34 @@
Transaction().setSidebandStream(layer, nullptr).apply();
}
+TEST_F(LayerTransactionTest, SetColorTransformBasic) {
+ sp<SurfaceControl> colorLayer;
+ ASSERT_NO_FATAL_FAILURE(
+ colorLayer = createLayer("test", 32, 32, ISurfaceComposerClient::eFXSurfaceColor));
+
+ Transaction().setLayer(colorLayer, mLayerZBase + 1).apply();
+ {
+ SCOPED_TRACE("default color");
+ screenshot()->expectColor(Rect(0, 0, 32, 32), Color::BLACK);
+ }
+
+ const half3 color(50.0f / 255.0f, 100.0f / 255.0f, 150.0f / 255.0f);
+ const Color expected = {90, 90, 90, 255};
+ // this is handwavy, but the precison loss scaled by 255 (8-bit per
+ // channel) should be less than one
+ const uint8_t tolerance = 1;
+ mat3 matrix;
+ matrix[0][0] = 0.3; matrix[1][0] = 0.59; matrix[2][0] = 0.11;
+ matrix[0][1] = 0.3; matrix[1][1] = 0.59; matrix[2][1] = 0.11;
+ matrix[0][2] = 0.3; matrix[1][2] = 0.59; matrix[2][2] = 0.11;
+ Transaction().setColor(colorLayer, color)
+ .setColorTransform(colorLayer, matrix, vec3()).apply();
+ {
+ SCOPED_TRACE("new color");
+ screenshot()->expectColor(Rect(0, 0, 32, 32), expected, tolerance);
+ }
+}
+
class LayerUpdateTest : public LayerTransactionTest {
protected:
virtual void SetUp() {
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 52e64d8..3caf1f6 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -325,7 +325,7 @@
EXPECT_CALL(*test->mRenderEngine, setOutputDataSpace(ui::Dataspace::UNKNOWN)).Times(1);
EXPECT_CALL(*test->mRenderEngine, setDisplayMaxLuminance(DEFAULT_DISPLAY_MAX_LUMINANCE))
.Times(1);
- EXPECT_CALL(*test->mRenderEngine, setupColorTransform(_)).Times(2);
+ EXPECT_CALL(*test->mRenderEngine, setColorTransform(_)).Times(2);
// These expectations retire on saturation as the code path these
// expectations are for appears to make an extra call to them.
// TODO: Investigate this extra call
diff --git a/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h b/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
index c29452c..6813cda 100644
--- a/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
+++ b/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
@@ -62,7 +62,7 @@
MOCK_METHOD1(setupLayerTexturing, void(const Texture&));
MOCK_METHOD0(setupLayerBlackedOut, void());
MOCK_METHOD4(setupFillWithColor, void(float, float, float, float));
- MOCK_METHOD1(setupColorTransform, void(const mat4&));
+ MOCK_METHOD1(setColorTransform, void(const mat4&));
MOCK_METHOD1(setSaturationMatrix, void(const mat4&));
MOCK_METHOD0(disableTexturing, void());
MOCK_METHOD0(disableBlending, void());