Pass display's colorTransform to RE even if HW handles it
Even if the DPU handles the colorTransform, RE needs to know about it
for A8 screen decorations layers. So pass the colorTransform to RE
always. Add a new boolean, specifying whether the DPU handles it, so
existing use cases can continue to ignore it.
RE uses a color matrix to turn transparent pixels in the A8 screen
decorations layer to black, providing antialiasing. If the DPU handles
the colorTransform, that black will then be converted to another color
(e.g. white, if the colorTransform comes from the "Color inversion"
accesibility feature). So RE needs to convert it to a color that the DPU
will then convert back to black, matching plastic next to the screen. Do
this by inverting the upper left 3x3 of the matrix and applying it to
black. If the color matrix is not invertible (note that the one used for
"Color inversion" *is*), leave black unchanged.
If the DPU does not handle the colorTransform, concat the A8 layer's
color matrix with the colorTransform.
Update libcompositionengine_test to use the proper colorTransform.
Add GTEST_SKIP if R8 buffers are not supported (follow-up from
I340de485fd62bea12a5a097f09073c4f7d58fa90).
Bug: 215706351
Bug: 216178319
Test: librenderengine_test
Test: libcompositionengine_test
Test: manual
Change-Id: Ib127620be4e5fc400ac2525c90dea1b9a29bd1f7
diff --git a/libs/renderengine/include/renderengine/DisplaySettings.h b/libs/renderengine/include/renderengine/DisplaySettings.h
index 2c51ccd..40ba5ad 100644
--- a/libs/renderengine/include/renderengine/DisplaySettings.h
+++ b/libs/renderengine/include/renderengine/DisplaySettings.h
@@ -54,6 +54,10 @@
// dataspace, in non-linear space.
mat4 colorTransform = mat4();
+ // If true, and colorTransform is non-identity, most client draw calls can
+ // ignore it. Some draws (e.g. screen decorations) may need it, though.
+ bool deviceHandlesColorTransform = false;
+
// An additional orientation flag to be applied after clipping the output.
// By way of example, this may be used for supporting fullscreen screenshot
// capture of a device in landscape while the buffer is in portrait
diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
index 763b82d..b467b35 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
@@ -792,7 +792,7 @@
// setup color filter if necessary
sk_sp<SkColorFilter> displayColorTransform;
- if (display.colorTransform != mat4()) {
+ if (display.colorTransform != mat4() && !display.deviceHandlesColorTransform) {
displayColorTransform = SkColorFilters::Matrix(toSkColorMatrix(display.colorTransform));
}
const bool ctModifiesAlpha =
@@ -1107,11 +1107,37 @@
if (imageTextureRef->colorType() == kAlpha_8_SkColorType) {
LOG_ALWAYS_FATAL_IF(layer.disableBlending, "Cannot disableBlending with A8");
- float matrix[] = { 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0,
- 0, 0, 0, 0, 0,
- 0, 0, 0, -1, 1 };
- paint.setColorFilter(SkColorFilters::Matrix(matrix));
+
+ // SysUI creates the alpha layer as a coverage layer, which is
+ // appropriate for the DPU. Use a color matrix to convert it to
+ // a mask.
+ // TODO (b/219525258): Handle input as a mask.
+ //
+ // The color matrix will convert A8 pixels with no alpha to
+ // black, as described by this vector. If the display handles
+ // the color transform, we need to invert it to find the color
+ // that will result in black after the DPU applies the transform.
+ SkV4 black{0.0f, 0.0f, 0.0f, 1.0f}; // r, g, b, a
+ if (display.colorTransform != mat4() && display.deviceHandlesColorTransform) {
+ SkM44 colorSpaceMatrix = getSkM44(display.colorTransform);
+ if (colorSpaceMatrix.invert(&colorSpaceMatrix)) {
+ black = colorSpaceMatrix * black;
+ } else {
+ // We'll just have to use 0,0,0 as black, which should
+ // be close to correct.
+ ALOGI("Could not invert colorTransform!");
+ }
+ }
+ SkColorMatrix colorMatrix(0, 0, 0, 0, black[0],
+ 0, 0, 0, 0, black[1],
+ 0, 0, 0, 0, black[2],
+ 0, 0, 0, -1, 1);
+ if (display.colorTransform != mat4() && !display.deviceHandlesColorTransform) {
+ // On the other hand, if the device doesn't handle it, we
+ // have to apply it ourselves.
+ colorMatrix.postConcat(toSkColorMatrix(display.colorTransform));
+ }
+ paint.setColorFilter(SkColorFilters::Matrix(colorMatrix));
}
} else {
ATRACE_NAME("DrawColor");
@@ -1134,8 +1160,8 @@
paint.setBlendMode(SkBlendMode::kSrc);
}
- // A color filter will have been set for an A8 buffer. Do not replace
- // it with the displayColorTransform, which shouldn't affect A8.
+ // An A8 buffer will already have the proper color filter attached to
+ // its paint, including the displayColorTransform as needed.
if (!paint.getColorFilter()) {
paint.setColorFilter(displayColorTransform);
}
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index e197150..add7a94 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -2627,6 +2627,7 @@
const auto r8Buffer = allocateR8Buffer(2, 1);
if (!r8Buffer) {
+ GTEST_SKIP() << "Test is only necessary on devices that support r8";
return;
}
{
@@ -2677,6 +2678,144 @@
expectBufferColor(Rect(0, 0, 1, 1), 0, 0, 0, 255);
expectBufferColor(Rect(1, 0, 2, 1), 0, 255, 0, 255);
}
+
+TEST_P(RenderEngineTest, r8_respects_color_transform) {
+ if (GetParam()->type() == renderengine::RenderEngine::RenderEngineType::GLES) {
+ return;
+ }
+
+ initializeRenderEngine();
+
+ const auto r8Buffer = allocateR8Buffer(2, 1);
+ if (!r8Buffer) {
+ GTEST_SKIP() << "Test is only necessary on devices that support r8";
+ return;
+ }
+ {
+ uint8_t* pixels;
+ r8Buffer->getBuffer()->lock(GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN,
+ reinterpret_cast<void**>(&pixels));
+ pixels[0] = 0;
+ pixels[1] = 255;
+ r8Buffer->getBuffer()->unlock();
+ }
+
+ const auto rect = Rect(0, 0, 2, 1);
+ const renderengine::DisplaySettings display{
+ .physicalDisplay = rect,
+ .clip = rect,
+ .outputDataspace = ui::Dataspace::SRGB,
+ // Verify that the R8 layer respects the color transform when
+ // deviceHandlesColorTransform is false. This transform converts
+ // pure red to pure green. That will occur when the R8 buffer is
+ // 255. When the R8 buffer is 0, it will still change to black, as
+ // with r8_behaves_as_mask.
+ .colorTransform = mat4(0, 1, 0, 0,
+ 0, 0, 0, 0,
+ 0, 0, 1, 0,
+ 0, 0, 0, 1),
+ .deviceHandlesColorTransform = false,
+ };
+
+ const auto redBuffer = allocateAndFillSourceBuffer(2, 1, ubyte4(255, 0, 0, 255));
+ const renderengine::LayerSettings redLayer{
+ .geometry.boundaries = rect.toFloatRect(),
+ .source =
+ renderengine::PixelSource{
+ .buffer =
+ renderengine::Buffer{
+ .buffer = redBuffer,
+ },
+ },
+ .alpha = 1.0f,
+ };
+ const renderengine::LayerSettings r8Layer{
+ .geometry.boundaries = rect.toFloatRect(),
+ .source =
+ renderengine::PixelSource{
+ .buffer =
+ renderengine::Buffer{
+ .buffer = r8Buffer,
+ },
+ },
+ .alpha = 1.0f,
+ };
+
+ std::vector<renderengine::LayerSettings> layers{redLayer, r8Layer};
+ invokeDraw(display, layers);
+
+ expectBufferColor(Rect(0, 0, 1, 1), 0, 0, 0, 255);
+ expectBufferColor(Rect(1, 0, 2, 1), 0, 255, 0, 255);
+}
+
+TEST_P(RenderEngineTest, r8_respects_color_transform_when_device_handles) {
+ if (GetParam()->type() == renderengine::RenderEngine::RenderEngineType::GLES) {
+ return;
+ }
+
+ initializeRenderEngine();
+
+ const auto r8Buffer = allocateR8Buffer(2, 1);
+ if (!r8Buffer) {
+ GTEST_SKIP() << "Test is only necessary on devices that support r8";
+ return;
+ }
+ {
+ uint8_t* pixels;
+ r8Buffer->getBuffer()->lock(GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN,
+ reinterpret_cast<void**>(&pixels));
+ pixels[0] = 0;
+ pixels[1] = 255;
+ r8Buffer->getBuffer()->unlock();
+ }
+
+ const auto rect = Rect(0, 0, 2, 1);
+ const renderengine::DisplaySettings display{
+ .physicalDisplay = rect,
+ .clip = rect,
+ .outputDataspace = ui::Dataspace::SRGB,
+ // If deviceHandlesColorTransform is true, pixels where the A8
+ // buffer is opaque are unaffected. If the colorTransform is
+ // invertible, pixels where the A8 buffer are transparent have the
+ // inverse applied to them so that the DPU will convert them back to
+ // black. Test with an arbitrary, invertible matrix.
+ .colorTransform = mat4(1, 0, 0, 2,
+ 3, 1, 2, 5,
+ 0, 5, 3, 0,
+ 0, 1, 0, 2),
+ .deviceHandlesColorTransform = true,
+ };
+
+ const auto redBuffer = allocateAndFillSourceBuffer(2, 1, ubyte4(255, 0, 0, 255));
+ const renderengine::LayerSettings redLayer{
+ .geometry.boundaries = rect.toFloatRect(),
+ .source =
+ renderengine::PixelSource{
+ .buffer =
+ renderengine::Buffer{
+ .buffer = redBuffer,
+ },
+ },
+ .alpha = 1.0f,
+ };
+ const renderengine::LayerSettings r8Layer{
+ .geometry.boundaries = rect.toFloatRect(),
+ .source =
+ renderengine::PixelSource{
+ .buffer =
+ renderengine::Buffer{
+ .buffer = r8Buffer,
+ },
+ },
+ .alpha = 1.0f,
+ };
+
+ std::vector<renderengine::LayerSettings> layers{redLayer, r8Layer};
+ invokeDraw(display, layers);
+
+ expectBufferColor(Rect(1, 0, 2, 1), 255, 0, 0, 255); // Still red.
+ expectBufferColor(Rect(0, 0, 1, 1), 0, 70, 0, 255);
+}
} // namespace renderengine
} // namespace android