Refactor context creation parameters into a struct.

This allows for easily adding new parameters to GL context creation, added parameters for creating contexts with EGL_CHROMIUM_create_context_bind_generates_resource and EGL_ANGLE_create_context_webgl_compatibility. Small related fix: disallow create-on-bind for vertex arrays and transform feedbacks in the passthrough command buffer, these objects do not allow create-on-bind by the ES 3.0 spec but the semantics were mistakenly added. BUG=angleproject:1518 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2480373002 Cr-Commit-Position: refs/heads/master@{#431412}
cbchan · Nov 11, 2016 · df7fff2 · df7fff2
1 parent 7e80889
commit df7fff2
Show file tree

Hide file tree

Showing 51 changed files with 291 additions and 151 deletions.
diff --git a/android_webview/browser/scoped_app_gl_state_restore.cc b/android_webview/browser/scoped_app_gl_state_restore.cc
@@ -25,7 +25,7 @@ class AppContextSurface {
       : surface(new gl::GLSurfaceStub),
         context(gl::init::CreateGLContext(nullptr,
                                           surface.get(),
-                                          gl::PreferDiscreteGpu)) {}
+                                          gl::GLContextAttribs())) {}
   void MakeCurrent() { context->MakeCurrent(surface.get()); }
 
  private:

diff --git a/android_webview/browser/test/fake_window.cc b/android_webview/browser/test/fake_window.cc
@@ -189,8 +189,8 @@ void FakeWindow::InitializeOnRT(base::WaitableEvent* sync) {
   surface_ = gl::init::CreateOffscreenGLSurface(surface_size_);
   DCHECK(surface_);
   DCHECK(surface_->GetHandle());
-  context_ =
-      gl::init::CreateGLContext(nullptr, surface_.get(), gl::PreferDiscreteGpu);
+  context_ = gl::init::CreateGLContext(nullptr, surface_.get(),
+                                       gl::GLContextAttribs());
   DCHECK(context_);
   sync->Signal();
 }

diff --git a/gpu/command_buffer/common/gles2_cmd_utils.cc b/gpu/command_buffer/common/gles2_cmd_utils.cc
@@ -1843,6 +1843,21 @@ const int32_t kContextType = 0x10004;
 
 }  // namespace
 
+bool IsWebGLContextType(ContextType context_type) {
+  // Switch statement to cause a compile-time error if we miss a case.
+  switch (context_type) {
+    case CONTEXT_TYPE_WEBGL1:
+    case CONTEXT_TYPE_WEBGL2:
+      return true;
+    case CONTEXT_TYPE_OPENGLES2:
+    case CONTEXT_TYPE_OPENGLES3:
+      return false;
+  }
+
+  NOTREACHED();
+  return false;
+}
+
 ContextCreationAttribHelper::ContextCreationAttribHelper()
     : gpu_preference(gl::PreferIntegratedGpu),
       alpha_size(-1),

diff --git a/gpu/command_buffer/common/gles2_cmd_utils.h b/gpu/command_buffer/common/gles2_cmd_utils.h
@@ -302,6 +302,7 @@ enum ContextType {
   CONTEXT_TYPE_OPENGLES3,
   CONTEXT_TYPE_LAST = CONTEXT_TYPE_OPENGLES3
 };
+GLES2_UTILS_EXPORT bool IsWebGLContextType(ContextType context_type);
 
 struct GLES2_UTILS_EXPORT ContextCreationAttribHelper {
   ContextCreationAttribHelper();

diff --git a/gpu/command_buffer/service/BUILD.gn b/gpu/command_buffer/service/BUILD.gn
@@ -123,6 +123,8 @@ target(link_target_type, "service_sources") {
     "renderbuffer_manager.h",
     "sampler_manager.cc",
     "sampler_manager.h",
+    "service_utils.cc",
+    "service_utils.h",
     "shader_manager.cc",
     "shader_manager.h",
     "shader_translator.cc",

diff --git a/gpu/command_buffer/service/feature_info.cc b/gpu/command_buffer/service/feature_info.cc
@@ -1405,6 +1405,11 @@ void FeatureInfo::InitializeFeatures() {
   feature_flags_.khr_debug = gl_version_info_->IsAtLeastGL(4, 3) ||
                              gl_version_info_->IsAtLeastGLES(3, 2) ||
                              extensions.Contains("GL_KHR_debug");
+
+  feature_flags_.chromium_bind_generates_resource =
+      extensions.Contains("GL_CHROMIUM_bind_generates_resource");
+  feature_flags_.angle_webgl_compatibility =
+      extensions.Contains("GL_ANGLE_webgl_compatibility");
 }
 
 bool FeatureInfo::IsES3Capable() const {
@@ -1489,18 +1494,7 @@ void FeatureInfo::EnableES3Validators() {
 }
 
 bool FeatureInfo::IsWebGLContext() const {
-  // Switch statement to cause a compile-time error if we miss a case.
-  switch (context_type_) {
-    case CONTEXT_TYPE_WEBGL1:
-    case CONTEXT_TYPE_WEBGL2:
-      return true;
-    case CONTEXT_TYPE_OPENGLES2:
-    case CONTEXT_TYPE_OPENGLES3:
-      return false;
-  }
-
-  NOTREACHED();
-  return false;
+  return IsWebGLContextType(context_type_);
 }
 
 bool FeatureInfo::IsWebGL1OrES2Context() const {

diff --git a/gpu/command_buffer/service/feature_info.h b/gpu/command_buffer/service/feature_info.h
@@ -96,6 +96,8 @@ class GPU_EXPORT FeatureInfo : public base::RefCounted<FeatureInfo> {
     bool chromium_color_buffer_float_rgba = false;
     bool angle_robust_client_memory = false;
     bool khr_debug = false;
+    bool chromium_bind_generates_resource = false;
+    bool angle_webgl_compatibility = false;
   };
 
   FeatureInfo();

diff --git a/gpu/command_buffer/service/gl_context_virtual.cc b/gpu/command_buffer/service/gl_context_virtual.cc
@@ -23,7 +23,7 @@ GLContextVirtual::GLContextVirtual(gl::GLShareGroup* share_group,
       decoder_(decoder) {}
 
 bool GLContextVirtual::Initialize(gl::GLSurface* compatible_surface,
-                                  gl::GpuPreference gpu_preference) {
+                                  const gl::GLContextAttribs& attribs) {
   SetGLStateRestorer(new GLStateRestorerImpl(decoder_));
   return shared_context_->MakeVirtuallyCurrent(this, compatible_surface);
 }

diff --git a/gpu/command_buffer/service/gl_context_virtual.h b/gpu/command_buffer/service/gl_context_virtual.h
@@ -34,7 +34,7 @@ class GPU_EXPORT GLContextVirtual : public gl::GLContext {
 
   // Implement GLContext.
   bool Initialize(gl::GLSurface* compatible_surface,
-                  gl::GpuPreference gpu_preference) override;
+                  const gl::GLContextAttribs& attribs) override;
   bool MakeCurrent(gl::GLSurface* surface) override;
   void ReleaseCurrent(gl::GLSurface* surface) override;
   bool IsCurrent(gl::GLSurface* surface) override;

diff --git a/gpu/command_buffer/service/gl_context_virtual_unittest.cc b/gpu/command_buffer/service/gl_context_virtual_unittest.cc
@@ -44,7 +44,7 @@ TEST_F(GLContextVirtualTest, Reinitialize) {
     share_group->SetSharedContext(GetGLSurface(), base_context.get());
     scoped_refptr<GLContextVirtual> context(new GLContextVirtual(
         share_group, base_context.get(), decoder_->AsWeakPtr()));
-    EXPECT_TRUE(context->Initialize(GetGLSurface(), gl::PreferIntegratedGpu));
+    EXPECT_TRUE(context->Initialize(GetGLSurface(), gl::GLContextAttribs()));
     EXPECT_TRUE(context->MakeCurrent(GetGLSurface()));
   }
   {
@@ -53,7 +53,7 @@ TEST_F(GLContextVirtualTest, Reinitialize) {
     share_group->SetSharedContext(GetGLSurface(), base_context.get());
     scoped_refptr<GLContextVirtual> context(new GLContextVirtual(
         share_group, base_context.get(), decoder_->AsWeakPtr()));
-    EXPECT_TRUE(context->Initialize(GetGLSurface(), gl::PreferIntegratedGpu));
+    EXPECT_TRUE(context->Initialize(GetGLSurface(), gl::GLContextAttribs()));
     EXPECT_TRUE(context->MakeCurrent(GetGLSurface()));
   }
 }

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc b/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc
@@ -158,7 +158,10 @@ bool GLES2DecoderPassthroughImpl::Initialize(
   }
 
   // Check for required extensions
-  if (!feature_info_->feature_flags().angle_robust_client_memory) {
+  if (!feature_info_->feature_flags().angle_robust_client_memory ||
+      !feature_info_->feature_flags().chromium_bind_generates_resource ||
+      (feature_info_->IsWebGLContext() !=
+       feature_info_->feature_flags().angle_webgl_compatibility)) {
     Destroy(true);
     return false;
   }
@@ -273,7 +276,11 @@ gpu::Capabilities GLES2DecoderPassthroughImpl::GetCapabilities() {
 
   PopulateNumericCapabilities(&caps, feature_info_.get());
 
-  caps.bind_generates_resource_chromium = group_->bind_generates_resource();
+  glGetIntegerv(GL_BIND_GENERATES_RESOURCE_CHROMIUM,
+                &caps.bind_generates_resource_chromium);
+  DCHECK_EQ(caps.bind_generates_resource_chromium != GL_FALSE,
+            group_->bind_generates_resource());
+
   caps.egl_image_external =
       feature_info_->feature_flags().oes_egl_image_external;
   caps.texture_format_astc =

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_passthrough_doers.cc b/gpu/command_buffer/service/gles2_cmd_decoder_passthrough_doers.cc
@@ -136,23 +136,13 @@ GLuint GetFramebufferServiceID(GLuint client_id,
 }
 
 GLuint GetTransformFeedbackServiceID(GLuint client_id,
-                                     ClientServiceMap<GLuint, GLuint>* id_map,
-                                     bool create_if_missing) {
-  return GetServiceID(client_id, id_map, create_if_missing, []() {
-    GLuint service_id = 0;
-    glGenTransformFeedbacks(1, &service_id);
-    return service_id;
-  });
+                                     ClientServiceMap<GLuint, GLuint>* id_map) {
+  return id_map->GetServiceIDOrInvalid(client_id);
 }
 
 GLuint GetVertexArrayServiceID(GLuint client_id,
-                               ClientServiceMap<GLuint, GLuint>* id_map,
-                               bool create_if_missing) {
-  return GetServiceID(client_id, id_map, create_if_missing, []() {
-    GLuint service_id = 0;
-    glGenVertexArraysOES(1, &service_id);
-    return service_id;
-  });
+                               ClientServiceMap<GLuint, GLuint>* id_map) {
+  return id_map->GetServiceIDOrInvalid(client_id);
 }
 
 GLuint GetProgramServiceID(GLuint client_id, PassthroughResources* resources) {
@@ -290,8 +280,7 @@ error::Error GLES2DecoderPassthroughImpl::DoBindTransformFeedback(
     GLuint transformfeedback) {
   glBindTransformFeedback(
       target, GetTransformFeedbackServiceID(transformfeedback,
-                                            &transform_feedback_id_map_,
-                                            bind_generates_resource_));
+                                            &transform_feedback_id_map_));
   return error::kNoError;
 }
 
@@ -723,20 +712,9 @@ error::Error GLES2DecoderPassthroughImpl::DoFramebufferRenderbuffer(
     GLenum attachment,
     GLenum renderbuffertarget,
     GLuint renderbuffer) {
-  // TODO(geofflang): Handle this case in ANGLE by adding a WebGL validation
-  // mode.
-  if (attachment == GL_DEPTH_STENCIL_ATTACHMENT) {
-    glFramebufferRenderbufferEXT(
-        target, GL_DEPTH_ATTACHMENT, renderbuffertarget,
-        GetRenderbufferServiceID(renderbuffer, resources_, false));
-    glFramebufferRenderbufferEXT(
-        target, GL_STENCIL_ATTACHMENT, renderbuffertarget,
-        GetRenderbufferServiceID(renderbuffer, resources_, false));
-  } else {
-    glFramebufferRenderbufferEXT(
-        target, attachment, renderbuffertarget,
-        GetRenderbufferServiceID(renderbuffer, resources_, false));
-  }
+  glFramebufferRenderbufferEXT(
+      target, attachment, renderbuffertarget,
+      GetRenderbufferServiceID(renderbuffer, resources_, false));
   return error::kNoError;
 }
 
@@ -1344,7 +1322,7 @@ error::Error GLES2DecoderPassthroughImpl::DoIsTransformFeedback(
     GLuint transformfeedback,
     uint32_t* result) {
   *result = glIsTransformFeedback(GetTransformFeedbackServiceID(
-      transformfeedback, &transform_feedback_id_map_, false));
+      transformfeedback, &transform_feedback_id_map_));
   return error::kNoError;
 }
 
@@ -2209,14 +2187,13 @@ error::Error GLES2DecoderPassthroughImpl::DoDeleteVertexArraysOES(
 
 error::Error GLES2DecoderPassthroughImpl::DoIsVertexArrayOES(GLuint array,
                                                              uint32_t* result) {
-  *result = glIsVertexArrayOES(
-      GetVertexArrayServiceID(array, &vertex_array_id_map_, false));
+  *result =
+      glIsVertexArrayOES(GetVertexArrayServiceID(array, &vertex_array_id_map_));
   return error::kNoError;
 }
 
 error::Error GLES2DecoderPassthroughImpl::DoBindVertexArrayOES(GLuint array) {
-  glBindVertexArrayOES(GetVertexArrayServiceID(array, &vertex_array_id_map_,
-                                               bind_generates_resource_));
+  glBindVertexArrayOES(GetVertexArrayServiceID(array, &vertex_array_id_map_));
   return error::kNoError;
 }
 

diff --git a/gpu/command_buffer/service/in_process_command_buffer.cc b/gpu/command_buffer/service/in_process_command_buffer.cc
@@ -37,6 +37,7 @@
 #include "gpu/command_buffer/service/memory_program_cache.h"
 #include "gpu/command_buffer/service/memory_tracking.h"
 #include "gpu/command_buffer/service/query_manager.h"
+#include "gpu/command_buffer/service/service_utils.h"
 #include "gpu/command_buffer/service/sync_point_manager.h"
 #include "gpu/command_buffer/service/transfer_buffer_manager.h"
 #include "ui/gfx/geometry/size.h"
@@ -363,20 +364,28 @@ bool InProcessCommandBuffer::InitializeOnGpuThread(
     context_ = gl_share_group_->GetSharedContext(surface_.get());
     if (!context_.get()) {
       context_ = gl::init::CreateGLContext(
-          gl_share_group_.get(), surface_.get(), params.attribs.gpu_preference);
+          gl_share_group_.get(), surface_.get(),
+          GenerateGLContextAttribs(
+              params.attribs, decoder_->GetContextGroup()->gpu_preferences()));
       gl_share_group_->SetSharedContext(surface_.get(), context_.get());
     }
 
     context_ = new GLContextVirtual(
         gl_share_group_.get(), context_.get(), decoder_->AsWeakPtr());
-    if (context_->Initialize(surface_.get(), params.attribs.gpu_preference)) {
+    if (context_->Initialize(
+            surface_.get(),
+            GenerateGLContextAttribs(
+                params.attribs,
+                decoder_->GetContextGroup()->gpu_preferences()))) {
       VLOG(1) << "Created virtual GL context.";
     } else {
       context_ = NULL;
     }
   } else {
-    context_ = gl::init::CreateGLContext(gl_share_group_.get(), surface_.get(),
-                                         params.attribs.gpu_preference);
+    context_ = gl::init::CreateGLContext(
+        gl_share_group_.get(), surface_.get(),
+        GenerateGLContextAttribs(
+            params.attribs, decoder_->GetContextGroup()->gpu_preferences()));
   }
 
   if (!context_.get()) {

diff --git a/gpu/command_buffer/service/service_utils.cc b/gpu/command_buffer/service/service_utils.cc
@@ -0,0 +1,27 @@
+// Copyright (c) 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/service_utils.h"
+
+#include "gpu/command_buffer/common/gles2_cmd_utils.h"
+#include "gpu/command_buffer/service/gpu_preferences.h"
+
+namespace gpu {
+namespace gles2 {
+
+gl::GLContextAttribs GenerateGLContextAttribs(
+    const ContextCreationAttribHelper& attribs_helper,
+    const GpuPreferences& gpu_preferences) {
+  gl::GLContextAttribs attribs;
+  attribs.gpu_preference = attribs_helper.gpu_preference;
+  if (gpu_preferences.use_passthrough_cmd_decoder) {
+    attribs.bind_generates_resource = attribs_helper.bind_generates_resource;
+    attribs.webgl_compatibility_context =
+        IsWebGLContextType(attribs_helper.context_type);
+  }
+  return attribs;
+}
+
+}  // namespace gles2
+}  // namespace gpu
diff --git a/gpu/command_buffer/service/service_utils.h b/gpu/command_buffer/service/service_utils.h
@@ -0,0 +1,24 @@
+// Copyright (c) 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.
+
+#ifndef GPU_COMMAND_BUFFER_SERVICE_SERVICE_UTILS_H_
+#define GPU_COMMAND_BUFFER_SERVICE_SERVICE_UTILS_H_
+
+#include "gpu/gpu_export.h"
+#include "ui/gl/gl_context.h"
+
+namespace gpu {
+struct GpuPreferences;
+
+namespace gles2 {
+struct ContextCreationAttribHelper;
+
+GPU_EXPORT gl::GLContextAttribs GenerateGLContextAttribs(
+    const ContextCreationAttribHelper& attribs_helper,
+    const GpuPreferences& gpu_preferences);
+
+}  // namespace gles2
+}  // namespace gpu
+
+#endif  // GPU_COMMAND_BUFFER_SERVICE_SERVICE_UTILS_H_
diff --git a/gpu/command_buffer/tests/gl_manager.cc b/gpu/command_buffer/tests/gl_manager.cc
@@ -34,6 +34,7 @@
 #include "gpu/command_buffer/service/image_manager.h"
 #include "gpu/command_buffer/service/mailbox_manager_impl.h"
 #include "gpu/command_buffer/service/memory_tracking.h"
+#include "gpu/command_buffer/service/service_utils.h"
 #include "gpu/command_buffer/service/sync_point_manager.h"
 #include "gpu/command_buffer/service/transfer_buffer_manager.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -325,16 +326,20 @@ void GLManager::InitializeWithCommandLine(
   if (base_context_) {
     context_ = scoped_refptr<gl::GLContext>(new gpu::GLContextVirtual(
         share_group_.get(), base_context_->get(), decoder_->AsWeakPtr()));
-    ASSERT_TRUE(context_->Initialize(surface_.get(), attribs.gpu_preference));
+    ASSERT_TRUE(context_->Initialize(
+        surface_.get(),
+        GenerateGLContextAttribs(attribs, context_group->gpu_preferences())));
   } else {
     if (real_gl_context) {
       context_ = scoped_refptr<gl::GLContext>(new gpu::GLContextVirtual(
           share_group_.get(), real_gl_context, decoder_->AsWeakPtr()));
-      ASSERT_TRUE(
-          context_->Initialize(surface_.get(), attribs.gpu_preference));
+      ASSERT_TRUE(context_->Initialize(
+          surface_.get(),
+          GenerateGLContextAttribs(attribs, context_group->gpu_preferences())));
     } else {
-      context_ = gl::init::CreateGLContext(share_group_.get(), surface_.get(),
-                                           attribs.gpu_preference);
+      context_ = gl::init::CreateGLContext(
+          share_group_.get(), surface_.get(),
+          GenerateGLContextAttribs(attribs, context_group->gpu_preferences()));
     }
   }
   ASSERT_TRUE(context_.get() != NULL) << "could not create GL context";
@@ -398,9 +403,9 @@ void GLManager::SetupBaseContext() {
     gfx::Size size(4, 4);
     base_surface_ = new scoped_refptr<gl::GLSurface>(
         gl::init::CreateOffscreenGLSurface(size));
-    gl::GpuPreference gpu_preference(gl::PreferDiscreteGpu);
     base_context_ = new scoped_refptr<gl::GLContext>(gl::init::CreateGLContext(
-        base_share_group_->get(), base_surface_->get(), gpu_preference));
+        base_share_group_->get(), base_surface_->get(),
+        gl::GLContextAttribs()));
     #endif
   }
   ++use_count_;

diff --git a/gpu/config/gpu_info_collector.cc b/gpu/config/gpu_info_collector.cc
@@ -45,7 +45,7 @@ scoped_refptr<gl::GLSurface> InitializeGLSurface() {
 
 scoped_refptr<gl::GLContext> InitializeGLContext(gl::GLSurface* surface) {
   scoped_refptr<gl::GLContext> context(
-      gl::init::CreateGLContext(nullptr, surface, gl::PreferIntegratedGpu));
+      gl::init::CreateGLContext(nullptr, surface, gl::GLContextAttribs()));
   if (!context.get()) {
     LOG(ERROR) << "gl::init::CreateGLContext failed";
     return NULL;