Add isTextureable parameter to GrContextThreadSafeProxy::createCharacterization

Change-Id: I08d263490982a7d5c1e48a5b31acca25405ab09a
Reviewed-on: https://skia-review.googlesource.com/c/188628
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

include/gpu/GrContextThreadSafeProxy.h

@@ -59,13 +59,16 @@ public:
      *                               allocated for mipmaps?
      *  @param willUseGLFBO0         Will the surface the DDL will be replayed into be backed by GL
      *                               FBO 0. This flag is only valid if using an GL backend.
+     *  @param isTextureable         Will the surface be able to act as a texture?
      */
     SkSurfaceCharacterization createCharacterization(
                                   size_t cacheMaxResourceBytes,
                                   const SkImageInfo& ii, const GrBackendFormat& backendFormat,
                                   int sampleCount, GrSurfaceOrigin origin,
                                   const SkSurfaceProps& surfaceProps,
-                                  bool isMipMapped, bool willUseGLFBO0 = false);
+                                  bool isMipMapped,
+                                  bool willUseGLFBO0 = false,
+                                  bool isTextureable = true);
 
     bool operator==(const GrContextThreadSafeProxy& that) const {
         // Each GrContext should only ever have a single thread-safe proxy.

src/gpu/GrBackendSurface.cpp

@@ -472,14 +472,15 @@ GrBackendRenderTarget::GrBackendRenderTarget(int width,
                                              int sampleCnt,
                                              int stencilBits,
                                              const GrGLFramebufferInfo& glInfo)
-        : fIsValid(true)
+        : fWidth(width)
-        , fWidth(width)
         , fHeight(height)
         , fSampleCnt(SkTMax(1, sampleCnt))
         , fStencilBits(stencilBits)
         , fConfig(kUnknown_GrPixelConfig)
         , fBackend(GrBackendApi::kOpenGL)
-        , fGLInfo(glInfo) {}
+        , fGLInfo(glInfo) {
+    fIsValid = SkToBool(glInfo.fFormat); // the glInfo must have a valid format
+}
 
 GrBackendRenderTarget::GrBackendRenderTarget(int width,
                                              int height,

src/gpu/GrContextThreadSafeProxy.cpp

@@ -36,7 +36,7 @@ SkSurfaceCharacterization GrContextThreadSafeProxy::createCharacterization(
                                      const SkImageInfo& ii, const GrBackendFormat& backendFormat,
                                      int sampleCnt, GrSurfaceOrigin origin,
                                      const SkSurfaceProps& surfaceProps,
-                                     bool isMipMapped, bool willUseGLFBO0) {
+                                     bool isMipMapped, bool willUseGLFBO0, bool isTextureable) {
     if (!backendFormat.isValid()) {
         return SkSurfaceCharacterization(); // return an invalid characterization
     }
@@ -69,16 +69,19 @@ SkSurfaceCharacterization GrContextThreadSafeProxy::createCharacterization(
         FSAAType = fCaps->usesMixedSamples() ? GrFSAAType::kMixedSamples : GrFSAAType::kUnifiedMSAA;
     }
 
-    // This surface characterization factory assumes that the resulting characterization is
+    if (willUseGLFBO0 && isTextureable) {
-    // textureable.
+        return SkSurfaceCharacterization(); // return an invalid characterization
-    if (!fCaps->isConfigTexturable(config)) {
+    }
+
+    if (isTextureable && !fCaps->isConfigTexturable(config)) {
+        // Skia doesn't agree that this is textureable.
         return SkSurfaceCharacterization(); // return an invalid characterization
     }
 
     return SkSurfaceCharacterization(sk_ref_sp<GrContextThreadSafeProxy>(this),
                                      cacheMaxResourceBytes, ii,
                                      origin, config, FSAAType, sampleCnt,
-                                     SkSurfaceCharacterization::Textureable(true),
+                                     SkSurfaceCharacterization::Textureable(isTextureable),
                                      SkSurfaceCharacterization::MipMapped(isMipMapped),
                                      SkSurfaceCharacterization::UsesGLFBO0(willUseGLFBO0),
                                      SkSurfaceCharacterization::VulkanSecondaryCBCompatible(false),

src/gpu/GrRenderTargetProxy.cpp

@@ -111,7 +111,8 @@ bool GrRenderTargetProxy::refsWrappedObjects() const {
 
 #ifdef SK_DEBUG
 void GrRenderTargetProxy::onValidateSurface(const GrSurface* surface) {
-    SkASSERT(!surface->asTexture());
+    // We do not check that surface->asTexture returns null since, when replaying DDLs we
+    // can fulfill a renderTarget-only proxy w/ a textureRenderTarget.
 
     // Anything that is checked here should be duplicated in GrTextureRenderTargetProxy's version
     SkASSERT(surface->asRenderTarget());

tests/DeferredDisplayListTest.cpp

@@ -251,12 +251,14 @@ static GrBackendFormat create_backend_format(GrContext* context,
 
 class SurfaceParameters {
 public:
-    static const int kNumParams = 9;
+    static const int kNumParams   = 11;
     static const int kSampleCount = 5;
     static const int kMipMipCount = 8;
+    static const int kFBO0Count   = 9;
 
-    SurfaceParameters(const GrCaps* caps)
+    SurfaceParameters(GrBackendApi backend)
-            : fWidth(64)
+            : fBackend(backend)
+            , fWidth(64)
             , fHeight(64)
             , fOrigin(kTopLeft_GrSurfaceOrigin)
             , fColorType(kRGBA_8888_SkColorType)
@@ -264,7 +266,9 @@ public:
             , fColorSpace(SkColorSpace::MakeSRGB())
             , fSampleCount(1)
             , fSurfaceProps(0x0, kUnknown_SkPixelGeometry)
-            , fShouldCreateMipMaps(true) {
+            , fShouldCreateMipMaps(true)
+            , fUsesGLFBO0(false)
+            , fIsTextureable(true) {
     }
 
     int sampleCount() const { return fSampleCount; }
@@ -272,6 +276,7 @@ public:
     void setColorType(SkColorType ct) { fColorType = ct; }
     void setColorSpace(sk_sp<SkColorSpace> cs) { fColorSpace = std::move(cs); }
     void setConfig(GrPixelConfig config) { fConfig = config; }
+    void setTextureable(bool isTextureable) { fIsTextureable = isTextureable; }
 
     // Modify the SurfaceParameters in just one way
     void modify(int i) {
@@ -308,6 +313,15 @@ public:
         case 8:
             fShouldCreateMipMaps = false;
             break;
+        case 9:
+            if (GrBackendApi::kOpenGL == fBackend) {
+                fUsesGLFBO0 = true;
+                fIsTextureable = false;
+            }
+            break;
+        case 10:
+            fIsTextureable = false;
+            break;
         }
     }
 
@@ -327,7 +341,8 @@ public:
 
         SkSurfaceCharacterization c = context->threadSafeProxy()->createCharacterization(
                                                 maxResourceBytes, ii, backendFormat, fSampleCount,
-                                                fOrigin, fSurfaceProps, fShouldCreateMipMaps);
+                                                fOrigin, fSurfaceProps, fShouldCreateMipMaps,
+                                                fUsesGLFBO0, fIsTextureable);
         return c;
     }
 
@@ -347,14 +362,33 @@ public:
     }
 
     // Create the surface with the current set of parameters
-    sk_sp<SkSurface> make(GrContext* context, GrBackendTexture* backend,
+    sk_sp<SkSurface> make(GrContext* context, GrBackendTexture* backend) const {
-                          bool nonTextureable) const {
         GrGpu* gpu = context->contextPriv().getGpu();
 
-        GrMipMapped mipmapped = nonTextureable
+        GrMipMapped mipmapped = !fIsTextureable
                                         ? GrMipMapped::kNo
                                         : GrMipMapped(fShouldCreateMipMaps);
 
+        if (fUsesGLFBO0) {
+            if (GrBackendApi::kOpenGL != context->backend()) {
+                return nullptr;
+            }
+
+            GrGLFramebufferInfo fboInfo;
+            fboInfo.fFBOID = 0;
+            fboInfo.fFormat = GR_GL_RGBA8;
+            static constexpr int kStencilBits = 8;
+            GrBackendRenderTarget backendRT(fWidth, fHeight, 1, kStencilBits, fboInfo);
+            backendRT.setPixelConfig(fConfig);
+
+            if (!backendRT.isValid()) {
+                return nullptr;
+            }
+
+            return SkSurface::MakeFromBackendRenderTarget(context, backendRT, fOrigin,
+                                                          fColorType, fColorSpace, &fSurfaceProps);
+        }
+
         *backend = gpu->createTestingOnlyBackendTexture(nullptr, fWidth, fHeight,
                                                         fColorType, true, mipmapped);
         if (!backend->isValid() || !gpu->isTestingOnlyBackendTexture(*backend)) {
@@ -362,7 +396,7 @@ public:
         }
 
         sk_sp<SkSurface> surface;
-        if (nonTextureable) {
+        if (!fIsTextureable) {
             // Create a surface w/ the current parameters but make it non-textureable
             surface = SkSurface::MakeFromBackendTextureAsRenderTarget(
                                             context, *backend, fOrigin, fSampleCount, fColorType,
@@ -382,12 +416,17 @@ public:
     }
 
     void cleanUpBackEnd(GrContext* context, const GrBackendTexture& backend) const {
+        if (!backend.isValid()) {
+            return;
+        }
+
         GrGpu* gpu = context->contextPriv().getGpu();
 
         gpu->deleteTestingOnlyBackendTexture(backend);
     }
 
 private:
+    GrBackendApi        fBackend;
     int                 fWidth;
     int                 fHeight;
     GrSurfaceOrigin     fOrigin;
@@ -397,6 +436,8 @@ private:
     int                 fSampleCount;
     SkSurfaceProps      fSurfaceProps;
     bool                fShouldCreateMipMaps;
+    bool                fUsesGLFBO0;
+    bool                fIsTextureable;
 };
 
 // Test out operator== && operator!=
@@ -404,7 +445,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLOperatorEqTest, reporter, ctxInfo) {
     GrContext* context = ctxInfo.grContext();
 
     for (int i = 0; i < SurfaceParameters::kNumParams; ++i) {
-        SurfaceParameters params1(context->contextPriv().caps());
+        SurfaceParameters params1(context->backend());
         params1.modify(i);
 
         SkSurfaceCharacterization char1 = params1.createCharacterization(context);
@@ -413,7 +454,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLOperatorEqTest, reporter, ctxInfo) {
         }
 
         for (int j = 0; j < SurfaceParameters::kNumParams; ++j) {
-            SurfaceParameters params2(context->contextPriv().caps());
+            SurfaceParameters params2(context->backend());
             params2.modify(j);
 
             SkSurfaceCharacterization char2 = params2.createCharacterization(context);
@@ -431,7 +472,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLOperatorEqTest, reporter, ctxInfo) {
     }
 
     {
-        SurfaceParameters params(context->contextPriv().caps());
+        SurfaceParameters params(context->backend());
 
         SkSurfaceCharacterization valid = params.createCharacterization(context);
         SkASSERT(valid.isValid());
@@ -461,14 +502,14 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLSurfaceCharacterizationTest, reporter, ctx
 
     // First, create a DDL using the stock SkSurface parameters
     {
-        SurfaceParameters params(context->contextPriv().caps());
+        SurfaceParameters params(context->backend());
 
         ddl = params.createDDL(context);
         SkAssertResult(ddl);
 
         // The DDL should draw into an SkSurface created with the same parameters
         GrBackendTexture backend;
-        sk_sp<SkSurface> s = params.make(context, &backend, false);
+        sk_sp<SkSurface> s = params.make(context, &backend);
         if (!s) {
             return;
         }
@@ -483,11 +524,11 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLSurfaceCharacterizationTest, reporter, ctx
 
     // Then, alter each parameter in turn and check that the DDL & surface are incompatible
     for (int i = 0; i < SurfaceParameters::kNumParams; ++i) {
-        SurfaceParameters params(context->contextPriv().caps());
+        SurfaceParameters params(context->backend());
         params.modify(i);
 
         GrBackendTexture backend;
-        sk_sp<SkSurface> s = params.make(context, &backend, false);
+        sk_sp<SkSurface> s = params.make(context, &backend);
         if (!s) {
             continue;
         }
@@ -519,6 +560,13 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLSurfaceCharacterizationTest, reporter, ctx
             continue;
         }
 
+        if (SurfaceParameters::kFBO0Count == i && context->backend() != GrBackendApi::kOpenGL) {
+            // FBO0 only affects the surface characterization when using OpenGL
+            s = nullptr;
+            params.cleanUpBackEnd(context, backend);
+            continue;
+        }
+
         REPORTER_ASSERT(reporter, !s->draw(ddl.get()),
                         "DDLSurfaceCharacterizationTest failed on parameter: %d\n", i);
 
@@ -530,10 +578,10 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLSurfaceCharacterizationTest, reporter, ctx
 
     // Next test the compatibility of resource cache parameters
     {
-        const SurfaceParameters params(context->contextPriv().caps());
+        const SurfaceParameters params(context->backend());
         GrBackendTexture backend;
 
-        sk_sp<SkSurface> s = params.make(context, &backend, false);
+        sk_sp<SkSurface> s = params.make(context, &backend);
 
         int maxResourceCount;
         size_t maxResourceBytes;
@@ -568,10 +616,12 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLSurfaceCharacterizationTest, reporter, ctx
     // Test that the textureability of the DDL characterization can block a DDL draw
     {
         GrBackendTexture backend;
-        const SurfaceParameters params(context->contextPriv().caps());
+        SurfaceParameters params(context->backend());
-        sk_sp<SkSurface> s = params.make(context, &backend, true);
+        params.setTextureable(false);
+
+        sk_sp<SkSurface> s = params.make(context, &backend);
         if (s) {
-            REPORTER_ASSERT(reporter, !s->draw(ddl.get()));
+            REPORTER_ASSERT(reporter, !s->draw(ddl.get())); // bc the DDL was made w/ textureability
 
             context->flush();
             gpu->testingOnly_flushGpuAndSync();
@@ -591,10 +641,10 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLSurfaceCharacterizationTest, reporter, ctx
 
     // Exercise the createResized method
     {
-        SurfaceParameters params(context->contextPriv().caps());
+        SurfaceParameters params(context->backend());
         GrBackendTexture backend;
 
-        sk_sp<SkSurface> s = params.make(context, &backend, false);
+        sk_sp<SkSurface> s = params.make(context, &backend);
         if (!s) {
             return;
         }
@@ -621,6 +671,54 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLSurfaceCharacterizationTest, reporter, ctx
     }
 }
 
+// Test that a DDL created w/o textureability can be replayed into both a textureable and
+// non-textureable destination. Note that DDLSurfaceCharacterizationTest tests that a
+// textureable DDL cannot be played into a non-textureable destination but can be replayed
+// into a textureable destination.
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLNonTextureabilityTest, reporter, ctxInfo) {
+    GrContext* context = ctxInfo.grContext();
+    GrGpu* gpu = context->contextPriv().getGpu();
+
+    // Create a bitmap that we can readback into
+    SkImageInfo imageInfo = SkImageInfo::Make(64, 64, kRGBA_8888_SkColorType,
+                                              kPremul_SkAlphaType);
+    SkBitmap bitmap;
+    bitmap.allocPixels(imageInfo);
+
+    for (bool textureability : { true, false }) {
+        std::unique_ptr<SkDeferredDisplayList> ddl;
+
+        // First, create a DDL w/o textureability. TODO: once we have reusable DDLs, move this
+        // outside of the loop.
+        {
+            SurfaceParameters params(context->backend());
+            params.setTextureable(false);
+
+            ddl = params.createDDL(context);
+            SkAssertResult(ddl);
+        }
+
+        // Then verify it can draw into either flavor of destination
+        SurfaceParameters params(context->backend());
+        params.setTextureable(textureability);
+
+        GrBackendTexture backend;
+        sk_sp<SkSurface> s = params.make(context, &backend);
+        if (!s) {
+            params.cleanUpBackEnd(context, backend);
+            continue;
+        }
+
+        REPORTER_ASSERT(reporter, s->draw(ddl.get()));
+        s->readPixels(imageInfo, bitmap.getPixels(), bitmap.rowBytes(), 0, 0);
+        context->flush();
+        gpu->testingOnly_flushGpuAndSync();
+        s = nullptr;
+        params.cleanUpBackEnd(context, backend);
+    }
+
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // This tests the SkSurface::MakeRenderTarget variant that takes an SkSurfaceCharacterization.
 // In particular, the SkSurface and the SkSurfaceCharacterization should always be compatible.
@@ -628,14 +726,14 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLMakeRenderTargetTest, reporter, ctxInfo) {
     GrContext* context = ctxInfo.grContext();
 
     for (int i = 0; i < SurfaceParameters::kNumParams; ++i) {
-        SurfaceParameters params(context->contextPriv().caps());
+        SurfaceParameters params(context->backend());
         params.modify(i);
 
         SkSurfaceCharacterization c = params.createCharacterization(context);
         GrBackendTexture backend;
 
         if (!c.isValid()) {
-            sk_sp<SkSurface> tmp = params.make(context, &backend, false);
+            sk_sp<SkSurface> tmp = params.make(context, &backend);
 
             // If we couldn't characterize the surface we shouldn't be able to create it either
             REPORTER_ASSERT(reporter, !tmp);
@@ -646,7 +744,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLMakeRenderTargetTest, reporter, ctxInfo) {
             continue;
         }
 
-        sk_sp<SkSurface> s = params.make(context, &backend, false);
+        sk_sp<SkSurface> s = params.make(context, &backend);
         if (!s) {
             REPORTER_ASSERT(reporter, !c.isValid());
             continue;
@@ -654,6 +752,12 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLMakeRenderTargetTest, reporter, ctxInfo) {
 
         REPORTER_ASSERT(reporter, c.isValid());
 
+        if (SurfaceParameters::kFBO0Count == i) {
+            // MakeRenderTarget doesn't support FBO0
+            params.cleanUpBackEnd(context, backend);
+            continue;
+        }
+
         s = SkSurface::MakeRenderTarget(context, c, SkBudgeted::kYes);
         REPORTER_ASSERT(reporter, s);
 
@@ -688,10 +792,10 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLWrapBackendTest, reporter, ctxInfo) {
         return;
     }
 
-    SurfaceParameters params(context->contextPriv().caps());
+    SurfaceParameters params(context->backend());
     GrBackendTexture backend;
 
-    sk_sp<SkSurface> s = params.make(context, &backend, false);
+    sk_sp<SkSurface> s = params.make(context, &backend);
     if (!s) {
         gpu->deleteTestingOnlyBackendTexture(backendTex);
         return;
@@ -917,7 +1021,7 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(DDLCompatibilityTest, reporter, ctxInfo) {
         for (int config = 0; config < kPrivateConfig1_GrPixelConfig; ++config) {
             GrPixelConfig pixelConfig = static_cast<GrPixelConfig>(config);
 
-            SurfaceParameters params(context->contextPriv().caps());
+            SurfaceParameters params(context->backend());
             params.setColorType(colorType);
             params.setConfig(pixelConfig);
             params.setColorSpace(nullptr);
@@ -942,7 +1046,7 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(DDLCompatibilityTest, reporter, ctxInfo) {
                 continue;
             }
 
-            sk_sp<SkSurface> s = params.make(context, &backend, false);
+            sk_sp<SkSurface> s = params.make(context, &backend);
             REPORTER_ASSERT(reporter, s);
             if (!s) {
                 s = nullptr;

tests/ProxyTest.cpp

@@ -20,6 +20,7 @@
 #include "GrTexture.h"
 #include "GrTextureProxy.h"
 #include "SkGr.h"
+#include "gl/GrGLDefines.h"
 
 // Check that the surface proxy's member vars are set as expected
 static void check_surface(skiatest::Reporter* reporter,
@@ -251,6 +252,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(WrappedProxyTest, reporter, ctxInfo) {
                 if (GrBackendApi::kOpenGL == ctxInfo.backend()) {
                     GrGLFramebufferInfo fboInfo;
                     fboInfo.fFBOID = 0;
+                    fboInfo.fFormat = GR_GL_RGBA8;
                     static constexpr int kStencilBits = 8;
                     GrBackendRenderTarget backendRT(kWidthHeight, kWidthHeight, numSamples,
                                                     kStencilBits, fboInfo);