Use Offscreen Surface for Split Screen

tools/viewer/sk_app/WindowContext.cpp

Index: tools/viewer/sk_app/WindowContext.cpp
diff --git a/tools/viewer/sk_app/WindowContext.cpp b/tools/viewer/sk_app/WindowContext.cpp
index 41bbd14576b37f604c8f65d7001b8de076932553..721639a09904c100b42053a56a2673d781192bd8 100755
--- a/tools/viewer/sk_app/WindowContext.cpp
+++ b/tools/viewer/sk_app/WindowContext.cpp
@@ -21,12 +21,13 @@
 
 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.
@@ -35,10 +36,17 @@ sk_sp<SkSurface> WindowContext::createRenderSurface(sk_sp<GrRenderTarget> rt, in
         // 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);
     }