Write YUV paths for mip mapping tests and fix color space tagging bug.

The tests that now run for YUV decoding are MipsAddedWhileOriginalInUse,
MipsAddedSubsequentDraw, and BasicMips.

This change adds a helper method to GpuImageDecodeCache called
GetUploadedPlaneForTesting and makes a few additional miscellaneous
fixes to the cache.

Bug: 900672
Change-Id: I7563206944125cdcea168ca3264c85ea5eb8e7fa
Reviewed-on: https://chromium-review.googlesource.com/c/1471261
Commit-Queue: Madeleine Barowsky <mbarowsky@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632750}

cc/tiles/gpu_image_decode_cache.cc

@@ -710,7 +710,7 @@ void GpuImageDecodeCache::UploadedImageData::SetImage(
   if (!represents_yuv_image && image_->isTextureBacked()) {
     gl_id_ = GlIdFromSkImage(image_.get());
   } else {
-    gl_id_ = -1;
+    gl_id_ = 0;
   }
   OnSetLockedData(false /* out_of_raster */);
 }
@@ -758,6 +758,7 @@ void GpuImageDecodeCache::UploadedImageData::Reset() {
   image_yuv_planes_.reset();
   gl_plane_ids_.reset();
   gl_id_ = 0;
+  is_alpha_ = false;
   transfer_cache_id_.reset();
   OnResetData();
 }
@@ -1683,7 +1684,7 @@ void GpuImageDecodeCache::DecodeImageIfNecessary(const DrawImage& draw_image,
     pixmap.setColorSpace(color_space);
 
     if (image_data->is_yuv) {
-      DLOG(WARNING) << "GpuImageDecodeCache wants to do YUV decoding/rendering";
+      DVLOG(3) << "GpuImageDecodeCache wants to do YUV decoding/rendering";
       SkPixmap pixmap_y;
       SkPixmap pixmap_u;
       SkPixmap pixmap_v;
@@ -1693,14 +1694,15 @@ void GpuImageDecodeCache::DecodeImageIfNecessary(const DrawImage& draw_image,
         DLOG(ERROR) << "DrawAndScaleImage failed.";
         backing_memory->Unlock();
         backing_memory.reset();
+      } else {
+        image_y = SkImage::MakeFromRaster(pixmap_y, release_proc, nullptr);
+        image_u = SkImage::MakeFromRaster(pixmap_u, release_proc, nullptr);
+        image_v = SkImage::MakeFromRaster(pixmap_v, release_proc, nullptr);
       }
-      image_y = SkImage::MakeFromRaster(pixmap_y, release_proc, nullptr);
-      image_u = SkImage::MakeFromRaster(pixmap_u, release_proc, nullptr);
-      image_v = SkImage::MakeFromRaster(pixmap_v, release_proc, nullptr);
     } else {  // RGBX decoding is the default path.
       if (!DrawAndScaleImage(draw_image, &pixmap, generator_client_id_,
                              image_data->is_yuv)) {
-        LOG(ERROR) << "DrawAndScaleImage failed.";
+        DLOG(ERROR) << "DrawAndScaleImage failed.";
         backing_memory->Unlock();
         backing_memory.reset();
       } else {
@@ -1833,7 +1835,6 @@ void GpuImageDecodeCache::UploadImageIfNecessary(const DrawImage& draw_image,
     sk_sp<SkImage> uploaded_y_image = image_data->decode.y_image();
     sk_sp<SkImage> uploaded_u_image = image_data->decode.u_image();
     sk_sp<SkImage> uploaded_v_image = image_data->decode.v_image();
-    image_data->decode.mark_used();
 
     // For kGpu, we upload and color convert (if necessary).
     if (image_data->mode == DecodedDataMode::kGpu) {
@@ -1862,7 +1863,7 @@ void GpuImageDecodeCache::UploadImageIfNecessary(const DrawImage& draw_image,
       uploaded_image = CreateImageFromYUVATexturesInternal(
           uploaded_y_image.get(), uploaded_u_image.get(),
           uploaded_v_image.get(), image_width, image_height, &yuva_color_space,
-          color_space);
+          decoded_target_colorspace);
     }
 
     // At-raster may have decoded this while we were unlocked. If so, ignore our
@@ -2003,9 +2004,8 @@ GpuImageDecodeCache::CreateImageData(const DrawImage& draw_image) {
   const bool is_bitmap_backed = !draw_image.paint_image().IsLazyGenerated() &&
                                 upload_scale_mip_level == 0 &&
                                 !cache_color_conversion_on_cpu;
-  SkYUVASizeInfo temp_yuva_size_info;
-  const bool is_yuv = draw_image.paint_image().IsYuv(&temp_yuva_size_info) &&
-                      mode == DecodedDataMode::kGpu;
+  const bool is_yuv =
+      draw_image.paint_image().IsYuv() && mode == DecodedDataMode::kGpu;
 
   // TODO(crbug.com/910276): Change after alpha support.
   // TODO(crbug.com/915972): Remove YUV420 assumption.
@@ -2362,6 +2362,24 @@ sk_sp<SkImage> GpuImageDecodeCache::GetSWImageDecodeForTesting(
   return image_data->decode.ImageForTesting();
 }
 
+sk_sp<SkImage> GpuImageDecodeCache::GetUploadedPlaneForTesting(
+    const DrawImage& draw_image,
+    size_t index) {
+  base::AutoLock lock(lock_);
+  ImageData* image_data = GetImageDataForDrawImage(
+      draw_image, InUseCacheKey::FromDrawImage(draw_image));
+  switch (index) {
+    case SkYUVAIndex::kY_Index:
+      return image_data->upload.y_image();
+    case SkYUVAIndex::kU_Index:
+      return image_data->upload.u_image();
+    case SkYUVAIndex::kV_Index:
+      return image_data->upload.v_image();
+    default:
+      return nullptr;
+  }
+}
+
 void GpuImageDecodeCache::OnMemoryPressure(
     base::MemoryPressureListener::MemoryPressureLevel level) {
   base::AutoLock lock(lock_);
@@ -2451,7 +2469,6 @@ sk_sp<SkImage> GpuImageDecodeCache::CreateImageFromYUVATexturesInternal(
 void GpuImageDecodeCache::UpdateMipsIfNeeded(const DrawImage& draw_image,
                                              ImageData* image_data) {
   CheckContextLockAcquiredIfNecessary();
-
   // If we already have mips, nothing to do.
   if (image_data->needs_mips)
     return;
@@ -2523,12 +2540,13 @@ void GpuImageDecodeCache::UpdateMipsIfNeeded(const DrawImage& draw_image,
     SkYUVColorSpace yuva_color_space = SkYUVColorSpace::kRec601_SkYUVColorSpace;
     size_t width = image_y_with_mips_owned->width();
     size_t height = image_y_with_mips_owned->height();
-    SkColorSpace* decoded_color_space = draw_image.paint_image().color_space();
+    sk_sp<SkColorSpace> decoded_color_space =
+        ColorSpaceForImageDecode(draw_image, image_data->mode);
     sk_sp<SkImage> yuv_image_with_mips_owned =
         CreateImageFromYUVATexturesInternal(
             image_y_with_mips_owned.get(), image_u_with_mips_owned.get(),
             image_v_with_mips_owned.get(), width, height, &yuva_color_space,
-            sk_ref_sp(decoded_color_space));
+            decoded_color_space);
     // In case of lost context
     if (!yuv_image_with_mips_owned) {
       DLOG(WARNING) << "TODO(crbug.com/740737): Context was lost. Early out.";

cc/tiles/gpu_image_decode_cache.h

@@ -170,6 +170,8 @@ class CC_EXPORT GpuImageDecodeCache
   bool IsInInUseCacheForTesting(const DrawImage& image) const;
   bool IsInPersistentCacheForTesting(const DrawImage& image) const;
   sk_sp<SkImage> GetSWImageDecodeForTesting(const DrawImage& image);
+  sk_sp<SkImage> GetUploadedPlaneForTesting(const DrawImage& draw_image,
+                                            size_t index);
   size_t paint_image_entries_count_for_testing() const {
     return paint_image_entries_.size();
   }

cc/tiles/gpu_image_decode_cache_unittest.cc

@@ -409,6 +409,24 @@ class GpuImageDecodeCacheTest
     return static_cast<ServiceImageTransferCacheEntry*>(entry)->image();
   }
 
+  void CompareAllPlanesToMippedVersions(GpuImageDecodeCache* cache,
+                                        const DrawImage& draw_image,
+                                        bool should_have_mips) {
+    for (size_t i = 0; i < SkYUVASizeInfo::kMaxCount; ++i) {
+      // TODO(crbug.com/910276): Skip alpha plane until supported in cache.
+      if (i != SkYUVAIndex::kA_Index) {
+        auto original_uploaded_plane =
+            cache->GetUploadedPlaneForTesting(draw_image, i);
+        ASSERT_TRUE(original_uploaded_plane);
+        auto plane_with_mips = original_uploaded_plane->makeTextureImage(
+            context_provider()->GrContext(), nullptr /* color space */,
+            GrMipMapped::kYes);
+        ASSERT_TRUE(plane_with_mips);
+        EXPECT_EQ(should_have_mips, original_uploaded_plane == plane_with_mips);
+      }
+    }
+  }
+
  protected:
   FakeDiscardableManager discardable_manager_;
   scoped_refptr<GPUImageDecodeTestMockContextProvider> context_provider_;
@@ -2558,10 +2576,6 @@ TEST_P(GpuImageDecodeCacheTest, DecodeToScaleNoneQuality) {
 }
 
 TEST_P(GpuImageDecodeCacheTest, BasicMips) {
-  if (do_yuv_decode_) {
-    // We will modify this test for YUV.
-    return;
-  }
   auto decode_and_check_mips = [this](SkFilterQuality filter_quality,
                                       SkSize scale,
                                       const gfx::ColorSpace& color_space,
@@ -2589,11 +2603,26 @@ TEST_P(GpuImageDecodeCacheTest, BasicMips) {
     EXPECT_TRUE(decoded_draw_image.image());
     EXPECT_TRUE(decoded_draw_image.image()->isTextureBacked());
 
-    sk_sp<SkImage> image_with_mips =
-        decoded_draw_image.image()->makeTextureImage(
-            context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
-    EXPECT_EQ(should_have_mips, image_with_mips == decoded_draw_image.image());
+    if (do_yuv_decode_) {
+      // As of M74, Skia will flatten a YUV SkImage upon calling
+      // makeTextureImage. Thus, we must separately request mips for each
+      // plane and compare to the original uploaded planes.
+      CompareAllPlanesToMippedVersions(cache.get(), draw_image,
+                                       should_have_mips);
+      EXPECT_TRUE(
+          SkColorSpace::Equals(cache->SupportsColorSpaceConversion()
+                                   ? color_space.ToSkColorSpace().get()
+                                   : nullptr,
+                               decoded_draw_image.image()->colorSpace()));
 
+    } else {
+      sk_sp<SkImage> image_with_mips =
+          decoded_draw_image.image()->makeTextureImage(
+              context_provider()->GrContext(), nullptr /* color space */,
+              GrMipMapped::kYes);
+      EXPECT_EQ(should_have_mips,
+                image_with_mips == decoded_draw_image.image());
+    }
     cache->DrawWithImageFinished(draw_image, decoded_draw_image);
     cache->UnrefImage(draw_image);
   };
@@ -2622,10 +2651,6 @@ TEST_P(GpuImageDecodeCacheTest, BasicMips) {
 }
 
 TEST_P(GpuImageDecodeCacheTest, MipsAddedSubsequentDraw) {
-  if (do_yuv_decode_) {
-    // We will modify this test for YUV.
-    return;
-  }
   auto cache = CreateCache();
   bool is_decomposable = true;
   auto filter_quality = kMedium_SkFilterQuality;
@@ -2654,12 +2679,20 @@ TEST_P(GpuImageDecodeCacheTest, MipsAddedSubsequentDraw) {
     EXPECT_TRUE(decoded_draw_image.image());
     EXPECT_TRUE(decoded_draw_image.image()->isTextureBacked());
 
-    // No mips should be generated
-    sk_sp<SkImage> image_with_mips =
-        decoded_draw_image.image()->makeTextureImage(
-            context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
-    EXPECT_NE(image_with_mips, decoded_draw_image.image());
-
+    // No mips should be generated.
+    if (do_yuv_decode_) {
+      // As of M74, Skia will flatten a YUV SkImage upon calling
+      // makeTextureImage. Thus, we must separately request mips for each
+      // plane and compare to the original uploaded planes.
+      CompareAllPlanesToMippedVersions(cache.get(), draw_image,
+                                       false /* should_have_mips */);
+    } else {
+      sk_sp<SkImage> image_with_mips =
+          decoded_draw_image.image()->makeTextureImage(
+              context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
+      ASSERT_TRUE(image_with_mips);
+      EXPECT_NE(image_with_mips, decoded_draw_image.image());
+    }
     cache->DrawWithImageFinished(draw_image, decoded_draw_image);
     cache->UnrefImage(draw_image);
   }
@@ -2690,20 +2723,26 @@ TEST_P(GpuImageDecodeCacheTest, MipsAddedSubsequentDraw) {
     EXPECT_TRUE(decoded_draw_image.image()->isTextureBacked());
 
     // Mips should be generated
-    sk_sp<SkImage> image_with_mips =
-        decoded_draw_image.image()->makeTextureImage(
-            context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
-    EXPECT_EQ(image_with_mips, decoded_draw_image.image());
+    if (do_yuv_decode_) {
+      CompareAllPlanesToMippedVersions(cache.get(), draw_image,
+                                       true /* should_have_mips */);
+      EXPECT_TRUE(
+          SkColorSpace::Equals(cache->SupportsColorSpaceConversion()
+                                   ? DefaultColorSpace().ToSkColorSpace().get()
+                                   : nullptr,
+                               decoded_draw_image.image()->colorSpace()));
+    } else {
+      sk_sp<SkImage> image_with_mips =
+          decoded_draw_image.image()->makeTextureImage(
+              context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
+      EXPECT_EQ(image_with_mips, decoded_draw_image.image());
+    }
     cache->DrawWithImageFinished(draw_image, decoded_draw_image);
     cache->UnrefImage(draw_image);
   }
 }
 
 TEST_P(GpuImageDecodeCacheTest, MipsAddedWhileOriginalInUse) {
-  if (do_yuv_decode_) {
-    // We will modify this test for YUV.
-    return;
-  }
   auto cache = CreateCache();
   bool is_decomposable = true;
   auto filter_quality = kMedium_SkFilterQuality;
@@ -2735,15 +2774,22 @@ TEST_P(GpuImageDecodeCacheTest, MipsAddedWhileOriginalInUse) {
     viz::ContextProvider::ScopedContextLock context_lock(context_provider());
     DecodedDrawImage decoded_draw_image =
         EnsureImageBacked(cache->GetDecodedImageForDraw(draw_image));
-    EXPECT_TRUE(decoded_draw_image.image());
-    EXPECT_TRUE(decoded_draw_image.image()->isTextureBacked());
-
-    // No mips should be generated
-    sk_sp<SkImage> image_with_mips =
-        decoded_draw_image.image()->makeTextureImage(
-            context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
-    EXPECT_NE(image_with_mips, decoded_draw_image.image());
+    ASSERT_TRUE(decoded_draw_image.image());
+    ASSERT_TRUE(decoded_draw_image.image()->isTextureBacked());
 
+    // No mips should be generated.
+    if (do_yuv_decode_) {
+      // As of M74, Skia will flatten a YUV SkImage upon calling
+      // makeTextureImage. Thus, we must separately request mips for each
+      // plane and compare to the original uploaded planes.
+      CompareAllPlanesToMippedVersions(cache.get(), draw_image,
+                                       false /* should_have_mips */);
+    } else {
+      sk_sp<SkImage> image_with_mips =
+          decoded_draw_image.image()->makeTextureImage(
+              context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
+      EXPECT_NE(image_with_mips, decoded_draw_image.image());
+    }
     images_to_unlock.push_back({draw_image, decoded_draw_image});
   }
 
@@ -2764,15 +2810,24 @@ TEST_P(GpuImageDecodeCacheTest, MipsAddedWhileOriginalInUse) {
     DecodedDrawImage decoded_draw_image =
         EnsureImageBacked(cache->GetDecodedImageForDraw(draw_image));
 
-    EXPECT_TRUE(decoded_draw_image.image());
-    EXPECT_TRUE(decoded_draw_image.image()->isTextureBacked());
-
-    // Mips should be generated
-    sk_sp<SkImage> image_with_mips =
-        decoded_draw_image.image()->makeTextureImage(
-            context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
-    EXPECT_EQ(image_with_mips, decoded_draw_image.image());
+    ASSERT_TRUE(decoded_draw_image.image());
+    ASSERT_TRUE(decoded_draw_image.image()->isTextureBacked());
 
+    // Mips should be generated.
+    if (do_yuv_decode_) {
+      CompareAllPlanesToMippedVersions(cache.get(), draw_image,
+                                       true /* should_have_mips */);
+      EXPECT_TRUE(
+          SkColorSpace::Equals(cache->SupportsColorSpaceConversion()
+                                   ? DefaultColorSpace().ToSkColorSpace().get()
+                                   : nullptr,
+                               decoded_draw_image.image()->colorSpace()));
+    } else {
+      sk_sp<SkImage> image_with_mips =
+          decoded_draw_image.image()->makeTextureImage(
+              context_provider()->GrContext(), nullptr, GrMipMapped::kYes);
+      EXPECT_EQ(image_with_mips, decoded_draw_image.image());
+    }
     images_to_unlock.push_back({draw_image, decoded_draw_image});
   }
 
@@ -2783,13 +2838,29 @@ TEST_P(GpuImageDecodeCacheTest, MipsAddedWhileOriginalInUse) {
   {
     // All images which are currently ref-ed must have locked textures.
     viz::ContextProvider::ScopedContextLock context_lock(context_provider());
-    for (const auto& decode : images_to_unlock) {
+    for (const auto& draw_and_decoded_draw_image : images_to_unlock) {
       if (!use_transfer_cache_) {
-        discardable_manager_.ExpectLocked(GpuImageDecodeCache::GlIdFromSkImage(
-            decode.decoded_image.image().get()));
+        if (do_yuv_decode_) {
+          DrawImage draw_image = draw_and_decoded_draw_image.image;
+          for (size_t i = 0; i < SkYUVASizeInfo::kMaxCount; ++i) {
+            // TODO(crbug.com/910276): Skip alpha plane until supported in
+            // cache.
+            if (i != SkYUVAIndex::kA_Index) {
+              SkImage* plane_image =
+                  cache->GetUploadedPlaneForTesting(draw_image, i).get();
+              discardable_manager_.ExpectLocked(
+                  GpuImageDecodeCache::GlIdFromSkImage(plane_image));
+            }
+          }
+        } else {
+          discardable_manager_.ExpectLocked(
+              GpuImageDecodeCache::GlIdFromSkImage(
+                  draw_and_decoded_draw_image.decoded_image.image().get()));
+        }
       }
-      cache->DrawWithImageFinished(decode.image, decode.decoded_image);
-      cache->UnrefImage(decode.image);
+      cache->DrawWithImageFinished(draw_and_decoded_draw_image.image,
+                                   draw_and_decoded_draw_image.decoded_image);
+      cache->UnrefImage(draw_and_decoded_draw_image.image);
     }
   }
 }