Commit 5a522f9e authored by Reza.Zakerinasab's avatar Reza.Zakerinasab Committed by Commit Bot

Minor code health improvements in ImageBitmap and ImageBuffer

This CL contains minor changes in ImageBitmap and ImageBuffer to:

- use blending in SkCanvas::drawImage() only if needed
- fix ImageBitmap->CopyImageData() behavior to match the signature
- use SkImageInfo::minRowBytes() in SkImage::readPixels() call sites

Bug: 785313,788879,788881
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I1473fc71ce1785f87c0f3e47441515f6cd3dd7a7
Reviewed-on: https://chromium-review.googlesource.com/793951
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Reviewed-by: default avatarJustin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519755}
parent 48aee9ac
...@@ -171,17 +171,20 @@ SkImageInfo GetSkImageInfo(const scoped_refptr<StaticBitmapImage>& image) { ...@@ -171,17 +171,20 @@ SkImageInfo GetSkImageInfo(const scoped_refptr<StaticBitmapImage>& image) {
} }
// This function results in a readback due to using SkImage::readPixels(). // This function results in a readback due to using SkImage::readPixels().
// Returns transparent black pixels if the input SkImageInfo.bounds() does
// not intersect with the input image boundaries.
scoped_refptr<Uint8Array> CopyImageData( scoped_refptr<Uint8Array> CopyImageData(
const scoped_refptr<StaticBitmapImage>& input, const scoped_refptr<StaticBitmapImage>& input,
const SkImageInfo& info) { const SkImageInfo& info,
const unsigned x = 0,
const unsigned y = 0) {
if (info.isEmpty()) if (info.isEmpty())
return nullptr; return nullptr;
sk_sp<SkImage> sk_image = input->PaintImageForCurrentFrame().GetSkImage(); sk_sp<SkImage> sk_image = input->PaintImageForCurrentFrame().GetSkImage();
if (sk_image->bounds().isEmpty()) if (sk_image->bounds().isEmpty())
return nullptr; return nullptr;
unsigned width = static_cast<unsigned>(input->width());
scoped_refptr<ArrayBuffer> dst_buffer = scoped_refptr<ArrayBuffer> dst_buffer =
ArrayBuffer::CreateOrNull(width * input->height(), info.bytesPerPixel()); ArrayBuffer::CreateOrNull(info.computeMinByteSize(), 1);
if (!dst_buffer) if (!dst_buffer)
return nullptr; return nullptr;
unsigned byte_length = dst_buffer->ByteLength(); unsigned byte_length = dst_buffer->ByteLength();
...@@ -189,8 +192,8 @@ scoped_refptr<Uint8Array> CopyImageData( ...@@ -189,8 +192,8 @@ scoped_refptr<Uint8Array> CopyImageData(
Uint8Array::Create(std::move(dst_buffer), 0, byte_length); Uint8Array::Create(std::move(dst_buffer), 0, byte_length);
if (!dst_pixels) if (!dst_pixels)
return nullptr; return nullptr;
bool read_pixels_successful = sk_image->readPixels( bool read_pixels_successful =
info, dst_pixels->Data(), width * info.bytesPerPixel(), 0, 0); sk_image->readPixels(info, dst_pixels->Data(), info.minRowBytes(), x, y);
DCHECK(read_pixels_successful); DCHECK(read_pixels_successful);
if (!read_pixels_successful) if (!read_pixels_successful)
return nullptr; return nullptr;
...@@ -260,7 +263,9 @@ scoped_refptr<StaticBitmapImage> FlipImageVertically( ...@@ -260,7 +263,9 @@ scoped_refptr<StaticBitmapImage> FlipImageVertically(
SkCanvas* canvas = surface->getCanvas(); SkCanvas* canvas = surface->getCanvas();
canvas->scale(1, -1); canvas->scale(1, -1);
canvas->translate(0, -input->height()); canvas->translate(0, -input->height());
canvas->drawImage(image.get(), 0, 0); SkPaint paint;
paint.setBlendMode(SkBlendMode::kSrc);
canvas->drawImage(image.get(), 0, 0, &paint);
return StaticBitmapImage::Create(surface->makeImageSnapshot(), return StaticBitmapImage::Create(surface->makeImageSnapshot(),
input->ContextProviderWrapper()); input->ContextProviderWrapper());
} }
...@@ -548,7 +553,9 @@ static scoped_refptr<StaticBitmapImage> CropImageAndApplyColorSpaceConversion( ...@@ -548,7 +553,9 @@ static scoped_refptr<StaticBitmapImage> CropImageAndApplyColorSpaceConversion(
sk_sp<SkSurface> surface = sk_sp<SkSurface> surface =
SkSurface::MakeRaster(GetSkImageInfo(StaticBitmapImage::Create( SkSurface::MakeRaster(GetSkImageInfo(StaticBitmapImage::Create(
skia_image, image->ContextProviderWrapper()))); skia_image, image->ContextProviderWrapper())));
surface->getCanvas()->drawImage(skia_image.get(), 0, 0); SkPaint paint;
paint.setBlendMode(SkBlendMode::kSrc);
surface->getCanvas()->drawImage(skia_image.get(), 0, 0, &paint);
skia_image = surface->makeImageSnapshot(); skia_image = surface->makeImageSnapshot();
} }
} }
......
...@@ -399,9 +399,8 @@ TEST_F(ImageBitmapTest, MAYBE_ImageBitmapColorSpaceConversionImageBitmap) { ...@@ -399,9 +399,8 @@ TEST_F(ImageBitmapTest, MAYBE_ImageBitmapColorSpaceConversionImageBitmap) {
std::unique_ptr<uint8_t[]> src_pixel( std::unique_ptr<uint8_t[]> src_pixel(
new uint8_t[raster_image_info.bytesPerPixel()]()); new uint8_t[raster_image_info.bytesPerPixel()]());
EXPECT_TRUE(image->readPixels( EXPECT_TRUE(image->readPixels(raster_image_info.makeWH(1, 1), src_pixel.get(),
raster_image_info.makeWH(1, 1), src_pixel.get(), raster_image_info.minRowBytes(), 5, 5));
image->width() * raster_image_info.bytesPerPixel(), 5, 5));
ImageResourceContent* source_image_resource = ImageResourceContent* source_image_resource =
ImageResourceContent::CreateLoaded( ImageResourceContent::CreateLoaded(
......
...@@ -274,14 +274,13 @@ bool ImageBuffer::GetImageData(const IntRect& rect, ...@@ -274,14 +274,13 @@ bool ImageBuffer::GetImageData(const IntRect& rect,
bool* is_gpu_readback_invoked) const { bool* is_gpu_readback_invoked) const {
uint8_t bytes_per_pixel = surface_->ColorParams().BytesPerPixel(); uint8_t bytes_per_pixel = surface_->ColorParams().BytesPerPixel();
CheckedNumeric<int> data_size = bytes_per_pixel; CheckedNumeric<int> data_size = bytes_per_pixel;
data_size *= rect.Width(); data_size *= rect.Size().Area();
data_size *= rect.Height();
if (!data_size.IsValid() || if (!data_size.IsValid() ||
data_size.ValueOrDie() > v8::TypedArray::kMaxLength) data_size.ValueOrDie() > v8::TypedArray::kMaxLength)
return false; return false;
if (!IsSurfaceValid()) { if (!IsSurfaceValid()) {
size_t alloc_size_in_bytes = rect.Width() * rect.Height() * bytes_per_pixel; size_t alloc_size_in_bytes = rect.Size().Area() * bytes_per_pixel;
auto data = WTF::ArrayBufferContents::CreateDataHandle( auto data = WTF::ArrayBufferContents::CreateDataHandle(
alloc_size_in_bytes, WTF::ArrayBufferContents::kZeroInitialize); alloc_size_in_bytes, WTF::ArrayBufferContents::kZeroInitialize);
if (!data) if (!data)
...@@ -304,7 +303,7 @@ bool ImageBuffer::GetImageData(const IntRect& rect, ...@@ -304,7 +303,7 @@ bool ImageBuffer::GetImageData(const IntRect& rect,
|| rect.X() < 0 || rect.Y() < 0 || || rect.X() < 0 || rect.Y() < 0 ||
rect.MaxX() > surface_->Size().Width() || rect.MaxX() > surface_->Size().Width() ||
rect.MaxY() > surface_->Size().Height(); rect.MaxY() > surface_->Size().Height();
size_t alloc_size_in_bytes = rect.Width() * rect.Height() * bytes_per_pixel; size_t alloc_size_in_bytes = rect.Size().Area() * bytes_per_pixel;
WTF::ArrayBufferContents::InitializationPolicy initialization_policy = WTF::ArrayBufferContents::InitializationPolicy initialization_policy =
may_have_stray_area ? WTF::ArrayBufferContents::kZeroInitialize may_have_stray_area ? WTF::ArrayBufferContents::kZeroInitialize
: WTF::ArrayBufferContents::kDontInitialize; : WTF::ArrayBufferContents::kDontInitialize;
...@@ -323,7 +322,7 @@ bool ImageBuffer::GetImageData(const IntRect& rect, ...@@ -323,7 +322,7 @@ bool ImageBuffer::GetImageData(const IntRect& rect,
kUnpremul_SkAlphaType); kUnpremul_SkAlphaType);
sk_sp<SkImage> sk_image = snapshot->PaintImageForCurrentFrame().GetSkImage(); sk_sp<SkImage> sk_image = snapshot->PaintImageForCurrentFrame().GetSkImage();
bool read_pixels_successful = sk_image->readPixels( bool read_pixels_successful = sk_image->readPixels(
info, result.Data(), bytes_per_pixel * rect.Width(), rect.X(), rect.Y()); info, result.Data(), info.minRowBytes(), rect.X(), rect.Y());
DCHECK(read_pixels_successful || DCHECK(read_pixels_successful ||
!sk_image->bounds().intersect(SkIRect::MakeXYWH( !sk_image->bounds().intersect(SkIRect::MakeXYWH(
rect.X(), rect.Y(), info.width(), info.height()))); rect.X(), rect.Y(), info.width(), info.height())));
...@@ -410,6 +409,8 @@ void ImageBuffer::SetSurface(std::unique_ptr<ImageBufferSurface> surface) { ...@@ -410,6 +409,8 @@ void ImageBuffer::SetSurface(std::unique_ptr<ImageBufferSurface> surface) {
image = nullptr; image = nullptr;
image = StaticBitmapImage::Create(texture_image->makeNonTextureImage()); image = StaticBitmapImage::Create(texture_image->makeNonTextureImage());
} }
SkPaint paint;
paint.setBlendMode(SkBlendMode::kSrc);
surface->Canvas()->drawImage(image->PaintImageForCurrentFrame(), 0, 0); surface->Canvas()->drawImage(image->PaintImageForCurrentFrame(), 0, 0);
surface->SetImageBuffer(this); surface->SetImageBuffer(this);
surface_ = std::move(surface); surface_ = std::move(surface);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment