commit 94ed83f3ab3a57191dc0a04d0ef7a774643be96c
author Greg Daniel <egdaniel@google.com> Fri Sep 27 13:05:43 2019 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Sep 27 17:34:45 2019 +0000
tree a27e9ebd5dd4f80afbe5c4f8f7feb02ac9507664
parent 666e4dae001389f531c933dafe51381df928691d

Don't execute OpsTask which will have no effect on their target.

Change-Id: Ia58555e7387ef0f80893a58150b3f76d77c26d6e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244896
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

src/gpu/GrOpsTask.cpp

diff --git a/src/gpu/GrOpsTask.cpp b/src/gpu/GrOpsTask.cpp
index b621dd4..aa25103 100644
--- a/src/gpu/GrOpsTask.cpp
+++ b/src/gpu/GrOpsTask.cpp
@@ -394,6 +394,13 @@
 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
     TRACE_EVENT0("skia.gpu", TRACE_FUNC);
 #endif
+    // TODO: remove the check for discard here once reduced op splitting is turned on. Currently we
+    // can end up with GrOpsTasks that only have a discard load op and no ops. For vulkan validation
+    // we need to keep that discard and not drop it. Once we have reduce op list splitting enabled
+    // we shouldn't end up with GrOpsTasks with only discard.
+    if (this->isNoOp() || (fClippedContentBounds.isEmpty() && fColorLoadOp != GrLoadOp::kDiscard)) {
+        return;
+    }
 
     flushState->setSampledProxyArray(&fSampledProxies);
     // Loop over the ops that haven't yet been prepared.
@@ -445,7 +452,11 @@
 // is at flush time). However, we need to store the RenderTargetProxy in the
 // Ops and instantiate them here.
 bool GrOpsTask::onExecute(GrOpFlushState* flushState) {
-    if (this->isNoOp()) {
+    // TODO: remove the check for discard here once reduced op splitting is turned on. Currently we
+    // can end up with GrOpsTasks that only have a discard load op and no ops. For vulkan validation
+    // we need to keep that discard and not drop it. Once we have reduce op list splitting enabled
+    // we shouldn't end up with GrOpsTasks with only discard.
+    if (this->isNoOp() || (fClippedContentBounds.isEmpty() && fColorLoadOp != GrLoadOp::kDiscard)) {
         return false;
     }
 
@@ -760,8 +771,11 @@
     this->forwardCombine(caps);
     if (!this->isNoOp()) {
         SkRect clippedContentBounds = SkRect::MakeIWH(fTarget->width(), fTarget->height());
+        // TODO: If we can fix up GLPrograms test to always intersect the fTarget bounds then we can
+        // simply assert here that the bounds intersect.
         if (clippedContentBounds.intersect(fTotalBounds)) {
-            clippedContentBounds.roundOut(targetUpdateBounds);
+            clippedContentBounds.roundOut(&fClippedContentBounds);
+            *targetUpdateBounds = fClippedContentBounds;
             return ExpectedOutcome::kTargetDirty;
         }
     }