Fix GPU explicit resource allocation bug

The explicit resource allocator assumes that it has complete control
over the resources it hands out thus, they can have no pre-existing
pending IO.

Change-Id: I59d8d079d8b8514688c458a54424329b86922cb6
Reviewed-on: https://skia-review.googlesource.com/143300
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

7 files changed, 45 insertions, 10 deletions

diff --git a/include/private/GrSurfaceProxy.h b/include/private/GrSurfaceProxy.h
index d69a294194..327ceb13a9 100644
--- a/include/private/GrSurfaceProxy.h
+++ b/include/private/GrSurfaceProxy.h
@@ -430,7 +430,7 @@ protected:
     void assign(sk_sp<GrSurface> surface);
 
     sk_sp<GrSurface> createSurfaceImpl(GrResourceProvider*, int sampleCnt, bool needsStencil,
-                                       GrSurfaceDescFlags descFlags, GrMipMapped) const;
+                                       GrSurfaceDescFlags, GrMipMapped) const;
 
     bool instantiateImpl(GrResourceProvider* resourceProvider, int sampleCnt, bool needsStencil,
                          GrSurfaceDescFlags descFlags, GrMipMapped, const GrUniqueKey*);
diff --git a/src/gpu/GrOpList.cpp b/src/gpu/GrOpList.cpp
index 313c38f14c..c724ea9ba4 100644
--- a/src/gpu/GrOpList.cpp
+++ b/src/gpu/GrOpList.cpp
@@ -145,8 +145,10 @@ static const char* op_to_name(GrLoadOp op) {
 
 void GrOpList::dump(bool printDependencies) const {
     SkDebugf("--------------------------------------------------------------\n");
-    SkDebugf("opListID: %d -> proxyID: %d\n", fUniqueID,
-             fTarget.get() ? fTarget.get()->uniqueID().asUInt() : -1);
+    SkDebugf("opListID: %d - proxyID: %d - surfaceID: %d\n", fUniqueID,
+             fTarget.get() ? fTarget.get()->uniqueID().asUInt() : -1,
+             fTarget.get() && fTarget.get()->priv().peekSurface()
+                                ? fTarget.get()->priv().peekSurface()->uniqueID().asUInt() : -1);
     SkDebugf("ColorLoadOp: %s %x StencilLoadOp: %s\n",
              op_to_name(fColorLoadOp),
              GrLoadOp::kClear == fColorLoadOp ? fLoadClearColor : 0x0,
diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp
index 81ad5840ed..9e79ead801 100644
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -115,10 +115,10 @@ bool GrSurfaceProxyPriv::AttachStencilIfNeeded(GrResourceProvider* resourceProvi
     return true;
 }
 
-sk_sp<GrSurface> GrSurfaceProxy::createSurfaceImpl(
-                                                GrResourceProvider* resourceProvider,
-                                                int sampleCnt, bool needsStencil,
-                                                GrSurfaceDescFlags descFlags, GrMipMapped mipMapped) const {
+sk_sp<GrSurface> GrSurfaceProxy::createSurfaceImpl(GrResourceProvider* resourceProvider,
+                                                   int sampleCnt, bool needsStencil,
+                                                   GrSurfaceDescFlags descFlags,
+                                                   GrMipMapped mipMapped) const {
     SkASSERT(GrSurfaceProxy::LazyState::kNot == this->lazyInstantiationState());
     SkASSERT(!fTarget);
     GrSurfaceDesc desc;
@@ -132,7 +132,10 @@ sk_sp<GrSurface> GrSurfaceProxy::createSurfaceImpl(
     desc.fSampleCnt = sampleCnt;
 
     GrResourceProvider::Flags resourceProviderFlags = GrResourceProvider::kNone_Flag;
-    if (fSurfaceFlags & GrInternalSurfaceFlags::kNoPendingIO) {
+    if (fSurfaceFlags & GrInternalSurfaceFlags::kNoPendingIO ||
+        resourceProvider->explicitlyAllocateGPUResources()) {
+        // The explicit resource allocator requires that any resources it pulls out of the
+        // cache have no pending IO.
         resourceProviderFlags = GrResourceProvider::kNoPendingIO_Flag;
     }
 
diff --git a/src/gpu/effects/GrYUVtoRGBEffect.cpp b/src/gpu/effects/GrYUVtoRGBEffect.cpp
index 55e7902739..905445e852 100644
--- a/src/gpu/effects/GrYUVtoRGBEffect.cpp
+++ b/src/gpu/effects/GrYUVtoRGBEffect.cpp
@@ -60,6 +60,19 @@ std::unique_ptr<GrFragmentProcessor> GrYUVtoRGBEffect::Make(sk_sp<GrTextureProxy
             std::move(yProxy), yTransform, std::move(uProxy), uTransform, std::move(vProxy),
             vTransform, mat, nv12, GrSamplerState(GrSamplerState::WrapMode::kClamp, uvFilterMode)));
 }
+
+SkString GrYUVtoRGBEffect::dumpInfo() const {
+    SkString str;
+    str.appendf("Y: %d %d U: %d %d V: %d %d\n",
+                fYSampler.proxy()->uniqueID().asUInt(),
+                fYSampler.proxy()->underlyingUniqueID().asUInt(),
+                fUSampler.proxy()->uniqueID().asUInt(),
+                fUSampler.proxy()->underlyingUniqueID().asUInt(),
+                fVSampler.proxy()->uniqueID().asUInt(),
+                fVSampler.proxy()->underlyingUniqueID().asUInt());
+
+    return str;
+}
 #include "glsl/GrGLSLFragmentProcessor.h"
 #include "glsl/GrGLSLFragmentShaderBuilder.h"
 #include "glsl/GrGLSLProgramBuilder.h"
diff --git a/src/gpu/effects/GrYUVtoRGBEffect.fp b/src/gpu/effects/GrYUVtoRGBEffect.fp
index e49fedae71..453800dd1b 100644
--- a/src/gpu/effects/GrYUVtoRGBEffect.fp
+++ b/src/gpu/effects/GrYUVtoRGBEffect.fp
@@ -41,6 +41,7 @@ layout(key) in bool nv12;
                                                      sk_sp<GrTextureProxy> uProxy,
                                                      sk_sp<GrTextureProxy> vProxy,
                                                      SkYUVColorSpace colorSpace, bool nv12);
+    SkString dumpInfo() const override;
 }
 
 @cpp {
@@ -105,6 +106,19 @@ layout(key) in bool nv12;
                                      GrSamplerState(GrSamplerState::WrapMode::kClamp,
                                                     uvFilterMode)));
     }
+
+    SkString GrYUVtoRGBEffect::dumpInfo() const {
+        SkString str;
+        str.appendf("Y: %d %d U: %d %d V: %d %d\n",
+            fYSampler.proxy()->uniqueID().asUInt(),
+            fYSampler.proxy()->underlyingUniqueID().asUInt(),
+            fUSampler.proxy()->uniqueID().asUInt(),
+            fUSampler.proxy()->underlyingUniqueID().asUInt(),
+            fVSampler.proxy()->uniqueID().asUInt(),
+            fVSampler.proxy()->underlyingUniqueID().asUInt());
+
+        return str;
+    }
 }
 
 void main() {
diff --git a/src/gpu/effects/GrYUVtoRGBEffect.h b/src/gpu/effects/GrYUVtoRGBEffect.h
index 3d44285bfe..6cc468edc0 100644
--- a/src/gpu/effects/GrYUVtoRGBEffect.h
+++ b/src/gpu/effects/GrYUVtoRGBEffect.h
@@ -19,6 +19,7 @@ public:
                                                      sk_sp<GrTextureProxy> uProxy,
                                                      sk_sp<GrTextureProxy> vProxy,
                                                      SkYUVColorSpace colorSpace, bool nv12);
+    SkString dumpInfo() const override;
     SkMatrix44 ySamplerTransform() const { return fYSamplerTransform; }
     SkMatrix44 uSamplerTransform() const { return fUSamplerTransform; }
     SkMatrix44 vSamplerTransform() const { return fVSamplerTransform; }
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 4ecfb22402..1dea80659a 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -425,8 +425,8 @@ sk_sp<SkImage> SkImage_Gpu::MakeFromYUVATexturesCopyImpl(GrContext* ctx,
     paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
     // TODO: Modify the fragment processor to sample from different channel instead of taking nv12
     // bool.
-    paint.addColorFragmentProcessor(
-            GrYUVtoRGBEffect::Make(yProxy, uProxy, vProxy, colorSpace, nv12));
+    paint.addColorFragmentProcessor(GrYUVtoRGBEffect::Make(std::move(yProxy), std::move(uProxy),
+                                                           std::move(vProxy), colorSpace, nv12));
 
     const SkRect rect = SkRect::MakeIWH(width, height);
 
@@ -435,6 +435,8 @@ sk_sp<SkImage> SkImage_Gpu::MakeFromYUVATexturesCopyImpl(GrContext* ctx,
     if (!renderTargetContext->asSurfaceProxy()) {
         return nullptr;
     }
+
+    // DDL TODO: in the promise image version we must not flush here
     ctx->contextPriv().flushSurfaceWrites(renderTargetContext->asSurfaceProxy());
 
     // MDB: this call is okay bc we know 'renderTargetContext' was exact