Refactor GL surface format handling

ui/gl/gl_surface_egl.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 "ui/gl/gl_surface_egl.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <map>
@@ skipping 221 matching lines @@
     LOG(ERROR) << "eglChooseConfig failed with error "
                << GetLastEGLErrorString();
     return false;
   }
   if (*num_configs == 0) {
     return false;
   }
   return true;
 }
 
-EGLConfig ChooseConfig(GLSurface::Format format) {
+EGLConfig ChooseConfig(GLSurfaceFormat format) {
   // Choose an EGL configuration.
   // On X this is only used for PBuffer surfaces.
   std::vector<EGLint> renderable_types;
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableES3GLContext)) {
     renderable_types.push_back(EGL_OPENGL_ES3_BIT);
   }
   renderable_types.push_back(EGL_OPENGL_ES2_BIT);
 
-  EGLint buffer_size = 32;
+  EGLint buffer_size = format.GetBufferSize();
   EGLint alpha_size = 8;
+  bool want_rgb565 = buffer_size == 16;
 
 #if defined(USE_X11) && !defined(OS_CHROMEOS)
   // If we're using ANGLE_NULL, we may not have a display, in which case we
   // can't use XVisualManager.
   if (g_native_display) {
     ui::XVisualManager::GetInstance()->ChooseVisualForWindow(
         true, nullptr, &buffer_size, nullptr, nullptr);
     alpha_size = buffer_size == 32 ? 8 : 0;
   }
 #endif
 
-  EGLint surface_type = (format == GLSurface::SURFACE_SURFACELESS)
+  EGLint surface_type = (format.IsSurfaceless()
                             ? EGL_DONT_CARE
-                            : EGL_WINDOW_BIT | EGL_PBUFFER_BIT;
+                            : EGL_WINDOW_BIT | EGL_PBUFFER_BIT);
 
   for (auto renderable_type : renderable_types) {
     EGLint config_attribs_8888[] = {EGL_BUFFER_SIZE,
                                     buffer_size,
                                     EGL_ALPHA_SIZE,
                                     alpha_size,
                                     EGL_BLUE_SIZE,
                                     8,
                                     EGL_GREEN_SIZE,
                                     8,
@@ skipping 13 matching lines @@
                                    6,
                                    EGL_RED_SIZE,
                                    5,
                                    EGL_RENDERABLE_TYPE,
                                    renderable_type,
                                    EGL_SURFACE_TYPE,
                                    surface_type,
                                    EGL_NONE};
 
     EGLint* choose_attributes = config_attribs_8888;
-    if (format == GLSurface::SURFACE_RGB565) {
+    if (want_rgb565) {
       choose_attributes = config_attribs_565;
     }
 
     EGLint num_configs;
     EGLint config_size = 1;
     EGLConfig config = nullptr;
     EGLConfig* config_data = &config;
     // Validate if there are any configs for given attribs.
     if (!ValidateEglConfig(g_display, choose_attributes, &num_configs)) {
       // Try the next renderable_type
       continue;
     }
 
     std::unique_ptr<EGLConfig[]> matching_configs(new EGLConfig[num_configs]);
-    if (format == GLSurface::SURFACE_RGB565) {
+    if (want_rgb565) {
       config_size = num_configs;
       config_data = matching_configs.get();
     }
 
     if (!eglChooseConfig(g_display, choose_attributes, config_data, config_size,
                          &num_configs)) {
       LOG(ERROR) << "eglChooseConfig failed with error "
                  << GetLastEGLErrorString();
       return config;
     }
 
-    if (format == GLSurface::SURFACE_RGB565) {
+    if (want_rgb565) {
       // Because of the EGL config sort order, we have to iterate
       // through all of them (it'll put higher sum(R,G,B) bits
       // first with the above attribs).
       bool match_found = false;
       for (int i = 0; i < num_configs; i++) {
         EGLint red, green, blue, alpha;
         // Read the relevant attributes of the EGLConfig.
         if (eglGetConfigAttrib(g_display, matching_configs[i], EGL_RED_SIZE,
                                &red) &&
             eglGetConfigAttrib(g_display, matching_configs[i], EGL_BLUE_SIZE,
@@ skipping 115 matching lines @@
   return result;
 }
 
 bool EGLSyncControlVSyncProvider::GetMscRate(int32_t* numerator,
                                              int32_t* denominator) {
   return false;
 }
 
 GLSurfaceEGL::GLSurfaceEGL() {}
 
-GLSurface::Format GLSurfaceEGL::GetFormat() {
+GLSurfaceFormat GLSurfaceEGL::GetFormat() {
   return format_;
 }
 
 EGLDisplay GLSurfaceEGL::GetDisplay() {
   return g_display;
 }
 
 EGLConfig GLSurfaceEGL::GetConfig() {
   if (!config_) {
     config_ = ChooseConfig(format_);
@@ skipping 227 matching lines @@
 
 #if defined(OS_WIN)
   vsync_override_ = false;
   swap_generation_ = 0;
   RECT windowRect;
   if (GetClientRect(window_, &windowRect))
     size_ = gfx::Rect(windowRect).size();
 #endif
 }
 
-bool NativeViewGLSurfaceEGL::Initialize(GLSurface::Format format) {
+bool NativeViewGLSurfaceEGL::Initialize(GLSurfaceFormat format) {
   format_ = format;
   return Initialize(nullptr);
 }
 
 bool NativeViewGLSurfaceEGL::Initialize(
     std::unique_ptr<gfx::VSyncProvider> sync_provider) {
   DCHECK(!surface_);
 
   if (!GetDisplay()) {
     LOG(ERROR) << "Trying to create surface with invalid display.";
@@ skipping 334 matching lines @@
 
 PbufferGLSurfaceEGL::PbufferGLSurfaceEGL(const gfx::Size& size)
     : size_(size),
       surface_(NULL) {
   // Some implementations of Pbuffer do not support having a 0 size. For such
   // cases use a (1, 1) surface.
   if (size_.GetArea() == 0)
     size_.SetSize(1, 1);
 }
 
-bool PbufferGLSurfaceEGL::Initialize() {
-  GLSurface::Format format = SURFACE_DEFAULT;
+bool PbufferGLSurfaceEGL::Initialize(GLSurfaceFormat format) {
+  EGLSurface old_surface = surface_;
+
 #if defined(OS_ANDROID)
   // This is to allow context virtualization which requires on- and offscreen
   // to use a compatible config. We expect the client to request RGB565
   // onscreen surface also for this to work (with the exception of
   // fullscreen video).
-  if (base::SysInfo::IsLowEndDevice())
-    format = SURFACE_RGB565;
+  if (format.IsDefault() && base::SysInfo::IsLowEndDevice())
+    format = GLSurfaceFormat(GLSurfaceFormat::SURFACE_RGB565);
 #endif
-  return Initialize(format);
-}
 
-bool PbufferGLSurfaceEGL::Initialize(GLSurface::Format format) {
-  EGLSurface old_surface = surface_;
   format_ = format;
 
   EGLDisplay display = GetDisplay();
   if (!display) {
     LOG(ERROR) << "Trying to create surface with invalid display.";
     return false;
   }
 
   // Allocate the new pbuffer surface before freeing the old one to ensure
   // they have different addresses. If they have the same address then a
@@ skipping 102 matching lines @@
   return handle;
 #endif
 }
 
 PbufferGLSurfaceEGL::~PbufferGLSurfaceEGL() {
   Destroy();
 }
 
 SurfacelessEGL::SurfacelessEGL(const gfx::Size& size)
     : size_(size) {
-  format_ = GLSurface::SURFACE_SURFACELESS;
+  format_ = GLSurfaceFormat(GLSurfaceFormat::SURFACE_SURFACELESS);
 }
 
-bool SurfacelessEGL::Initialize() {
-  return Initialize(SURFACE_SURFACELESS);
-}
-
-bool SurfacelessEGL::Initialize(GLSurface::Format format) {
+bool SurfacelessEGL::Initialize(GLSurfaceFormat format) {
+  if (format.IsDefault()) {

[bajones, 2017/01/04 22:31:55]

This feels weird, though I can certainly see how you arrived here. Seems like
it's probably an error to request a surfaceless surface with anything other than
the default settings. Maybe turn the if into a DCHECK and always override to
SURFACE_SURFACELESS?

[klausw, 2017/01/05 00:55:13]

Changed to set surfaceless unconditionally.

+    format = GLSurfaceFormat(GLSurfaceFormat::SURFACE_SURFACELESS);
+  }
   format_ = format;
   return true;
 }
 
 void SurfacelessEGL::Destroy() {
 }
 
 bool SurfacelessEGL::IsOffscreen() {
   return true;
 }
@@ skipping 23 matching lines @@
 }
 
 void* SurfacelessEGL::GetShareHandle() {
   return NULL;
 }
 
 SurfacelessEGL::~SurfacelessEGL() {
 }
 
 }  // namespace gl