Merge "Ensure SkiaPipeline always has a valid colorspace."

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 aa8bac9362ab..91f9447a3d59 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());