Add glCopyCompressedTextureCHROMIUM

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 29d4d5d14e6925ecb99c98dca01097106bcc69b1..7be1648ce0392e790212c4ce5cf9268f08d66b0d 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -969,6 +969,13 @@ class GLES2DecoderImpl : public GLES2Decoder,
                                 GLint xoffset,
                                 GLint yoffset);
 
+  void DoCopyCompressedTextureCHROMIUM(GLenum target,
+                                       GLuint source_id,
+                                       GLuint dest_id,
+                                       GLenum internal_format,
+                                       GLenum dest_type,
+                                       GLsizei source_size);
+
   // Wrapper for TexStorage2DEXT.
   void DoTexStorage2DEXT(
       GLenum target,
@@ -1697,6 +1704,11 @@ class GLES2DecoderImpl : public GLES2Decoder,
                                    TextureRef* source_texture_ref,
                                    TextureRef* dest_texture_ref,
                                    GLenum dest_internal_format);
+  bool ValidateCopyCompressedTextureCHROMIUM(const char* function_name,
+                                             GLenum target,
+                                             TextureRef* source_texture_ref,
+                                             TextureRef* dest_texture_ref,
+                                             GLenum dest_internal_format);
 
   void RenderWarning(const char* filename, int line, const std::string& msg);
   void PerformanceWarning(
@@ -11523,6 +11535,65 @@ bool GLES2DecoderImpl::ValidateCopyTextureCHROMIUM(
   return true;
 }
 
+bool GLES2DecoderImpl::ValidateCopyCompressedTextureCHROMIUM(
+    const char* function_name,
+    GLenum target,
+    TextureRef* source_texture_ref,
+    TextureRef* dest_texture_ref,
+    GLenum dest_internal_format) {
+  if (!source_texture_ref || !dest_texture_ref) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "unknown texture id");
+    return false;
+  }
+
+  if (GL_TEXTURE_2D != target) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name,
+                       "invalid texture target");
+    return false;
+  }
+
+  Texture* source_texture = source_texture_ref->texture();
+  Texture* dest_texture = dest_texture_ref->texture();
+  if (source_texture == dest_texture) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, function_name,
+                       "source and destination textures are the same");
+    return false;
+  }
+
+  if (dest_texture->target() != GL_TEXTURE_2D ||
+      (source_texture->target() != GL_TEXTURE_2D &&
+       source_texture->target() != GL_TEXTURE_RECTANGLE_ARB &&
+       source_texture->target() != GL_TEXTURE_EXTERNAL_OES)) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name,
+                       "invalid texture target binding");
+    return false;
+  }
+
+  GLenum source_type = 0;
+  GLenum source_internal_format = 0;
+  source_texture->GetLevelType(source_texture->target(), 0, &source_type,
+                               &source_internal_format);
+
+  bool valid_dest_format =
+      dest_internal_format == GL_ATC_RGB_AMD ||
+      dest_internal_format == GL_ATC_RGBA_INTERPOLATED_ALPHA_AMD ||
+      dest_internal_format == GL_COMPRESSED_RGB_S3TC_DXT1_EXT ||
+      dest_internal_format == GL_COMPRESSED_RGBA_S3TC_DXT5_EXT ||
+      dest_internal_format == GL_ETC1_RGB8_OES;
+  bool valid_source_format = source_internal_format == GL_ALPHA ||
+                             source_internal_format == GL_RGB ||
+                             source_internal_format == GL_RGBA ||
+                             source_internal_format == GL_LUMINANCE ||
+                             source_internal_format == GL_LUMINANCE_ALPHA ||
+                             source_internal_format == GL_BGRA_EXT;

[reveman, 2015/04/30 13:36:14]

hm, doesn't source format have to match destination format?

[christiank, 2015/05/05 13:42:22]

Conceptually, maybe it should. However, it appears as if the source format is
GL_RGBA when I am testing (using one-copy). The reason seems to be the following
line in ResourceProvider::ScopedWriteLockGpuMemoryBuffer::   
~ScopedWriteLockGpuMemoryBuffer():

gl->CreateImageCHROMIUM(gpu_memory_buffer_->AsClientBuffer(),
                        size_.width(),
                        size_.height(),
                        GL_RGBA);

I don't think we want to change this format. I vaguely remember a discussion
about this somewhere.

I agree this doesn't look right, but I am not really sure what the right
approach is?

I have tried to find any instances where the other (non GL_RGBA)
source_internal_formats are set, but I couldn't find any. The other formats have
been added simply by following the behavior of ValidateCopyTextureCHROMIUM.

[reveman, 2015/05/05 16:12:09]

This changed recently, CreateImageCHROMIUM now needs to specific a more exact
internal format. e.g. GL_RGBA, GL_BGRA_EXT, GL_R8, GL_ETC1_RGB8_OES, etc. Are
you using ToT?

[christiank, 2015/05/07 12:47:00]

Ah, you're right. My Chromium revision was a little bit too old, I had to
rebase. Rebasing fixed part of the problem. I also found that
BindTexImage2DCHROMIUM causes a conflict of format. In
GLES2DecoderImpl::DoBindTexImage2DCHROMIUM calls SetLevelInfo with GL_RGBA.

I am not sure why it always sets the format to GL_RGBA, could it be laziness?
What do you think about adding a format parameter to BindTexImage2DCHROMIUM?
That would fix the problem raised here. I could verify that the source and
destination have the same compressed format. If my understanding is correct, it
would also make the level information of the Texture object more accurate.

[reveman, 2015/05/07 18:35:49]

It used to be that GL_RGBA was the only supported format but I guess we forgot
to fix this when adding support for other formats. Do you mind fixing this in a
separate patch?
I think it would be better to add a GetFormat function to the GLImage API.
Yes, sounds like a good plan.

[christiank, 2015/05/08 13:04:23]

Sure, I'll address it in a separate patch.

[christiank, 2015/05/12 11:08:58]

I have now fixed the format validation.

+  if (!valid_source_format || !valid_dest_format) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, function_name,
+                       "invalid internal format");
+    return false;
+  }
+  return true;
+}
+
 void GLES2DecoderImpl::DoCopyTextureCHROMIUM(GLenum target,
                                              GLuint source_id,
                                              GLuint dest_id,
@@ -11813,6 +11884,111 @@ void GLES2DecoderImpl::DoCopySubTextureCHROMIUM(GLenum target,
   DoDidUseTexImageIfNeeded(source_texture, source_texture->target());
 }
 
+void GLES2DecoderImpl::DoCopyCompressedTextureCHROMIUM(GLenum target,
+                                                       GLuint source_id,
+                                                       GLuint dest_id,
+                                                       GLenum internal_format,
+                                                       GLenum dest_type,
+                                                       GLsizei source_size) {
+  TRACE_EVENT0("gpu", "GLES2DecoderImpl::DoCopyCompressedTextureCHROMIUM");
+
+  TextureRef* source_texture_ref = GetTexture(source_id);
+  TextureRef* dest_texture_ref = GetTexture(dest_id);
+  Texture* source_texture = source_texture_ref->texture();
+  Texture* dest_texture = dest_texture_ref->texture();
+  gfx::GLImage* image =
+      source_texture->GetLevelImage(source_texture->target(), 0);
+  if (!image) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glCopyCompressedTextureCHROMIUM",
+                       "no mage");
+    return;
+  }
+
+  gfx::Size size = image->GetSize();
+  int source_width = size.width();
+  int source_height = size.height();
+  if (source_width <= 0 || source_height <= 0) {
+    LOCAL_SET_GL_ERROR(
+        GL_INVALID_VALUE,
+        "glCopyCompressedTextureCHROMIUM", "invalid image size");
+    return;
+  }
+
+  GLenum source_type = 0;
+  GLenum source_internal_format = 0;
+  source_texture->GetLevelType(
+      source_texture->target(), 0, &source_type, &source_internal_format);
+
+  if (dest_texture->IsImmutable()) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION,
+                       "glCopyCompressedTextureCHROMIUM",
+                       "texture is immutable");
+    return;
+  }
+
+  if (!ValidateCopyCompressedTextureCHROMIUM(
+          "glCopyCompressedTextureCHROMIUM",
+          target,
+          source_texture_ref, dest_texture_ref,
+          internal_format)) {
+    return;
+  }
+
+  // Clear the source texture if necessary.
+  if (!texture_manager()->ClearTextureLevel(this, source_texture_ref,
+                                            source_texture->target(), 0)) {
+    LOCAL_SET_GL_ERROR(GL_OUT_OF_MEMORY, "glCopyCompressedTextureCHROMIUM",
+                       "dimensions too big");
+    return;
+  }
+
+  GLenum dest_type_previous = dest_type;
+  GLenum dest_internal_format = internal_format;
+  int dest_width = 0;
+  int dest_height = 0;
+  bool dest_level_defined =
+      dest_texture->GetLevelSize(GL_TEXTURE_2D, 0, &dest_width, &dest_height);
+
+  if (dest_level_defined) {
+    dest_texture->GetLevelType(GL_TEXTURE_2D, 0, &dest_type_previous,
+                               &dest_internal_format);
+  }
+
+  // Resize the destination texture to the dimensions of the source texture.
+  if (!dest_level_defined || dest_width != source_width ||
+      dest_height != source_height ||
+      dest_internal_format != internal_format ||
+      dest_type_previous != dest_type) {
+    // Ensure that the glCompressedTexImage2D succeeds.
+    LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glCopyCompressedTextureCHROMIUM");
+    glBindTexture(GL_TEXTURE_2D, dest_texture->service_id());
+    glCompressedTexImage2D(GL_TEXTURE_2D, 0, internal_format, source_width,
+                           source_height, 0, source_size, NULL);
+    GLenum error = LOCAL_PEEK_GL_ERROR("glCopyCompressedTextureCHROMIUM");
+    if (error != GL_NO_ERROR) {
+      RestoreCurrentTextureBindings(&state_, GL_TEXTURE_2D);
+      return;
+    }
+
+    texture_manager()->SetLevelInfo(
+        dest_texture_ref, GL_TEXTURE_2D, 0, internal_format, source_width,
+        source_height, 1, 0, internal_format, dest_type, true);
+  } else {
+    texture_manager()->SetLevelCleared(dest_texture_ref, GL_TEXTURE_2D, 0,
+                                       true);
+  }
+
+  ScopedModifyPixels modify(dest_texture_ref);
+
+  DCHECK(!unpack_flip_y_);
+  DCHECK(!(unpack_premultiply_alpha_ ^ unpack_unpremultiply_alpha_));
+
+  // Copying compressed textures is only supported using GLImage::CopyTexImage.
+  glBindTexture(GL_TEXTURE_2D, dest_texture->service_id());
+  bool copy_result = image->CopyTexImage(GL_TEXTURE_2D);
+  DCHECK(copy_result);

[reveman, 2015/04/30 13:36:14]

How do we handle this failure? CopyTexImage is allowed to fail at which time we
need to implement some fallback. Or make sure this copy-compressed-image
extension is not available unless it's supported by all available GLImage types.

Could we use ARB_copy_image here if available?

[christiank, 2015/05/05 13:42:22]

Tile compression should only be enabled when using the GL_TEXTURE_EXTERNAL_OES
target and we "should" be using a non-failing CopyTexImage implementation. But I
see what you mean. Looking at glCopyCompressedTextureCHROMIUM in isolation, it
should be able to handle other cases as well.

ARB_copy_image could possibly work. I experimented with it a bit, but I could
never get it to work after disabling CopyTexImage. It failed with
GL_ILLEGAL_OPERATION, probably thanks to the source format being incompatible
with the destination format (see the other discussion).

[reveman, 2015/05/05 16:12:09]

It's hard to see how this is enforced right now. If someone decides to add
compressed texture format support to one of the native gpu-memory-buffer
implementations then this is not going to work. I guess we need ARB_copy_image
support before we can do that..
OK, we can save ARB_copy_image support for a follow up. I think we should also
be using that extension for normal CopyTexture if available.

Can you log an error here instead of the DCHECK?

[christiank, 2015/05/07 12:47:00]

Yes you're right. It certainly feels like a non-complete implementation. Then we
have the case when ARB_copy_image isn't supported either.
Absolutely, I'll address this as soon as I know how to proceed with the
validation issue.

[reveman, 2015/05/07 18:35:49]

Ideally we'd be able to say that CopyCompressedTexture is only supported when
ARB_copy_image is present but it looks like we need this to be supported using
GpuMemoryBuffers in cases where ARB_copy_image is unavailable.

What if we fallback to GetCompressedTexImage+CompressedTexImage2D when
ARB_copy_image is not supported?

[christiank, 2015/05/08 13:04:23]

Sure, I imagine GetCompressedTexImage + CompressedTexImage2D could work as a
fallback. GetCompressedTexImage is OpenGL 3+ as far as I can tell though. I'll
give it a shot.

[christiank, 2015/05/12 11:08:59]

Is it okay if I log an error for now? I'd rather address the
GetCompressedTexImage and CopyTexImage fallbacks separately if possible.

+}
+
 static GLenum ExtractTypeFromStorageFormat(GLenum internalformat) {
   switch (internalformat) {
     case GL_RGB565: