Fix WritePixels test on ANGLE.
Don't upload BGRA to RGBA if not supported (ES2 w/ EXT BGRA extension).
R=senorblanco@chromium.org
Review URL: https://codereview.appspot.com/7305046
git-svn-id: http://skia.googlecode.com/svn/trunk@7622 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index dc69766..3b5e9c4 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -531,8 +531,11 @@
* @param rowBytes number of bytes between consecutive rows. Zero means rows are tightly
* packed.
* @param pixelOpsFlags see PixelOpsFlags enum above.
+ *
+ * @return true if the write succeeded, false if not. The write can fail because of an
+ * unsupported combination of target and pixel configs.
*/
- void writeRenderTargetPixels(GrRenderTarget* target,
+ bool writeRenderTargetPixels(GrRenderTarget* target,
int left, int top, int width, int height,
GrPixelConfig config, const void* buffer,
size_t rowBytes = 0,
@@ -572,8 +575,10 @@
* @param rowBytes number of bytes between consecutive rows. Zero
* means rows are tightly packed.
* @param pixelOpsFlags see PixelOpsFlags enum above.
+ * @return true if the write succeeded, false if not. The write can fail because of an
+ * unsupported combination of texture and pixel configs.
*/
- void writeTexturePixels(GrTexture* texture,
+ bool writeTexturePixels(GrTexture* texture,
int left, int top, int width, int height,
GrPixelConfig config, const void* buffer,
size_t rowBytes,
diff --git a/include/gpu/GrTypes.h b/include/gpu/GrTypes.h
index 9f1eb03..6b484de 100644
--- a/include/gpu/GrTypes.h
+++ b/include/gpu/GrTypes.h
@@ -310,30 +310,8 @@
// This alias is deprecated and will be removed.
static const GrPixelConfig kSkia8888_PM_GrPixelConfig = kSkia8888_GrPixelConfig;
-// Returns true if the pixel config has 8bit r,g,b,a components in that byte
-// order
-static inline bool GrPixelConfigIsRGBA8888(GrPixelConfig config) {
- switch (config) {
- case kRGBA_8888_GrPixelConfig:
- return true;
- default:
- return false;
- }
-}
-
-// Returns true if the pixel config has 8bit b,g,r,a components in that byte
-// order
-static inline bool GrPixelConfigIsBGRA8888(GrPixelConfig config) {
- switch (config) {
- case kBGRA_8888_GrPixelConfig:
- return true;
- default:
- return false;
- }
-}
-
// Returns true if the pixel config is 32 bits per pixel
-static inline bool GrPixelConfigIs32Bit(GrPixelConfig config) {
+static inline bool GrPixelConfigIs8888(GrPixelConfig config) {
switch (config) {
case kRGBA_8888_GrPixelConfig:
case kBGRA_8888_GrPixelConfig:
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 0480c08..21bdbf7 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -1217,23 +1217,29 @@
}
}
-void GrContext::writeTexturePixels(GrTexture* texture,
+bool GrContext::writeTexturePixels(GrTexture* texture,
int left, int top, int width, int height,
GrPixelConfig config, const void* buffer, size_t rowBytes,
uint32_t flags) {
SK_TRACE_EVENT0("GrContext::writeTexturePixels");
ASSERT_OWNED_RESOURCE(texture);
- // TODO: use scratch texture to perform conversion
- if (kUnpremul_PixelOpsFlag & flags) {
- return;
+ if ((kUnpremul_PixelOpsFlag & flags) || !fGpu->canWriteTexturePixels(texture, config)) {
+ if (NULL != texture->asRenderTarget()) {
+ return this->writeRenderTargetPixels(texture->asRenderTarget(),
+ left, top, width, height,
+ config, buffer, rowBytes, flags);
+ } else {
+ return false;
+ }
}
+
if (!(kDontFlush_PixelOpsFlag & flags)) {
this->flush();
}
- fGpu->writeTexturePixels(texture, left, top, width, height,
- config, buffer, rowBytes);
+ return fGpu->writeTexturePixels(texture, left, top, width, height,
+ config, buffer, rowBytes);
}
bool GrContext::readTexturePixels(GrTexture* texture,
@@ -1308,7 +1314,7 @@
bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
int left, int top, int width, int height,
- GrPixelConfig config, void* buffer, size_t rowBytes,
+ GrPixelConfig dstConfig, void* buffer, size_t rowBytes,
uint32_t flags) {
SK_TRACE_EVENT0("GrContext::readRenderTargetPixels");
ASSERT_OWNED_RESOURCE(target);
@@ -1329,25 +1335,27 @@
// If fGpu->readPixels would incur a y-flip cost then we will read the pixels upside down. We'll
// either do the flipY by drawing into a scratch with a matrix or on the cpu after the read.
bool flipY = fGpu->readPixelsWillPayForYFlip(target, left, top,
- width, height, config,
+ width, height, dstConfig,
rowBytes);
- bool swapRAndB = fGpu->preferredReadPixelsConfig(config) == GrPixelConfigSwapRAndB(config);
+ // We ignore the preferred config if it is different than our config unless it is an R/B swap.
+ // In that case we'll perform an R and B swap while drawing to a scratch texture of the swapped
+ // config. Then we will call readPixels on the scratch with the swapped config. The swaps during
+ // the draw cancels out the fact that we call readPixels with a config that is R/B swapped from
+ // dstConfig.
+ GrPixelConfig readConfig = dstConfig;
+ bool swapRAndB = false;
+ if (GrPixelConfigSwapRAndB(dstConfig) == fGpu->preferredReadPixelsConfig(dstConfig)) {
+ readConfig = GrPixelConfigSwapRAndB(readConfig);
+ swapRAndB = true;
+ }
bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & flags);
- if (unpremul && kRGBA_8888_GrPixelConfig != config && kBGRA_8888_GrPixelConfig != config) {
+ if (unpremul && !GrPixelConfigIs8888(dstConfig)) {
// The unpremul flag is only allowed for these two configs.
return false;
}
- GrPixelConfig readConfig;
- if (swapRAndB) {
- readConfig = GrPixelConfigSwapRAndB(config);
- GrAssert(kUnknown_GrPixelConfig != config);
- } else {
- readConfig = config;
- }
-
// If the src is a texture and we would have to do conversions after read pixels, we instead
// do the conversions by drawing the src to a scratch texture. If we handle any of the
// conversions in the draw we set the corresponding bool to false so that we don't reapply it
@@ -1364,7 +1372,7 @@
desc.fConfig = readConfig;
desc.fOrigin = kTopLeft_GrSurfaceOrigin;
- // When a full readback is faster than a partial we could always make the scratch exactly
+ // When a full read back is faster than a partial we could always make the scratch exactly
// match the passed rect. However, if we see many different size rectangles we will trash
// our texture cache and pay the cost of creating and destroying many textures. So, we only
// request an exact match when the caller is reading an entire RT.
@@ -1388,7 +1396,7 @@
if (unpremul) {
effect.reset(this->createPMToUPMEffect(src, swapRAndB, textureMatrix));
if (NULL != effect) {
- unpremul = false; // we no longer need to do this on CPU after the readback.
+ unpremul = false; // we no longer need to do this on CPU after the read back.
}
}
// If we failed to create a PM->UPM effect and have no other conversions to perform then
@@ -1429,8 +1437,8 @@
SkCanvas::Config8888 srcC8888 = SkCanvas::kNative_Premul_Config8888;
SkCanvas::Config8888 dstC8888 = SkCanvas::kNative_Premul_Config8888;
- SkDEBUGCODE(bool c8888IsValid =) grconfig_to_config8888(config, false, &srcC8888);
- grconfig_to_config8888(config, unpremul, &dstC8888);
+ SkDEBUGCODE(bool c8888IsValid =) grconfig_to_config8888(dstConfig, false, &srcC8888);
+ grconfig_to_config8888(dstConfig, unpremul, &dstC8888);
if (swapRAndB) {
GrAssert(c8888IsValid); // we should only do r/b swap on 8888 configs
@@ -1486,9 +1494,9 @@
fGpu->drawSimpleRect(dstR, NULL);
}
-void GrContext::writeRenderTargetPixels(GrRenderTarget* target,
+bool GrContext::writeRenderTargetPixels(GrRenderTarget* target,
int left, int top, int width, int height,
- GrPixelConfig config,
+ GrPixelConfig srcConfig,
const void* buffer,
size_t rowBytes,
uint32_t flags) {
@@ -1498,7 +1506,7 @@
if (NULL == target) {
target = fDrawState->getRenderTarget();
if (NULL == target) {
- return;
+ return false;
}
}
@@ -1517,31 +1525,33 @@
// At least some drivers on the Mac get confused when glTexImage2D is called on a texture
// attached to an FBO. The FBO still sees the old image. TODO: determine what OS versions and/or
// HW is affected.
- if (NULL != target->asTexture() && !(kUnpremul_PixelOpsFlag & flags)) {
- this->writeTexturePixels(target->asTexture(),
- left, top, width, height,
- config, buffer, rowBytes, flags);
- return;
+ if (NULL != target->asTexture() && !(kUnpremul_PixelOpsFlag & flags) &&
+ fGpu->canWriteTexturePixels(target->asTexture(), srcConfig)) {
+ return this->writeTexturePixels(target->asTexture(),
+ left, top, width, height,
+ srcConfig, buffer, rowBytes, flags);
}
#endif
- bool swapRAndB = (fGpu->preferredReadPixelsConfig(config) == GrPixelConfigSwapRAndB(config));
-
- GrPixelConfig textureConfig;
- if (swapRAndB) {
- textureConfig = GrPixelConfigSwapRAndB(config);
- } else {
- textureConfig = config;
+ // We ignore the preferred config unless it is a R/B swap of the src config. In that case
+ // we will upload the original src data to a scratch texture but we will spoof it as the swapped
+ // config. This scratch will then have R and B swapped. We correct for this by swapping again
+ // when drawing the scratch to the dst using a conversion effect.
+ bool swapRAndB = false;
+ GrPixelConfig writeConfig = srcConfig;
+ if (fGpu->preferredWritePixelsConfig(srcConfig) == GrPixelConfigSwapRAndB(srcConfig)) {
+ writeConfig = GrPixelConfigSwapRAndB(srcConfig);
+ swapRAndB = true;
}
GrTextureDesc desc;
desc.fWidth = width;
desc.fHeight = height;
- desc.fConfig = textureConfig;
+ desc.fConfig = writeConfig;
GrAutoScratchTexture ast(this, desc);
GrTexture* texture = ast.texture();
if (NULL == texture) {
- return;
+ return false;
}
SkAutoTUnref<const GrEffectRef> effect;
@@ -1552,17 +1562,18 @@
SkAutoSTMalloc<128 * 128, uint32_t> tmpPixels(0);
if (kUnpremul_PixelOpsFlag & flags) {
- if (kRGBA_8888_GrPixelConfig != config && kBGRA_8888_GrPixelConfig != config) {
- return;
+ if (!GrPixelConfigIs8888(srcConfig)) {
+ return false;
}
effect.reset(this->createUPMToPMEffect(texture, swapRAndB, textureMatrix));
+ // handle the unpremul step on the CPU if we couldn't create an effect to do it.
if (NULL == effect) {
SkCanvas::Config8888 srcConfig8888, dstConfig8888;
GR_DEBUGCODE(bool success = )
- grconfig_to_config8888(config, true, &srcConfig8888);
+ grconfig_to_config8888(srcConfig, true, &srcConfig8888);
GrAssert(success);
GR_DEBUGCODE(success = )
- grconfig_to_config8888(config, false, &dstConfig8888);
+ grconfig_to_config8888(srcConfig, false, &dstConfig8888);
GrAssert(success);
const uint32_t* src = reinterpret_cast<const uint32_t*>(buffer);
tmpPixels.reset(width * height);
@@ -1580,10 +1591,12 @@
textureMatrix));
}
- this->writeTexturePixels(texture,
- 0, 0, width, height,
- textureConfig, buffer, rowBytes,
- flags & ~kUnpremul_PixelOpsFlag);
+ if (!this->writeTexturePixels(texture,
+ 0, 0, width, height,
+ writeConfig, buffer, rowBytes,
+ flags & ~kUnpremul_PixelOpsFlag)) {
+ return false;
+ }
GrDrawTarget::AutoStateRestore asr(fGpu, GrDrawTarget::kReset_ASRInit);
GrDrawState* drawState = fGpu->drawState();
@@ -1596,6 +1609,7 @@
drawState->setRenderTarget(target);
fGpu->drawSimpleRect(GrRect::MakeWH(SkIntToScalar(width), SkIntToScalar(height)), NULL);
+ return true;
}
////////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index d695e16..c9aa046 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -237,13 +237,13 @@
config, buffer, rowBytes);
}
-void GrGpu::writeTexturePixels(GrTexture* texture,
+bool GrGpu::writeTexturePixels(GrTexture* texture,
int left, int top, int width, int height,
GrPixelConfig config, const void* buffer,
size_t rowBytes) {
this->handleDirtyContext();
- this->onWriteTexturePixels(texture, left, top, width, height,
- config, buffer, rowBytes);
+ return this->onWriteTexturePixels(texture, left, top, width, height,
+ config, buffer, rowBytes);
}
void GrGpu::resolveRenderTarget(GrRenderTarget* target) {
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 38c109d..d0ca377 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -165,23 +165,17 @@
void forceRenderTargetFlush();
/**
- * readPixels with some configs may be slow. Given a desired config this
- * function returns a fast-path config. The returned config must have the
- * same components and component sizes. The caller is free to ignore the
- * result and call readPixels with the original config.
+ * Gets a preferred 8888 config to use for writing / reading pixel data. The returned config
+ * must have at least as many bits per channel as the config param.
*/
- virtual GrPixelConfig preferredReadPixelsConfig(GrPixelConfig config)
- const {
- return config;
- }
+ virtual GrPixelConfig preferredReadPixelsConfig(GrPixelConfig config) const { return config; }
+ virtual GrPixelConfig preferredWritePixelsConfig(GrPixelConfig config) const { return config; }
/**
- * Same as above but applies to writeTexturePixels
+ * Called before uploading writing pixels to a GrTexture when the src pixel config doesn't
+ * match the texture's config.
*/
- virtual GrPixelConfig preferredWritePixelsConfig(GrPixelConfig config)
- const {
- return config;
- }
+ virtual bool canWriteTexturePixels(const GrTexture*, GrPixelConfig srcConfig) const = 0;
/**
* OpenGL's readPixels returns the result bottom-to-top while the skia
@@ -248,7 +242,7 @@
* @param rowBytes number of bytes between consecutive rows. Zero
* means rows are tightly packed.
*/
- void writeTexturePixels(GrTexture* texture,
+ bool writeTexturePixels(GrTexture* texture,
int left, int top, int width, int height,
GrPixelConfig config, const void* buffer,
size_t rowBytes);
@@ -475,7 +469,7 @@
size_t rowBytes) = 0;
// overridden by backend-specific derived class to perform the texture update
- virtual void onWriteTexturePixels(GrTexture* texture,
+ virtual bool onWriteTexturePixels(GrTexture* texture,
int left, int top, int width, int height,
GrPixelConfig config, const void* buffer,
size_t rowBytes) = 0;
diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp
index 5c2e0cb..ff3941d 100644
--- a/src/gpu/gl/GrGpuGL.cpp
+++ b/src/gpu/gl/GrGpuGL.cpp
@@ -345,19 +345,44 @@
}
}
-GrPixelConfig GrGpuGL::preferredReadPixelsConfig(GrPixelConfig config) const {
- if (GR_GL_RGBA_8888_PIXEL_OPS_SLOW && GrPixelConfigIsRGBA8888(config)) {
- return GrPixelConfigSwapRAndB(config);
+namespace {
+GrPixelConfig preferred_pixel_ops_config(GrPixelConfig config) {
+ if (GR_GL_RGBA_8888_PIXEL_OPS_SLOW && kRGBA_8888_GrPixelConfig == config) {
+ return kBGRA_8888_GrPixelConfig;
} else {
return config;
}
}
+}
+
+GrPixelConfig GrGpuGL::preferredReadPixelsConfig(GrPixelConfig config) const {
+ return preferred_pixel_ops_config(config);
+}
GrPixelConfig GrGpuGL::preferredWritePixelsConfig(GrPixelConfig config) const {
- if (GR_GL_RGBA_8888_PIXEL_OPS_SLOW && GrPixelConfigIsRGBA8888(config)) {
- return GrPixelConfigSwapRAndB(config);
+ return preferred_pixel_ops_config(config);
+}
+
+bool GrGpuGL::canWriteTexturePixels(const GrTexture* texture, GrPixelConfig srcConfig) const {
+ if (kIndex_8_GrPixelConfig == srcConfig || kIndex_8_GrPixelConfig == texture->config()) {
+ return false;
+ }
+ if (srcConfig != texture->config() && kES2_GrGLBinding == this->glBinding()) {
+ // In general ES2 requires the internal format of the texture and the format of the src
+ // pixels to match. However, It may or may not be possible to upload BGRA data to a RGBA
+ // texture. It depends upon which extension added BGRA. The Apple extension allows it
+ // (BGRA's internal format is RGBA) while the EXT extension does not (BGRA is its own
+ // internal format).
+ if (this->glCaps().bgraFormatSupport() &&
+ !this->glCaps().bgraIsInternalFormat() &&
+ kBGRA_8888_GrPixelConfig == srcConfig &&
+ kRGBA_8888_GrPixelConfig == texture->config()) {
+ return true;
+ } else {
+ return false;
+ }
} else {
- return config;
+ return true;
}
}
@@ -597,12 +622,12 @@
////////////////////////////////////////////////////////////////////////////////
-void GrGpuGL::onWriteTexturePixels(GrTexture* texture,
+bool GrGpuGL::onWriteTexturePixels(GrTexture* texture,
int left, int top, int width, int height,
GrPixelConfig config, const void* buffer,
size_t rowBytes) {
if (NULL == buffer) {
- return;
+ return false;
}
GrGLTexture* glTex = static_cast<GrGLTexture*>(texture);
@@ -617,9 +642,9 @@
desc.fTextureID = glTex->textureID();
desc.fOrigin = glTex->origin();
- this->uploadTexData(desc, false,
- left, top, width, height,
- config, buffer, rowBytes);
+ return this->uploadTexData(desc, false,
+ left, top, width, height,
+ config, buffer, rowBytes);
}
namespace {
diff --git a/src/gpu/gl/GrGpuGL.h b/src/gpu/gl/GrGpuGL.h
index 71cb699..d9ad1f4 100644
--- a/src/gpu/gl/GrGpuGL.h
+++ b/src/gpu/gl/GrGpuGL.h
@@ -42,10 +42,9 @@
bool programUnitTest(int maxStages);
// GrGpu overrides
- virtual GrPixelConfig preferredReadPixelsConfig(GrPixelConfig config)
- const SK_OVERRIDE;
- virtual GrPixelConfig preferredWritePixelsConfig(GrPixelConfig config)
- const SK_OVERRIDE;
+ virtual GrPixelConfig preferredReadPixelsConfig(GrPixelConfig config) const SK_OVERRIDE;
+ virtual GrPixelConfig preferredWritePixelsConfig(GrPixelConfig config) const SK_OVERRIDE;
+ virtual bool canWriteTexturePixels(const GrTexture*, GrPixelConfig srcConfig) const SK_OVERRIDE;
virtual bool readPixelsWillPayForYFlip(
GrRenderTarget* renderTarget,
int left, int top,
@@ -88,7 +87,7 @@
void* buffer,
size_t rowBytes) SK_OVERRIDE;
- virtual void onWriteTexturePixels(GrTexture* texture,
+ virtual bool onWriteTexturePixels(GrTexture* texture,
int left, int top, int width, int height,
GrPixelConfig config, const void* buffer,
size_t rowBytes) SK_OVERRIDE;
diff --git a/tests/GpuBitmapCopyTest.cpp b/tests/GpuBitmapCopyTest.cpp
index cf9139c..22141e5 100644
--- a/tests/GpuBitmapCopyTest.cpp
+++ b/tests/GpuBitmapCopyTest.cpp
@@ -107,11 +107,6 @@
if (!GrContextFactory::IsRenderingGLContext(glType)) {
continue;
}
-#if SK_ANGLE // This test breaks ANGLE: memcmps fail and readpixels return value is false.
- if (type == GrContextFactory::kANGLE_GLContextType) {
- continue;
- }
-#endif
GrContext* grContext = factory->get(glType);
if (NULL == grContext) {
diff --git a/tests/WritePixelsTest.cpp b/tests/WritePixelsTest.cpp
index 83e0784..2d4cc64 100644
--- a/tests/WritePixelsTest.cpp
+++ b/tests/WritePixelsTest.cpp
@@ -425,11 +425,6 @@
if (isGPUDevice) {
GrContextFactory::GLContextType type =
static_cast<GrContextFactory::GLContextType>(glCtxType);
-#if SK_ANGLE // This test breaks ANGLE with GL errors in texsubimage2D. Disable until debugged.
- if (type == GrContextFactory::kANGLE_GLContextType) {
- continue;
- }
-#endif
if (!GrContextFactory::IsRenderingGLContext(type)) {
continue;
}