diff options
author | bsalomon <bsalomon@google.com> | 2016-03-04 07:06:43 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-04 07:06:43 -0800 |
commit | d3312595c86289113ea5994234844388523499e8 (patch) | |
tree | 73f596e64d80fa963952d0367de1bf5c2edd2da4 | |
parent | 0032a302ca8cddc7f6ca500b8096afa4888fc614 (diff) |
Revert of Don't allow nullptr in texels array params (unless using a transfer buffer). (patchset #3 id:60001 of https://codereview.chromium.org/1765633002/ )
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
-rw-r--r-- | include/gpu/GrTextureProvider.h | 2 | ||||
-rw-r--r-- | src/gpu/GrGpu.cpp | 30 | ||||
-rw-r--r-- | src/gpu/GrGpu.h | 21 | ||||
-rw-r--r-- | src/gpu/GrTextureProvider.cpp | 31 | ||||
-rw-r--r-- | src/gpu/gl/GrGLGpu.cpp | 101 | ||||
-rw-r--r-- | src/gpu/vk/GrVkGpu.cpp | 3 |
6 files changed, 95 insertions, 93 deletions
diff --git a/include/gpu/GrTextureProvider.h b/include/gpu/GrTextureProvider.h index 7c12ebd6db..83efb5bf7b 100644 --- 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 efd687d6a8..4e0464a4ce 100644 --- 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 5b4c05a0fd..5d35fcf859 100644 --- 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 f1775aba78..2d422f717f 100644 --- 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 fe803de0a9..dc50c1429c 100644 --- 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 4f4d7c9909..a208aed456 100644 --- 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(); |