SkiaRE: Use RGB_888x for isOpaque
Bug: 181130369
Test: manual
Test: librenderengine_test
The old method of blending with black doesn't work if we need to do
color conversion. The Skia pipeline premultiplies the shader prior to
blending. We can claim the image is premul to avoid that, but if we need
to do color conversion, we'll unpremultiply and convert, leading to the
current bug.
Instead, use SkColorTypes that ignore the alpha bits where possible. At
the moment, this only works for kRGBA_8888_SkColorType. There are two
other input types that we might see: kRGBA_1010102_SkColorType and
kRGBA_F16_SkColorType. While the former has an "ignore alpha" version,
it is not yet supported as a source to the GPU. The latter does not yet
have an "ignore alpha" version. Adding support for both is tracked in
skbug.com/12048. In the meantime, keep the old behavior for the
unsupported types, rather than breaking them even worse.
Change-Id: I7b6c4ff92a39a5e1c510a0a91117f55d7ce5115f
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index 1ca7a16..dfab6e8 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -54,6 +54,7 @@
virtual std::unique_ptr<renderengine::gl::GLESRenderEngine> createGLESRenderEngine() {
return nullptr;
}
+ virtual bool useColorManagement() const = 0;
};
class GLESRenderEngineFactory : public RenderEngineFactory {
@@ -82,6 +83,8 @@
.build();
return renderengine::gl::GLESRenderEngine::create(reCreationArgs);
}
+
+ bool useColorManagement() const override { return false; }
};
class GLESCMRenderEngineFactory : public RenderEngineFactory {
@@ -110,6 +113,8 @@
.build();
return renderengine::gl::GLESRenderEngine::create(reCreationArgs);
}
+
+ bool useColorManagement() const override { return true; }
};
class SkiaGLESRenderEngineFactory : public RenderEngineFactory {
@@ -130,9 +135,16 @@
.setSupportsBackgroundBlur(true)
.setContextPriority(renderengine::RenderEngine::ContextPriority::MEDIUM)
.setRenderEngineType(type())
+ // FIXME (b/189935602): This version is currently color managed.
+ // We should change it and fix the tests that fail.
+ //.setUseColorManagerment(false)
.build();
return renderengine::skia::SkiaGLRenderEngine::create(reCreationArgs);
}
+
+ // FIXME (b/189935602): This version is currently color managed.
+ // We should change it and fix the tests that fail.
+ bool useColorManagement() const override { return true; }
};
class SkiaGLESCMRenderEngineFactory : public RenderEngineFactory {
@@ -157,6 +169,8 @@
.build();
return renderengine::skia::SkiaGLRenderEngine::create(reCreationArgs);
}
+
+ bool useColorManagement() const override { return true; }
};
class RenderEngineTest : public ::testing::TestWithParam<std::shared_ptr<RenderEngineFactory>> {
@@ -295,6 +309,7 @@
const uint8_t expected[4] = {r, g, b, a};
bool equal = colorCompare(src, expected);
EXPECT_TRUE(equal)
+ << GetParam()->name().c_str() << ": "
<< "pixel @ (" << region.left + i << ", " << region.top + j << "): "
<< "expected (" << static_cast<uint32_t>(r) << ", "
<< static_cast<uint32_t>(g) << ", " << static_cast<uint32_t>(b) << ", "
@@ -2015,6 +2030,56 @@
expectBufferColor(rect, 0, 128, 0, 128);
}
+TEST_P(RenderEngineTest, test_isOpaque) {
+ initializeRenderEngine();
+
+ const auto rect = Rect(0, 0, 1, 1);
+ const renderengine::DisplaySettings display{
+ .physicalDisplay = rect,
+ .clip = rect,
+ .outputDataspace = ui::Dataspace::DISPLAY_P3,
+ };
+
+ // Create an unpremul buffer that is green with no alpha. Using isOpaque
+ // should make the green show.
+ const auto buf = allocateSourceBuffer(1, 1);
+ {
+ uint8_t* pixels;
+ buf->getBuffer()->lock(GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN,
+ reinterpret_cast<void**>(&pixels));
+ pixels[0] = 0;
+ pixels[1] = 255;
+ pixels[2] = 0;
+ pixels[3] = 0;
+ buf->getBuffer()->unlock();
+ }
+
+ const renderengine::LayerSettings greenLayer{
+ .geometry.boundaries = rect.toFloatRect(),
+ .source =
+ renderengine::PixelSource{
+ .buffer =
+ renderengine::Buffer{
+ .buffer = buf,
+ // Although the pixels are not
+ // premultiplied in practice, this
+ // matches the input we see.
+ .usePremultipliedAlpha = true,
+ .isOpaque = true,
+ },
+ },
+ .alpha = 1.0f,
+ };
+
+ std::vector<const renderengine::LayerSettings*> layers{&greenLayer};
+ invokeDraw(display, layers);
+
+ if (GetParam()->useColorManagement()) {
+ expectBufferColor(rect, 117, 251, 76, 255);
+ } else {
+ expectBufferColor(rect, 0, 255, 0, 255);
+ }
+}
} // namespace android
// TODO(b/129481165): remove the #pragma below and fix conversion issues