Revert of Don't allow nullptr in texels array params (unless using a transfer buffer...

Reason for revert:
breaks vk build

Original issue's description:
> Don't allow nullptr in texels array params (unless using a transfer buffer).
>
> Require all levels in writePixels to have a non-nullptr.
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1765633002
>
> Committed: https://skia.googlesource.com/skia/+/8ee78f31b2a29a5f76403755ea17bad9be74a3ec

TBR=jvanverth@google.com,cblume@google.com,cblume@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1760343003

diff --git a/include/gpu/GrTextureProvider.h b/include/gpu/GrTextureProvider.h
index 7c12ebd6db36152535185c8e15d05d7839e937be..83efb5bf7b0622c3568a4f3d3e0b69e972606c5f 100644 (file)
--- a/include/gpu/GrTextureProvider.h
+++ b/include/gpu/GrTextureProvider.h
@@ -45,7 +45,7 @@ public:
 
     /** Shortcut for creating a texture with no initial data to upload. */
     GrTexture* createTexture(const GrSurfaceDesc& desc, SkBudgeted budgeted) {
-        return this->createTexture(desc, budgeted, nullptr, 0);
+        return this->createTexture(desc, budgeted, NULL, 0);
     }
 
     /** Assigns a unique key to the texture. The texture will be findable via this key using

diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index efd687d6a858923db669e28984f69c5e3e51ef49..4e0464a4ce6cb74f0e2c734b5fef8b1f3685e391 100644 (file)
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -98,7 +98,7 @@ static GrSurfaceOrigin resolve_origin(GrSurfaceOrigin origin, bool renderTarget)
  * @param isRT Indicates if the texture can be a render target.
  */
 static bool check_texture_creation_params(const GrCaps& caps, const GrSurfaceDesc& desc,
-                                          bool* isRT, const SkTArray<GrMipLevel>& texels) {
+                                          bool* isRT) {
     if (!caps.isConfigTexturable(desc.fConfig)) {
         return false;
     }
@@ -124,12 +124,6 @@ static bool check_texture_creation_params(const GrCaps& caps, const GrSurfaceDes
             return false;
         }
     }
-
-    for (int i = 0; i < texels.count(); ++i) {
-        if (!texels[i].fPixels) {
-            return false;
-        }
-    }
     return true;
 }
 
@@ -139,7 +133,7 @@ GrTexture* GrGpu::createTexture(const GrSurfaceDesc& origDesc, SkBudgeted budget
 
     const GrCaps* caps = this->caps();
     bool isRT = false;
-    bool textureCreationParamsValid = check_texture_creation_params(*caps, desc, &isRT, texels);
+    bool textureCreationParamsValid = check_texture_creation_params(*caps, desc, &isRT);
     if (!textureCreationParamsValid) {
         return nullptr;
     }
@@ -185,6 +179,17 @@ GrTexture* GrGpu::createTexture(const GrSurfaceDesc& origDesc, SkBudgeted budget
     return tex;
 }
 
+GrTexture* GrGpu::createTexture(const GrSurfaceDesc& desc, SkBudgeted budgeted,
+                                const void* srcData, size_t rowBytes) {
+    GrMipLevel level;
+    level.fPixels = srcData;
+    level.fRowBytes = rowBytes;
+    SkSTArray<1, GrMipLevel> levels;
+    levels.push_back(level);
+
+    return this->createTexture(desc, budgeted, levels);
+}
+
 GrTexture* GrGpu::wrapBackendTexture(const GrBackendTextureDesc& desc, GrWrapOwnership ownership) {
     this->handleDirtyContext();
     if (!this->caps()->isConfigTexturable(desc.fConfig)) {
@@ -386,11 +391,16 @@ bool GrGpu::writePixels(GrSurface* surface,
     if (!surface) {
         return false;
     }
+    bool validMipDataFound = false;
     for (int currentMipLevel = 0; currentMipLevel < texels.count(); currentMipLevel++) {
-        if (!texels[currentMipLevel].fPixels ) {
-            return false;
+        if (texels[currentMipLevel].fPixels != nullptr) {
+            validMipDataFound = true;
+            break;
         }
     }
+    if (!validMipDataFound) {
+        return false;
+    }
 
     this->handleDirtyContext();
     if (this->onWritePixels(surface, left, top, width, height, config, texels)) {

diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 5b4c05a0fd6b86d46d534d4743c5ef9fe1d15baa..5d35fcf859bca3792681e9e6ff5037e92fc9fdf0 100644 (file)
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -96,13 +96,22 @@ public:
                              const SkTArray<GrMipLevel>& texels);
 
     /**
-     * Simplified createTexture() interface when there is no initial texel data
-     * to upload.
+     * This function is a shim which creates a SkTArGrMipLevell> of size 1.
+     * It then calls createTexture with that SkTArray.
+     *
+     * @param srcData  texel data to load texture. Begins with full-size
+     *                 palette data for paletted texture. For compressed
+     *                 formats it contains the compressed pixel data. Otherwise,
+     *                 it contains width*height texels. If nullptr texture data
+     *                 is uninitialized.
+     * @param rowBytes the number of bytes between consecutive rows. Zero
+     *                 means rows are tightly packed. This field is ignored
+     *                 for compressed pixel formats.
+     * @return    The texture object if successful, otherwise, nullptr.
      */
-    GrTexture* createTexture(const GrSurfaceDesc& desc, SkBudgeted budgeted) {
-        return this->createTexture(desc, budgeted, SkTArray<GrMipLevel>());
-    }
-    
+    GrTexture* createTexture(const GrSurfaceDesc& desc, SkBudgeted budgeted,
+                             const void* srcData, size_t rowBytes);
+
     /**
      * Implements GrTextureProvider::wrapBackendTexture
      */

diff --git a/src/gpu/GrTextureProvider.cpp b/src/gpu/GrTextureProvider.cpp
index f1775aba78f38c869611184cbca3fae00c4842c0..2d422f717f5e777ff8af2c2dc486465b963a47e9 100644 (file)
--- a/src/gpu/GrTextureProvider.cpp
+++ b/src/gpu/GrTextureProvider.cpp
@@ -37,15 +37,6 @@ GrTexture* GrTextureProvider::createMipMappedTexture(const GrSurfaceDesc& desc,
     if (this->isAbandoned()) {
         return nullptr;
     }
-    if (mipLevelCount && !texels) {
-        return nullptr;
-    }
-    for (int i = 0; i < mipLevelCount; ++i) {
-        if (!texels[i].fPixels) {
-            return nullptr;
-        }
-    }
-
     if ((desc.fFlags & kRenderTarget_GrSurfaceFlag) &&
         !fGpu->caps()->isConfigRenderable(desc.fConfig, desc.fSampleCnt > 0)) {
         return nullptr;
@@ -57,8 +48,8 @@ GrTexture* GrTextureProvider::createMipMappedTexture(const GrSurfaceDesc& desc,
             static const uint32_t kFlags = kExact_ScratchTextureFlag |
                                            kNoCreate_ScratchTextureFlag;
             if (GrTexture* texture = this->refScratchTexture(desc, kFlags)) {
-                if (!texels || texture->writePixels(0, 0, desc.fWidth, desc.fHeight, desc.fConfig,
-                                                    baseMipLevel.fPixels, baseMipLevel.fRowBytes)) {
+                if (texture->writePixels(0, 0, desc.fWidth, desc.fHeight, desc.fConfig,
+                                         baseMipLevel.fPixels, baseMipLevel.fRowBytes)) {
                     if (SkBudgeted::kNo == budgeted) {
                         texture->resourcePriv().makeUnbudgeted();
                     }
@@ -78,16 +69,12 @@ GrTexture* GrTextureProvider::createMipMappedTexture(const GrSurfaceDesc& desc,
 
 GrTexture* GrTextureProvider::createTexture(const GrSurfaceDesc& desc, SkBudgeted budgeted,
                                             const void* srcData, size_t rowBytes) {
-    GrMipLevel tempTexels;
-    GrMipLevel* texels = nullptr;
-    int levelCount = 0;
-    if (srcData) {
-      tempTexels.fPixels = srcData;
-      tempTexels.fRowBytes = rowBytes;
-      texels = &tempTexels;
-      levelCount = 1;
-    }
-    return this->createMipMappedTexture(desc, budgeted, texels, levelCount);
+    const int mipLevelCount = 1;
+    GrMipLevel texels[mipLevelCount];
+    texels[0].fPixels = srcData;
+    texels[0].fRowBytes = rowBytes;
+
+    return this->createMipMappedTexture(desc, budgeted, texels, mipLevelCount);
 }
 
 GrTexture* GrTextureProvider::createApproxTexture(const GrSurfaceDesc& desc) {
@@ -150,7 +137,7 @@ GrTexture* GrTextureProvider::refScratchTexture(const GrSurfaceDesc& inDesc,
     }
 
     if (!(kNoCreate_ScratchTextureFlag & flags)) {
-        return fGpu->createTexture(*desc, SkBudgeted::kYes);
+        return fGpu->createTexture(*desc, SkBudgeted::kYes, nullptr, 0);
     }
 
     return nullptr;

diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index fe803de0a903bfb367116fc5fce42d6ef243a70e..dc50c1429ce44ea940c65a31b85693b1b45d6010 100644 (file)
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -926,7 +926,7 @@ static inline GrGLenum check_alloc_error(const GrSurfaceDesc& desc,
  * @param succeeded      Set to true if allocating and populating the texture completed
  *                       without error.
  */
-static bool allocate_and_populate_uncompressed_texture(const GrSurfaceDesc& desc,
+static void allocate_and_populate_uncompressed_texture(const GrSurfaceDesc& desc,
                                                        const GrGLInterface& interface,
                                                        const GrGLCaps& caps,
                                                        GrGLenum target,
@@ -934,7 +934,8 @@ static bool allocate_and_populate_uncompressed_texture(const GrSurfaceDesc& desc
                                                        GrGLenum externalFormat,
                                                        GrGLenum externalType,
                                                        const SkTArray<GrMipLevel>& texels,
-                                                       int baseWidth, int baseHeight) {
+                                                       int baseWidth, int baseHeight,
+                                                       bool* succeeded) {
     CLEAR_ERROR_BEFORE_ALLOC(&interface);
 
     bool useTexStorage = caps.isConfigTexSupportEnabled(desc.fConfig);
@@ -954,7 +955,7 @@ static bool allocate_and_populate_uncompressed_texture(const GrSurfaceDesc& desc
                                    desc.fWidth, desc.fHeight));
         GrGLenum error = check_alloc_error(desc, &interface);
         if (error != GR_GL_NO_ERROR) {
-            return  false;
+            *succeeded = false;
         } else {
             for (int currentMipLevel = 0; currentMipLevel < texels.count(); currentMipLevel++) {
                 const void* currentMipData = texels[currentMipLevel].fPixels;
@@ -975,48 +976,33 @@ static bool allocate_and_populate_uncompressed_texture(const GrSurfaceDesc& desc
                                          externalFormat, externalType,
                                          currentMipData));
             }
-            return true;
+            *succeeded = true;
         }
     } else {
-        if (texels.empty()) {
+        *succeeded = true;
+        for (int currentMipLevel = 0; currentMipLevel < texels.count(); currentMipLevel++) {
+            int twoToTheMipLevel = 1 << currentMipLevel;
+            int currentWidth = SkTMax(1, baseWidth / twoToTheMipLevel);
+            int currentHeight = SkTMax(1, baseHeight / twoToTheMipLevel);
+            const void* currentMipData = texels[currentMipLevel].fPixels;
+            // Even if curremtMipData is nullptr, continue to call TexImage2D.
+            // This will allocate texture memory which we can later populate.
             GL_ALLOC_CALL(&interface,
                           TexImage2D(target,
-                                     0,
+                                     currentMipLevel,
                                      internalFormat,
-                                     baseWidth,
-                                     baseHeight,
+                                     currentWidth,
+                                     currentHeight,
                                      0, // border
                                      externalFormat, externalType,
-                                     nullptr));
+                                     currentMipData));
             GrGLenum error = check_alloc_error(desc, &interface);
             if (error != GR_GL_NO_ERROR) {
-                return false;
-            }
-        } else {
-            for (int currentMipLevel = 0; currentMipLevel < texels.count(); currentMipLevel++) {
-                int twoToTheMipLevel = 1 << currentMipLevel;
-                int currentWidth = SkTMax(1, baseWidth / twoToTheMipLevel);
-                int currentHeight = SkTMax(1, baseHeight / twoToTheMipLevel);
-                const void* currentMipData = texels[currentMipLevel].fPixels;
-                // Even if curremtMipData is nullptr, continue to call TexImage2D.
-                // This will allocate texture memory which we can later populate.
-                GL_ALLOC_CALL(&interface,
-                              TexImage2D(target,
-                                         currentMipLevel,
-                                         internalFormat,
-                                         currentWidth,
-                                         currentHeight,
-                                         0, // border
-                                         externalFormat, externalType,
-                                         currentMipData));
-                GrGLenum error = check_alloc_error(desc, &interface);
-                if (error != GR_GL_NO_ERROR) {
-                    return false;
-                }
+                *succeeded = false;
+                break;
             }
         }
     }
-    return true;
 }
 
 /**
@@ -1150,7 +1136,8 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
 
     for (int currentMipLevel = texelsShallowCopy.count() - 1; currentMipLevel >= 0;
          currentMipLevel--) {
-        SkASSERT(texelsShallowCopy[currentMipLevel].fPixels || kTransfer_UploadType == uploadType);
+        SkASSERT(texelsShallowCopy[currentMipLevel].fPixels ||
+                 kNewTexture_UploadType == uploadType || kTransfer_UploadType == uploadType);
     }
 
     const GrGLInterface* interface = this->glInterface();
@@ -1167,6 +1154,10 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
         int currentWidth = SkTMax(1, width / twoToTheMipLevel);
         int currentHeight = SkTMax(1, height / twoToTheMipLevel);
 
+        if (texelsShallowCopy[currentMipLevel].fPixels == nullptr) {
+            continue;
+        }
+
         if (currentHeight > SK_MaxS32 ||
             currentWidth > SK_MaxS32) {
             return false;
@@ -1202,7 +1193,7 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
     bool swFlipY = false;
     bool glFlipY = false;
 
-    if (kBottomLeft_GrSurfaceOrigin == desc.fOrigin && !texelsShallowCopy.empty()) {
+    if (kBottomLeft_GrSurfaceOrigin == desc.fOrigin) {
         if (caps.unpackFlipYSupport()) {
             glFlipY = true;
         } else {
@@ -1228,6 +1219,10 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
     char* buffer = (char*)tempStorage.reset(combined_buffer_size);
 
     for (int currentMipLevel = 0; currentMipLevel < texelsShallowCopy.count(); currentMipLevel++) {
+        if (texelsShallowCopy[currentMipLevel].fPixels == nullptr) {
+            continue;
+        }
+
         int twoToTheMipLevel = 1 << currentMipLevel;
         int currentWidth = SkTMax(1, width / twoToTheMipLevel);
         int currentHeight = SkTMax(1, height / twoToTheMipLevel);
@@ -1274,9 +1269,6 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
         } else {
             return false;
         }
-    }
-
-    if (!texelsShallowCopy.empty()) {
         if (glFlipY) {
             GR_GL_CALL(interface, PixelStorei(GR_GL_UNPACK_FLIP_Y, GR_GL_TRUE));
         }
@@ -1289,10 +1281,10 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
         0 == left && 0 == top &&
         desc.fWidth == width && desc.fHeight == height &&
         !desc.fTextureStorageAllocator.fAllocateTextureStorage) {
-        succeeded = allocate_and_populate_uncompressed_texture(desc, *interface, caps, target,
-                                                               internalFormat, externalFormat,
-                                                               externalType, texelsShallowCopy,
-                                                               width, height);
+        allocate_and_populate_uncompressed_texture(desc, *interface, caps, target,
+                                                   internalFormat, externalFormat,
+                                                   externalType, texelsShallowCopy,
+                                                   width, height, &succeeded);
     } else {
         if (swFlipY || glFlipY) {
             top = desc.fHeight - (top + height);
@@ -1302,6 +1294,9 @@ bool GrGLGpu::uploadTexData(const GrSurfaceDesc& desc,
             int twoToTheMipLevel = 1 << currentMipLevel;
             int currentWidth = SkTMax(1, width / twoToTheMipLevel);
             int currentHeight = SkTMax(1, height / twoToTheMipLevel);
+            if (texelsShallowCopy[currentMipLevel].fPixels == nullptr) {
+                continue;
+            }
 
             GL_CALL(TexSubImage2D(target,
                                   currentMipLevel,
@@ -1329,6 +1324,8 @@ bool GrGLGpu::uploadCompressedTexData(const GrSurfaceDesc& desc,
                                       UploadType uploadType,
                                       int left, int top, int width, int height) {
     SkASSERT(this->caps()->isConfigTexturable(desc.fConfig));
+    SkASSERT(kTransfer_UploadType != uploadType &&
+             (texels[0].fPixels || kNewTexture_UploadType != uploadType));
 
     // No support for software flip y, yet...
     SkASSERT(kBottomLeft_GrSurfaceOrigin != desc.fOrigin);
@@ -1369,7 +1366,9 @@ bool GrGLGpu::uploadCompressedTexData(const GrSurfaceDesc& desc,
             return false;
         }
         for (int currentMipLevel = 0; currentMipLevel < texels.count(); currentMipLevel++) {
-            SkASSERT(texels[currentMipLevel].fPixels || kTransfer_UploadType == uploadType);
+            if (texels[currentMipLevel].fPixels == nullptr) {
+                continue;
+            }
 
             int twoToTheMipLevel = 1 << currentMipLevel;
             int currentWidth = SkTMax(1, width / twoToTheMipLevel);
@@ -1848,15 +1847,14 @@ bool GrGLGpu::createTextureExternalAllocatorImpl(const GrSurfaceDesc& desc,
     // We do not make SkTArray available outside of Skia,
     // and so we do not want to allow mipmaps to external
     // allocators just yet.
-    SkASSERT(texels.count() < 2);
+    SkASSERT(texels.count() == 1);
+    SkSTArray<1, GrMipLevel> texelsShallowCopy(1);
+    texelsShallowCopy.push_back(texels[0]);
 
-    const void* pixels = nullptr;
-    if (!texels.empty()) {
-        pixels = texels.begin()->fPixels;
-    }
     switch (desc.fTextureStorageAllocator.fAllocateTextureStorage(
                     desc.fTextureStorageAllocator.fCtx, reinterpret_cast<GrBackendObject>(info),
-                    desc.fWidth, desc.fHeight, desc.fConfig, pixels, desc.fOrigin)) {
+                    desc.fWidth, desc.fHeight, desc.fConfig, texelsShallowCopy[0].fPixels,
+                    desc.fOrigin)) {
         case GrTextureStorageAllocator::Result::kSucceededAndUploaded:
             return true;
         case GrTextureStorageAllocator::Result::kFailed:
@@ -1867,7 +1865,7 @@ bool GrGLGpu::createTextureExternalAllocatorImpl(const GrSurfaceDesc& desc,
 
     if (!this->uploadTexData(desc, info->fTarget, kNewTexture_UploadType, 0, 0,
                              desc.fWidth, desc.fHeight,
-                             desc.fConfig, texels)) {
+                             desc.fConfig, texelsShallowCopy)) {
         desc.fTextureStorageAllocator.fDeallocateTextureStorage(
                 desc.fTextureStorageAllocator.fCtx, reinterpret_cast<GrBackendObject>(info));
         return false;
@@ -2481,8 +2479,7 @@ bool GrGLGpu::readPixelsSupported(GrPixelConfig rtConfig, GrPixelConfig readConf
         desc.fConfig = rtConfig;
         desc.fWidth = desc.fHeight = 16;
         desc.fFlags = kRenderTarget_GrSurfaceFlag;
-        SkAutoTUnref<GrTexture> temp(this->createTexture(desc,
-                                     SkBudgeted::kNo));
+        SkAutoTUnref<GrTexture> temp(this->createTexture(desc, SkBudgeted::kNo, nullptr, 0));
         if (!temp) {
             return false;
         }

diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 4f4d7c99094ac9287a2f8a8620eb36a34e217bde..a208aed456405f8184f14cfc7eaed0e9c6dbda2b 100644 (file)
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -542,8 +542,7 @@ GrTexture* GrVkGpu::onCreateTexture(const GrSurfaceDesc& desc, GrGpuResource::Li
     }
 
     // TODO: We're ignoring MIP levels here.
-    if (!texels.empty()) {
-        SkASSERT(texels.begin()->fPixels);
+    if (!texels.empty() && texels.begin()->fPixels) {
         if (!this->uploadTexData(tex, 0, 0, desc.fWidth, desc.fHeight, desc.fConfig,
                                  texels.begin()->fPixels, texels.begin()->fRowBytes)) {
             tex->unref();