aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar bsalomon <bsalomon@google.com>2016-03-04 07:06:43 -0800
committerGravatar Commit bot <commit-bot@chromium.org>2016-03-04 07:06:43 -0800
commitd3312595c86289113ea5994234844388523499e8 (patch)
tree73f596e64d80fa963952d0367de1bf5c2edd2da4
parent0032a302ca8cddc7f6ca500b8096afa4888fc614 (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.h2
-rw-r--r--src/gpu/GrGpu.cpp30
-rw-r--r--src/gpu/GrGpu.h21
-rw-r--r--src/gpu/GrTextureProvider.cpp31
-rw-r--r--src/gpu/gl/GrGLGpu.cpp101
-rw-r--r--src/gpu/vk/GrVkGpu.cpp3
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();