diff options
author | Derek Sollenberger <djsollen@google.com> | 2020-02-05 15:41:51 -0500 |
---|---|---|
committer | Derek Sollenberger <djsollen@google.com> | 2020-02-06 07:41:50 -0500 |
commit | 1863d94e9a32a210cfb50d7c22bbac30dc33e010 (patch) | |
tree | 72e3c07f29692eaa994c888feaefc018b08e7dce /libs/hwui | |
parent | 929de647969cf5efa1778fe4b54ee968bff4d613 (diff) |
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
Diffstat (limited to 'libs/hwui')
-rw-r--r-- | libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp | 6 | ||||
-rw-r--r-- | libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h | 2 | ||||
-rw-r--r-- | libs/hwui/pipeline/skia/SkiaPipeline.cpp | 2 | ||||
-rw-r--r-- | libs/hwui/pipeline/skia/SkiaPipeline.h | 4 | ||||
-rw-r--r-- | libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp | 5 | ||||
-rw-r--r-- | libs/hwui/pipeline/skia/SkiaVulkanPipeline.h | 2 | ||||
-rw-r--r-- | libs/hwui/renderthread/CanvasContext.cpp | 6 | ||||
-rw-r--r-- | libs/hwui/renderthread/CanvasContext.h | 1 | ||||
-rw-r--r-- | libs/hwui/renderthread/IRenderPipeline.h | 4 | ||||
-rw-r--r-- | libs/hwui/tests/unit/SkiaPipelineTests.cpp | 2 |
10 files changed, 18 insertions, 16 deletions
diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp index e7efe2ff798b..8d5acc631274 100644 --- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp +++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp @@ -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; } diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h index 3fe0f92b1924..e482cad6c953 100644 --- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h +++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h @@ -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; diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.cpp b/libs/hwui/pipeline/skia/SkiaPipeline.cpp index 6f4af3d26b3a..29b4dd7f32e7 100644 --- a/libs/hwui/pipeline/skia/SkiaPipeline.cpp +++ b/libs/hwui/pipeline/skia/SkiaPipeline.cpp @@ -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(); diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.h b/libs/hwui/pipeline/skia/SkiaPipeline.h index af8414de4bc1..8341164edc19 100644 --- a/libs/hwui/pipeline/skia/SkiaPipeline.h +++ b/libs/hwui/pipeline/skia/SkiaPipeline.h @@ -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; diff --git a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp index ad7c70614239..535a19956e03 100644 --- a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp +++ b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp @@ -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); } diff --git a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h index 31734783de7f..c8bf233d8e1c 100644 --- a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h +++ b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.h @@ -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; diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp index c1435d1ea2d5..0f3901cb225b 100644 --- a/libs/hwui/renderthread/CanvasContext.cpp +++ b/libs/hwui/renderthread/CanvasContext.cpp @@ -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() { diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h index 0967b20e44ee..629c741e8757 100644 --- a/libs/hwui/renderthread/CanvasContext.h +++ b/libs/hwui/renderthread/CanvasContext.h @@ -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}; diff --git a/libs/hwui/renderthread/IRenderPipeline.h b/libs/hwui/renderthread/IRenderPipeline.h index ef0aa98d4220..ba0d64c5492d 100644 --- a/libs/hwui/renderthread/IRenderPipeline.h +++ b/libs/hwui/renderthread/IRenderPipeline.h @@ -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; diff --git a/libs/hwui/tests/unit/SkiaPipelineTests.cpp b/libs/hwui/tests/unit/SkiaPipelineTests.cpp index 307d13606cb8..90bcd1c0e370 100644 --- a/libs/hwui/tests/unit/SkiaPipelineTests.cpp +++ b/libs/hwui/tests/unit/SkiaPipelineTests.cpp @@ -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()); |