commit 222a354c3b4cfa9920460c478e04fddf87a0c7a7
author Michael Ludwig <michaelludwig@google.com> Wed Apr 24 16:07:03 2024 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Fri Apr 26 12:45:47 2024 +0000
tree f6923fd4b579c0c2abdd84495c6c2597e791c8f2
parent 36e3b537588f03dae5b2a3479d349e737e1c8fdb

[graphite] Assume sources are already instantiated in copies

Change-Id: If265731bf1d6f94437321e5c99561a4de6210f76
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/845339
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>

src/gpu/graphite/task/CopyTask.cpp

diff --git a/src/gpu/graphite/task/CopyTask.cpp b/src/gpu/graphite/task/CopyTask.cpp
index 7348667..7f18991 100644
--- a/src/gpu/graphite/task/CopyTask.cpp
+++ b/src/gpu/graphite/task/CopyTask.cpp
@@ -64,6 +64,9 @@
                                                              sk_sp<Buffer> buffer,
                                                              size_t bufferOffset,
                                                              size_t bufferRowBytes) {
+    if (!textureProxy) {
+        return nullptr;
+    }
     return sk_sp<CopyTextureToBufferTask>(new CopyTextureToBufferTask(std::move(textureProxy),
                                                                       srcRect,
                                                                       std::move(buffer),
@@ -87,14 +90,9 @@
 
 Task::Status CopyTextureToBufferTask::prepareResources(ResourceProvider* resourceProvider,
                                                        const RuntimeEffectDictionary*) {
-    if (!fTextureProxy) {
-        SKGPU_LOG_E("No texture proxy specified for CopyTextureToBufferTask");
-        return Status::kFail;
-    }
-    if (!TextureProxy::InstantiateIfNotLazy(resourceProvider, fTextureProxy.get())) {
-        SKGPU_LOG_E("Could not instantiate texture proxy for CopyTextureToBufferTask!");
-        return Status::kFail;
-    }
+    // If the source texture hasn't been instantiated yet, it means there was no prior task that
+    // could have initialized its contents so a readback to a buffer does not make sense.
+    SkASSERT(fTextureProxy->isInstantiated() || fTextureProxy->isLazy());
     return Status::kSuccess;
 }
 
@@ -121,6 +119,9 @@
                                                                sk_sp<TextureProxy> dstProxy,
                                                                SkIPoint dstPoint,
                                                                int dstLevel) {
+    if (!srcProxy || !dstProxy) {
+        return nullptr;
+    }
     return sk_sp<CopyTextureToTextureTask>(new CopyTextureToTextureTask(std::move(srcProxy),
                                                                         srcRect,
                                                                         std::move(dstProxy),
@@ -143,18 +144,18 @@
 
 Task::Status CopyTextureToTextureTask::prepareResources(ResourceProvider* resourceProvider,
                                                         const RuntimeEffectDictionary*) {
-    if (!fSrcProxy) {
-        SKGPU_LOG_E("No src texture proxy specified for CopyTextureToTextureTask");
-        return Status::kFail;
-    }
+    // If the source texture hasn't been instantiated yet, it means there was no prior task that
+    // could have initialized its contents so propagating the undefined contents to the dst
+    // does not make sense.
+    // TODO(b/333729316): Assert that fSrcProxy is instantiated or lazy; right now it may not be
+    // instantatiated if this is a dst readback copy for a scratch Device. In that case, a
+    // RenderPassTask will immediately follow this copy task and instantiate the source proxy so
+    // that addCommands() has a texture to operate on. That said, the texture's contents will be
+    // undefined when the copy is executed ideally it just shouldn't happen.
     if (!TextureProxy::InstantiateIfNotLazy(resourceProvider, fSrcProxy.get())) {
         SKGPU_LOG_E("Could not instantiate src texture proxy for CopyTextureToTextureTask!");
         return Status::kFail;
     }
-    if (!fDstProxy) {
-        SKGPU_LOG_E("No dst texture proxy specified for CopyTextureToTextureTask");
-        return Status::kFail;
-    }
     if (!TextureProxy::InstantiateIfNotLazy(resourceProvider, fDstProxy.get())) {
         SKGPU_LOG_E("Could not instantiate dst texture proxy for CopyTextureToTextureTask!");
         return Status::kFail;
@@ -165,7 +166,9 @@
 Task::Status CopyTextureToTextureTask::addCommands(Context*,
                                                    CommandBuffer* commandBuffer,
                                                    ReplayTargetData) {
-
+    // prepareResources() doesn't instantiate the source assuming that a prior task will have do so
+    // as part of initializing the texture contents.
+    SkASSERT(fSrcProxy->isInstantiated());
     if (commandBuffer->copyTextureToTexture(fSrcProxy->refTexture(),
                                             fSrcRect,
                                             fDstProxy->refTexture(),

src/gpu/graphite/task/UploadTask.cpp

diff --git a/src/gpu/graphite/task/UploadTask.cpp b/src/gpu/graphite/task/UploadTask.cpp
index 1812b8a..d8577985 100644
--- a/src/gpu/graphite/task/UploadTask.cpp
+++ b/src/gpu/graphite/task/UploadTask.cpp
@@ -329,10 +329,9 @@
 }
 
 bool UploadInstance::prepareResources(ResourceProvider* resourceProvider) {
-    if (!fTextureProxy) {
-        SKGPU_LOG_E("No texture proxy specified for UploadTask");
-        return false;
-    }
+    // While most uploads are to already instantiated proxies (e.g. for client-created texture
+    // images) it is possible that writePixels() was issued as the first operation on a scratch
+    // Device, or that this is the first upload to the raster or text atlas proxies.
     if (!TextureProxy::InstantiateIfNotLazy(resourceProvider, fTextureProxy.get())) {
         SKGPU_LOG_E("Could not instantiate texture proxy for UploadTask!");
         return false;