Reland of Select correct copy method for DoCopySubTexture

gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.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 "gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.h"
 
 #include "base/logging.h"
 #include "gpu/command_buffer/service/framebuffer_manager.h"
 #include "gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
@@ skipping 238 matching lines @@
       const bool rgba_immutable =
           texture->texture()->IsImmutable() &&
           TextureManager::ExtractFormatFromStorageFormat(internal_format) ==
               GL_RGBA;
       const bool do_copy = !rgba_immutable;
 
       // CMAA Effect
       if (do_copy) {
         ApplyCMAAEffectTexture(source_texture, rgba8_texture_, do_copy);
 
+        // Source format for DoCopySubTexture is always GL_RGBA8.
+        CopyTextureMethod method = DIRECT_COPY;
+        bool copy_tex_image_format_valid =
+            !GLES2Util::IsIntegerFormat(internal_format) &&
+            GLES2Util::GetColorEncodingFromInternalFormat(internal_format) !=
+                GL_SRGB &&
+            internal_format != GL_BGRA_EXT && internal_format != GL_BGRA8_EXT;
+        if (GLES2Util::IsSizedColorFormat(internal_format)) {

[Zhenyao Mo, 2017/02/07 18:58:07]

I have a question: if the format is float, even if the channel bits do not
match, can we still do the copying?  As far as I understand, the bits matching
rule only applies to fixed point to fixed point copying, not fixed point to
float point copying.

[qiankun, 2017/02/08 03:44:23]

I didn't see the bits matching rule only applies to fixed-point-to-fixed-point
copying, see validation for CopyTexImage2D:
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_dec....

But there is another problem, see my commetns:
https://codereview.chromium.org/2543123003/#msg35. Some formats (e.g. RGBA16I,
RGBA16UI, RGBA32I, RGBA32UI) are not supported by DoCopySubTexture yet. This is
still a big potential issue to be fixed.

[dshwang, 2017/02/10 01:25:28]

We don't have plan to use CMAA on float texture. Currently, RGB, RGBA, BGRA,
SRGB support are enough, which DrawingBuffer requires. In the future, skia might
use it for default framebuffer, and I think those format are still enough.

Thanks and lgtm after fact. :)

+          int dr, dg, db, da;
+          GLES2Util::GetColorFormatComponentSizes(internal_format, 0, &dr, &dg,
+                                                  &db, &da);
+          if ((dr > 0 && dr != 8) || (dg > 0 && dg != 8) ||
+              (db > 0 && db != 8) || (da > 0 && da != 8)) {
+            copy_tex_image_format_valid = false;
+          }
+        }
+        if (!copy_tex_image_format_valid)
+          method = DIRECT_DRAW;
+        bool color_renderable = Texture::ColorRenderable(
+            decoder->GetFeatureInfo(), internal_format, false);
+#if defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY)
+        // glDrawArrays is faster than glCopyTexSubImage2D on IA Mesa driver,
+        // although opposite in Android.
+        // TODO(dshwang): After Mesa fixes this issue, remove this hack.
+        // https://bugs.freedesktop.org/show_bug.cgi?id=98478, crbug.com/535198.
+        if (color_renderable)
+          method = DIRECT_DRAW;
+#endif
+        if (method == DIRECT_DRAW && !color_renderable)
+          method = DRAW_AND_COPY;
+
         copier->DoCopySubTexture(
             decoder, GL_TEXTURE_2D, rgba8_texture_, 0, GL_RGBA8, GL_TEXTURE_2D,
             source_texture, 0, internal_format, 0, 0, 0, 0, width_, height_,

[qiankun, 2017/02/07 10:46:33]

internal_format may be non-color-renderable. It caused CopySubTexture hit
NOTREACHED() at
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_cop....
See bug: https://bugs.chromium.org/p/chromium/issues/detail?id=687180.

-            width_, height_, width_, height_, false, false, false, DIRECT_DRAW);
+            width_, height_, width_, height_, false, false, false, method);
       } else {
         ApplyCMAAEffectTexture(source_texture, source_texture, do_copy);
       }
 
       decoder->RestoreTextureState(source_texture);
     }
   }
 
   // Restore state
   decoder->RestoreAllAttributes();
@@ skipping 1577 matching lines @@
     \n#endif\n
     \n#if defined DISPLAY_EDGES\n
       DisplayEdges();
     \n#endif\n
     }
   );
 /* clang-format on */
 
 }  // namespace gles2
 }  // namespace gpu