On emulators, render onscreen (with warnings) if any of the MakeS32 based...

On emulators, render onscreen (with warnings) if any of the MakeS32 based SkSurface creation calls fail. (#3947)

flow/raster_cache.cc

@@ -83,10 +83,10 @@ RasterCacheResult RasterizePicture(SkPicture* picture,
                      std::ceil(std::fabs(logical_rect.height() * scale.y())));
 
   const SkImageInfo image_info =
-      SkImageInfo::MakeN32Premul(physical_rect.width(),     // physical width
-                                 physical_rect.height(),    // physical height
-                                 sk_ref_sp(dst_color_space) // colorspace
-                                 );
+      SkImageInfo::MakeN32Premul(physical_rect.width(),      // physical width
+                                 physical_rect.height(),     // physical height
+                                 sk_ref_sp(dst_color_space)  // colorspace
+      );
 
   sk_sp<SkSurface> surface =
       context
@@ -94,7 +94,6 @@ RasterCacheResult RasterizePicture(SkPicture* picture,
           : SkSurface::MakeRaster(image_info);
 
   if (!surface) {
-    FTL_DCHECK(false);
     return {};
   }
 
@@ -161,15 +160,10 @@ RasterCacheResult RasterCache::GetPrerolledImage(
   }
 
   if (!entry.image.is_valid()) {
-    entry.image =
-        RasterizePicture(picture, context, matrix, dst_color_space,
-                         checkerboard_images_);
+    entry.image = RasterizePicture(picture, context, matrix, dst_color_space,
+                                   checkerboard_images_);
   }
 
-  // We are not considering unrasterizable images. So if we don't have an image
-  // by now, we know that rasterization itself failed.
-  FTL_DCHECK(entry.image.is_valid());
-
   return entry.image;
 }
 

shell/gpu/gpu_surface_gl.cc

@@ -23,9 +23,7 @@ static const int kGrCacheMaxCount = 8192;
 static const size_t kGrCacheMaxByteSize = 512 * (1 << 20);
 
 GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate)
-    : delegate_(delegate),
-      onscreen_surface_supports_srgb_(delegate_->SurfaceSupportsSRGB()),
-      weak_factory_(this) {
+    : delegate_(delegate), weak_factory_(this) {
   if (!delegate_->GLContextMakeCurrent()) {
     FTL_LOG(ERROR)
         << "Could not make the context current to setup the gr context.";
@@ -78,6 +76,18 @@ bool GPUSurfaceGL::IsValid() {
   return valid_;
 }
 
+static GrPixelConfig FirstSupportedNonSRGBConfig(GrContext* context) {
+#define RETURN_IF_RENDERABLE(x)                          \
+  if (context->caps()->isConfigRenderable((x), false)) { \
+    return (x);                                          \
+  }
+
+  RETURN_IF_RENDERABLE(kRGBA_8888_GrPixelConfig);
+  RETURN_IF_RENDERABLE(kRGBA_4444_GrPixelConfig);
+  RETURN_IF_RENDERABLE(kRGB_565_GrPixelConfig);
+  return kUnknown_GrPixelConfig;
+}
+
 static sk_sp<SkSurface> WrapOnscreenSurface(GrContext* context,
                                             const SkISize& size,
                                             intptr_t fbo,
@@ -86,8 +96,9 @@ static sk_sp<SkSurface> WrapOnscreenSurface(GrContext* context,
       .fFBOID = static_cast<GrGLuint>(fbo),
   };
 
-  const GrPixelConfig pixel_config =
-      supports_srgb ? kSRGBA_8888_GrPixelConfig : kRGBA_8888_GrPixelConfig;
+  const GrPixelConfig pixel_config = supports_srgb
+                                         ? kSRGBA_8888_GrPixelConfig
+                                         : FirstSupportedNonSRGBConfig(context);
 
   GrBackendRenderTarget render_target(size.fWidth,      // width
                                       size.fHeight,     // height
@@ -95,7 +106,7 @@ static sk_sp<SkSurface> WrapOnscreenSurface(GrContext* context,
                                       0,                // stencil bits (TODO)
                                       pixel_config,     // pixel config
                                       framebuffer_info  // framebuffer info
-                                      );
+  );
 
   sk_sp<SkColorSpace> colorspace =
       supports_srgb ? SkColorSpace::MakeSRGB() : nullptr;
@@ -109,7 +120,7 @@ static sk_sp<SkSurface> WrapOnscreenSurface(GrContext* context,
       GrSurfaceOrigin::kBottomLeft_GrSurfaceOrigin,  // origin
       colorspace,                                    // colorspace
       &surface_props                                 // surface properties
-      );
+  );
 }
 
 static sk_sp<SkSurface> CreateOffscreenSurface(GrContext* context,
@@ -127,7 +138,7 @@ static sk_sp<SkSurface> CreateOffscreenSurface(GrContext* context,
       0,                            // sample count
       kBottomLeft_GrSurfaceOrigin,  // surface origin
       &surface_props                // surface props
-      );
+  );
 }
 
 bool GPUSurfaceGL::CreateOrUpdateSurfaces(const SkISize& size) {
@@ -154,19 +165,34 @@ bool GPUSurfaceGL::CreateOrUpdateSurfaces(const SkISize& size) {
 
   sk_sp<SkSurface> onscreen_surface, offscreen_surface;
 
-  onscreen_surface =
-      WrapOnscreenSurface(context_.get(), size, delegate_->GLContextFBO(),
-                          onscreen_surface_supports_srgb_);
+  const bool surface_supports_srgb = delegate_->SurfaceSupportsSRGB();
+
+  onscreen_surface = WrapOnscreenSurface(
+      context_.get(), size, delegate_->GLContextFBO(), surface_supports_srgb);
+
   if (onscreen_surface == nullptr) {
+    // If the onscreen surface could not be wrapped. There is absolutely no
+    // point in moving forward.
     FTL_LOG(ERROR) << "Could not wrap onscreen surface.";
     return false;
   }
 
-  if (!onscreen_surface_supports_srgb_) {
+  if (!surface_supports_srgb) {
     offscreen_surface = CreateOffscreenSurface(context_.get(), size);
     if (offscreen_surface == nullptr) {
-      FTL_LOG(ERROR) << "Could not create offscreen surface.";
-      return false;
+      // If the offscreen surface was needed but could not be wrapped. Render to
+      // the onscreen surface directly but warn the user that color correctness
+      // is not available.
+      static bool warned_once = false;
+      if (!warned_once) {
+        warned_once = true;
+        FTL_LOG(ERROR) << "WARNING: Could not create offscreen surface. This "
+                          "device or emulator does not support "
+                          "color correct rendering. Fallbacks are in effect. "
+                          "Colors on this device will differ from those "
+                          "displayed on most other devices. This warning will "
+                          "only be logged once.";
+      }
     }
   }
 
@@ -208,11 +234,10 @@ bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) {
     return false;
   }
 
-  if (!onscreen_surface_supports_srgb_) {
+  if (offscreen_surface_ != nullptr) {
     // Because the surface did not support sRGB, we rendered to an offscreen
     // surface. Now we must ensure that the texture is copied onscreen.
     TRACE_EVENT0("flutter", "CopyTextureOnscreen");
-    FTL_DCHECK(offscreen_surface_ != nullptr);
     SkPaint paint;
     const GrCaps* caps = context_->caps();
     if (caps->srgbSupport() && !caps->srgbDecodeDisableSupport()) {
@@ -223,7 +248,7 @@ bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) {
         0,                                        // left
         0,                                        // top
         &paint                                    // paint
-        );
+    );
   }
 
   {
@@ -241,8 +266,7 @@ sk_sp<SkSurface> GPUSurfaceGL::AcquireRenderSurface(const SkISize& size) {
     return nullptr;
   }
 
-  return onscreen_surface_supports_srgb_ ? onscreen_surface_
-                                         : offscreen_surface_;
+  return offscreen_surface_ != nullptr ? offscreen_surface_ : onscreen_surface_;
 }
 
 GrContext* GPUSurfaceGL::GetContext() {

shell/gpu/gpu_surface_gl.h

@@ -40,7 +40,6 @@ class GPUSurfaceGL : public Surface {
 
  private:
   GPUSurfaceGLDelegate* delegate_;
-  bool onscreen_surface_supports_srgb_;
   sk_sp<GrContext> context_;
   sk_sp<SkSurface> onscreen_surface_;
   sk_sp<SkSurface> offscreen_surface_;