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

libs/renderengine/skia/AutoBackendTexture.cpp

+8 −1

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

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

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

libs/renderengine/skia/AutoBackendTexture.h

+2 −0

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

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

	DISALLOW_COPY_AND_ASSIGN(LocalRef);		DISALLOW_COPY_AND_ASSIGN(LocalRef);

	private:		private:

libs/renderengine/skia/SkiaGLRenderEngine.cpp

+23 −26

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

	sk_sp<SkImage> image =		// isOpaque means we need to ignore the alpha in the image,
	imageTextureRef->makeImage(layerDataspace,		// replacing it with the alpha specified by the LayerSettings. See
	item.usePremultipliedAlpha ? kPremul_SkAlphaType		// https://developer.android.com/reference/android/view/SurfaceControl.Builder#setOpaque(boolean)
	: kUnpremul_SkAlphaType,		// The proper way to do this is to use an SkColorType that ignores
	grContext);		// 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();		auto texMatrix = getSkM44(item.textureTransform).asM33();
	// textureTansform was intended to be passed directly into a shader, so when		// 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);		shader = image->makeShader(SkSamplingOptions(), matrix);
	}		}

	// Handle opaque images - it's a little nonstandard how we do this.		if (useIsOpaqueWorkaround) {
	// 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) {
	shader = SkShaders::Blend(SkBlendMode::kPlus, shader,		shader = SkShaders::Blend(SkBlendMode::kPlus, shader,
	SkShaders::Color(SkColors::kBlack,		SkShaders::Color(SkColors::kBlack,
	toSkColorSpace(layerDataspace)));		toSkColorSpace(layerDataspace)));

libs/renderengine/tests/RenderEngineTest.cpp

+65 −0

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

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

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

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

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

	class SkiaGLESRenderEngineFactory : public RenderEngineFactory {		class SkiaGLESRenderEngineFactory : public RenderEngineFactory {
	@@ -130,9 +135,16 @@ public:
	.setSupportsBackgroundBlur(true)		.setSupportsBackgroundBlur(true)
	.setContextPriority(renderengine::RenderEngine::ContextPriority::MEDIUM)		.setContextPriority(renderengine::RenderEngine::ContextPriority::MEDIUM)
	.setRenderEngineType(type())		.setRenderEngineType(type())
			// FIXME (b/189935602): This version is currently color managed.
			// We should change it and fix the tests that fail.
			//.setUseColorManagerment(false)
	.build();		.build();
	return renderengine::skia::SkiaGLRenderEngine::create(reCreationArgs);		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 {		class SkiaGLESCMRenderEngineFactory : public RenderEngineFactory {
	@@ -157,6 +169,8 @@ public:
	.build();		.build();
	return renderengine::skia::SkiaGLRenderEngine::create(reCreationArgs);		return renderengine::skia::SkiaGLRenderEngine::create(reCreationArgs);
	}		}

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

	class RenderEngineTest : public ::testing::TestWithParam<std::shared_ptr<RenderEngineFactory>> {		class RenderEngineTest : public ::testing::TestWithParam<std::shared_ptr<RenderEngineFactory>> {
	@@ -295,6 +309,7 @@ public:
	const uint8_t expected[4] = {r, g, b, a};		const uint8_t expected[4] = {r, g, b, a};
	bool equal = colorCompare(src, expected);		bool equal = colorCompare(src, expected);
	EXPECT_TRUE(equal)		EXPECT_TRUE(equal)
			<< GetParam()->name().c_str() << ": "
	<< "pixel @ (" << region.left + i << ", " << region.top + j << "): "		<< "pixel @ (" << region.left + i << ", " << region.top + j << "): "
	<< "expected (" << static_cast<uint32_t>(r) << ", "		<< "expected (" << static_cast<uint32_t>(r) << ", "
	<< static_cast<uint32_t>(g) << ", " << static_cast<uint32_t>(b) << ", "		<< static_cast<uint32_t>(g) << ", " << static_cast<uint32_t>(b) << ", "
	@@ -2015,6 +2030,56 @@ TEST_P(RenderEngineTest, testDisableBlendingBuffer) {
	expectBufferColor(rect, 0, 128, 0, 128);		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		} // namespace android

	// TODO(b/129481165): remove the #pragma below and fix conversion issues		// TODO(b/129481165): remove the #pragma below and fix conversion issues