Implement WEBGL_compressed_texture_s3tc_srgb

gpu/command_buffer/service/feature_info.cc

 // Copyright (c) 2012 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/feature_info.h"
 
 #include <stddef.h>
 
 #include <set>
 #include <vector>
@@ skipping 573 matching lines @@
        (extensions.Contains("GL_EXT_framebuffer_sRGB") ||
         extensions.Contains("GL_ARB_framebuffer_sRGB")))) {
     feature_flags_.desktop_srgb_support = true;
   }
   // With EXT_sRGB, unsized SRGB_EXT and SRGB_ALPHA_EXT are accepted by the
   // <format> and <internalformat> parameter of TexImage2D. GLES3 adds support
   // for SRGB Textures but the accepted internal formats for TexImage2D are only
   // sized formats GL_SRGB8 and GL_SRGB8_ALPHA8. Also, SRGB_EXT isn't a valid
   // <format> in this case. So, even with GLES3 explicitly check for
   // GL_EXT_sRGB.
+  bool have_srgb = false;
   if ((((gl_version_info_->is_es3 ||
          extensions.Contains("GL_OES_rgb8_rgba8")) &&
         extensions.Contains("GL_EXT_sRGB")) ||
        feature_flags_.desktop_srgb_support) &&
        IsWebGL1OrES2Context()) {
+    have_srgb = true;
     AddExtensionString("GL_EXT_sRGB");
     validators_.texture_internal_format.AddValue(GL_SRGB_EXT);
     validators_.texture_internal_format.AddValue(GL_SRGB_ALPHA_EXT);
     validators_.texture_format.AddValue(GL_SRGB_EXT);
     validators_.texture_format.AddValue(GL_SRGB_ALPHA_EXT);
     validators_.render_buffer_format.AddValue(GL_SRGB8_ALPHA8_EXT);
     validators_.framebuffer_parameter.AddValue(
         GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING_EXT);
     validators_.texture_unsized_internal_format.AddValue(GL_SRGB_EXT);
     validators_.texture_unsized_internal_format.AddValue(GL_SRGB_ALPHA_EXT);
   }
 
+  if ((have_s3tc || (enable_dxt1 && enable_dxt3 && enable_dxt5)) && have_srgb) {

[jbauman, 2016/09/15 00:14:25]

Will this actually guarantee that these SRGB formats are supported? 

It looks to me like this works with GL_EXT_texture_compression_s3tc and
GL_EXT_texture_sRGB, but is it defined with other combinations of extensions?

[Kai Ninomiya, 2016/09/15 20:45:35]

You're right. It should be enabled if the desktop GL context has both
EXT_texture_compression_s3tc and EXT_texture_sRGB.

But I'm not sure whether (have_s3tc && have_srgb) would be correct either, since
these extension checks are at the ES level. In ES, EXT_sRGB plus
EXT_texture_compression_s3tc does not provide s3tc-srgb. Do you know?

+    AddExtensionString("GL_NV_sRGB_formats");
+
+    validators_.compressed_texture_format.AddValue(
+        GL_COMPRESSED_SRGB_S3TC_DXT1_EXT);
+    validators_.compressed_texture_format.AddValue(
+        GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT);
+    validators_.compressed_texture_format.AddValue(
+        GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT);
+    validators_.compressed_texture_format.AddValue(
+        GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT);
+
+    validators_.texture_internal_format_storage.AddValue(
+        GL_COMPRESSED_SRGB_S3TC_DXT1_EXT);
+    validators_.texture_internal_format_storage.AddValue(
+        GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT);
+    validators_.texture_internal_format_storage.AddValue(
+        GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT);
+    validators_.texture_internal_format_storage.AddValue(
+        GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT);

[jbauman, 2016/09/15 00:14:25]

I guess we're not going to bother with GL_SLUMINANCE_NV? If so, we should at
least put a note in that we're not supporting the complete extension.

[Ken Russell (switch to Gerrit), 2016/09/15 00:50:35]

Agreed.

Should we consider exposing the extension string
EXT_texture_compression_s3tc_srgb instead, even though it's still a draft OpenGL
ES extension? Or add a TODO before the AddExtensionString call above indicating
that it should be changed once that extension is published?

[Kai Ninomiya, 2016/09/15 20:45:35]

I have tried replacing GL_NV_sRGB_formats with
GL_EXT_texture_compression_s3tc_srgb in both places, but that disables the
feature in WebGL. I'm guessing this is being queried somewhere else?

[Kai Ninomiya, 2016/09/15 21:54:08]

Never mind, it actually works fine.

We decided offline to use the currently-draft extension
GL_EXT_texture_compression_s3tc_srgb since it's what we're actually doing.

+
+    //validators_.texture_internal_format.AddValue(_);
+    //validators_.texture_format.AddValue(_);
+    //validators_.texture_unsized_internal_format.AddValue(_);

[jbauman, 2016/09/15 00:14:25]

Unneeded?

[Kai Ninomiya, 2016/09/15 20:45:35]

Done.

+  }
+
   // Note: Only APPLE_texture_format_BGRA8888 extension allows BGRA8_EXT in
   // ES3's glTexStorage2D, whereas EXT_texture_format_BGRA8888 doesn't provide
   // that compatibility. So if EXT_texture_format_BGRA8888 (but not
   // APPLE_texture_format_BGRA8888) is present on an underlying ES3 context, we
   // have to choose which one of BGRA vs texture storage we expose.
   // When creating ES2 contexts, we prefer support BGRA to texture storage, so
   // we disable texture storage if only EXT_texture_format_BGRA8888 is present.
   // If neither is present, we expose texture storage.
   // When creating ES3 contexts, we do need to expose texture storage, so we
   // disable BGRA if we have to.
@@ skipping 876 matching lines @@
   if (pos == std::string::npos) {
     extensions_ += (extensions_.empty() ? "" : " ") + str;
   }
 }
 
 FeatureInfo::~FeatureInfo() {
 }
 
 }  // namespace gles2
 }  // namespace gpu