Force cube map texture complete on Linux AMD

gpu/command_buffer/service/texture_manager.cc

Index: gpu/command_buffer/service/texture_manager.cc
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index 68cd6bf3e182ae549100085bb16a2ca101962ac9..4104a52a7b24c59b4d11a01ea51fe387aad15da5 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -395,6 +395,24 @@ class ScopedResetPixelUnpackBuffer{
 
 }  // namespace anonymous
 
+DecoderTextureState::DecoderTextureState(
+    const GpuDriverBugWorkarounds& workarounds)
+    : tex_image_failed(false),
+      texture_upload_count(0),
+      texsubimage_faster_than_teximage(
+          workarounds.texsubimage_faster_than_teximage),
+      force_cube_map_positive_x_allocation(
+          workarounds.force_cube_map_positive_x_allocation),
+      force_cube_complete(workarounds.force_cube_complete),
+      force_int_cube_texture_complete(
+          workarounds.force_int_cube_texture_complete),
+      unpack_alignment_workaround_with_unpack_buffer(
+          workarounds.unpack_alignment_workaround_with_unpack_buffer),
+      unpack_overlapping_rows_separately_unpack_buffer(
+          workarounds.unpack_overlapping_rows_separately_unpack_buffer),
+      unpack_image_height_workaround_with_unpack_buffer(
+          workarounds.unpack_image_height_workaround_with_unpack_buffer) {}
+
 TextureManager::DestructionObserver::DestructionObserver() {}
 
 TextureManager::DestructionObserver::~DestructionObserver() {}
@@ -2527,13 +2545,15 @@ void TextureManager::DoCubeMapWorkaround(
 
   std::vector<GLenum> undefined_faces;
   Texture* texture = texture_ref->texture();
-  if (texture_state->force_cube_complete) {
+  if (texture_state->force_cube_complete ||
+      texture_state->force_int_cube_texture_complete) {
     int width = 0;
     int height = 0;
     for (unsigned i = 0; i < 6; i++) {
       GLenum target = static_cast<GLenum>(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i);
       bool defined = texture->GetLevelSize(
           target, args.level, &width, &height, nullptr);
+      defined = defined && width == args.width && height == args.height;

[Zhenyao Mo, 2017/04/18 17:48:02]

We should not modify a face level if it's already defined. Otherwise you
internally change something users define.

I think in this case we should just return and don't apply workaround.

[qiankun, 2017/04/19 12:26:35]

If we don't apply workaround, the bug still exists. 
Per my understanding, all the faces of a cube map texture should be in same size
in the buggy driver. If we will update one face with a different size when the
driver requires force_cube_complete workaround, then it is supposed all other
faces will be updated in future. Otherwise, users will not get what they want
but GL errors. So, we can apply the workaround. But, as your mentioned, it
changs content of other faces users define.

       if (!defined && target != args.target)
         undefined_faces.push_back(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i);
     }
@@ -2585,6 +2605,13 @@ void TextureManager::ValidateAndDoTexImage(
       (texture_state->force_cube_complete ||
        (texture_state->force_cube_map_positive_x_allocation &&
         args.target != GL_TEXTURE_CUBE_MAP_POSITIVE_X));
+  // Force integer cube map texture complete, see crbug.com/712117.
+  need_cube_map_workaround = need_cube_map_workaround ||
+                             (feature_info_->IsWebGL2OrES3Context() &&
+                              texture->target() == GL_TEXTURE_CUBE_MAP &&
+                              texture_state->force_int_cube_texture_complete &&

[qiankun, 2017/04/18 16:45:10]

With this workaround, we can fix the integer texture related conformance test
failures on Linux after GPU-GPU path was enabled in blink. I just found the srgb
related failures are similar as integer related failures. So, I would like to
reuse force_cube_complete workaround and check internal_formats are integer
formats and srgb formats here. What do you think?

[Zhenyao Mo, 2017/04/18 17:48:02]

This sounds fine.

[qiankun, 2017/04/19 12:26:35]

I found we can not reuse force_cube_complete workaround, since there are not the
same situation on Linux AMD as other platforms. I added a standalone
force_int_or_srgb_cube_complete workaround for Linux AMD. This workaround should
only be applied on Linux AMD. We use the same method DoCubeMapWrokaround to do
the real things.

+                              GLES2Util::IsIntegerFormat(args.internal_format));
+
   if (need_cube_map_workaround && !buffer) {
     DoCubeMapWorkaround(texture_state, state, framebuffer_state,
                         texture_ref, function_name, args);