From 2d9155e174d83e9646c951dbbc85fe1eda8b20e9 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Tue, 27 Jun 2017 13:30:10 -0400 Subject: [PATCH] Reland "Run Flutter on iOS and Android with color correct Skia (#3826) * Revert "Revert "Reland "Run Flutter on iOS and Android with color correct Skia" (#3818)" (#3823)" This reverts commit db8d8a9979901d05b011368226ad5bf61b1da13f. * Fix test code to match internal API change --- flow/compositor_context.h | 2 +- flow/layers/layer.h | 1 + flow/layers/layer_tree.cc | 6 +- flow/layers/picture_layer.cc | 3 +- flow/raster_cache.cc | 7 +- flow/raster_cache.h | 2 + flow/raster_cache_unittests.cc | 25 ++-- lib/ui/painting/image_decoding.cc | 5 +- shell/common/platform_view.cc | 1 + shell/gpu/gpu_surface_gl.cc | 36 ++++-- shell/gpu/gpu_surface_gl.h | 3 + shell/platform/android/android_context_gl.cc | 121 +++++++++++------- shell/platform/android/android_context_gl.h | 16 +-- shell/platform/android/android_surface_gl.cc | 7 +- shell/platform/android/android_surface_gl.h | 9 ++ shell/platform/darwin/ios/ios_gl_context.h | 5 + shell/platform/darwin/ios/ios_gl_context.mm | 34 ++++- shell/platform/darwin/ios/ios_surface_gl.h | 4 + .../darwin/ios/ios_surface_software.mm | 5 +- 19 files changed, 197 insertions(+), 95 deletions(-) diff --git a/flow/compositor_context.h b/flow/compositor_context.h index c441f11dd35..da0312362c2 100644 --- a/flow/compositor_context.h +++ b/flow/compositor_context.h @@ -21,7 +21,7 @@ class CompositorContext { public: class ScopedFrame { public: - SkCanvas& canvas() { return *canvas_; } + SkCanvas* canvas() { return canvas_; } CompositorContext& context() const { return context_; } diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 8bf78777742..c42abe802c9 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -35,6 +35,7 @@ class Layer { struct PrerollContext { RasterCache* raster_cache; GrContext* gr_context; + SkColorSpace* dst_color_space; SkRect child_paint_bounds; }; diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index 0bbbd582235..aab6df5e844 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -27,11 +27,13 @@ void LayerTree::Raster(CompositorContext::ScopedFrame& frame, void LayerTree::Preroll(CompositorContext::ScopedFrame& frame, bool ignore_raster_cache) { TRACE_EVENT0("flutter", "LayerTree::Preroll"); + SkColorSpace* color_space = + frame.canvas() ? frame.canvas()->imageInfo().colorSpace() : nullptr; frame.context().raster_cache().SetCheckboardCacheImages( checkerboard_raster_cache_images_); Layer::PrerollContext context = { ignore_raster_cache ? nullptr : &frame.context().raster_cache(), - frame.gr_context(), SkRect::MakeEmpty(), + frame.gr_context(), color_space, SkRect::MakeEmpty(), }; root_layer_->Preroll(&context, SkMatrix::I()); } @@ -51,7 +53,7 @@ void LayerTree::UpdateScene(SceneUpdateContext& context, #endif void LayerTree::Paint(CompositorContext::ScopedFrame& frame) { - Layer::PaintContext context = {frame.canvas(), frame.context().frame_time(), + Layer::PaintContext context = {*frame.canvas(), frame.context().frame_time(), frame.context().engine_time(), frame.context().memory_usage(), checkerboard_offscreen_layers_}; diff --git a/flow/layers/picture_layer.cc b/flow/layers/picture_layer.cc index cc6a4a86c52..66bb07f4218 100644 --- a/flow/layers/picture_layer.cc +++ b/flow/layers/picture_layer.cc @@ -23,7 +23,8 @@ PictureLayer::~PictureLayer() { void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { if (auto cache = context->raster_cache) { raster_cache_result_ = cache->GetPrerolledImage( - context->gr_context, picture_.get(), matrix, is_complex_, will_change_); + context->gr_context, picture_.get(), matrix, context->dst_color_space, + is_complex_, will_change_); } SkRect bounds = picture_->cullRect().makeOffset(offset_.x(), offset_.y()); diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index eb11e1cdb5e..ee0b1742146 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -71,6 +71,7 @@ static bool IsPictureWorthRasterizing(SkPicture* picture, RasterCacheResult RasterizePicture(SkPicture* picture, GrContext* context, const MatrixDecomposition& matrix, + SkColorSpace* dst_color_space, bool checkerboard) { TRACE_EVENT0("flutter", "RasterCachePopulate"); @@ -81,7 +82,7 @@ RasterCacheResult RasterizePicture(SkPicture* picture, std::ceil(logical_rect.width() * std::abs(scale.x())), // physical width std::ceil(logical_rect.height() * std::abs(scale.y())), // physical height - nullptr // colorspace + sk_ref_sp(dst_color_space) // colorspace ); sk_sp surface = @@ -130,6 +131,7 @@ RasterCacheResult RasterCache::GetPrerolledImage( GrContext* context, SkPicture* picture, const SkMatrix& transformation_matrix, + SkColorSpace* dst_color_space, bool is_complex, bool will_change) { if (!IsPictureWorthRasterizing(picture, will_change, is_complex)) { @@ -159,7 +161,8 @@ RasterCacheResult RasterCache::GetPrerolledImage( if (!entry.image.is_valid()) { entry.image = - RasterizePicture(picture, context, matrix, checkerboard_images_); + RasterizePicture(picture, context, matrix, dst_color_space, + checkerboard_images_); } // We are not considering unrasterizable images. So if we don't have an image diff --git a/flow/raster_cache.h b/flow/raster_cache.h index 9601082cbc7..b5389311959 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -53,8 +53,10 @@ class RasterCache { RasterCacheResult GetPrerolledImage(GrContext* context, SkPicture* picture, const SkMatrix& transformation_matrix, + SkColorSpace* dst_color_space, bool is_complex, bool will_change); + void SweepAfterFrame(); void Clear(); diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 361a06c34e9..b361edf67ec 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -32,14 +32,15 @@ TEST(RasterCache, ThresholdIsRespected) { sk_sp image; + sk_sp srgb = SkColorSpace::MakeSRGB(); ASSERT_FALSE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 1 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 1 cache.SweepAfterFrame(); ASSERT_FALSE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 2 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 2 cache.SweepAfterFrame(); ASSERT_TRUE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 3 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 3 cache.SweepAfterFrame(); } @@ -53,14 +54,15 @@ TEST(RasterCache, ThresholdIsRespectedWhenZero) { sk_sp image; + sk_sp srgb = SkColorSpace::MakeSRGB(); ASSERT_FALSE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 1 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 1 cache.SweepAfterFrame(); ASSERT_FALSE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 2 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 2 cache.SweepAfterFrame(); ASSERT_FALSE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 3 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 3 cache.SweepAfterFrame(); } @@ -74,19 +76,20 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) { sk_sp image; + sk_sp srgb = SkColorSpace::MakeSRGB(); ASSERT_FALSE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 1 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 1 cache.SweepAfterFrame(); ASSERT_FALSE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 2 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 2 cache.SweepAfterFrame(); ASSERT_TRUE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 3 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 3 cache.SweepAfterFrame(); ASSERT_TRUE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 4 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 4 cache.SweepAfterFrame(); cache.SweepAfterFrame(); // Extra frame without a preroll image access. ASSERT_FALSE( - cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 5 + cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb.get(), true, false)); // 5 } diff --git a/lib/ui/painting/image_decoding.cc b/lib/ui/painting/image_decoding.cc index 50a4bc8f461..9c9a3518675 100644 --- a/lib/ui/painting/image_decoding.cc +++ b/lib/ui/painting/image_decoding.cc @@ -32,9 +32,10 @@ sk_sp DecodeImage(sk_sp buffer) { GrContext* context = ResourceContext::Get(); if (context) { - // TODO: Supply actual destination color space once available + // This acts as a flag to indicate that we want a color space aware decode. + sk_sp dstColorSpace = SkColorSpace::MakeSRGB(); return SkImage::MakeCrossContextFromEncoded(context, std::move(buffer), - false, nullptr); + false, dstColorSpace.get()); } else { return SkImage::MakeFromEncoded(std::move(buffer)); } diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index 88e34607f86..f7baf456436 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -171,6 +171,7 @@ void PlatformView::SetupResourceContextOnIOThreadPerform( // other threads correctly, so the textures end up blank. For now, suppress // that feature, which will cause texture uploads to do CPU YUV conversion. options.fDisableGpuYUVConversion = true; + options.fRequireDecodeDisableForSRGB = false; blink::ResourceContext::Set(GrContext::Create( GrBackend::kOpenGL_GrBackend, diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc index 8380b6bd2ce..ea4ea321e07 100644 --- a/shell/gpu/gpu_surface_gl.cc +++ b/shell/gpu/gpu_surface_gl.cc @@ -8,6 +8,7 @@ #include "lib/ftl/arraysize.h" #include "lib/ftl/logging.h" #include "third_party/skia/include/core/SkSurface.h" +#include "third_party/skia/include/gpu/GrContextOptions.h" #include "third_party/skia/include/gpu/gl/GrGLInterface.h" namespace shell { @@ -44,9 +45,12 @@ bool GPUSurfaceGL::Setup() { auto backend_context = reinterpret_cast(GrGLCreateNativeInterface()); - context_ = - sk_sp(GrContext::Create(kOpenGL_GrBackend, backend_context)); + GrContextOptions options; + options.fRequireDecodeDisableForSRGB = false; + context_ = + sk_sp(GrContext::Create(kOpenGL_GrBackend, backend_context, + options)); if (context_ == nullptr) { FTL_LOG(INFO) << "Failed to setup Skia Gr context."; return false; @@ -104,15 +108,23 @@ bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) { } bool GPUSurfaceGL::SelectPixelConfig(GrPixelConfig* config) { - static const GrPixelConfig kConfigOptions[] = { - kSkia8888_GrPixelConfig, kRGBA_4444_GrPixelConfig, - }; + if (delegate_->ColorSpace() && delegate_->ColorSpace()->gammaCloseToSRGB()) { + FTL_DCHECK(context_->caps()->isConfigRenderable(kSRGBA_8888_GrPixelConfig, + false)); + *config = kSRGBA_8888_GrPixelConfig; + return true; + } - for (size_t i = 0; i < arraysize(kConfigOptions); i++) { - if (context_->caps()->isConfigRenderable(kConfigOptions[i], false)) { - *config = kConfigOptions[i]; - return true; - } + // FIXME: + // If sRGB support is not available, we should instead fall back to software. + if (context_->caps()->isConfigRenderable(kRGBA_8888_GrPixelConfig, false)) { + *config = kRGBA_8888_GrPixelConfig; + return true; + } + + if (context_->caps()->isConfigRenderable(kRGBA_4444_GrPixelConfig, false)) { + *config = kRGBA_4444_GrPixelConfig; + return true; } return false; @@ -124,7 +136,6 @@ sk_sp GPUSurfaceGL::CreateSurface(const SkISize& size) { } GrBackendRenderTargetDesc desc; - if (!SelectPixelConfig(&desc.fConfig)) { return nullptr; } @@ -135,7 +146,8 @@ sk_sp GPUSurfaceGL::CreateSurface(const SkISize& size) { desc.fOrigin = kBottomLeft_GrSurfaceOrigin; desc.fRenderTargetHandle = delegate_->GLContextFBO(); - return SkSurface::MakeFromBackendRenderTarget(context_.get(), desc, nullptr); + return SkSurface::MakeFromBackendRenderTarget(context_.get(), desc, + delegate_->ColorSpace(), nullptr); } sk_sp GPUSurfaceGL::AcquireSurface(const SkISize& size) { diff --git a/shell/gpu/gpu_surface_gl.h b/shell/gpu/gpu_surface_gl.h index 95a6527ba55..219d8fdd176 100644 --- a/shell/gpu/gpu_surface_gl.h +++ b/shell/gpu/gpu_surface_gl.h @@ -22,6 +22,9 @@ class GPUSurfaceGLDelegate { virtual bool GLContextPresent() = 0; virtual intptr_t GLContextFBO() const = 0; + + // TODO: Update Mac desktop and make this pure virtual. + virtual sk_sp ColorSpace() const { return nullptr; } }; class GPUSurfaceGL : public Surface { diff --git a/shell/platform/android/android_context_gl.cc b/shell/platform/android/android_context_gl.cc index 9ae9dfb8e21..a8e14c87fc7 100644 --- a/shell/platform/android/android_context_gl.cc +++ b/shell/platform/android/android_context_gl.cc @@ -3,9 +3,17 @@ // found in the LICENSE file. #include "flutter/shell/platform/android/android_context_gl.h" - +#include #include +#ifndef EGL_GL_COLORSPACE_KHR +#define EGL_GL_COLORSPACE_KHR 0x309D +#endif + +#ifndef EGL_GL_COLORSPACE_SRGB_KHR +#define EGL_GL_COLORSPACE_SRGB_KHR 0x3089 +#endif + namespace shell { template @@ -112,43 +120,66 @@ static bool TeardownSurface(EGLDisplay display, EGLSurface surface) { } // For onscreen rendering. -static EGLResult CreateWindowSurface( - EGLDisplay display, - EGLConfig config, - AndroidNativeWindow::Handle window_handle) { +bool AndroidContextGL::CreateWindowSurface( + ftl::RefPtr window) { // The configurations are only required when dealing with extensions or VG. // We do neither. - EGLSurface surface = eglCreateWindowSurface( - display, config, reinterpret_cast(window_handle), - nullptr); - return {surface != EGL_NO_SURFACE, surface}; + + window_ = std::move(window); + EGLDisplay display = environment_->Display(); + + const EGLint srgb_attribs[] = { + EGL_GL_COLORSPACE_KHR, EGL_GL_COLORSPACE_SRGB_KHR, + EGL_NONE + }; + const EGLint default_attribs[] = { + EGL_NONE + }; + + const EGLint* attribs = default_attribs; + if (srgb_support_) { + attribs = srgb_attribs; + } + + surface_ = eglCreateWindowSurface( + display, config_, + reinterpret_cast(window_->handle()), attribs); + return surface_ != EGL_NO_SURFACE; } // For offscreen rendering. -static EGLResult CreatePBufferSurface(EGLDisplay display, - EGLConfig config) { +bool AndroidContextGL::CreatePBufferSurface() { // We only ever create pbuffer surfaces for background resource loading // contexts. We never bind the pbuffer to anything. - const EGLint attribs[] = {EGL_WIDTH, 1, EGL_HEIGHT, 1, EGL_NONE}; - EGLSurface surface = eglCreatePbufferSurface(display, config, attribs); - return {surface != EGL_NO_SURFACE, surface}; -} -static EGLResult CreateSurface( - EGLDisplay display, - EGLConfig config, - ftl::RefPtr window) { - return window && window->IsValid() - ? CreateWindowSurface(display, config, window->handle()) - : CreatePBufferSurface(display, config); + EGLDisplay display = environment_->Display(); + + const EGLint srgb_attribs[] = { + EGL_WIDTH, 1, + EGL_HEIGHT, 1, + EGL_GL_COLORSPACE_KHR, EGL_GL_COLORSPACE_SRGB_KHR, + EGL_NONE + }; + const EGLint default_attribs[] = { + EGL_WIDTH, 1, + EGL_HEIGHT, 1, + EGL_NONE + }; + + const EGLint* attribs = default_attribs; + if (srgb_support_) { + attribs = srgb_attribs; + } + + surface_ = eglCreatePbufferSurface(display, config_, attribs); + return surface_ != EGL_NO_SURFACE; } AndroidContextGL::AndroidContextGL(ftl::RefPtr env, - ftl::RefPtr window, PlatformView::SurfaceConfig config, const AndroidContextGL* share_context) : environment_(env), - window_(window), + window_(nullptr), config_(nullptr), surface_(EGL_NO_SURFACE), context_(EGL_NO_CONTEXT), @@ -170,17 +201,6 @@ AndroidContextGL::AndroidContextGL(ftl::RefPtr env, return; } - // Create a surface for the configuration. - - std::tie(success, surface_) = - CreateSurface(environment_->Display(), config_, window_); - - if (!success) { - FTL_LOG(ERROR) << "Could not create the EGL surface."; - LogLastEGLError(); - return; - } - // Create a context for the configuration. std::tie(success, context_) = CreateContext( @@ -193,15 +213,22 @@ AndroidContextGL::AndroidContextGL(ftl::RefPtr env, return; } + // On its own, this is not enough to guarantee that we will render in + // sRGB mode. We also need to query GL using the GrContext. + + const char* exts = eglQueryString(environment_->Display(), EGL_EXTENSIONS); + srgb_support_ = strstr(exts, "EGL_KHR_gl_colorspace"); + + if (!this->CreatePBufferSurface()) { + FTL_LOG(ERROR) << "Could not create the EGL surface."; + LogLastEGLError(); + return; + } + // All done! valid_ = true; } -AndroidContextGL::AndroidContextGL(ftl::RefPtr env, - PlatformView::SurfaceConfig config, - const AndroidContextGL* share_context) - : AndroidContextGL(env, nullptr, config, share_context) {} - AndroidContextGL::~AndroidContextGL() { if (!TeardownContext(environment_->Display(), context_)) { FTL_LOG(ERROR) << "Could not tear down the EGL context. Possible resource leak."; @@ -270,18 +297,18 @@ bool AndroidContextGL::Resize(const SkISize& size) { TeardownSurface(environment_->Display(), surface_); - bool success = false; - std::tie(success, surface_) = - CreateSurface(environment_->Display(), config_, window_); - - MakeCurrent(); - - if (!success) { + if (!this->CreateWindowSurface(window_)) { FTL_LOG(ERROR) << "Unable to create EGL window surface on resize."; return false; } + MakeCurrent(); + return true; } +bool AndroidContextGL::SupportsSRGB() const { + return srgb_support_; +} + } // namespace shell diff --git a/shell/platform/android/android_context_gl.h b/shell/platform/android/android_context_gl.h index aa2a081a510..0cad635a7e1 100644 --- a/shell/platform/android/android_context_gl.h +++ b/shell/platform/android/android_context_gl.h @@ -17,6 +17,11 @@ namespace shell { class AndroidContextGL : public ftl::RefCountedThreadSafe { public: + + bool CreateWindowSurface(ftl::RefPtr window); + + bool CreatePBufferSurface(); + ftl::RefPtr Environment() const; bool IsValid() const; @@ -31,22 +36,17 @@ class AndroidContextGL : public ftl::RefCountedThreadSafe { bool Resize(const SkISize& size); + bool SupportsSRGB() const; + private: ftl::RefPtr environment_; ftl::RefPtr window_; EGLConfig config_; EGLSurface surface_; EGLContext context_; + bool srgb_support_; bool valid_; - /// Creates a window surface context tied to the window handle for on-screen - /// rendering. - AndroidContextGL(ftl::RefPtr env, - ftl::RefPtr window, - PlatformView::SurfaceConfig config, - const AndroidContextGL* share_context = nullptr); - - /// Creates a pbuffer surface context for offscreen rendering. AndroidContextGL(ftl::RefPtr env, PlatformView::SurfaceConfig config, const AndroidContextGL* share_context = nullptr); diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index 9e86824f460..fbfd03cb0b6 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -117,7 +117,7 @@ bool AndroidSurfaceGL::SetNativeWindow(ftl::RefPtr window, // Create the onscreen context. onscreen_context_ = ftl::MakeRefCounted( - offscreen_context_->Environment(), std::move(window), config, + offscreen_context_->Environment(), config, offscreen_context_.get() /* sharegroup */); if (!onscreen_context_->IsValid()) { @@ -125,6 +125,11 @@ bool AndroidSurfaceGL::SetNativeWindow(ftl::RefPtr window, return false; } + if (!onscreen_context_->CreateWindowSurface(std::move(window))) { + onscreen_context_ = nullptr; + return false; + } + return true; } diff --git a/shell/platform/android/android_surface_gl.h b/shell/platform/android/android_surface_gl.h index 21a5768db74..29ef4e8e7b2 100644 --- a/shell/platform/android/android_surface_gl.h +++ b/shell/platform/android/android_surface_gl.h @@ -47,12 +47,21 @@ class AndroidSurfaceGL : public GPUSurfaceGLDelegate, public AndroidSurface { intptr_t GLContextFBO() const override; + sk_sp ColorSpace() const override { + // TODO: + // We can render more consistently across devices when Android makes it + // possible to query for the color space of the display. + return onscreen_context_->SupportsSRGB() ? SkColorSpace::MakeSRGB() + : nullptr; + } + void SetFlutterView( const fml::jni::JavaObjectWeakGlobalRef& flutter_view) override; private: ftl::RefPtr onscreen_context_; ftl::RefPtr offscreen_context_; + sk_sp gr_context_; FTL_DISALLOW_COPY_AND_ASSIGN(AndroidSurfaceGL); }; diff --git a/shell/platform/darwin/ios/ios_gl_context.h b/shell/platform/darwin/ios/ios_gl_context.h index 51ad00fde79..52506758d54 100644 --- a/shell/platform/darwin/ios/ios_gl_context.h +++ b/shell/platform/darwin/ios/ios_gl_context.h @@ -34,6 +34,10 @@ class IOSGLContext { bool ResourceMakeCurrent(); + sk_sp ColorSpace() const { + return color_space_; + } + private: fml::scoped_nsobject layer_; fml::scoped_nsobject context_; @@ -45,6 +49,7 @@ class IOSGLContext { GLuint depth_stencil_packed_buffer_; GLint storage_size_width_; GLint storage_size_height_; + sk_sp color_space_; bool valid_; FTL_DISALLOW_COPY_AND_ASSIGN(IOSGLContext); diff --git a/shell/platform/darwin/ios/ios_gl_context.mm b/shell/platform/darwin/ios/ios_gl_context.mm index 4bb90aed433..082e3a11372 100644 --- a/shell/platform/darwin/ios/ios_gl_context.mm +++ b/shell/platform/darwin/ios/ios_gl_context.mm @@ -3,6 +3,10 @@ // found in the LICENSE file. #include "flutter/shell/platform/darwin/ios/ios_gl_context.h" +#include "third_party/skia/include/gpu/GrContextOptions.h" +#include "third_party/skia/include/gpu/gl/GrGLInterface.h" + +#include namespace shell { @@ -96,14 +100,32 @@ IOSGLContext::IOSGLContext(PlatformView::SurfaceConfig config, CAEAGLLayer* laye } } - // The default is RGBA - NSString* drawableColorFormat = kEAGLColorFormatRGBA8; - - if (config.red_bits <= 5 && config.green_bits <= 6 && config.blue_bits <= 5 && - config.alpha_bits == 0) { - drawableColorFormat = kEAGLColorFormatRGB565; + // TODO: + // iOS displays are more variable than just P3 or sRGB. Reading the display + // gamut just tells us what color space it makes sense to render into. We + // should use iOS APIs to perform the final correction step based on the + // device properties. Ex: We can indicate that we have rendered in P3, and + // the framework will do the final adjustment for us. + NSOperatingSystemVersion version = [[NSProcessInfo processInfo] + operatingSystemVersion]; + color_space_ = SkColorSpace::MakeSRGB(); + if (version.majorVersion >= 10) { + UIDisplayGamut displayGamut = + [UIScreen mainScreen].traitCollection.displayGamut; + switch (displayGamut) { + case UIDisplayGamutP3: + // Should we consider using more than 8-bits of precision given that + // P3 specifies a wider range of colors? + color_space_ = SkColorSpace::MakeRGB( + SkColorSpace::kSRGB_RenderTargetGamma, + SkColorSpace::kDCIP3_D65_Gamut); + break; + default: + break; + } } + NSString* drawableColorFormat = kEAGLColorFormatSRGBA8; layer_.get().drawableProperties = @{ kEAGLDrawablePropertyColorFormat : drawableColorFormat, kEAGLDrawablePropertyRetainedBacking : @(NO), diff --git a/shell/platform/darwin/ios/ios_surface_gl.h b/shell/platform/darwin/ios/ios_surface_gl.h index 5f2abb9b3d1..c7900480b14 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.h +++ b/shell/platform/darwin/ios/ios_surface_gl.h @@ -36,6 +36,10 @@ class IOSSurfaceGL : public IOSSurface, public GPUSurfaceGLDelegate { intptr_t GLContextFBO() const override; + sk_sp ColorSpace() const override { + return context_.ColorSpace(); + } + private: IOSGLContext context_; diff --git a/shell/platform/darwin/ios/ios_surface_software.mm b/shell/platform/darwin/ios/ios_surface_software.mm index 7ce5f97b568..759571696c1 100644 --- a/shell/platform/darwin/ios/ios_surface_software.mm +++ b/shell/platform/darwin/ios/ios_surface_software.mm @@ -63,8 +63,9 @@ sk_sp IOSSurfaceSoftware::AcquireBackingStore(const SkISize& size) { return sk_surface_; } - sk_surface_ = SkSurface::MakeRasterN32Premul(size.fWidth, size.fHeight, - nullptr /* SkSurfaceProps as out */); + SkImageInfo info = SkImageInfo::MakeS32(size.fWidth, size.fHeight, + kPremul_SkAlphaType); + sk_surface_ = SkSurface::MakeRaster(info, nullptr); return sk_surface_; }