Implement support for rendering to 32-bit float textures on ES3

ui/gl/gl_gl_api_implementation.cc

Index: ui/gl/gl_gl_api_implementation.cc
diff --git a/ui/gl/gl_gl_api_implementation.cc b/ui/gl/gl_gl_api_implementation.cc
index ce8a711cdff0d0ba5ec55189599e4b2036604edb..f795862563cd250429cad3fa19ec7dda7ae023ba 100644
--- a/ui/gl/gl_gl_api_implementation.cc
+++ b/ui/gl/gl_gl_api_implementation.cc
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <vector>
 
+#include "base/at_exit.h"
 #include "base/command_line.h"
 #include "base/strings/string_util.h"
 #include "ui/gl/gl_context.h"
@@ -14,6 +15,7 @@
 #include "ui/gl/gl_state_restorer.h"
 #include "ui/gl/gl_surface.h"
 #include "ui/gl/gl_switches.h"
+#include "ui/gl/gl_version_info.h"
 
 namespace gfx {
 
@@ -23,6 +25,8 @@ static GLApi* g_gl;
 static RealGLApi* g_real_gl;
 // A GL Api that calls TRACE and then calls another GL api.
 static TraceGLApi* g_trace_gl;
+// GL version used when initializing dynamic bindings.
+static GLVersionInfo* g_version_info = NULL;
 
 namespace {
 
@@ -40,8 +44,31 @@ static inline GLenum GetTexInternalFormat(GLenum internal_format,
                                           GLenum type) {
   GLenum gl_internal_format = GetInternalFormat(internal_format);
 
-  if (gfx::GetGLImplementation() == gfx::kGLImplementationEGLGLES2)
+  if (gfx::g_version_info && gfx::g_version_info->is_es) {
+    if (gfx::g_version_info->is_es3) {

[piman, 2014/01/21 22:37:12]

I think we want to limit the amount of magic that happens at this level.

We expose GLES2 to the clients. This function is just here to share the code
when using desktop GL vs GLES on the service side (since they have different
requirements wrt internal_format), but I don't think we want to go beyond that.

I think what we want is expose extensions to the client so that it can choose to
allocate textures with (say) a RGBA32F internal_format if the extension is
present, and pass it through here (modulo differences between GLES vs GL).
GLES2Decoder would do the validation.

Otherwise, the same code (that'd do render-to-float-texture) might work on one
implementation and not another one, without being able to tell them apart.

[oetuaho-nv, 2014/01/22 09:46:54]

Traditionally in OpenGL, the mechanism for the client to determine whether a
format is renderable is to check for framebuffer completeness. And whether a
given framebuffer is complete is subject to implementation-dependent set of
restrictions, see GLES2 spec section 4.4.5. I don't think that we should change
this to an extension mechanism in WebGL. It seems to go against the design
philosophy that developers can transfer knowledge of OpenGL to WebGL with as few
difficulties as possible, and the way the spec is worded, we might need a lot of
extensions for different formats, not just for float textures.

Also, having to use different enums based on extension support sounds like
needlessly exposing complexity to the client. I think we should treat mobile
APIs equally to desktop GL and shouldn't make supporting them an extra hassle
for content developers.

+      // Use sized internal formats from core ES3 to get support for rendering

[no sievers, 2014/01/21 22:21:13]

Why is the format being renderable relevant in this function?

[oetuaho-nv, 2014/01/22 09:46:54]

When creating textures, it's better to choose a renderable format in case the
client later decides to attach it to a framebuffer. I don't see any reasonable
alternatives, changing the format when the framebuffer attachment happens would
require costly copying. I can add a comment about this.

+      // to 32-bit float render targets using EXT_color_buffer_float. ES2
+      // extension is still relied on to provide the float LUMINANCE/ALPHA
+      // formats missing from ES3.
+      if (type == GL_FLOAT) {
+        switch(format) {
+          case GL_RGBA:
+            gl_internal_format = GL_RGBA32F;
+            break;
+          case GL_RGB:
+            gl_internal_format = GL_RGB32F;
+            break;
+          default:
+            break;
+        }
+      }
+      // GL_HALF_FLOAT_OES textures can be renderable with

[no sievers, 2014/01/21 22:21:13]

same here

+      // EXT_color_buffer_half_float, GL_HALF_FLOAT textures would require
+      // EXT_color_buffer_float and similar enum conversions as above.
+      DCHECK(type != GL_HALF_FLOAT);

[no sievers, 2014/01/21 22:21:13]

Does this compile? Isn't it GL_HALF_FLOAT_OES?

[no sievers, 2014/01/21 22:21:13]

Don't we need to allow this? Looking at feature_info.cc we seem to allow and
expose GL_OES_texture_half_float on GLES.

Wouldn't we just want to pass it through and it would choose its own internal
format?

Note that the validators would take care of failing TexImage2D() if the
extension was *not* supported.

[oetuaho-nv, 2014/01/22 09:46:54]

Both of these enums exist, and have different values. Right now the higher-level
code is exclusively using GL_HALF_FLOAT_OES for textures. This check is here to
ensure that the higher-level code doesn't switch to the other enum for whatever
reason without making the requisite changes here as well. I'll clarify the
comment.

+    }
     return gl_internal_format;
+  }
 
   if (type == GL_FLOAT) {
     switch (format) {
@@ -144,6 +171,13 @@ static void GL_BINDING_CALL CustomRenderbufferStorageMultisampleEXT(
       target, samples, gl_internal_format, width, height);
 }
 
+static void CleanupVersionInfo(void* unused) {
+  if (g_version_info) {
+    delete g_version_info;
+    g_version_info = NULL;
+  }
+}
+
 }  // anonymous namespace
 
 void DriverGL::InitializeCustomDynamicBindings(GLContext* context) {
@@ -193,6 +227,9 @@ void SetGLToRealGLApi() {
 
 void InitializeDynamicGLBindingsGL(GLContext* context) {
   g_driver_gl.InitializeCustomDynamicBindings(context);
+  DCHECK(context && context->IsCurrent(NULL) && !g_version_info);
+  g_version_info = new GLVersionInfo(context->GetGLVersion().c_str());
+  base::AtExitManager::RegisterCallback(CleanupVersionInfo, NULL);

[no sievers, 2014/01/21 22:21:13]

Hmm, are you doing this because we are actually not calling ClearGLBindingsGL()?
Maybe we should rather fix that then (separately). Seems like we'd already be
leaking other stuff.

[oetuaho-nv, 2014/01/22 09:46:54]

Ah, I should have realized from the code that putting this to ClearGLBindingsGL
should be sufficient. I'll remove this. I'm not aware of any bugs related to
this.

 }
 
 void InitializeDebugGLBindingsGL() {
@@ -218,6 +255,7 @@ void ClearGLBindingsGL() {
     delete g_current_gl_context_tls;
     g_current_gl_context_tls = NULL;
   }
+  CleanupVersionInfo(NULL);
 }
 
 GLApi::GLApi() {