Commit 9e1fa761 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[graphics] Remove direct use of ArrayBufferContents

In the context of an ongoing refactoring of WTF::ArrayBuffers we move
the implementation out of libwtf. The idea is to merge it into
DOMArrayBuffer eventually and get rid of WTF::ArrayBuffer completely.
StaticBitmapImage included WTF::ArrayBufferContents but cannot include
anything in third_party/blink/renderer/core/typed_arrays/*. With this CL
I remove the direct creation of ArrayBufferContents in
StaticBitmapImage.

The idea behind the CL is that instead of creating an
ArrayBufferContents in ConvertToArrayBufferContents, we just pass a
pointer to the backing store of the ArrayBufferContents, and let
StaticBitmapImage just fill that backing store with data. I renamed the
function therefore to CopyToByteArray.

To be able to allocate an ArrayBufferContents of the right size,
StaticBitmapImage now also provides a SizeInBytes function, and
MayHaveStrayArea to indicate if the backing store needs to be
zero-initialized.

Alternative implementations would be to just fill a WTF::Vector, and
then copy the result into an ArrayBufferContents, or to provide a
producer function to StaticBitmapImage so that it can allocate an
ArrayBufferContents in an opaque way. I did not like both, the former
requires extra copying, the latter is too complicated.

R=jbroman@chromium.org

Bug: chromium:1008840
Change-Id: I0b283dd9605f31905f3cb0d65615fc85d84e8f63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1871674Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709065}
parent 621ae172
...@@ -262,8 +262,21 @@ static bool ExtractImageData(Image* image, ...@@ -262,8 +262,21 @@ static bool ExtractImageData(Image* image,
kDoNotRespectImageOrientation, kDoNotRespectImageOrientation,
Image::kDoNotClampImageToSourceRect, Image::kSyncDecode); Image::kDoNotClampImageToSourceRect, Image::kSyncDecode);
return StaticBitmapImage::ConvertToArrayBufferContents( size_t size_in_bytes =
StaticBitmapImage::Create(surface->makeImageSnapshot()), contents, StaticBitmapImage::GetSizeInBytes(image_dest_rect, color_params);
if (size_in_bytes > v8::TypedArray::kMaxLength)
return false;
WTF::ArrayBufferContents result(size_in_bytes, 1,
WTF::ArrayBufferContents::kNotShared,
WTF::ArrayBufferContents::kZeroInitialize);
if (result.DataLength() != size_in_bytes)
return false;
result.Transfer(contents);
return StaticBitmapImage::CopyToByteArray(
StaticBitmapImage::Create(surface->makeImageSnapshot()),
base::span<uint8_t>(reinterpret_cast<uint8_t*>(contents.Data()),
contents.DataLength()),
image_dest_rect, color_params); image_dest_rect, color_params);
} }
......
...@@ -1594,8 +1594,6 @@ ImageData* BaseRenderingContext2D::getImageData( ...@@ -1594,8 +1594,6 @@ ImageData* BaseRenderingContext2D::getImageData(
return result; return result;
} }
WTF::ArrayBufferContents contents;
const CanvasColorParams& color_params = ColorParams(); const CanvasColorParams& color_params = ColorParams();
// Deferred offscreen canvases might have recorded commands, make sure // Deferred offscreen canvases might have recorded commands, make sure
// that those get drawn here // that those get drawn here
...@@ -1606,12 +1604,35 @@ ImageData* BaseRenderingContext2D::getImageData( ...@@ -1606,12 +1604,35 @@ ImageData* BaseRenderingContext2D::getImageData(
if (IsAccelerated()) if (IsAccelerated())
DisableAcceleration(); DisableAcceleration();
if (!StaticBitmapImage::ConvertToArrayBufferContents( size_t size_in_bytes =
snapshot, contents, image_data_rect, color_params, IsAccelerated())) { StaticBitmapImage::GetSizeInBytes(image_data_rect, color_params);
if (size_in_bytes > v8::TypedArray::kMaxLength)
return nullptr;
bool may_have_stray_area =
IsAccelerated() // GPU readback may fail silently.
|| StaticBitmapImage::MayHaveStrayArea(snapshot, image_data_rect);
WTF::ArrayBufferContents::InitializationPolicy initialization_policy =
may_have_stray_area ? WTF::ArrayBufferContents::kZeroInitialize
: WTF::ArrayBufferContents::kDontInitialize;
WTF::ArrayBufferContents contents(size_in_bytes, 1,
WTF::ArrayBufferContents::kNotShared,
initialization_policy);
if (contents.DataLength() != size_in_bytes) {
exception_state.ThrowRangeError("Out of memory at ImageData creation"); exception_state.ThrowRangeError("Out of memory at ImageData creation");
return nullptr; return nullptr;
} }
if (!StaticBitmapImage::CopyToByteArray(
snapshot,
base::span<uint8_t>(reinterpret_cast<uint8_t*>(contents.Data()),
contents.DataLength()),
image_data_rect, color_params)) {
exception_state.ThrowRangeError("Failed to copy image data");
return nullptr;
}
// Convert pixels to proper storage format if needed // Convert pixels to proper storage format if needed
if (PixelFormat() != kRGBA8CanvasPixelFormat) { if (PixelFormat() != kRGBA8CanvasPixelFormat) {
ImageDataStorageFormat storage_format = ImageDataStorageFormat storage_format =
......
...@@ -82,49 +82,35 @@ scoped_refptr<StaticBitmapImage> StaticBitmapImage::ConvertToColorSpace( ...@@ -82,49 +82,35 @@ scoped_refptr<StaticBitmapImage> StaticBitmapImage::ConvertToColorSpace(
: nullptr); : nullptr);
} }
bool StaticBitmapImage::ConvertToArrayBufferContents( size_t StaticBitmapImage::GetSizeInBytes(
scoped_refptr<StaticBitmapImage> src_image,
WTF::ArrayBufferContents& dest_contents,
const IntRect& rect, const IntRect& rect,
const CanvasColorParams& color_params, const CanvasColorParams& color_params) {
bool is_accelerated) {
uint8_t bytes_per_pixel = color_params.BytesPerPixel(); uint8_t bytes_per_pixel = color_params.BytesPerPixel();
base::CheckedNumeric<int> data_size = bytes_per_pixel; base::CheckedNumeric<size_t> data_size = bytes_per_pixel;
data_size *= rect.Size().Area(); data_size *= rect.Size().Area();
if (!data_size.IsValid() || return data_size.ValueOrDefault(0);
data_size.ValueOrDie() > v8::TypedArray::kMaxLength) }
return false;
int alloc_size_in_bytes = data_size.ValueOrDie();
if (!src_image) {
WTF::ArrayBufferContents result(alloc_size_in_bytes, 1,
WTF::ArrayBufferContents::kNotShared,
WTF::ArrayBufferContents::kZeroInitialize);
// Check if the ArrayBuffer backing store was allocated correctly. bool StaticBitmapImage::MayHaveStrayArea(
if (result.DataLength() != static_cast<size_t>(alloc_size_in_bytes)) { scoped_refptr<StaticBitmapImage> src_image,
const IntRect& rect) {
if (!src_image)
return false; return false;
}
result.Transfer(dest_contents);
return true;
}
const bool may_have_stray_area = return rect.X() < 0 || rect.Y() < 0 ||
is_accelerated // GPU readback may fail silently
|| rect.X() < 0 || rect.Y() < 0 ||
rect.MaxX() > src_image->Size().Width() || rect.MaxX() > src_image->Size().Width() ||
rect.MaxY() > src_image->Size().Height(); rect.MaxY() > src_image->Size().Height();
WTF::ArrayBufferContents::InitializationPolicy initialization_policy = }
may_have_stray_area ? WTF::ArrayBufferContents::kZeroInitialize
: WTF::ArrayBufferContents::kDontInitialize; bool StaticBitmapImage::CopyToByteArray(
scoped_refptr<StaticBitmapImage> src_image,
WTF::ArrayBufferContents result(alloc_size_in_bytes, 1, base::span<uint8_t> dst,
WTF::ArrayBufferContents::kNotShared, const IntRect& rect,
initialization_policy); const CanvasColorParams& color_params) {
// Check if the ArrayBuffer backing store was allocated correctly. DCHECK_EQ(dst.size(), GetSizeInBytes(rect, color_params));
if (result.DataLength() != static_cast<size_t>(alloc_size_in_bytes)) {
return false; if (!src_image)
} return true;
SkColorType color_type = SkColorType color_type =
(color_params.GetSkColorType() == kRGBA_F16_SkColorType) (color_params.GetSkColorType() == kRGBA_F16_SkColorType)
...@@ -137,11 +123,10 @@ bool StaticBitmapImage::ConvertToArrayBufferContents( ...@@ -137,11 +123,10 @@ bool StaticBitmapImage::ConvertToArrayBufferContents(
if (!sk_image) if (!sk_image)
return false; return false;
bool read_pixels_successful = sk_image->readPixels( bool read_pixels_successful = sk_image->readPixels(
info, result.Data(), info.minRowBytes(), rect.X(), rect.Y()); info, dst.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())));
result.Transfer(dest_contents);
return true; return true;
} }
......
...@@ -15,12 +15,6 @@ ...@@ -15,12 +15,6 @@
#include "third_party/khronos/GLES2/gl2.h" #include "third_party/khronos/GLES2/gl2.h"
#include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/skia/include/core/SkRefCnt.h"
namespace WTF {
class ArrayBufferContents;
} // namespace WTF
namespace gpu { namespace gpu {
namespace gles2 { namespace gles2 {
class GLES2Interface; class GLES2Interface;
...@@ -114,12 +108,16 @@ class PLATFORM_EXPORT StaticBitmapImage : public Image { ...@@ -114,12 +108,16 @@ class PLATFORM_EXPORT StaticBitmapImage : public Image {
sk_sp<SkColorSpace>, sk_sp<SkColorSpace>,
SkColorType = kN32_SkColorType); SkColorType = kN32_SkColorType);
static bool ConvertToArrayBufferContents( static size_t GetSizeInBytes(const IntRect& rect,
scoped_refptr<StaticBitmapImage> src_image, const CanvasColorParams& color_params);
WTF::ArrayBufferContents& dest_contents,
static bool MayHaveStrayArea(scoped_refptr<StaticBitmapImage> src_image,
const IntRect& rect);
static bool CopyToByteArray(scoped_refptr<StaticBitmapImage> src_image,
base::span<uint8_t> dst,
const IntRect&, const IntRect&,
const CanvasColorParams&, const CanvasColorParams&);
bool is_accelerated = false);
protected: protected:
// Helper for sub-classes // Helper for sub-classes
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#include "third_party/blink/renderer/platform/graphics/static_bitmap_image.h" #include "third_party/blink/renderer/platform/graphics/static_bitmap_image.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_contents.h"
#include "third_party/skia/include/core/SkSurface.h" #include "third_party/skia/include/core/SkSurface.h"
#include "v8/include/v8.h" #include "v8/include/v8.h"
...@@ -27,9 +26,9 @@ TEST_F(StaticBitmapImageTest, ...@@ -27,9 +26,9 @@ TEST_F(StaticBitmapImageTest,
IntRect too_big_rect(IntPoint(0, 0), IntRect too_big_rect(IntPoint(0, 0),
IntSize(1, (v8::TypedArray::kMaxLength / 4) + 1)); IntSize(1, (v8::TypedArray::kMaxLength / 4) + 1));
WTF::ArrayBufferContents contents; EXPECT_GT(
EXPECT_FALSE(StaticBitmapImage::ConvertToArrayBufferContents( StaticBitmapImage::GetSizeInBytes(too_big_rect, CanvasColorParams()),
image, contents, too_big_rect, CanvasColorParams())); v8::TypedArray::kMaxLength);
} }
} // namespace blink } // namespace blink
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