gpu: Fix pixmap alignment issue

When working with YUV images, uses the stride of the plane when creating
pixmaps for each plane.

Bug: 919627
Bug: 1023090
Change-Id: I1bf1dd5f0ac96a2eac5a8f4bb2f73453042fe7df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906954
Commit-Queue: Sasha McIntosh <sashamcintosh@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715096}

cc/paint/image_transfer_cache_entry.cc

@@ -216,6 +216,7 @@ ClientImageTransferCacheEntry::ClientImageTransferCacheEntry(
   safe_size += decoded_color_space_size + align;
   safe_size += num_planes_ * sizeof(uint64_t);  // plane widths
   safe_size += num_planes_ * sizeof(uint64_t);  // plane heights
+  safe_size += num_planes_ * sizeof(uint64_t);  // plane strides
   safe_size +=
       num_planes_ * (sizeof(uint64_t) + align);  // pixels size + alignment
   // Include 4 bytes of padding before each plane data chunk so we can always
@@ -249,6 +250,7 @@ void ClientImageTransferCacheEntry::ValidateYUVDataBeforeSerializing() const {
     const SkPixmap* plane = yuv_pixmaps_->at(i);
     DCHECK_GT(plane->width(), 0);
     DCHECK_GT(plane->height(), 0);
+    DCHECK_GT(plane->rowBytesAsPixels(), 0);
   }
 }
 
@@ -272,6 +274,7 @@ bool ClientImageTransferCacheEntry::Serialize(base::span<uint8_t> data) const {
       const SkPixmap* plane = yuv_pixmaps_->at(i);
       writer.Write(plane->width());
       writer.Write(plane->height());
+      writer.Write(plane->rowBytesAsPixels());
       size_t plane_size = plane->computeByteSize();
       if (plane_size == SIZE_MAX)
         return false;
@@ -402,6 +405,8 @@ bool ServiceImageTransferCacheEntry::Deserialize(
       reader.Read(&plane_width);
       uint32_t plane_height = 0;
       reader.Read(&plane_height);
+      uint32_t plane_stride = 0;
+      reader.Read(&plane_stride);
       // Because Skia does not support YUV rasterization from software planes,
       // we require that each pixmap fits in a GPU texture. In the
       // GpuImageDecodeCache, we veto YUV decoding if the planes would be too
@@ -434,7 +439,7 @@ bool ServiceImageTransferCacheEntry::Deserialize(
       // are OK with this as the worst case scenario is visual corruption.
       SkPixmap plane_pixmap(plane_pixmap_info,
                             const_cast<const void*>(plane_pixel_data),
-                            plane_pixmap_info.minRowBytes());
+                            plane_stride);
 
       // Nothing should read the colorspace of individual planes because that
       // information is stored in image_, so we pass nullptr.

cc/paint/image_transfer_cache_entry_unittest.cc

@@ -46,9 +46,10 @@ void MarkTextureAsReleased(SkImage::ReleaseContext context) {
   *released = true;
 }
 
-// Checks if all the pixels in |image| are |expected_color|.
-bool CheckImageIsSolidColor(const sk_sp<SkImage>& image,
-                            SkColor expected_color) {
+// Checks if all the pixels in the |subset| of |image| are |expected_color|.
+bool CheckRectIsSolidColor(const sk_sp<SkImage>& image,
+                           SkColor expected_color,
+                           const SkIRect& subset) {
   DCHECK_GE(image->width(), 1);
   DCHECK_GE(image->height(), 1);
   SkBitmap bitmap;
@@ -59,8 +60,8 @@ bool CheckImageIsSolidColor(const sk_sp<SkImage>& image,
     return false;
   if (!image->readPixels(pixmap, 0 /* srcX */, 0 /* srcY */))
     return false;
-  for (int y = 0; y < image->height(); y++) {
-    for (int x = 0; x < image->width(); x++) {
+  for (int y = subset.fTop; y < subset.fBottom; y++) {
+    for (int x = subset.fLeft; x < subset.fRight; x++) {
       if (bitmap.getColor(x, y) != expected_color)
         return false;
     }
@@ -68,6 +69,13 @@ bool CheckImageIsSolidColor(const sk_sp<SkImage>& image,
   return true;
 }
 
+// Checks if all the pixels in |image| are |expected_color|.
+bool CheckImageIsSolidColor(const sk_sp<SkImage>& image,
+                            SkColor expected_color) {
+  return CheckRectIsSolidColor(
+      image, expected_color, SkIRect::MakeWH(image->width(), image->height()));
+}
+
 class ImageTransferCacheEntryTest
     : public testing::TestWithParam<YUVDecodeFormat> {
  public:
@@ -198,6 +206,74 @@ class ImageTransferCacheEntryTest
   sk_sp<GrContext> gr_context_;
 };
 
+TEST_P(ImageTransferCacheEntryTest, Deserialize) {
+#if defined(OS_ANDROID)
+  // TODO(crbug.com/985458): this test is failing on Android for NV12 and we
+  // don't understand why yet. Revisit this once Skia supports an RG8
+  // SkColorType.
+  if (GetParam() == YUVDecodeFormat::kYUV2)
+    return;
+#endif
+
+  // Create a client-side entry from YUV planes. Use a different stride than the
+  // width to test that alignment works correctly.
+  const int image_width = 12;
+  const int image_height = 10;
+  const size_t y_stride = 16;
+  const size_t uv_stride = 8;
+
+  const size_t y_bytes = y_stride * image_height;
+  const size_t uv_bytes = uv_stride * image_height / 2;
+  const size_t planes_size = y_bytes + 2 * uv_bytes;
+  std::unique_ptr<char[]> planes_data(new char[planes_size]);
+
+  void* planes[3];
+  planes[0] = reinterpret_cast<void*>(planes_data.get());
+  planes[1] = ((char*)planes[0]) + y_bytes;
+  planes[2] = ((char*)planes[1]) + uv_bytes;
+
+  auto info = SkImageInfo::Make(image_width, image_height, kGray_8_SkColorType,
+                                kUnknown_SkAlphaType);
+  SkPixmap y_pixmap(info, planes[0], y_stride);
+  SkPixmap u_pixmap(info.makeWH(image_width / 2, image_height / 2), planes[1],
+                    uv_stride);
+  SkPixmap v_pixmap(info.makeWH(image_width / 2, image_height / 2), planes[2],
+                    uv_stride);
+
+  // rgb (255, 121, 255) -> yuv (255, 255, 255)
+  const SkIRect bottom_color_rect =
+      SkIRect::MakeXYWH(0, image_height / 2, image_width, image_height / 2);
+  ASSERT_TRUE(y_pixmap.erase(SkColors::kWhite));
+  ASSERT_TRUE(u_pixmap.erase(SkColors::kWhite));
+  ASSERT_TRUE(v_pixmap.erase(SkColors::kWhite));
+  // rgb (178, 0, 225) -> yuv (0, 255, 255)
+  const SkIRect top_color_rect = SkIRect::MakeWH(image_width, image_height / 2);
+  ASSERT_TRUE(y_pixmap.erase(SkColors::kBlack, &top_color_rect));
+
+  auto client_entry(std::make_unique<ClientImageTransferCacheEntry>(
+      &y_pixmap, &u_pixmap, &v_pixmap, nullptr, kJpegYUVColorSpace,
+      true /* needs_mips */));
+  uint32_t size = client_entry->SerializedSize();
+  std::vector<uint8_t> data(size);
+  ASSERT_TRUE(client_entry->Serialize(
+      base::make_span(static_cast<uint8_t*>(data.data()), size)));
+
+  // Create service-side entry from the client-side serialize info
+  auto entry(std::make_unique<ServiceImageTransferCacheEntry>());
+  ASSERT_TRUE(entry->Deserialize(
+      gr_context(), base::make_span(static_cast<uint8_t*>(data.data()), size)));
+  ASSERT_TRUE(entry->is_yuv());
+
+  // Check color of pixels
+  ASSERT_TRUE(CheckRectIsSolidColor(entry->image(), SkColorSetRGB(178, 0, 225),
+                                    top_color_rect));
+  ASSERT_TRUE(CheckRectIsSolidColor(
+      entry->image(), SkColorSetRGB(255, 121, 255), bottom_color_rect));
+
+  client_entry.reset();
+  entry.reset();
+}
+
 TEST_P(ImageTransferCacheEntryTest, HardwareDecodedNoMipsAtCreation) {
   std::unique_ptr<bool[]> release_flags;
   std::vector<sk_sp<SkImage>> plane_images = CreateTestYUVImage(&release_flags);