Use Offscreen Surface for Split Screen

tools/viewer/sk_app/WindowContext.cpp

 
 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "GrContext.h"
 #include "SkSurface.h"
 #include "WindowContext.h"
 
 #include "gl/GrGLDefines.h"
 
 #include "gl/GrGLUtil.h"
 #include "GrRenderTarget.h"
 #include "GrContext.h"
 
 #include "SkCanvas.h"
 #include "SkImage_Base.h"
 
 namespace sk_app {
 
-sk_sp<SkSurface> WindowContext::createRenderSurface(sk_sp<GrRenderTarget> rt, int colorBits) {
+sk_sp<SkSurface> WindowContext::createRenderSurface(
+        sk_sp<GrRenderTarget> rt, int colorBits, bool offscreen, bool sRGB) {
     auto flags = (fSurfaceProps.flags() & ~SkSurfaceProps::kGammaCorrect_Flag) |
                  (GrPixelConfigIsSRGB(fPixelConfig) ? SkSurfaceProps::kGammaCorrect_Flag : 0);

[Brian Osman, 2016/06/15 13:41:59]

Need to also set this flag if sRGB is true:

((GrPixelConfigIsSRGB(fPixelConfig) || sRGB) ?
skSurfaceProps::kGammaCorrect_Flag : 0);

[liyuqian, 2016/06/15 14:09:15]

Done.

     SkSurfaceProps props(flags, fSurfaceProps.pixelGeometry());
 
-    if (!this->isGpuContext() || colorBits > 24 ||
+    if (!this->isGpuContext() || colorBits > 24 || offscreen ||
         kRGBA_F16_SkColorType == fDisplayParams.fColorType) {
         // If we're rendering to F16, we need an off-screen surface - the current render
         // target is most likely the wrong format.
         //
         // If we're rendering raster data or using a deep (10-bit or higher) surface, we probably
         // need an off-screen surface. 10-bit, in particular, has strange gamma behavior.
         SkImageInfo info = SkImageInfo::Make(fWidth, fHeight,
                                              fDisplayParams.fColorType,
-                                             kUnknown_SkAlphaType,
-                                             fDisplayParams.fProfileType);
-        return SkSurface::MakeRenderTarget(fContext, SkBudgeted::kNo, info,
-                                           fDisplayParams.fMSAASampleCount, &props);
+                                             // kUnknown_SkAlphaType won't pass

[msarett, 2016/06/15 13:48:10]

Sorry I don't understand what you're saying here?  But I don't typically use
kUnknown anyway.  If you're sure it's opaque, kOpaque makes sense.  Otherwise,
it's probably best to use kPremul.

[liyuqian, 2016/06/15 14:09:15]

I was referring to the previous implementation before my CL. I think that the
previous implementation has never executed this line because MakeRenderTarget
with kUnknown_SkAlphaType will cause crash by CheckAlphaAndGetFlags.

I was using kOpaque in the Raster backend when I create the Surface. Should I
also use kPremul there?

[msarett, 2016/06/15 14:20:40]

Depends on whether what you are drawing is opaque or premul.  Typically, you
should use premul if you're not sure.  If it's relevant, jpegs are opaque.

+                                             // CheckAlphaTypeAndGetFlags
+                                             kOpaque_SkAlphaType,
+                                             sRGB ? kSRGB_SkColorProfileType

[msarett, 2016/06/15 13:48:10]

Please use a SkColorSpace object here instead of the SkColorProfileType.

Ex:

(sRGB || fDisplayParams.fProfileType == kSRGB) ? SkColorSpace::NewSRGB() :
nullptr.

We currently try to keep the SkColorSpace and SkColorProfileType in sync, with
the eventual goal being to delete the SkProfileType.

[liyuqian, 2016/06/15 14:09:15]

The difficulty here is that DisplayParams maintains a SkProfileType which shall
be used if sRGB is not set to true. I'll add Jim to reviewers to let him decide
how to change all SkColorProfileType to SkColorSpace.

[msarett, 2016/06/15 14:20:40]

SGTM.

I still think it's possible to use SkColorSpace here without an API change, you
would just need to check the sRGB bool and the fDisplayParams.
(sRGB || fDisplayParams.fProfileType == kSRGB) ? SkColorSpace::NewSRGB() :
nullptr

But I'm ok if you want to avoid this clutter and stick with the current API.

[liyuqian, 2016/06/15 16:43:12]

Done. I've adopted your approach.

+                                                  : fDisplayParams.fProfileType);
+        if (this->isGpuContext()) {
+            return SkSurface::MakeRenderTarget(fContext, SkBudgeted::kNo, info,
+                                               fDisplayParams.fMSAASampleCount, &props);
+        } else {
+            return SkSurface::MakeRaster(info, &props);
+        }
     } else {
         return SkSurface::MakeRenderTargetDirect(rt.get(), &props);
     }
 }
 
 void WindowContext::presentRenderSurface(sk_sp<SkSurface> renderSurface, sk_sp<GrRenderTarget> rt,
                                          int colorBits) {
     if (!this->isGpuContext() || colorBits > 24 ||
         kRGBA_F16_SkColorType == fDisplayParams.fColorType) {
         // We made/have an off-screen surface. Get the contents as an SkImage:
@@ skipping 13 matching lines @@
 
         // With ten-bit output, we need to manually apply the gamma of the output device
         // (unless we're in non-gamma correct mode, in which case our data is already
         // fake-sRGB, like we're expected to put in the 10-bit buffer):
         bool doGamma = (colorBits == 30) && SkImageInfoIsGammaCorrect(info);
         fContext->applyGamma(rt.get(), texture, doGamma ? 1.0f / 2.2f : 1.0f);
     }
 }
 
 }   //namespace sk_app