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

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 633 matching lines @@
   // 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.
   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");
   bool has_texture_storage =
       !workarounds_.disable_texture_storage &&
       (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 =
       gl_version_info_->is_es3 && !has_ext_texture_storage && !has_apple_bgra;
   if (texture_storage_incompatible_with_bgra &&
       enable_texture_format_bgra8888 && enable_texture_storage) {
     switch (context_type_) {
       case CONTEXT_TYPE_OPENGLES2:
       case CONTEXT_TYPE_WEBGL1:
         enable_texture_storage = false;
         break;
       case CONTEXT_TYPE_OPENGLES3:
       case CONTEXT_TYPE_WEBGL2:
         enable_texture_format_bgra8888 = false;
         break;
     }
   }
 
   if (enable_texture_format_bgra8888) {
     feature_flags_.ext_texture_format_bgra8888 = true;
     AddExtensionString("GL_EXT_texture_format_BGRA8888");
     validators_.texture_internal_format.AddValue(GL_BGRA_EXT);
     validators_.texture_format.AddValue(GL_BGRA_EXT);
     validators_.texture_unsized_internal_format.AddValue(GL_BGRA_EXT);
+
+    // GL_APPLE_texture_format_BGRA8888 explicitly declare GL_BGRA8_EXT is
+    // added to internal format on ES3 context, but
+    // GL_EXT_texture_format_BGRA8888 doesn't.
+    if (gl_version_info_->is_es3 && has_apple_bgra) {
+      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
   // BGRA internal formats are converted to RGBA in the API implementation).
   // For ES, there is no extension that exposes BGRA renderbuffers, however
   // Angle does support these.
   bool enable_render_buffer_bgra =
       gl_version_info_->is_angle || !gl_version_info_->is_es;
 
   if (enable_render_buffer_bgra) {
@@ skipping 399 matching lines @@
   if (extensions.Contains("GL_ANGLE_texture_usage")) {
     feature_flags_.angle_texture_usage = true;
     AddExtensionString("GL_ANGLE_texture_usage");
     validators_.texture_parameter.AddValue(GL_TEXTURE_USAGE_ANGLE);
   }
 
   if (enable_texture_storage) {
     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);
+      // GL_APPLE_texture_format_BGRA8888 can add it above.
+      if (!validators_.texture_sized_color_renderable_internal_format.IsValid(

[Zhenyao Mo, 2016/10/10 20:30:29]

I am not sure I follow the logic of this block.  texture_storage should have
nothing to do with these validators.

[dshwang, 2016/10/10 20:57:10]

Sorry for insufficient explanation.
|texture_sized_xxx_internal_format| contains list of sized formats.
GL_BGRA8 should be in the list, if both |enable_texture_storage| and
|enable_texture_format_bgra8888| are true, because gpu client can call
glTexStorage2D(GL_TEXTURE_2D, 1, GL_BGRA8, 256, 256). CC actually calls it on
linux.

[Zhenyao Mo, 2016/10/10 21:00:27]

But this also enables TexImage calls to use these sized internalformats, which
could be illegal unless it's APPLE_texture_format_BGRA8888 on ES3.

[dshwang, 2016/10/10 21:18:29]

Oh, I didn't know.

Even if EXT_texture_format_BGRA8888 and EXT_texture_storage are supported,
TexImage(GL_BGRA8) is illegal?

EXT_texture_storage spec say
"""
If EXT_texture_format_BGRA8888 or APPLE_texture_format_BGRA8888 is supported:
BGRA8_EXT
"""
https://www.khronos.org/registry/gles/extensions/EXT/EXT_texture_storage.txt

If it's illegal, Texture::CanRenderTo() should be implemented differently for
storage texture and regular texture.
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag...

If it's illegal, I'll change Texture::CanRenderTo() implementation.

+              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);
         validators_.texture_internal_format_storage.AddValue(GL_ALPHA32F_EXT);
         validators_.texture_internal_format_storage.AddValue(
             GL_LUMINANCE32F_EXT);
         validators_.texture_internal_format_storage.AddValue(
             GL_LUMINANCE_ALPHA32F_EXT);
     }
     if (enable_texture_half_float) {
@@ skipping 418 matching lines @@
   if (pos == std::string::npos) {
     extensions_ += (extensions_.empty() ? "" : " ") + str;
   }
 }
 
 FeatureInfo::~FeatureInfo() {
 }
 
 }  // namespace gles2
 }  // namespace gpu