Revert "Revert "Expand partial render target write pixels workaround.""

New version limits the workaround to unorm configs.

This reverts commit fdd117025fb846771e9e47cbf825d9071745fa98.

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

8 files changed, 89 insertions, 35 deletions

diff --git a/src/gpu/GrCaps.cpp b/src/gpu/GrCaps.cpp
index ef6d5bb086..82d32cac18 100644
--- a/src/gpu/GrCaps.cpp
+++ b/src/gpu/GrCaps.cpp
@@ -72,8 +72,6 @@ GrCaps::GrCaps(const GrContextOptions& options) {
     fSuppressPrints = options.fSuppressPrints;
     fWireframeMode = options.fWireframeMode;
     fBufferMapThreshold = options.fBufferMapThreshold;
-    fUseDrawInsteadOfPartialRenderTargetWrite = options.fUseDrawInsteadOfPartialRenderTargetWrite;
-    fUseDrawInsteadOfAllRenderTargetWrites = false;
     fAvoidInstancedDrawsToFPTargets = false;
     fAvoidStencilBuffers = false;
 
@@ -142,8 +140,6 @@ SkString GrCaps::dump() const {
     r.appendf("Cross context texture support      : %s\n", gNY[fCrossContextTextureSupport]);
 
     r.appendf("Draw Instead of Clear [workaround] : %s\n", gNY[fUseDrawInsteadOfClear]);
-    r.appendf("Draw Instead of TexSubImage [workaround] : %s\n",
-              gNY[fUseDrawInsteadOfPartialRenderTargetWrite]);
     r.appendf("Prefer VRAM Use over flushes [workaround] : %s\n", gNY[fPreferVRAMUseOverFlushes]);
 
     if (this->advancedBlendEquationSupport()) {
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index e9f4f93b19..6cec47775c 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -291,15 +291,6 @@ bool GrGpu::getWritePixelsInfo(GrSurface* dstSurface, int width, int height,
     SkASSERT(dstSurface);
     SkASSERT(kGpuPrefersDraw_DrawPreference != *drawPreference);
 
-    if (SkToBool(dstSurface->asRenderTarget())) {
-        if (this->caps()->useDrawInsteadOfAllRenderTargetWrites()) {
-            ElevateDrawPreference(drawPreference, kRequireDraw_DrawPreference);
-        } else if (this->caps()->useDrawInsteadOfPartialRenderTargetWrite() &&
-                   (width < dstSurface->width() || height < dstSurface->height())) {
-            ElevateDrawPreference(drawPreference, kRequireDraw_DrawPreference);
-        }
-    }
-
     if (!this->onGetWritePixelsInfo(dstSurface, width, height, srcConfig, drawPreference,
                                     tempDrawInfo)) {
         return false;
diff --git a/src/gpu/GrOpFlushState.cpp b/src/gpu/GrOpFlushState.cpp
index c6f5d38383..72deabe197 100644
--- a/src/gpu/GrOpFlushState.cpp
+++ b/src/gpu/GrOpFlushState.cpp
@@ -8,7 +8,7 @@
 #include "GrOpFlushState.h"
 
 #include "GrDrawOpAtlas.h"
-#include "GrPipeline.h"
+#include "GrResourceProvider.h"
 
 GrOpFlushState::GrOpFlushState(GrGpu* gpu, GrResourceProvider* resourceProvider)
         : fGpu(gpu)
@@ -29,3 +29,33 @@ uint16_t* GrOpFlushState::makeIndexSpace(int indexCount,
                                             const GrBuffer** buffer, int* startIndex) {
     return reinterpret_cast<uint16_t*>(fIndexPool.makeSpace(indexCount, buffer, startIndex));
 }
+
+void GrOpFlushState::doUpload(GrDrawOp::DeferredUploadFn& upload) {
+    GrDrawOp::WritePixelsFn wp = [this](GrSurface* surface, int left, int top, int width,
+                                        int height, GrPixelConfig config, const void* buffer,
+                                        size_t rowBytes) {
+        GrGpu::DrawPreference drawPreference = GrGpu::kNoDraw_DrawPreference;
+        GrGpu::WritePixelTempDrawInfo tempInfo;
+        fGpu->getWritePixelsInfo(surface, width, height, surface->config(), &drawPreference,
+                                 &tempInfo);
+        if (GrGpu::kNoDraw_DrawPreference == drawPreference) {
+            return this->fGpu->writePixels(surface, left, top, width, height, config, buffer,
+                                           rowBytes);
+        }
+        GrSurfaceDesc desc;
+        desc.fConfig = surface->config();
+        desc.fWidth = width;
+        desc.fHeight = height;
+        desc.fOrigin = surface->origin();
+        sk_sp<GrTexture> temp(this->fResourceProvider->createApproxTexture(
+                desc, GrResourceProvider::kNoPendingIO_Flag));
+        if (!temp) {
+            return false;
+        }
+        if (!fGpu->writePixels(temp.get(), 0, 0, width, height, desc.fConfig, buffer, rowBytes)) {
+            return false;
+        }
+        return fGpu->copySurface(surface, temp.get(), SkIRect::MakeWH(width, height), {left, top});
+    };
+    upload(wp);
+}
diff --git a/src/gpu/GrOpFlushState.h b/src/gpu/GrOpFlushState.h
index 85a356e44f..402bac5325 100644
--- a/src/gpu/GrOpFlushState.h
+++ b/src/gpu/GrOpFlushState.h
@@ -72,16 +72,7 @@ public:
         fAsapUploads.reset();
     }
 
-    void doUpload(GrDrawOp::DeferredUploadFn& upload) {
-        GrDrawOp::WritePixelsFn wp = [this] (GrSurface* surface,
-                int left, int top, int width, int height,
-                GrPixelConfig config, const void* buffer,
-                size_t rowBytes) -> bool {
-            return this->fGpu->writePixels(surface, left, top, width, height, config, buffer,
-                                           rowBytes);
-        };
-        upload(wp);
-    }
+    void doUpload(GrDrawOp::DeferredUploadFn&);
 
     void putBackIndices(size_t indices) { fIndexPool.putBack(indices * sizeof(uint16_t)); }
 
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 8287580fbe..d52b73b2e8 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -57,6 +57,8 @@ GrGLCaps::GrGLCaps(const GrContextOptions& contextOptions,
     fClearTextureSupport = false;
     fDrawArraysBaseVertexIsBroken = false;
     fUseDrawToClearStencilClip = false;
+    fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO = false;
+    fUseDrawInsteadOfAllRenderTargetWrites = false;
     fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines = false;
 
     fBlitFramebufferFlags = kNoSupport_BlitFramebufferFlag;
@@ -523,12 +525,12 @@ void GrGLCaps::init(const GrContextOptions& contextOptions,
     }
 
     if (kAdreno4xx_GrGLRenderer == ctxInfo.renderer()) {
-        fUseDrawInsteadOfPartialRenderTargetWrite = true;
         // This is known to be fixed sometime between driver 145.0 and 219.0
         if (ctxInfo.driver() == kQualcomm_GrGLDriver &&
             ctxInfo.driverVersion() <= GR_GL_DRIVER_VER(219, 0)) {
             fUseDrawToClearStencilClip = true;
         }
+        fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO = true;
     }
 
     // This was reproduced on the following configurations:
@@ -546,7 +548,7 @@ void GrGLCaps::init(const GrContextOptions& contextOptions,
 
     // Texture uploads sometimes seem to be ignored to textures bound to FBOS on Tegra3.
     if (kTegra3_GrGLRenderer == ctxInfo.renderer()) {
-        fUseDrawInsteadOfPartialRenderTargetWrite = true;
+        fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO = true;
         fUseDrawInsteadOfAllRenderTargetWrites = true;
     }
 
@@ -1298,6 +1300,10 @@ SkString GrGLCaps::dump() const {
     r.appendf("Texture swizzle support: %s\n", (fTextureSwizzleSupport ? "YES" : "NO"));
     r.appendf("BGRA to RGBA readback conversions are slow: %s\n",
               (fRGBAToBGRAReadbackConversionsAreSlow ? "YES" : "NO"));
+    r.appendf("Intermediate texture for partial updates of unorm textures ever bound to FBOs: %s\n",
+              fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO ? "YES" : "NO");
+    r.appendf("Intermediate texture for all updates of textures bound to FBOs: %s\n",
+              fUseDrawInsteadOfAllRenderTargetWrites ? "YES" : "NO");
 
     r.append("Configs\n-------\n");
     for (int i = 0; i < kGrPixelConfigCnt; ++i) {
@@ -2217,4 +2223,7 @@ void GrGLCaps::onApplyOptionsOverrides(const GrContextOptions& options) {
         fAvoidInstancedDrawsToFPTargets = true;
 #endif
     }
+    if (options.fUseDrawInsteadOfPartialRenderTargetWrite) {
+        fUseDrawInsteadOfAllRenderTargetWrites = true;
+    }
 }
diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index 7d81d23481..43da5f86cd 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -374,6 +374,18 @@ public:
     /// op instead of using glClear seems to resolve the issue.
     bool useDrawToClearStencilClip() const { return fUseDrawToClearStencilClip; }
 
+    // If true then we must use an intermediate surface to perform partial updates to unorm textures
+    // that have ever been bound to a FBO.
+    bool disallowTexSubImageForUnormConfigTexturesEverBoundToFBO() const {
+        return fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO;
+    }
+
+    // Use an intermediate surface to write pixels (full or partial overwrite) to into a texture
+    // that is bound to an FBO.
+    bool useDrawInsteadOfAllRenderTargetWrites() const {
+        return fUseDrawInsteadOfAllRenderTargetWrites;
+    }
+
     // At least some Adreno 3xx drivers draw lines incorrectly after drawing non-lines. Toggling
     // face culling on and off seems to resolve this.
     bool requiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines() const {
@@ -456,6 +468,8 @@ private:
     bool fClearTextureSupport : 1;
     bool fDrawArraysBaseVertexIsBroken : 1;
     bool fUseDrawToClearStencilClip : 1;
+    bool fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO : 1;
+    bool fUseDrawInsteadOfAllRenderTargetWrites : 1;
     bool fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines : 1;
 
     uint32_t fBlitFramebufferFlags;
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index a2efeaa047..d7a1f96462 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -583,7 +583,10 @@ sk_sp<GrTexture> GrGLGpu::onWrapBackendTexture(const GrBackendTexture& backendTe
         if (!this->createRenderTargetObjects(surfDesc, idDesc.fInfo, &rtIDDesc)) {
             return nullptr;
         }
-        return GrGLTextureRenderTarget::MakeWrapped(this, surfDesc, idDesc, rtIDDesc);
+        sk_sp<GrGLTextureRenderTarget> texRT(
+                GrGLTextureRenderTarget::MakeWrapped(this, surfDesc, idDesc, rtIDDesc));
+        texRT->baseLevelWasBoundToFBO();
+        return texRT;
     }
 
     return GrGLTexture::MakeWrapped(this, surfDesc, idDesc);
@@ -663,16 +666,28 @@ bool GrGLGpu::onGetWritePixelsInfo(GrSurface* dstSurface, int width, int height,
                                    GrPixelConfig srcConfig,
                                    DrawPreference* drawPreference,
                                    WritePixelTempDrawInfo* tempDrawInfo) {
-    // This subclass only allows writes to textures. If the dst is not a texture we have to draw
-    // into it. We could use glDrawPixels on GLs that have it, but we don't today.
-    if (!dstSurface->asTexture()) {
-        ElevateDrawPreference(drawPreference, kRequireDraw_DrawPreference);
-    } else {
-        GrGLTexture* texture = static_cast<GrGLTexture*>(dstSurface->asTexture());
+    if (SkToBool(dstSurface->asRenderTarget())) {
+        if (this->glCaps().useDrawInsteadOfAllRenderTargetWrites()) {
+            ElevateDrawPreference(drawPreference, kRequireDraw_DrawPreference);
+        }
+    }
+
+    GrGLTexture* texture = static_cast<GrGLTexture*>(dstSurface->asTexture());
+
+    if (texture) {
         if (GR_GL_TEXTURE_EXTERNAL == texture->target()) {
              // We don't currently support writing pixels to EXTERNAL textures.
              return false;
         }
+        if (GrPixelConfigIsUnorm(texture->config()) && texture->hasBaseLevelBeenBoundToFBO() &&
+            this->glCaps().disallowTexSubImageForUnormConfigTexturesEverBoundToFBO() &&
+            (width < dstSurface->width() || height < dstSurface->height())) {
+            ElevateDrawPreference(drawPreference, kRequireDraw_DrawPreference);
+        }
+    } else {
+        // This subclass only allows writes to textures. If the dst is not a texture we have to draw
+        // into it. We could use glDrawPixels on GLs that have it, but we don't today.
+        ElevateDrawPreference(drawPreference, kRequireDraw_DrawPreference);
     }
 
     // If the dst is MSAA, we have to draw, or we'll just be writing to the resolve target.
@@ -1373,6 +1388,7 @@ sk_sp<GrTexture> GrGLGpu::onCreateTexture(const GrSurfaceDesc& desc,
         }
         tex = sk_make_sp<GrGLTextureRenderTarget>(this, budgeted, desc, idDesc, rtIDDesc,
                                                   wasMipMapDataProvided);
+        tex->baseLevelWasBoundToFBO();
     } else {
         tex = sk_make_sp<GrGLTexture>(this, budgeted, desc, idDesc, wasMipMapDataProvided);
     }
@@ -3274,8 +3290,9 @@ void GrGLGpu::bindSurfaceFBOForPixelOps(GrSurface* surface, GrGLenum fboTarget,
     GrGLRenderTarget* rt = static_cast<GrGLRenderTarget*>(surface->asRenderTarget());
     if (!rt) {
         SkASSERT(surface->asTexture());
-        GrGLuint texID = static_cast<GrGLTexture*>(surface->asTexture())->textureID();
-        GrGLenum target = static_cast<GrGLTexture*>(surface->asTexture())->target();
+        GrGLTexture* texture = static_cast<GrGLTexture*>(surface->asTexture());
+        GrGLuint texID = texture->textureID();
+        GrGLenum target = texture->target();
         GrGLuint* tempFBOID;
         tempFBOID = kSrc_TempFBOTarget == tempFBOTarget ? &fTempSrcFBOID : &fTempDstFBOID;
 
@@ -3290,6 +3307,7 @@ void GrGLGpu::bindSurfaceFBOForPixelOps(GrSurface* surface, GrGLenum fboTarget,
                                                              target,
                                                              texID,
                                                              0));
+        texture->baseLevelWasBoundToFBO();
         viewport->fLeft = 0;
         viewport->fBottom = 0;
         viewport->fWidth = surface->width();
diff --git a/src/gpu/gl/GrGLTexture.h b/src/gpu/gl/GrGLTexture.h
index 74358914ec..85ada01c87 100644
--- a/src/gpu/gl/GrGLTexture.h
+++ b/src/gpu/gl/GrGLTexture.h
@@ -66,7 +66,11 @@ public:
 
     GrGLenum target() const { return fInfo.fTarget; }
 
+    bool hasBaseLevelBeenBoundToFBO() const { return fBaseLevelHasBeenBoundToFBO; }
+    void baseLevelWasBoundToFBO() { fBaseLevelHasBeenBoundToFBO = true; }
+
     static sk_sp<GrGLTexture> MakeWrapped(GrGLGpu*, const GrSurfaceDesc&, const IDDesc&);
+
 protected:
     // Constructor for subclasses.
     GrGLTexture(GrGLGpu*, const GrSurfaceDesc&, const IDDesc&, bool wasMipMapDataProvided);
@@ -96,6 +100,7 @@ private:
     // direct interaction with the GL object.
     GrGLTextureInfo                 fInfo;
     GrBackendObjectOwnership        fTextureIDOwnership;
+    bool                            fBaseLevelHasBeenBoundToFBO = false;
 
     ReleaseProc                     fReleaseProc = nullptr;
     ReleaseCtx                      fReleaseCtx = nullptr;