Add fExternalFormatForTexImage to GrGLGpu::ConfigEntry. Use it in GrGLGpu::uploadTexData.

src/gpu/gl/GrGLGpu.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "GrGLGpu.h"
 #include "GrGLGLSL.h"
@@ skipping 683 matching lines @@
                             bool isNewTexture,
                             int left, int top, int width, int height,
                             GrPixelConfig dataConfig,
                             const void* data,
                             size_t rowBytes) {
     SkASSERT(data || isNewTexture);
 
     // If we're uploading compressed data then we should be using uploadCompressedTexData
     SkASSERT(!GrPixelConfigIsCompressed(dataConfig));
 
+    SkASSERT(this->caps()->isConfigTexturable(desc.fConfig));
+
     size_t bpp = GrBytesPerPixel(dataConfig);
     if (!GrSurfacePriv::AdjustWritePixelParams(desc.fWidth, desc.fHeight, bpp, &left, &top,
                                                &width, &height, &data, &rowBytes)) {
         return false;
     }
     size_t trimRowBytes = width * bpp;
 
     // in case we need a temporary, trimmed copy of the src pixels
 #if defined(GOOGLE3)
     // Stack frame size is limited in GOOGLE3.
@@ skipping 11 matching lines @@
                          this->glCaps().texStorageSupport();
 
     if (useTexStorage && kGL_GrGLStandard == this->glStandard()) {
         // 565 is not a sized internal format on desktop GL. So on desktop with
         // 565 we always use an unsized internal format to let the system pick
         // the best sized format to convert the 565 data to. Since TexStorage
         // only allows sized internal formats we will instead use TexImage2D.
         useTexStorage = desc.fConfig != kRGB_565_GrPixelConfig;
     }
 
-    GrGLenum internalFormat = 0x0; // suppress warning
-    GrGLenum externalFormat = 0x0; // suppress warning
-    GrGLenum externalType = 0x0;   // suppress warning
 
     bool useSizedFormat = use_sized_format_for_texture(useTexStorage, this->ctxInfo(),
                                                        desc.fConfig);
 
-    if (!this->configToGLFormats(desc.fConfig, useSizedFormat, &internalFormat,
-                                 &externalFormat, &externalType)) {
-        return false;
-    }
+    // Internal format comes from the texture desc.
+    GrGLenum internalFormat = useSizedFormat ?
+                                fConfigTable[desc.fConfig].fSizedInternalFormat:
+                                fConfigTable[desc.fConfig].fBaseInternalFormat;
 
-    if (dataConfig != desc.fConfig) {
-        // call this again if we're going to upload a different config than the texture's config.
-        if (!this->configToGLFormats(dataConfig, false, nullptr, &externalFormat,
-                                     &externalType)) {
-            return false;
-        }
-    }
+    // External format and type come from the upload data.
+    GrGLenum externalFormat = fConfigTable[dataConfig].fExternalFormatForTexImage;
+    GrGLenum externalType = fConfigTable[dataConfig].fExternalType;
 
     /*
-     *  check whether to allocate a temporary buffer for flipping y or
+     *  Check whether to allocate a temporary buffer for flipping y or
      *  because our srcData has extra bytes past each row. If so, we need
      *  to trim those off here, since GL ES may not let us specify
      *  GL_UNPACK_ROW_LENGTH.
      */
     bool restoreGLRowLength = false;
     bool swFlipY = false;
     bool glFlipY = false;
     if (data) {
         if (kBottomLeft_GrSurfaceOrigin == desc.fOrigin) {
             if (this->glCaps().unpackFlipYSupport()) {
@@ skipping 1279 matching lines @@
         return false;
     }
 
     GrGLenum format = 0;
     GrGLenum type = 0;
     bool flipY = kBottomLeft_GrSurfaceOrigin == surface->origin();
     if (!this->configToGLFormats(config, false, nullptr, &format, &type)) {
         return false;
     }
 
-    // glReadPixels does not allow GL_SRGB_ALPHA. Instead use GL_RGBA. This will not trigger a
-    // conversion when the src is srgb.
-    if (GR_GL_SRGB_ALPHA == format) {
-        format = GR_GL_RGBA;
-    }
-
     // resolve the render target if necessary
     switch (tgt->getResolveType()) {
         case GrGLRenderTarget::kCantResolve_ResolveType:
             return false;
         case GrGLRenderTarget::kAutoResolves_ResolveType:
             this->flushRenderTarget(tgt, &SkIRect::EmptyIRect());
             break;
         case GrGLRenderTarget::kCanResolve_ResolveType:
             this->onResolveRenderTarget(tgt);
             // we don't track the state of the READ FBO ID.
@@ skipping 616 matching lines @@
     } else {
         fConfigTable[kBGRA_8888_GrPixelConfig].fBaseInternalFormat = GR_GL_RGBA;
         fConfigTable[kBGRA_8888_GrPixelConfig].fSizedInternalFormat = GR_GL_RGBA8;
     }
     fConfigTable[kBGRA_8888_GrPixelConfig].fExternalFormat= GR_GL_BGRA;
     fConfigTable[kBGRA_8888_GrPixelConfig].fExternalType  = GR_GL_UNSIGNED_BYTE;
 
 
     fConfigTable[kSRGBA_8888_GrPixelConfig].fBaseInternalFormat = GR_GL_SRGB_ALPHA;
     fConfigTable[kSRGBA_8888_GrPixelConfig].fSizedInternalFormat = GR_GL_SRGB8_ALPHA8;
-    // OpenGL ES 2.0 + GL_EXT_sRGB allows GL_SRGB_ALPHA to be specified as the <format>
-    // param to Tex(Sub)Image2D. ES 2.0 requires the internalFormat and format to match.
-    // Thus, on ES 2.0 we will use GL_SRGB_ALPHA as the externalFormat. However,
-    // onReadPixels needs code to override that because GL_SRGB_ALPHA is not allowed as a
-    // glReadPixels format.
-    // On OpenGL and ES 3.0 GL_SRGB_ALPHA does not work for the <format> param to
-    // glReadPixels nor does it work with Tex(Sub)Image2D So we always set the externalFormat
-    // return to GL_RGBA.
-    if (this->glStandard() == kGLES_GrGLStandard &&
-        this->glVersion() == GR_GL_VER(2,0)) {
-        fConfigTable[kSRGBA_8888_GrPixelConfig].fExternalFormat = GR_GL_SRGB_ALPHA;
-    } else {
-        fConfigTable[kSRGBA_8888_GrPixelConfig].fExternalFormat = GR_GL_RGBA;
-    }
+    // GL does not do srgb<->rgb conversions when transferring between cpu and gpu. Thus, the
+    // external format is GL_RGBA. See below for note about ES2.0 and glTex[Sub]Image.
+    fConfigTable[kSRGBA_8888_GrPixelConfig].fExternalFormat = GR_GL_RGBA;
     fConfigTable[kSRGBA_8888_GrPixelConfig].fExternalType = GR_GL_UNSIGNED_BYTE;
 
 
     fConfigTable[kRGB_565_GrPixelConfig].fBaseInternalFormat = GR_GL_RGB;
     if (this->glCaps().ES2CompatibilitySupport()) {
         fConfigTable[kRGB_565_GrPixelConfig].fSizedInternalFormat = GR_GL_RGB565;
     } else {
         fConfigTable[kRGB_565_GrPixelConfig].fSizedInternalFormat = GR_GL_RGB5;
     }
     fConfigTable[kRGB_565_GrPixelConfig].fExternalFormat = GR_GL_RGB;
@@ skipping 79 matching lines @@
     fConfigTable[kR11_EAC_GrPixelConfig].fBaseInternalFormat = GR_GL_COMPRESSED_R11_EAC;
     fConfigTable[kR11_EAC_GrPixelConfig].fSizedInternalFormat = GR_GL_COMPRESSED_R11_EAC;
     fConfigTable[kR11_EAC_GrPixelConfig].fExternalFormat = 0;
     fConfigTable[kR11_EAC_GrPixelConfig].fExternalType = 0;
 
     fConfigTable[kASTC_12x12_GrPixelConfig].fBaseInternalFormat = GR_GL_COMPRESSED_RGBA_ASTC_12x12;
     fConfigTable[kASTC_12x12_GrPixelConfig].fSizedInternalFormat = GR_GL_COMPRESSED_RGBA_ASTC_12x12;
     fConfigTable[kASTC_12x12_GrPixelConfig].fExternalFormat = 0;
     fConfigTable[kASTC_12x12_GrPixelConfig].fExternalType = 0;
 
+
+    // Almost always we want to pass fExternalFormat as the <format> param to glTex[Sub]Image.
+    for (int i = 0; i < kGrPixelConfigCnt; ++i) {
+        fConfigTable[i].fExternalFormatForTexImage = fConfigTable[i].fExternalFormat;
+    }

[jvanverth1, 2015/12/17 19:21:50]

What about iOS, BGRA, and ES 3.0? Didn't we see something odd there as well?

[bsalomon, 2015/12/17 19:35:11]

I don't recall that. AFAIK there is no sRGB BGRA format. I think this produces
the same behavior as the previous code: on desktop/ES 3.0+ we will pass GL_RGBA
for the format param to tex[Sub]Image and readPixels. On ES2 we pass
GL_SRGB_ALPHA for format to tex[Sub]Image and GL_RGBA to readPixels.

+    // OpenGL ES 2.0 + GL_EXT_sRGB allows GL_SRGB_ALPHA to be specified as the <format>
+    // param to Tex(Sub)Image. ES 2.0 requires the <internalFormat> and <format> params to match.
+    // Thus, on ES 2.0 we will use GL_SRGB_ALPHA as the <format> param.
+    // On OpenGL and ES 3.0+ GL_SRGB_ALPHA does not work for the <format> param to glTexImage.
+    if (this->glStandard() == kGLES_GrGLStandard && this->glVersion() == GR_GL_VER(2,0)) {
+        fConfigTable[kSRGBA_8888_GrPixelConfig].fExternalFormatForTexImage = GR_GL_SRGB_ALPHA;
+    }
+
 #ifdef SK_DEBUG
     // Make sure we initialized everything.
     ConfigEntry defaultEntry;
-    for (int i = 0; i < kLast_GrPixelConfig; ++i) {
+    for (int i = 0; i < kGrPixelConfigCnt; ++i) {
         SkASSERT(defaultEntry.fBaseInternalFormat != fConfigTable[i].fBaseInternalFormat);
         SkASSERT(defaultEntry.fSizedInternalFormat != fConfigTable[i].fSizedInternalFormat);
         SkASSERT(defaultEntry.fExternalFormat != fConfigTable[i].fExternalFormat);
         SkASSERT(defaultEntry.fExternalType != fConfigTable[i].fExternalType);
     }
 #endif
 }
 
 bool GrGLGpu::configToGLFormats(GrPixelConfig config,
                                 bool getSizedInternalFormat,
@@ skipping 817 matching lines @@
             this->setVertexArrayID(gpu, 0);
         }
         int attrCount = gpu->glCaps().maxVertexAttributes();
         if (fDefaultVertexArrayAttribState.count() != attrCount) {
             fDefaultVertexArrayAttribState.resize(attrCount);
         }
         attribState = &fDefaultVertexArrayAttribState;
     }
     return attribState;
 }