Merge "SkiaRE: Use RGB_888x for isOpaque" into sc-dev am: f824b53807 (26506954) · Commits · e / os / platform_frameworks_native

libs/renderengine/skia/AutoBackendTexture.cpp

+8 −1

Original line number	Diff line number	Diff line
		@@ -87,8 +87,15 @@ sk_sp<SkImage> AutoBackendTexture::makeImage(ui::Dataspace dataspace, SkAlphaTyp
		mUpdateProc(mImageCtx, context);
		}

		auto colorType = mColorType;
		if (alphaType == kOpaque_SkAlphaType) {
		if (colorType == kRGBA_8888_SkColorType) {
		colorType = kRGB_888x_SkColorType;
		}
		}

		sk_sp<SkImage> image =
		SkImage::MakeFromTexture(context, mBackendTexture, kTopLeft_GrSurfaceOrigin, mColorType,
		SkImage::MakeFromTexture(context, mBackendTexture, kTopLeft_GrSurfaceOrigin, colorType,
		alphaType, toSkColorSpace(dataspace), releaseImageProc, this);
		if (image.get()) {
		// The following ref will be counteracted by releaseProc, when SkImage is discarded.

libs/renderengine/skia/AutoBackendTexture.h

+2 −0

Original line number	Diff line number	Diff line
		@@ -65,6 +65,8 @@ public:
		return mTexture->getOrCreateSurface(dataspace, context);
		}

		SkColorType colorType() const { return mTexture->mColorType; }

		DISALLOW_COPY_AND_ASSIGN(LocalRef);

		private:

libs/renderengine/skia/SkiaGLRenderEngine.cpp

+23 −26

Original line number	Diff line number	Diff line
		@@ -971,11 +971,28 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
		false);
		}

		sk_sp<SkImage> image =
		imageTextureRef->makeImage(layerDataspace,
		item.usePremultipliedAlpha ? kPremul_SkAlphaType
		: kUnpremul_SkAlphaType,
		grContext);
		// isOpaque means we need to ignore the alpha in the image,
		// replacing it with the alpha specified by the LayerSettings. See
		// https://developer.android.com/reference/android/view/SurfaceControl.Builder#setOpaque(boolean)
		// The proper way to do this is to use an SkColorType that ignores
		// alpha, like kRGB_888x_SkColorType, and that is used if the
		// incoming image is kRGBA_8888_SkColorType. However, the incoming
		// image may be kRGBA_F16_SkColorType, for which there is no RGBX
		// SkColorType, or kRGBA_1010102_SkColorType, for which we have
		// kRGB_101010x_SkColorType, but it is not yet supported as a source
		// on the GPU. (Adding both is tracked in skbug.com/12048.) In the
		// meantime, we'll use a workaround that works unless we need to do
		// any color conversion. The workaround requires that we pretend the
		// image is already premultiplied, so that we do not premultiply it
		// before applying SkBlendMode::kPlus.
		const bool useIsOpaqueWorkaround = item.isOpaque &&
		(imageTextureRef->colorType() == kRGBA_1010102_SkColorType \|\|
		imageTextureRef->colorType() == kRGBA_F16_SkColorType);
		const auto alphaType = useIsOpaqueWorkaround ? kPremul_SkAlphaType
		: item.isOpaque ? kOpaque_SkAlphaType
		: item.usePremultipliedAlpha ? kPremul_SkAlphaType
		: kUnpremul_SkAlphaType;
		sk_sp<SkImage> image = imageTextureRef->makeImage(layerDataspace, alphaType, grContext);

		auto texMatrix = getSkM44(item.textureTransform).asM33();
		// textureTansform was intended to be passed directly into a shader, so when
		@@ -1004,27 +1021,7 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display,
		shader = image->makeShader(SkSamplingOptions(), matrix);
		}

		// Handle opaque images - it's a little nonstandard how we do this.
		// Fundamentally we need to support SurfaceControl.Builder#setOpaque:
		// https://developer.android.com/reference/android/view/SurfaceControl.Builder#setOpaque(boolean)
		// The important language is that when isOpaque is set, opacity is not sampled from the
		// alpha channel, but blending may still be supported on a transaction via setAlpha. So,
		// here's the conundrum:
		// 1. We can't force the SkImage alpha type to kOpaque_SkAlphaType, because it's treated
		// as an internal hint - composition is undefined when there are alpha bits present.
		// 2. We can try to lie about the pixel layout, but that only works for RGBA8888
		// buffers, i.e., treating them as RGBx8888 instead. But we can't do the same for
		// RGBA1010102 because RGBx1010102 is not supported as a pixel layout for SkImages. It's
		// also not clear what to use for F16 either, and lying about the pixel layout is a bit
		// of a hack anyways.
		// 3. We can't change the blendmode to src, because while this satisfies the requirement
		// for ignoring the alpha channel, it doesn't quite satisfy the blending requirement
		// because src always clobbers the destination content.
		//
		// So, what we do here instead is an additive blend mode where we compose the input
		// image with a solid black. This might need to be reassess if this does not support
		// FP16 incredibly well, but FP16 end-to-end isn't well supported anyway at the moment.
		if (item.isOpaque) {
		if (useIsOpaqueWorkaround) {
		shader = SkShaders::Blend(SkBlendMode::kPlus, shader,
		SkShaders::Color(SkColors::kBlack,
		toSkColorSpace(layerDataspace)));

libs/renderengine/tests/RenderEngineTest.cpp

+65 −0

Original line number	Diff line number	Diff line
		@@ -54,6 +54,7 @@ public:
		virtual std::unique_ptr<renderengine::gl::GLESRenderEngine> createGLESRenderEngine() {
		return nullptr;
		}
		virtual bool useColorManagement() const = 0;
		};

		class GLESRenderEngineFactory : public RenderEngineFactory {
		@@ -82,6 +83,8 @@ public:
		.build();
		return renderengine::gl::GLESRenderEngine::create(reCreationArgs);
		}

		bool useColorManagement() const override { return false; }
		};

		class GLESCMRenderEngineFactory : public RenderEngineFactory {
		@@ -110,6 +113,8 @@ public:
		.build();
		return renderengine::gl::GLESRenderEngine::create(reCreationArgs);
		}

		bool useColorManagement() const override { return true; }
		};

		class SkiaGLESRenderEngineFactory : public RenderEngineFactory {
		@@ -130,9 +135,16 @@ public:
		.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 @@ public:
		.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 @@ public:
		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 @@ TEST_P(RenderEngineTest, testDisableBlendingBuffer) {
		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