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
This commit is contained in:
Derek Sollenberger
2020-02-05 15:41:51 -05:00
parent 929de64796
commit 1863d94e9a
12 changed files with 26 additions and 21 deletions

View File

@@ -147,10 +147,12 @@ static jlong android_view_ThreadedRenderer_createRootRenderNode(JNIEnv* env, job
}
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 @@ static const JNINativeMethod gMethods[] = {
{ "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 },

View File

@@ -157,7 +157,7 @@ public class HardwareRenderer {
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 @@ public class HardwareRenderer {
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);

View File

@@ -171,17 +171,15 @@ static void setBufferCount(ANativeWindow* window, uint32_t extraBuffers) {
}
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;
}

View File

@@ -44,7 +44,7 @@ public:
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;

View File

@@ -44,6 +44,7 @@ namespace uirenderer {
namespace skiapipeline {
SkiaPipeline::SkiaPipeline(RenderThread& thread) : mRenderThread(thread) {
setSurfaceColorProperties(mColorMode);
}
SkiaPipeline::~SkiaPipeline() {
@@ -584,6 +585,7 @@ void SkiaPipeline::dumpResourceCacheUsage() const {
}
void SkiaPipeline::setSurfaceColorProperties(ColorMode colorMode) {
mColorMode = colorMode;
if (colorMode == ColorMode::SRGB) {
mSurfaceColorType = SkColorType::kN32_SkColorType;
mSurfaceColorSpace = SkColorSpace::MakeSRGB();

View File

@@ -50,6 +50,7 @@ public:
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 @@ public:
protected:
void dumpResourceCacheUsage() const;
void setSurfaceColorProperties(renderthread::ColorMode colorMode);
renderthread::RenderThread& mRenderThread;
renderthread::ColorMode mColorMode = renderthread::ColorMode::SRGB;
SkColorType mSurfaceColorType;
sk_sp<SkColorSpace> mSurfaceColorSpace;

View File

@@ -117,17 +117,16 @@ DeferredLayerUpdater* SkiaVulkanPipeline::createTextureLayer() {
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);
}

View File

@@ -43,7 +43,7 @@ public:
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;

View File

@@ -161,9 +161,8 @@ void CanvasContext::setSurface(sp<Surface>&& surface, bool enableTimeout) {
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::setOpaque(bool opaque) {
}
void CanvasContext::setWideGamut(bool wideGamut) {
mWideColorGamut = wideGamut;
ColorMode colorMode = wideGamut ? ColorMode::WideColorGamut : ColorMode::SRGB;
mRenderPipeline->setSurfaceColorProperties(colorMode);
}
bool CanvasContext::makeCurrent() {

View File

@@ -251,7 +251,6 @@ private:
nsecs_t mLastDropVsync = 0;
bool mOpaque;
bool mWideColorGamut = false;
bool mUseForceDark = false;
LightInfo mLightInfo;
LightGeometry mLightGeometry = {{0, 0, 0}, 0};

View File

@@ -66,7 +66,7 @@ public:
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 @@ public:
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;

View File

@@ -398,7 +398,7 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaPipeline, context_lost) {
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());