Ensure SkiaPipeline always has a valid colorspace.
Previously we didn't assign a colorspace to the pipeline until it
was provided a surface to render into. This resulted in undefined
behavior if the application attempted to render an offscreen layer
before the OS provided the main window with its surface. Now instead
of deferring setting whether or not the application is wide gamut we
do initialize it to a default setting when the pipeline is created.
Bug: 148042673
Test: apct/device_boot_health_check_extra_postsubmit
Change-Id: I84d743511e949ac977486470bb14eec936de7f88
diff --git a/core/jni/android_view_ThreadedRenderer.cpp b/core/jni/android_view_ThreadedRenderer.cpp
index 69ca17c..5a8225c 100644
--- a/core/jni/android_view_ThreadedRenderer.cpp
+++ b/core/jni/android_view_ThreadedRenderer.cpp
@@ -147,10 +147,12 @@
}
static jlong android_view_ThreadedRenderer_createProxy(JNIEnv* env, jobject clazz,
- jboolean translucent, jlong rootRenderNodePtr) {
+ jboolean translucent, jboolean isWideGamut, jlong rootRenderNodePtr) {
RootRenderNode* rootRenderNode = reinterpret_cast<RootRenderNode*>(rootRenderNodePtr);
ContextFactoryImpl factory(rootRenderNode);
- return (jlong) new RenderProxy(translucent, rootRenderNode, &factory);
+ RenderProxy* proxy = new RenderProxy(translucent, rootRenderNode, &factory);
+ proxy->setWideGamut(isWideGamut);
+ return (jlong) proxy;
}
static void android_view_ThreadedRenderer_deleteProxy(JNIEnv* env, jobject clazz,
@@ -627,7 +629,7 @@
{ "nSetProcessStatsBuffer", "(I)V", (void*) android_view_ThreadedRenderer_setProcessStatsBuffer },
{ "nGetRenderThreadTid", "(J)I", (void*) android_view_ThreadedRenderer_getRenderThreadTid },
{ "nCreateRootRenderNode", "()J", (void*) android_view_ThreadedRenderer_createRootRenderNode },
- { "nCreateProxy", "(ZJ)J", (void*) android_view_ThreadedRenderer_createProxy },
+ { "nCreateProxy", "(ZZJ)J", (void*) android_view_ThreadedRenderer_createProxy },
{ "nDeleteProxy", "(J)V", (void*) android_view_ThreadedRenderer_deleteProxy },
{ "nLoadSystemProperties", "(J)Z", (void*) android_view_ThreadedRenderer_loadSystemProperties },
{ "nSetName", "(JLjava/lang/String;)V", (void*) android_view_ThreadedRenderer_setName },
diff --git a/graphics/java/android/graphics/HardwareRenderer.java b/graphics/java/android/graphics/HardwareRenderer.java
index 3b86413..d08bfcf 100644
--- a/graphics/java/android/graphics/HardwareRenderer.java
+++ b/graphics/java/android/graphics/HardwareRenderer.java
@@ -157,7 +157,7 @@
public HardwareRenderer() {
mRootNode = RenderNode.adopt(nCreateRootRenderNode());
mRootNode.setClipToBounds(false);
- mNativeProxy = nCreateProxy(!mOpaque, mRootNode.mNativeRenderNode);
+ mNativeProxy = nCreateProxy(!mOpaque, mIsWideGamut, mRootNode.mNativeRenderNode);
if (mNativeProxy == 0) {
throw new OutOfMemoryError("Unable to create hardware renderer");
}
@@ -1085,7 +1085,8 @@
private static native long nCreateRootRenderNode();
- private static native long nCreateProxy(boolean translucent, long rootRenderNode);
+ private static native long nCreateProxy(boolean translucent, boolean isWideGamut,
+ long rootRenderNode);
private static native void nDeleteProxy(long nativeProxy);
diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
index e7efe2f..8d5acc6 100644
--- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
@@ -171,17 +171,15 @@
}
bool SkiaOpenGLPipeline::setSurface(ANativeWindow* surface, SwapBehavior swapBehavior,
- ColorMode colorMode, uint32_t extraBuffers) {
+ uint32_t extraBuffers) {
if (mEglSurface != EGL_NO_SURFACE) {
mEglManager.destroySurface(mEglSurface);
mEglSurface = EGL_NO_SURFACE;
}
- setSurfaceColorProperties(colorMode);
-
if (surface) {
mRenderThread.requireGlContext();
- auto newSurface = mEglManager.createSurface(surface, colorMode, mSurfaceColorSpace);
+ auto newSurface = mEglManager.createSurface(surface, mColorMode, mSurfaceColorSpace);
if (!newSurface) {
return false;
}
diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h
index 3fe0f92..e482cad 100644
--- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h
+++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h
@@ -44,7 +44,7 @@
FrameInfo* currentFrameInfo, bool* requireSwap) override;
DeferredLayerUpdater* createTextureLayer() override;
bool setSurface(ANativeWindow* surface, renderthread::SwapBehavior swapBehavior,
- renderthread::ColorMode colorMode, uint32_t extraBuffers) override;
+ uint32_t extraBuffers) override;
void onStop() override;
bool isSurfaceReady() override;
bool isContextReady() override;
diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.cpp b/libs/hwui/pipeline/skia/SkiaPipeline.cpp
index 6f4af3d..29b4dd7 100644
--- a/libs/hwui/pipeline/skia/SkiaPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaPipeline.cpp
@@ -44,6 +44,7 @@
namespace skiapipeline {
SkiaPipeline::SkiaPipeline(RenderThread& thread) : mRenderThread(thread) {
+ setSurfaceColorProperties(mColorMode);
}
SkiaPipeline::~SkiaPipeline() {
@@ -584,6 +585,7 @@
}
void SkiaPipeline::setSurfaceColorProperties(ColorMode colorMode) {
+ mColorMode = colorMode;
if (colorMode == ColorMode::SRGB) {
mSurfaceColorType = SkColorType::kN32_SkColorType;
mSurfaceColorSpace = SkColorSpace::MakeSRGB();
diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.h b/libs/hwui/pipeline/skia/SkiaPipeline.h
index af8414d..8341164 100644
--- a/libs/hwui/pipeline/skia/SkiaPipeline.h
+++ b/libs/hwui/pipeline/skia/SkiaPipeline.h
@@ -50,6 +50,7 @@
bool createOrUpdateLayer(RenderNode* node, const DamageAccumulator& damageAccumulator,
ErrorHandler* errorHandler) override;
+ void setSurfaceColorProperties(renderthread::ColorMode colorMode) override;
SkColorType getSurfaceColorType() const override { return mSurfaceColorType; }
sk_sp<SkColorSpace> getSurfaceColorSpace() override { return mSurfaceColorSpace; }
@@ -72,9 +73,10 @@
protected:
void dumpResourceCacheUsage() const;
- void setSurfaceColorProperties(renderthread::ColorMode colorMode);
renderthread::RenderThread& mRenderThread;
+
+ renderthread::ColorMode mColorMode = renderthread::ColorMode::SRGB;
SkColorType mSurfaceColorType;
sk_sp<SkColorSpace> mSurfaceColorSpace;
diff --git a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
index ad7c706..535a199 100644
--- a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
@@ -117,17 +117,16 @@
void SkiaVulkanPipeline::onStop() {}
bool SkiaVulkanPipeline::setSurface(ANativeWindow* surface, SwapBehavior swapBehavior,
- ColorMode colorMode, uint32_t extraBuffers) {
+ uint32_t extraBuffers) {
if (mVkSurface) {
mVkManager.destroySurface(mVkSurface);
mVkSurface = nullptr;
}
- setSurfaceColorProperties(colorMode);
if (surface) {
mRenderThread.requireVkContext();
mVkSurface =
- mVkManager.createSurface(surface, colorMode, mSurfaceColorSpace, mSurfaceColorType,
+ mVkManager.createSurface(surface, mColorMode, mSurfaceColorSpace, mSurfaceColorType,
mRenderThread.getGrContext(), extraBuffers);
}
diff --git a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h
index 3173478..c8bf233 100644
--- a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h
+++ b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h
@@ -43,7 +43,7 @@
FrameInfo* currentFrameInfo, bool* requireSwap) override;
DeferredLayerUpdater* createTextureLayer() override;
bool setSurface(ANativeWindow* surface, renderthread::SwapBehavior swapBehavior,
- renderthread::ColorMode colorMode, uint32_t extraBuffers) override;
+ uint32_t extraBuffers) override;
void onStop() override;
bool isSurfaceReady() override;
bool isContextReady() override;
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index c1435d1e..0f3901cb 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -161,9 +161,8 @@
mRenderAheadCapacity = mRenderAheadDepth;
}
- ColorMode colorMode = mWideColorGamut ? ColorMode::WideColorGamut : ColorMode::SRGB;
bool hasSurface = mRenderPipeline->setSurface(
- mNativeSurface ? mNativeSurface->getNativeWindow() : nullptr, mSwapBehavior, colorMode,
+ mNativeSurface ? mNativeSurface->getNativeWindow() : nullptr, mSwapBehavior,
mRenderAheadCapacity);
mFrameNumber = -1;
@@ -225,7 +224,8 @@
}
void CanvasContext::setWideGamut(bool wideGamut) {
- mWideColorGamut = wideGamut;
+ ColorMode colorMode = wideGamut ? ColorMode::WideColorGamut : ColorMode::SRGB;
+ mRenderPipeline->setSurfaceColorProperties(colorMode);
}
bool CanvasContext::makeCurrent() {
diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h
index 0967b20..629c741 100644
--- a/libs/hwui/renderthread/CanvasContext.h
+++ b/libs/hwui/renderthread/CanvasContext.h
@@ -251,7 +251,6 @@
nsecs_t mLastDropVsync = 0;
bool mOpaque;
- bool mWideColorGamut = false;
bool mUseForceDark = false;
LightInfo mLightInfo;
LightGeometry mLightGeometry = {{0, 0, 0}, 0};
diff --git a/libs/hwui/renderthread/IRenderPipeline.h b/libs/hwui/renderthread/IRenderPipeline.h
index ef0aa98..ba0d64c 100644
--- a/libs/hwui/renderthread/IRenderPipeline.h
+++ b/libs/hwui/renderthread/IRenderPipeline.h
@@ -66,7 +66,7 @@
virtual bool swapBuffers(const Frame& frame, bool drew, const SkRect& screenDirty,
FrameInfo* currentFrameInfo, bool* requireSwap) = 0;
virtual DeferredLayerUpdater* createTextureLayer() = 0;
- virtual bool setSurface(ANativeWindow* window, SwapBehavior swapBehavior, ColorMode colorMode,
+ virtual bool setSurface(ANativeWindow* window, SwapBehavior swapBehavior,
uint32_t extraBuffers) = 0;
virtual void onStop() = 0;
virtual bool isSurfaceReady() = 0;
@@ -80,6 +80,8 @@
virtual bool pinImages(std::vector<SkImage*>& mutableImages) = 0;
virtual bool pinImages(LsaVector<sk_sp<Bitmap>>& images) = 0;
virtual void unpinImages() = 0;
+
+ virtual void setSurfaceColorProperties(ColorMode colorMode) = 0;
virtual SkColorType getSurfaceColorType() const = 0;
virtual sk_sp<SkColorSpace> getSurfaceColorSpace() = 0;
virtual GrSurfaceOrigin getSurfaceOrigin() = 0;
diff --git a/libs/hwui/tests/unit/SkiaPipelineTests.cpp b/libs/hwui/tests/unit/SkiaPipelineTests.cpp
index 307d136..90bcd1c 100644
--- a/libs/hwui/tests/unit/SkiaPipelineTests.cpp
+++ b/libs/hwui/tests/unit/SkiaPipelineTests.cpp
@@ -398,7 +398,7 @@
auto surface = context.surface();
auto pipeline = std::make_unique<SkiaOpenGLPipeline>(renderThread);
EXPECT_FALSE(pipeline->isSurfaceReady());
- EXPECT_TRUE(pipeline->setSurface(surface.get(), SwapBehavior::kSwap_default, ColorMode::SRGB, 0));
+ EXPECT_TRUE(pipeline->setSurface(surface.get(), SwapBehavior::kSwap_default, 0));
EXPECT_TRUE(pipeline->isSurfaceReady());
renderThread.destroyRenderingContext();
EXPECT_FALSE(pipeline->isSurfaceReady());