gpu: Clarify sized texture format is available only if ES3 context or immutable texture is supported

gpu/command_buffer/service/feature_info.cc

Index: gpu/command_buffer/service/feature_info.cc
diff --git a/gpu/command_buffer/service/feature_info.cc b/gpu/command_buffer/service/feature_info.cc
index 06ce1cef4294f85a4642eeb8d3bc751c37b3e05e..61bbc964346f8455a1e08bf4447680e791ccc999 100644
--- a/gpu/command_buffer/service/feature_info.cc
+++ b/gpu/command_buffer/service/feature_info.cc
@@ -651,7 +651,8 @@ void FeatureInfo::InitializeFeatures() {
   // disable BGRA if we have to.
   bool has_apple_bgra = extensions.Contains("GL_APPLE_texture_format_BGRA8888");
   bool has_ext_bgra = extensions.Contains("GL_EXT_texture_format_BGRA8888");
-  bool has_bgra = has_ext_bgra || has_apple_bgra || !gl_version_info_->is_es;
+  bool enable_texture_format_bgra8888 =
+      has_ext_bgra || has_apple_bgra || !gl_version_info_->is_es;
 
   bool has_ext_texture_storage = extensions.Contains("GL_EXT_texture_storage");
   bool has_arb_texture_storage = extensions.Contains("GL_ARB_texture_storage");
@@ -660,7 +661,6 @@ void FeatureInfo::InitializeFeatures() {
       (has_ext_texture_storage || has_arb_texture_storage ||
        gl_version_info_->is_es3 || gl_version_info_->IsAtLeastGL(4, 2));
 
-  bool enable_texture_format_bgra8888 = has_bgra;
   bool enable_texture_storage = has_texture_storage;
 
   bool texture_storage_incompatible_with_bgra =
@@ -685,6 +685,14 @@ void FeatureInfo::InitializeFeatures() {
     validators_.texture_internal_format.AddValue(GL_BGRA_EXT);
     validators_.texture_format.AddValue(GL_BGRA_EXT);
     validators_.texture_unsized_internal_format.AddValue(GL_BGRA_EXT);
+
+    if (IsES3Capable()) {

[Zhenyao Mo, 2016/10/13 21:23:38]

This should be IsES3Enabled().  Otherwise the command buffer is still ES2 and
sized internalformat isn't allowed.

[dshwang, 2016/10/14 12:03:34]

Here is inside FeatureInfo::InitializeFeatures()
FeatureInfo::EnableES3Validators() sets IsES3Enabled() to true, which is later
procedure.

Let me move this code to FeatureInfo::EnableES3Validators()

+      validators_.texture_internal_format.AddValue(GL_BGRA8_EXT);
+      validators_.texture_sized_color_renderable_internal_format.AddValue(
+          GL_BGRA8_EXT);
+      validators_.texture_sized_texture_filterable_internal_format.AddValue(
+          GL_BGRA8_EXT);
+    }
   }
 
   // On desktop, all devices support BGRA render buffers (note that on desktop
@@ -1104,8 +1112,20 @@ void FeatureInfo::InitializeFeatures() {
     feature_flags_.ext_texture_storage = true;
     AddExtensionString("GL_EXT_texture_storage");
     validators_.texture_parameter.AddValue(GL_TEXTURE_IMMUTABLE_FORMAT_EXT);
-    if (enable_texture_format_bgra8888)
-        validators_.texture_internal_format_storage.AddValue(GL_BGRA8_EXT);
+    if (enable_texture_format_bgra8888) {
+      validators_.texture_internal_format_storage.AddValue(GL_BGRA8_EXT);
+      // It can be added above if the context is ES3 capable.
+      if (!validators_.texture_sized_color_renderable_internal_format.IsValid(

[Zhenyao Mo, 2016/10/13 21:23:38]

You don't need this at all.  AddValue() will do this check anyway, so it's
duplicated efforts.

[dshwang, 2016/10/14 12:03:34]

Done.

+              GL_BGRA8_EXT)) {
+        validators_.texture_sized_color_renderable_internal_format.AddValue(
+            GL_BGRA8_EXT);
+      }
+      if (!validators_.texture_sized_texture_filterable_internal_format.IsValid(
+              GL_BGRA8_EXT)) {
+        validators_.texture_sized_texture_filterable_internal_format.AddValue(
+            GL_BGRA8_EXT);
+      }
+    }
     if (enable_texture_float) {
         validators_.texture_internal_format_storage.AddValue(GL_RGBA32F_EXT);
         validators_.texture_internal_format_storage.AddValue(GL_RGB32F_EXT);