Manually generated sRGB mipmaps.

src/gpu/gl/GrGLGpu.cpp

Index: src/gpu/gl/GrGLGpu.cpp
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 6daf9551706bc335f8d6a5588efe82f4633536a9..a7deb28ecd11657f9f21b39e942337f5d75fb6c6 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -1981,6 +1981,14 @@ void GrGLGpu::flushMinSampleShading(float minSampleShading) {
 }
 
 bool GrGLGpu::flushGLState(const GrPipeline& pipeline, const GrPrimitiveProcessor& primProc) {
+    SkAutoTUnref<GrGLProgram> program(fProgramCache->refProgram(this, pipeline, primProc));
+    if (!program) {
+        GrCapsDebugf(this->caps(), "Failed to create program!\n");
+        return false;
+    }
+
+    program->generateMipmaps(primProc, pipeline);
+
     GrXferProcessor::BlendInfo blendInfo;
     pipeline.getXferProcessor().getBlendInfo(&blendInfo);
 
@@ -1988,12 +1996,6 @@ bool GrGLGpu::flushGLState(const GrPipeline& pipeline, const GrPrimitiveProcesso
     this->flushDrawFace(pipeline.getDrawFace());
     this->flushMinSampleShading(primProc.getSampleShading());
 
-    SkAutoTUnref<GrGLProgram> program(fProgramCache->refProgram(this, pipeline, primProc));
-    if (!program) {
-        GrCapsDebugf(this->caps(), "Failed to create program!\n");
-        return false;
-    }
-
     GrGLuint programID = program->programID();
     if (fHWProgramID != programID) {
         GL_CALL(UseProgram(programID));
@@ -2645,19 +2647,22 @@ void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target, const SkIRect* bounds,
     }
 
     if (this->glCaps().srgbWriteControl()) {
-        bool enableSRGBWrite = GrPixelConfigIsSRGB(target->config()) && !disableSRGB;
-        if (enableSRGBWrite && kYes_TriState != fHWSRGBFramebuffer) {
-            GL_CALL(Enable(GR_GL_FRAMEBUFFER_SRGB));
-            fHWSRGBFramebuffer = kYes_TriState;
-        } else if (!enableSRGBWrite && kNo_TriState != fHWSRGBFramebuffer) {
-            GL_CALL(Disable(GR_GL_FRAMEBUFFER_SRGB));
-            fHWSRGBFramebuffer = kNo_TriState;
-        }
+        setFramebufferSRGB(GrPixelConfigIsSRGB(target->config()) && !disableSRGB);

[bsalomon, 2016/05/31 16:35:02]

this->

[Brian Osman, 2016/05/31 17:33:05]

Done.

     }
 
     this->didWriteToSurface(target, bounds);
 }
 
+void GrGLGpu::setFramebufferSRGB(bool enable) {
+    if (enable && kYes_TriState != fHWSRGBFramebuffer) {
+        GL_CALL(Enable(GR_GL_FRAMEBUFFER_SRGB));
+        fHWSRGBFramebuffer = kYes_TriState;
+    } else if (!enable && kNo_TriState != fHWSRGBFramebuffer) {
+        GL_CALL(Disable(GR_GL_FRAMEBUFFER_SRGB));
+        fHWSRGBFramebuffer = kNo_TriState;
+    }
+}
+
 void GrGLGpu::flushViewport(const GrGLIRect& viewport) {
     if (fHWViewport != viewport) {
         viewport.pushToGLViewport(this->glInterface());
@@ -3138,17 +3143,6 @@ void GrGLGpu::bindTexture(int unitIdx, const GrTextureParams& params, bool allow
     bool setAll = timestamp < this->getResetTimestamp();
     GrGLTexture::TexParams newTexParams;
 
-    if (this->caps()->srgbSupport()) {
-        // By default, the decision to allow SRGB decode is based on the destination config.
-        // A texture can override that by specifying a value in GrTextureParams.
-        newTexParams.fSRGBDecode = allowSRGBInputs ? GR_GL_DECODE_EXT : GR_GL_SKIP_DECODE_EXT;
-
-        if (setAll || newTexParams.fSRGBDecode != oldTexParams.fSRGBDecode) {
-            this->setTextureUnit(unitIdx);
-            GL_CALL(TexParameteri(target, GR_GL_TEXTURE_SRGB_DECODE_EXT, newTexParams.fSRGBDecode));
-        }
-    }
-
     static GrGLenum glMinFilterModes[] = {
         GR_GL_NEAREST,
         GR_GL_LINEAR,
@@ -3170,15 +3164,26 @@ void GrGLGpu::bindTexture(int unitIdx, const GrTextureParams& params, bool allow
     newTexParams.fMinFilter = glMinFilterModes[filterMode];
     newTexParams.fMagFilter = glMagFilterModes[filterMode];
 
-    if (GrTextureParams::kMipMap_FilterMode == filterMode) {
-        if (texture->texturePriv().mipMapsAreDirty()) {
+    bool enableSRGBDecode = false;
+    if (GrPixelConfigIsSRGB(texture->config())) {
+        enableSRGBDecode = allowSRGBInputs;
+
+        newTexParams.fSRGBDecode = enableSRGBDecode ? GR_GL_DECODE_EXT : GR_GL_SKIP_DECODE_EXT;
+        if (setAll || newTexParams.fSRGBDecode != oldTexParams.fSRGBDecode) {
             this->setTextureUnit(unitIdx);
-            GL_CALL(GenerateMipmap(target));
-            texture->texturePriv().dirtyMipMaps(false);
-            texture->texturePriv().setMaxMipMapLevel(SkMipMap::ComputeLevelCount(
-                texture->width(), texture->height()));
+            GL_CALL(TexParameteri(target, GR_GL_TEXTURE_SRGB_DECODE_EXT, newTexParams.fSRGBDecode));
+        }
+    }
+
+#ifdef SK_DEBUG
+    // We were supposed to ensure MipMaps were up-to-date and built correctly before getting here.
+    if (GrTextureParams::kMipMap_FilterMode == filterMode) {
+        SkASSERT(!texture->texturePriv().mipMapsAreDirty());
+        if (GrPixelConfigIsSRGB(texture->config())) {
+            SkASSERT(texture->texturePriv().mipMapsAreSRGBCorrect() == enableSRGBDecode);
         }
     }
+#endif
 
     newTexParams.fMaxMipMapLevel = texture->texturePriv().maxMipMapLevel();
 
@@ -3278,6 +3283,67 @@ void GrGLGpu::bindTexelBuffer(int unitIdx, intptr_t offsetInBytes, GrPixelConfig
     }
 }
 
+void GrGLGpu::generateMipmaps(const GrTextureParams& params, bool allowSRGBInputs,
+                              GrGLTexture* texture) {
+    SkASSERT(texture);
+
+    // First, figure out if we need mips for this texture at all:
+    GrTextureParams::FilterMode filterMode = params.filterMode();
+
+    if (GrTextureParams::kMipMap_FilterMode == filterMode) {
+        if (!this->caps()->mipMapSupport() || GrPixelConfigIsCompressed(texture->config())) {
+            filterMode = GrTextureParams::kBilerp_FilterMode;
+        }
+    }
+
+    if (GrTextureParams::kMipMap_FilterMode != filterMode) {
+        return;
+    }
+
+    // If this is an sRGB texture and the mips were previously built the "other" way
+    // (gamma-correct vs. not), then we need to rebuild them. We don't need to check for
+    // srgbSupport - we'll *never* get an sRGB pixel config if we don't support it.
+    if (GrPixelConfigIsSRGB(texture->config()) &&
+        allowSRGBInputs != texture->texturePriv().mipMapsAreSRGBCorrect()) {
+        texture->texturePriv().dirtyMipMaps(true);
+    }
+
+    // If the mips aren't dirty, we're done:
+    if (!texture->texturePriv().mipMapsAreDirty()) {
+        return;
+    }
+
+    // If we created a rt/tex and rendered to it without using a texture and now we're texturing
+    // from the rt it will still be the last bound texture, but it needs resolving.
+    GrGLRenderTarget* texRT = static_cast<GrGLRenderTarget*>(texture->asRenderTarget());
+    if (texRT) {
+        this->onResolveRenderTarget(texRT);
+    }
+
+    GrGLenum target = texture->target();
+    this->setScratchTextureUnit();
+    GL_CALL(BindTexture(target, texture->textureID()));
+
+    // Configure sRGB decode, if necessary. This state is the only thing needed for the driver
+    // call (glGenerateMipmap) to work correctly. Our manual method dirties other state, too.
+    if (GrPixelConfigIsSRGB(texture->config())) {
+        GL_CALL(TexParameteri(target, GR_GL_TEXTURE_SRGB_DECODE_EXT,
+                              allowSRGBInputs ? GR_GL_DECODE_EXT : GR_GL_SKIP_DECODE_EXT));
+    }
+
+    // Either do manual mipmap generation or (if that fails), just rely on the driver:
+    if (!this->generateMipmap(texture, allowSRGBInputs)) {
+        GL_CALL(GenerateMipmap(target));
+    }
+
+    texture->texturePriv().dirtyMipMaps(false, allowSRGBInputs);
+    texture->texturePriv().setMaxMipMapLevel(SkMipMap::ComputeLevelCount(
+        texture->width(), texture->height()));
+
+    // We have potentially set lots of state on the texture. Easiest to dirty it all:
+    texture->textureParamsModified();
+}
+
 void GrGLGpu::setTextureSwizzle(int unitIdx, GrGLenum target, const GrGLenum swizzle[]) {
     this->setTextureUnit(unitIdx);
     if (this->glStandard() == kGLES_GrGLStandard) {
@@ -4068,6 +4134,139 @@ bool GrGLGpu::copySurfaceAsBlitFramebuffer(GrSurface* dst,
     return true;
 }
 
+bool gManualMipmaps = true;
+
+// Manual implementation of glGenerateMipmap, to work around driver bugs w/sRGB
+// Uses glBlitFramebuffer to do a series of downsample operations to successive mips.
+bool GrGLGpu::generateMipmap(GrGLTexture* texture, bool gammaCorrect) {
+    // Global switch for manual mipmap generation:
+    if (!gManualMipmaps) {
+        return false;
+    }
+
+    // We only handle 2D textures for now:

[bsalomon, 2016/05/31 16:35:03]

Do we need to check the format? (for renderability).

[Brian Osman, 2016/05/31 17:33:05]

Yes.

+    if (GR_GL_TEXTURE_2D != texture->target()) {
+        return false;
+    }
+
+    // Our iterative downsample requires the ability to limit which level we're sampling:
+    if (!this->glCaps().mipMapLevelAndLodControlSupport()) {
+        return false;
+    }
+
+    // Get and bind program:
+    int progIdx = TextureTargetToCopyProgramIdx(texture->target());
+    if (!fCopyPrograms[progIdx].fProgram) {
+        if (!this->createCopyProgram(progIdx)) {
+            SkDebugf("Failed to create copy program.\n");
+            return false;
+        }
+    }
+    GL_CALL(UseProgram(fCopyPrograms[progIdx].fProgram));
+    fHWProgramID = fCopyPrograms[progIdx].fProgram;
+
+    int width = texture->width();
+    int height = texture->height();
+    int levelCount = SkMipMap::ComputeLevelCount(width, height) + 1;
+
+    // Define all mips, if we haven't previously done so:
+    if (0 == texture->texturePriv().maxMipMapLevel()) {
+        GrGLenum internalFormat;
+        GrGLenum externalFormat;
+        GrGLenum externalType;
+        if (!this->glCaps().getTexImageFormats(texture->desc().fConfig, texture->desc().fConfig,
+                                               &internalFormat, &externalFormat, &externalType)) {
+            return false;
+        }
+
+        for (GrGLint level = 1; level < levelCount; ++level) {
+            // Define the next mip:

[bsalomon, 2016/05/31 16:35:03]

Aren't we supposed to round up the odds?

[bsalomon, 2016/05/31 16:38:41]

Err, duh, withdrawn.

+            width = SkTMax(1, width / 2);
+            height = SkTMax(1, height / 2);
+            GL_ALLOC_CALL(this->glInterface(), TexImage2D(GR_GL_TEXTURE_2D, level, internalFormat,
+                                                          width, height, 0,
+                                                          externalFormat, externalType, nullptr));
+        }
+    }
+
+    // Create (if necessary), then bind temporary FBO:
+    if (0 == fTempDstFBOID) {
+        GL_CALL(GenFramebuffers(1, &fTempDstFBOID));
+    }
+    GL_CALL(BindFramebuffer(GR_GL_FRAMEBUFFER, fTempDstFBOID));
+    fHWBoundRenderTargetUniqueID = SK_InvalidUniqueID;
+
+    // Turn FB sRGB on or off:
+    if (this->glCaps().srgbWriteControl()) {
+        this->setFramebufferSRGB(gammaCorrect);
+    }
+
+    // Bind the texture, to get things basically configured. We'll be mucking with state further.
+    this->setTextureUnit(0);

[bsalomon, 2016/05/31 16:35:03]

I think we have some notion of a scratch texture unit used for non-GrGLProgram
related things.

[Brian Osman, 2016/05/31 17:33:05]

We do, although the index of that unit isn't a simple constant:

  fHWBoundTextureUniqueIDs.count() - 1

I was using the scratch unit in some other code (and I could bring it
back here). I was basing this code off copySurfaceAsDraw, which uses
unit zero for the source texture.

[bsalomon, 2016/05/31 19:45:46]

sgtm

+    GrTextureParams params(SkShader::kClamp_TileMode, GrTextureParams::kBilerp_FilterMode);
+    this->bindTexture(0, params, gammaCorrect, texture);
+
+    // Vertex data:
+    fHWVertexArrayState.setVertexArrayID(this, 0);
+
+    GrGLAttribArrayState* attribs = fHWVertexArrayState.bindInternalVertexArray(this);
+    attribs->set(this, 0, fCopyProgramArrayBuffer, kVec2f_GrVertexAttribType, 2 * sizeof(GrGLfloat),
+                 0);
+    attribs->disableUnusedArrays(this, 0x1);
+
+    // Set "simple" state once:
+    GrXferProcessor::BlendInfo blendInfo;
+    blendInfo.reset();
+    this->flushBlend(blendInfo, GrSwizzle::RGBA());
+    this->flushColorWrite(true);
+    this->flushDrawFace(GrPipelineBuilder::kBoth_DrawFace);
+    this->flushHWAAState(nullptr, false, false);
+    this->disableScissor();
+    GrStencilSettings stencil;
+    stencil.setDisabled();
+    this->flushStencil(stencil);
+
+    // Configure rects to always map entire previous mip to next mip:
+    GL_CALL(Uniform4f(fCopyPrograms[progIdx].fPosXformUniform, 2.0f, 2.0f, -1.0f, -1.0f));
+    GL_CALL(Uniform4f(fCopyPrograms[progIdx].fTexCoordXformUniform, 1.0f, 1.0f, 0.0f, 0.0f));
+    GL_CALL(Uniform1i(fCopyPrograms[progIdx].fTextureUniform, 0));
+
+    // Do all the blits:
+    width = texture->width();
+    height = texture->height();
+    GrGLIRect viewport;
+    viewport.fLeft = 0;
+    viewport.fBottom = 0;
+    for (GrGLint level = 1; level < levelCount; ++level) {
+        // Only sample from previous mip
+        GL_CALL(TexParameteri(GR_GL_TEXTURE_2D, GR_GL_TEXTURE_BASE_LEVEL, level - 1));
+
+        GL_CALL(FramebufferTexture2D(GR_GL_FRAMEBUFFER, GR_GL_COLOR_ATTACHMENT0,
+                                     GR_GL_TEXTURE_2D, texture->textureID(), level));
+
+        width = SkTMax(1, width / 2);
+        height = SkTMax(1, height / 2);
+        viewport.fWidth = width;
+        viewport.fHeight = height;
+        this->flushViewport(viewport);
+
+        GrGLenum status;
+        GL_CALL_RET(status, CheckFramebufferStatus(GR_GL_FRAMEBUFFER));
+        if (status != GR_GL_FRAMEBUFFER_COMPLETE) {
+            SkDebugf("FB status: 0x%08x\n", status);
+        }
+
+        GL_CALL(DrawArrays(GR_GL_TRIANGLE_STRIP, 0, 4));
+    }
+
+    // Unbind:
+    GL_CALL(FramebufferTexture2D(GR_GL_FRAMEBUFFER, GR_GL_COLOR_ATTACHMENT0,
+                                 GR_GL_TEXTURE_2D, 0, 0));
+    this->didWriteToSurface(texture, nullptr, levelCount);

[bsalomon, 2016/05/31 16:35:03]

Do we need this? IIRC we use this to track whether we need to resolve or
generate MIP levels, neither of which should be true here.

[Brian Osman, 2016/05/31 17:33:05]

Ah, true. I was basing this off the copySurfaceAsXXX methods, and
originally thinking that resolve would be an issue, but it's obviously
not. Good catch.

+
+    return true;
+}
+
 void GrGLGpu::onGetMultisampleSpecs(GrRenderTarget* rt,
                                     const GrStencilSettings& stencil,
                                     int* effectiveSampleCnt,