Add R_8 GPU memory buffers format.

ui/gl/gl_image_memory.cc

 // Copyright 2014 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_image_memory.h"
 
 #include "base/logging.h"
 #include "base/trace_event/trace_event.h"
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/scoped_binders.h"
 
 #if defined(OS_WIN) || defined(USE_X11) || defined(OS_ANDROID) || \
     defined(USE_OZONE)
 #include "ui/gl/gl_surface_egl.h"
 #endif
 
 namespace gfx {
 namespace {
 
-bool ValidInternalFormat(unsigned internalformat) {
-  switch (internalformat) {
-    case GL_RGBA:
-      return true;
-    default:
-      return false;
-  }
-}
-
-bool ValidFormat(gfx::GpuMemoryBuffer::Format format) {
+bool ValidateFormats(gfx::GpuMemoryBuffer::Format format,

[reveman, 2015/04/01 13:14:33]

Please make this function consistent with the gl_image_linux_dma_buffer
function.

bool ValidFormat(unsigned internalformat, gfx::GpuMemoryBuffer::Format format) {
...

[Daniele Castagna, 2015/04/01 21:58:34]

Done.

+                     unsigned internalformat) {
   switch (format) {
     case gfx::GpuMemoryBuffer::ATC:
     case gfx::GpuMemoryBuffer::ATCIA:
     case gfx::GpuMemoryBuffer::DXT1:
     case gfx::GpuMemoryBuffer::DXT5:
     case gfx::GpuMemoryBuffer::ETC1:
     case gfx::GpuMemoryBuffer::RGBA_8888:
     case gfx::GpuMemoryBuffer::BGRA_8888:
+      if (internalformat != GL_RGBA) {
+        LOG(ERROR) << "Invalid internal format " << internalformat
+                   << "for the specified format: " << format;
+        return false;
+      }
+      return true;
+    case gfx::GpuMemoryBuffer::R_8:
+      if (internalformat != GL_LUMINANCE && internalformat != GL_R8) {
+        LOG(ERROR) << "Invalid internal format " << internalformat
+                   << "for the specified format: " << format;
+      }
       return true;
     case gfx::GpuMemoryBuffer::RGBX_8888:
+      LOG(ERROR) << "Invalid format: " << format;
       return false;
   }
-
   NOTREACHED();
   return false;
 }
 
 bool IsCompressedFormat(gfx::GpuMemoryBuffer::Format format) {
   switch (format) {
     case gfx::GpuMemoryBuffer::ATC:
     case gfx::GpuMemoryBuffer::ATCIA:
     case gfx::GpuMemoryBuffer::DXT1:
     case gfx::GpuMemoryBuffer::DXT5:
     case gfx::GpuMemoryBuffer::ETC1:
       return true;
+    case gfx::GpuMemoryBuffer::R_8:
     case gfx::GpuMemoryBuffer::RGBA_8888:
     case gfx::GpuMemoryBuffer::BGRA_8888:
     case gfx::GpuMemoryBuffer::RGBX_8888:
       return false;
   }
 
   NOTREACHED();
   return false;
 }
 
-GLenum TextureFormat(gfx::GpuMemoryBuffer::Format format) {
+GLenum TextureFormat(gfx::GpuMemoryBuffer::Format format,
+                     unsigned internalformat) {
   switch (format) {

[reveman, 2015/04/01 13:14:33]

can you move this switch (format) statement into a switch (internalformat)
statement to make it more clear what the expected values of internalformat is
not just for R_8?

[Daniele Castagna, 2015/04/01 21:58:34]

Split in two functions TextureFormat and InternalTextureFormat.

     case gfx::GpuMemoryBuffer::ATC:
       return GL_ATC_RGB_AMD;
     case gfx::GpuMemoryBuffer::ATCIA:
       return GL_ATC_RGBA_INTERPOLATED_ALPHA_AMD;
     case gfx::GpuMemoryBuffer::DXT1:
       return GL_COMPRESSED_RGB_S3TC_DXT1_EXT;
     case gfx::GpuMemoryBuffer::DXT5:
       return GL_COMPRESSED_RGBA_S3TC_DXT5_EXT;
     case gfx::GpuMemoryBuffer::ETC1:
       return GL_ETC1_RGB8_OES;
+    case gfx::GpuMemoryBuffer::R_8:
+      DCHECK(internalformat == GL_R8 || internalformat == GL_LUMINANCE);
+      return internalformat;
     case gfx::GpuMemoryBuffer::RGBA_8888:
       return GL_RGBA;
     case gfx::GpuMemoryBuffer::BGRA_8888:
       return GL_BGRA_EXT;
     case gfx::GpuMemoryBuffer::RGBX_8888:
       NOTREACHED();
       return 0;
   }
 
   NOTREACHED();
   return 0;
 }
 
-GLenum DataFormat(gfx::GpuMemoryBuffer::Format format) {
-  return TextureFormat(format);
+GLenum DataFormat(gfx::GpuMemoryBuffer::Format format,
+                  unsigned internalformat) {

[reveman, 2015/04/01 13:14:33]

hm, I'm not a fan of passing internalformat to this function as it should not be
needed to determine the DataFormat. Maybe we can split these functions into
something like this:

GLenum TextureFormat(gfx::GpuMemoryBuffer::Format format) {
  // As before this patch.
}

GLenum InternalTextureFormat(unsigned internalformat,
gfx::GpuMemoryBuffer::Format format) {
  switch (internalformat) {
    case GL_RGB:
    case GL_RGBA:
      return TextureFormat(format);
    case GL_R8:
    case GL_LUMINANCE:
     return internalformat;
  }
  ...
}

GLenum DataFormat(gfx::GpuMemoryBuffer::Format format) {
  switch (format) {
    case gfx::GpuMemoryBuffer::ATC:
    ...
    case gfx::GpuMemoryBuffer::BGRA_8888:
      return TextureFormat(format);
    case gfx::GpuMemoryBuffer::R_8:
      return GL_RED;
  }
}

[Daniele Castagna, 2015/04/01 21:58:35]

I'm afraid DataFormat needs to know about internalformat_.
DataFormat should return GL_RED if the internal format is GL_R8 (ARB_texture_rg
available) and GL_LUMINANCE if GL_LUMINANCE.

+  GLenum data_format = TextureFormat(format, internalformat);
+  return data_format == GL_R8 ? GL_RED : data_format;
 }
 
 GLenum DataType(gfx::GpuMemoryBuffer::Format format) {
   switch (format) {
     case gfx::GpuMemoryBuffer::RGBA_8888:
     case gfx::GpuMemoryBuffer::BGRA_8888:
+    case gfx::GpuMemoryBuffer::R_8:
       return GL_UNSIGNED_BYTE;
     case gfx::GpuMemoryBuffer::ATC:
     case gfx::GpuMemoryBuffer::ATCIA:
     case gfx::GpuMemoryBuffer::DXT1:
     case gfx::GpuMemoryBuffer::DXT5:
     case gfx::GpuMemoryBuffer::ETC1:
     case gfx::GpuMemoryBuffer::RGBX_8888:
       NOTREACHED();
       return 0;
   }
@@ skipping 32 matching lines @@
 
 GLImageMemory::~GLImageMemory() {
 #if defined(OS_WIN) || defined(USE_X11) || defined(OS_ANDROID) || \
     defined(USE_OZONE)
   DCHECK_EQ(EGL_NO_IMAGE_KHR, egl_image_);
   DCHECK_EQ(0u, egl_texture_id_);
 #endif
 }
 
 // static
 bool GLImageMemory::StrideInBytes(size_t width,

[reveman, 2015/04/01 13:14:33]

Just make sure this function is consistent with
GpuMemoryBufferImpl::StrideInBytes after making adjustments there..

[Daniele Castagna, 2015/04/01 21:58:35]

If by "make sure this function is consistent" you mean: "remember to copy and
paste". :)

Done.

                                   gfx::GpuMemoryBuffer::Format format,
                                   size_t* stride_in_bytes) {
-  base::CheckedNumeric<size_t> s = width;
+  base::CheckedNumeric<size_t> checked_stride = width;
   switch (format) {
-    case gfx::GpuMemoryBuffer::ATCIA:
-    case gfx::GpuMemoryBuffer::DXT5:
-      *stride_in_bytes = width;
-      return true;
-    case gfx::GpuMemoryBuffer::ATC:
-    case gfx::GpuMemoryBuffer::DXT1:
-    case gfx::GpuMemoryBuffer::ETC1:
+    case GpuMemoryBuffer::R_8:
+      checked_stride = (checked_stride + 3) % 4;
+      break;
+    case GpuMemoryBuffer::ATCIA:
+    case GpuMemoryBuffer::DXT5:  // 'checked_stride' is the same as 'width'.
+      break;
+    case GpuMemoryBuffer::ATC:
+    case GpuMemoryBuffer::DXT1:
+    case GpuMemoryBuffer::ETC1:
       DCHECK_EQ(width % 2, 0U);
-      s /= 2;
-      if (!s.IsValid())
-        return false;
-
-      *stride_in_bytes = s.ValueOrDie();
-      return true;
-    case gfx::GpuMemoryBuffer::RGBA_8888:
-    case gfx::GpuMemoryBuffer::BGRA_8888:
-      s *= 4;
-      if (!s.IsValid())
-        return false;
-
-      *stride_in_bytes = s.ValueOrDie();
-      return true;
-    case gfx::GpuMemoryBuffer::RGBX_8888:
-      NOTREACHED();
-      return false;
+      checked_stride /= 2;
+      break;
+    case GpuMemoryBuffer::RGBA_8888:
+    case GpuMemoryBuffer::RGBX_8888:
+    case GpuMemoryBuffer::BGRA_8888:
+      checked_stride *= 4;
+      break;
   }
-
-  NOTREACHED();
-  return false;
+  if (!checked_stride.IsValid())
+    return false;
+  *stride_in_bytes = checked_stride.ValueOrDie();
+  return true;
 }
 
 bool GLImageMemory::Initialize(const unsigned char* memory,
                                gfx::GpuMemoryBuffer::Format format) {
-  if (!ValidInternalFormat(internalformat_)) {

[reveman, 2015/04/01 13:14:33]

Please make this function consistent with GLImageLinuxDMABuffer::Initialize.

[Daniele Castagna, 2015/04/01 21:58:34]

Done.

-    LOG(ERROR) << "Invalid internalformat: " << internalformat_;
+  if (!ValidateFormats(format, internalformat_)) {
     return false;
   }
 
-  if (!ValidFormat(format)) {
-    LOG(ERROR) << "Invalid format: " << format;
-    return false;
-  }
-
   DCHECK(memory);
   DCHECK(!memory_);
   DCHECK_IMPLIES(IsCompressedFormat(format), size_.width() % 4 == 0);
   DCHECK_IMPLIES(IsCompressedFormat(format), size_.height() % 4 == 0);
   memory_ = memory;
   format_ = format;
   return true;
 }
 
 void GLImageMemory::Destroy(bool have_context) {
@@ skipping 41 matching lines @@
   if (target == GL_TEXTURE_EXTERNAL_OES)
     return false;
 
   DCHECK(memory_);
   if (IsCompressedFormat(format_)) {
     glCompressedTexSubImage2D(target,
                               0,  // level
                               0,  // x-offset
                               0,  // y-offset
                               size_.width(), size_.height(),
-                              DataFormat(format_), SizeInBytes(size_, format_),
-                              memory_);
+                              DataFormat(format_, internalformat_),
+                              SizeInBytes(size_, format_), memory_);
   } else {
     glTexSubImage2D(target, 0,  // level
                     0,          // x
                     0,          // y
-                    size_.width(), size_.height(), DataFormat(format_),
-                    DataType(format_), memory_);
+                    size_.width(), size_.height(),
+                    DataFormat(format_, internalformat_), DataType(format_),
+                    memory_);
   }
 
   return true;
 }
 
 void GLImageMemory::WillUseTexImage() {
   DCHECK(!in_use_);
   in_use_ = true;
 
   if (!need_do_bind_tex_image_)
@@ skipping 32 matching lines @@
 
       {
         ScopedTextureBinder texture_binder(GL_TEXTURE_2D, egl_texture_id_);
 
         glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
         glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
         glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
         if (IsCompressedFormat(format_)) {
           glCompressedTexImage2D(GL_TEXTURE_2D,
                                  0,  // mip level
-                                 TextureFormat(format_), size_.width(),
-                                 size_.height(),
+                                 TextureFormat(format_, internalformat_),
+                                 size_.width(), size_.height(),
                                  0,  // border
                                  SizeInBytes(size_, format_), memory_);
         } else {
           glTexImage2D(GL_TEXTURE_2D,
                        0,  // mip level
-                       TextureFormat(format_),
-                       size_.width(),
+                       TextureFormat(format_, internalformat_), size_.width(),
                        size_.height(),
                        0,  // border
-                       DataFormat(format_),
-                       DataType(format_),
+                       DataFormat(format_, internalformat_), DataType(format_),
                        memory_);
         }
       }
 
       EGLint attrs[] = {EGL_IMAGE_PRESERVED_KHR, EGL_TRUE, EGL_NONE};
       // Need to pass current EGL rendering context to eglCreateImageKHR for
       // target type EGL_GL_TEXTURE_2D_KHR.
       egl_image_ =
           eglCreateImageKHR(GLSurfaceEGL::GetHardwareDisplay(),
                             eglGetCurrentContext(),
                             EGL_GL_TEXTURE_2D_KHR,
                             reinterpret_cast<EGLClientBuffer>(egl_texture_id_),
                             attrs);
       DCHECK_NE(EGL_NO_IMAGE_KHR, egl_image_)
           << "Error creating EGLImage: " << eglGetError();
     } else {
       ScopedTextureBinder texture_binder(GL_TEXTURE_2D, egl_texture_id_);
 
       if (IsCompressedFormat(format_)) {
         glCompressedTexSubImage2D(GL_TEXTURE_2D,
                                   0,  // mip level
                                   0,  // x-offset
                                   0,  // y-offset
                                   size_.width(), size_.height(),
-                                  DataFormat(format_),
-                                  SizeInBytes(size_, format_),
-                                  memory_);
+                                  DataFormat(format_, internalformat_),
+                                  SizeInBytes(size_, format_), memory_);
       } else {
         glTexSubImage2D(GL_TEXTURE_2D,
                         0,  // mip level
                         0,  // x-offset
                         0,  // y-offset
-                        size_.width(),
-                        size_.height(),
-                        DataFormat(format_),
-                        DataType(format_),
+                        size_.width(), size_.height(),
+                        DataFormat(format_, internalformat_), DataType(format_),
                         memory_);
       }
     }
 
     glEGLImageTargetTexture2DOES(target, egl_image_);
     DCHECK_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());
     return;
   }
 #endif
 
   DCHECK_NE(static_cast<GLenum>(GL_TEXTURE_EXTERNAL_OES), target);
   if (IsCompressedFormat(format_)) {
     glCompressedTexImage2D(target,
                            0,  // mip level
-                           TextureFormat(format_), size_.width(),
-                           size_.height(),
+                           TextureFormat(format_, internalformat_),
+                           size_.width(), size_.height(),
                            0,  // border
                            SizeInBytes(size_, format_), memory_);
   } else {
-    glTexImage2D(target,
-                 0,  // mip level
-                 TextureFormat(format_),
-                 size_.width(),
-                 size_.height(),
-                 0,  // border
-                 DataFormat(format_),
-                 DataType(format_),
-                 memory_);
+    glTexImage2D(
+        target,
+        0,  // mip level
+        TextureFormat(format_, internalformat_), size_.width(), size_.height(),
+        0,  // border
+        DataFormat(format_, internalformat_), DataType(format_), memory_);
   }
 }
 
 }  // namespace gfx