[Remoting Android] Make EglThreadContext create OpenGL ES 3 context

remoting/client/jni/egl_thread_context.cc

 // Copyright 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 "remoting/client/jni/egl_thread_context.h"
 
 #include "base/logging.h"
 
 namespace remoting {
 
 EglThreadContext::EglThreadContext() {
   DCHECK(thread_checker_.CalledOnValidThread());
   CHECK(eglGetCurrentDisplay() == EGL_NO_DISPLAY);
   CHECK(eglGetCurrentContext() == EGL_NO_CONTEXT);
   display_ = eglGetDisplay(EGL_DEFAULT_DISPLAY);
   if (!display_ || !eglInitialize(display_, NULL, NULL)) {
-    LOG(FATAL) << "Failed to initialize EGL display.";
+    LOG(FATAL) << "Failed to initialize EGL display: " << eglGetError();
   }
 
-  const EGLint config_attribs[] = {
-    EGL_RED_SIZE, 8,
-    EGL_GREEN_SIZE, 8,
-    EGL_BLUE_SIZE, 8,
-    EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
-    EGL_NONE
-  };
-
-  EGLint numConfigs;
-  if (!eglChooseConfig(display_, config_attribs, &config_, 1, &numConfigs)) {
-    LOG(FATAL) << "Failed to choose config.";
+  if (CreateContextWithClientVersion(EGL_OPENGL_ES3_BIT, 3)) {
+    client_version_ = 3;
+    return;
   }
-  const EGLint context_attribs[] = {
-    EGL_CONTEXT_CLIENT_VERSION, 2,
-    EGL_NONE
-  };
-  context_ = eglCreateContext(display_, config_, EGL_NO_CONTEXT,
-                              context_attribs);
-  if (!context_) {
-    LOG(FATAL) << "Failed to create context.";
+  LOG(WARNING) << "Failed to create context for OpenGL ES 3. Falling back to "
+               << "OpenGL ES 2.";
+  if (CreateContextWithClientVersion(EGL_OPENGL_ES2_BIT, 2)) {
+    client_version_ = 2;
+  } else {

[Sergey Ulanov, 2016/07/22 20:12:34]

nit: for consistency return above and remove else.
Or alternatively you can format this code as follows:

if (init_opengl_3) {
 version = 3;
} else (init_opengl_2) {
 LOG(WARNING) << "using OGL2";
 version = 2;
} else {
 LOG(FATAL).
}

[Yuwei, 2016/07/22 21:22:39]

Done.

+    LOG(FATAL) << "Failed to create context: " << eglGetError();
   }
 }
 
 EglThreadContext::~EglThreadContext() {
   DCHECK(thread_checker_.CalledOnValidThread());
   eglMakeCurrent(display_, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
   eglDestroyContext(display_, context_);
   if (surface_) {
     eglDestroySurface(display_, surface_);
   }
   eglTerminate(display_);
 }
 
 void EglThreadContext::BindToWindow(EGLNativeWindowType window) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (surface_) {
     eglDestroySurface(display_, surface_);
     surface_ = EGL_NO_SURFACE;
   }
   if (window) {
     surface_ = eglCreateWindowSurface(display_, config_, window, NULL);
     if (!surface_) {
-      LOG(FATAL) << "Failed to create window surface.";
+      LOG(FATAL) << "Failed to create window surface: " << eglGetError();
     }
   } else {
     surface_ = EGL_NO_SURFACE;
   }
   if (!eglMakeCurrent(display_, surface_, surface_, context_)) {
-    LOG(FATAL) << "Failed to make current.";
+    LOG(FATAL) << "Failed to make current: " << eglGetError();
   }
 }
 
 bool EglThreadContext::IsWindowBound() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return surface_ != EGL_NO_SURFACE;
 }
 
-void EglThreadContext::SwapBuffers() {
+bool EglThreadContext::SwapBuffers() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(IsWindowBound());
+  if (!IsWindowBound()) {
+    return false;
+  }
   if (!eglSwapBuffers(display_, surface_)) {
-    LOG(FATAL) << "Failed to swap buffer.";
+    // Not fatal since the surface may be destroyed on a different thread
+    // earlier than the window is unbound. The context can still be reused
+    // after rebinding to the right window.
+    LOG(WARNING) << "Failed to swap buffer: " << eglGetError();
+    return false;
   }
+  return true;
+}
+
+bool EglThreadContext::CreateContextWithClientVersion(
+    unsigned int renderable_type,
+    int client_version) {
+  EGLint config_attribs[] = {
+     EGL_RED_SIZE, 8,
+     EGL_GREEN_SIZE, 8,
+     EGL_BLUE_SIZE, 8,
+     EGL_RENDERABLE_TYPE, renderable_type,
+     EGL_NONE
+   };
+
+   EGLint numConfigs;

[Sergey Ulanov, 2016/07/22 20:12:34]

num_configs

[Yuwei, 2016/07/22 21:22:39]

Done.

+   if (!eglChooseConfig(display_, config_attribs, &config_, 1, &numConfigs)) {
+     LOG(WARNING) << "Failed to choose config: " << eglGetError();
+     return false;
+   }
+
+  EGLint context_attribs[] = {
+    EGL_CONTEXT_CLIENT_VERSION, client_version,
+    EGL_NONE
+  };
+  context_ = eglCreateContext(display_, config_, EGL_NO_CONTEXT,
+                              context_attribs);
+  if (!context_) {
+    LOG(WARNING) << "Failed to create context: " << eglGetError();

[Lambros, 2016/07/22 20:36:07]

You can maybe remove this log, as you are already logging failure in the ctor at
line 24.

[Yuwei, 2016/07/22 21:22:39]

Done.

+    return false;
+  }
+  return true;
+}
+
+int EglThreadContext::GetClientVersion() const {
+  return client_version_;
 }
 
 }  // namespace remoting