cmaa, gpu: use R8 image texture when GL_NV_image_formats is available.

gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.h"
 
 #include "base/logging.h"
 #include "gpu/command_buffer/service/framebuffer_manager.h"
 #include "gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "gpu/command_buffer/service/texture_manager.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_gl_api_implementation.h"
 #include "ui/gl/gl_version_info.h"
 
 #define SHADER(Src) #Src
 
 namespace gpu {
 namespace gles2 {
 
 ApplyFramebufferAttachmentCMAAINTELResourceManager::
     ApplyFramebufferAttachmentCMAAINTELResourceManager()
     : initialized_(false),
       textures_initialized_(false),
       is_in_gamma_correct_mode_(false),
       supports_usampler_(true),
       supports_r8_image_(true),
-      supports_r8_read_format_(true),
       is_gles31_compatible_(false),
       frame_id_(0),
       width_(0),
       height_(0),
       edges0_shader_(0),
       edges1_shader_(0),
       edges_combine_shader_(0),
       process_and_apply_shader_(0),
       debug_display_edges_shader_(0),
       cmaa_framebuffer_(0),
@@ skipping 13 matching lines @@
     ~ApplyFramebufferAttachmentCMAAINTELResourceManager() {
   Destroy();
 }
 
 void ApplyFramebufferAttachmentCMAAINTELResourceManager::Initialize(
     gles2::GLES2Decoder* decoder) {
   DCHECK(decoder);
   is_gles31_compatible_ =
       decoder->GetGLContext()->GetVersionInfo()->IsAtLeastGLES(3, 1);
 
-  // Check if RGBA8UI is supported as an FBO colour target with depth.
-  // If not supported, GLSL needs to convert the data to/from float so there is
-  // a small extra cost.
-  {
-    GLuint rgba8ui_texture = 0, depth_texture = 0;
-    glGenTextures(1, &rgba8ui_texture);
-    glBindTexture(GL_TEXTURE_2D, rgba8ui_texture);
-    glTexStorage2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8UI, 4, 4);
+  if (is_gles31_compatible_) {
+    supports_r8_image_ =
+        decoder->GetGLContext()->HasExtension("GL_NV_image_formats");
 
-    glGenTextures(1, &depth_texture);
-    glBindTexture(GL_TEXTURE_2D, depth_texture);
-    glTexStorage2DEXT(GL_TEXTURE_2D, 1, GL_DEPTH_COMPONENT16, 4, 4);
+    // ES 3.0 requires GL_RGBA8UI is color renderable.
+    supports_usampler_ = true;
+  } else {
+    // CMAA requires GL_ARB_shader_image_load_store for GL, and it requires r8
+    // image texture.
+    DCHECK(decoder->GetGLContext()->HasExtension(

[Zhenyao Mo, 2017/03/01 23:52:33]

Is it validated somewhere before this that it's either ES 3.1 or above, or it's
GL with this extension?

[dshwang, 2017/03/02 20:27:03]

yes, here
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/feature_info....

+        "GL_ARB_shader_image_load_store"));
+    supports_r8_image_ = true;
 
-    // Create the FBO
-    GLuint rgba8ui_framebuffer = 0;
-    glGenFramebuffersEXT(1, &rgba8ui_framebuffer);
-    glBindFramebufferEXT(GL_FRAMEBUFFER, rgba8ui_framebuffer);
+    // Check if RGBA8UI is supported as an FBO colour target with depth.
+    // If not supported, GLSL needs to convert the data to/from float so there
+    // is a small extra cost.
+    {
+      GLuint rgba8ui_texture = 0, depth_texture = 0;
+      glGenTextures(1, &rgba8ui_texture);
+      glBindTexture(GL_TEXTURE_2D, rgba8ui_texture);

[Zhenyao Mo, 2017/03/01 23:52:33]

A question: although not new to this CL, but here we changed texture/fbo
bindings. Do we need to restore them?

[dshwang, 2017/03/02 20:27:03]

below code anyway bind texture/fbo, so not necessary. In addition, as far as I
know, glDeleteFramebuffersEXT and glDeleteTextures unbind by itself.

[Zhenyao Mo, 2017/03/06 03:36:54]

What I mean is the bindings before entering this code is lost. Later bind/delete
doesn't change this fact.   If we don't recover them, we will leak state changes
into other code.

[dshwang, 2017/03/07 02:44:09]

Before leaving CMAA code, CMAA always restore texture unit binding.
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_app...

+      glTexStorage2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8UI, 4, 4);

[Zhenyao Mo, 2017/03/01 23:52:33]

tex storage isn't part of core 4.1 or lower.  I think it's much safer to call
TexImage2D.  Again, not new to this CL, but since you are touching this code and
I am reading it... (or if we made sure only on 4.2 or above, or check
tex_storage extension, then we are also good)

[dshwang, 2017/03/02 20:27:03]

Good point! The implementation uses glTexStorage2DEXT() repeatedly. I missed
"GL_ARB_texture_storage" check when landing the initial commit, because this
extensions is widely supported. New patch set has this check in feature_info.cc

 
-    // Bind to the FBO to test support
-    glFramebufferTexture2DEXT(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
-                              GL_TEXTURE_2D, rgba8ui_texture, 0);
-    glFramebufferTexture2DEXT(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT,
-                              GL_TEXTURE_2D, depth_texture, 0);
-    GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER);
+      glGenTextures(1, &depth_texture);
+      glBindTexture(GL_TEXTURE_2D, depth_texture);
+      glTexStorage2DEXT(GL_TEXTURE_2D, 1, GL_DEPTH_COMPONENT16, 4, 4);
 
-    supports_usampler_ = (status == GL_FRAMEBUFFER_COMPLETE);
+      // Create the FBO
+      GLuint rgba8ui_framebuffer = 0;
+      glGenFramebuffersEXT(1, &rgba8ui_framebuffer);
+      glBindFramebufferEXT(GL_FRAMEBUFFER, rgba8ui_framebuffer);
 
-    glDeleteFramebuffersEXT(1, &rgba8ui_framebuffer);
-    glDeleteTextures(1, &rgba8ui_texture);
-    glDeleteTextures(1, &depth_texture);
-  }
+      // Bind to the FBO to test support
+      glFramebufferTexture2DEXT(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                                GL_TEXTURE_2D, rgba8ui_texture, 0);
+      glFramebufferTexture2DEXT(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT,
+                                GL_TEXTURE_2D, depth_texture, 0);
+      GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER);
 
-  // Check to see if R8 images are supported
-  // If not supported, images are bound as R32F for write targets, not R8.
-  {
-    GLuint r8_texture = 0;
-    glGenTextures(1, &r8_texture);
-    glBindTexture(GL_TEXTURE_2D, r8_texture);
-    glTexStorage2DEXT(GL_TEXTURE_2D, 1, GL_R8, 4, 4);
+      supports_usampler_ = (status == GL_FRAMEBUFFER_COMPLETE);
 
-    glGetError();  // reset all previous errors
-    glBindImageTextureEXT(0, r8_texture, 0, GL_FALSE, 0, GL_WRITE_ONLY, GL_R8);
-    if (glGetError() != GL_NO_ERROR)
-      supports_r8_image_ = false;
-
-    glDeleteTextures(1, &r8_texture);
-  }
-
-  // Check if R8 GLSL read formats are supported.
-  // If not supported, r32f is used instead.
-  {
-    const char shader_source[] =
-        SHADER(layout(r8) restrict writeonly uniform highp image2D g_r8Image;
-               void main() {
-                 imageStore(g_r8Image, ivec2(0, 0), vec4(1.0, 0.0, 0.0, 0.0));
-               });
-
-    GLuint shader = CreateShader(GL_FRAGMENT_SHADER, "", shader_source);
-    supports_r8_read_format_ = (shader != 0);
-    if (shader != 0) {
-      glDeleteShader(shader);
+      glDeleteFramebuffersEXT(1, &rgba8ui_framebuffer);
+      glDeleteTextures(1, &rgba8ui_texture);
+      glDeleteTextures(1, &depth_texture);
     }
   }
 
   VLOG(1) << "ApplyFramebufferAttachmentCMAAINTEL: "
           << "Supports USampler is " << (supports_usampler_ ? "true" : "false");
   VLOG(1) << "ApplyFramebufferAttachmentCMAAINTEL: "
           << "Supports R8 Images is "
           << (supports_r8_image_ ? "true" : "false");
-  VLOG(1) << "ApplyFramebufferAttachmentCMAAINTEL: "
-          << "Supports R8 Read Format is "
-          << (supports_r8_read_format_ ? "true" : "false");
 
   // Create the shaders
   std::ostringstream defines, edge1, edge2, combineEdges, blur, displayEdges,
       cmaa_frag;
 
   cmaa_frag << cmaa_frag_s1_ << cmaa_frag_s2_;
   std::string cmaa_frag_string = cmaa_frag.str();
   const char* cmaa_frag_c_str = cmaa_frag_string.c_str();
 
   if (supports_usampler_) {
     defines << "#define SUPPORTS_USAMPLER2D\n";
   }
 
   if (is_in_gamma_correct_mode_) {
     defines << "#define IN_GAMMA_CORRECT_MODE\n";
   }
 
-  if (supports_r8_read_format_) {
+  if (supports_r8_image_) {
     defines << "#define EDGE_READ_FORMAT r8\n";
   } else {
     defines << "#define EDGE_READ_FORMAT r32f\n";
   }
 
   displayEdges << defines.str() << "#define DISPLAY_EDGES\n";
   debug_display_edges_shader_ =
       CreateProgram(displayEdges.str().c_str(), vert_str_, cmaa_frag_c_str);
 
   edge1 << defines.str() << "#define DETECT_EDGES1\n";
@@ skipping 428 matching lines @@
 }
 
 GLuint ApplyFramebufferAttachmentCMAAINTELResourceManager::CreateShader(
     GLenum type,
     const char* defines,
     const char* source) {
   GLuint shader = glCreateShader(type);
 
   const char header_es31[] =
       "#version 310 es                                                      \n";
   const char header_gl30[] =

[Zhenyao Mo, 2017/03/01 23:52:33]

shouldn't this name be gl130 instead?

[dshwang, 2017/03/02 20:27:03]

Done.

       "#version 130                                                         \n"
       "#extension GL_ARB_shading_language_420pack  : require                \n"
       "#extension GL_ARB_texture_gather            : require                \n"
       "#extension GL_ARB_explicit_uniform_location : require                \n"
       "#extension GL_ARB_explicit_attrib_location  : require                \n"
       "#extension GL_ARB_shader_image_load_store   : require                \n";
 
-  const char* header = NULL;
+  std::ostringstream header;
   if (is_gles31_compatible_) {
-    header = header_es31;
+    header << header_es31;
+    if (supports_r8_image_)
+      header << "#extension GL_NV_image_formats : require\n";
   } else {
-    header = header_gl30;
+    header << header_gl30;
   }
 
-  const char* source_array[4] = {header, defines, "\n", source};
+  const char* source_array[4] = {header.str().c_str(), defines, "\n", source};
   glShaderSource(shader, 4, source_array, NULL);
 
   glCompileShader(shader);
 
   GLint compile_result;
   glGetShaderiv(shader, GL_COMPILE_STATUS, &compile_result);
   if (compile_result == 0) {
 #if DCHECK_IS_ON()
     GLint info_log_length;
     glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &info_log_length);
@@ skipping 1224 matching lines @@
     \n#endif\n
     \n#if defined DISPLAY_EDGES\n
       DisplayEdges();
     \n#endif\n
     }
   );
 /* clang-format on */
 
 }  // namespace gles2
 }  // namespace gpu