Skip to content

Commit

Permalink
WebGL: Make TexImageImpl use generic mechanisms
Browse files Browse the repository at this point in the history 
Add an overload of WebGLRenderingContextBase::TexImageImpl that takes a
TexImageParams, and call that from the non-TexImageParams version of
the function.

Change ImagePixelLocker to expose the underlying SkPixmap, and change
ImageExtractor to expose its ImagePixelLocker's SkPixmap.

Change TexImageParams to call TexImageSkPixmap.

Bug: 1310815
Change-Id: I217ddb5c918571dc78fc400ef535c37078d792b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3570398
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: ccameron chromium <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989576}
  • Loading branch information
@ccameron-chromium
ccameron-chromium authored and Chromium LUCI CQ committed Apr 6, 2022
1 parent 66dc513 commit badb39a
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 125 deletions.
154 changes: 67 additions & 87 deletions third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
View file
Expand Up @@ -5125,96 +5125,81 @@ void WebGLRenderingContextBase::TexImageImpl(
GLenum type,
Image* image,
WebGLImageConversion::ImageHtmlDomSource dom_source,
bool flip_y,
bool premultiply_alpha,
bool source_has_flip_y,
const absl::optional<gfx::Rect>& source_image_rect,
GLsizei depth,
GLint unpack_image_height) {
const char* func_name = GetTexImageFunctionName(function_id);
// All calling functions check isContextLost, so a duplicate check is not
// needed here.
if (type == GL_UNSIGNED_INT_10F_11F_11F_REV) {
// The UNSIGNED_INT_10F_11F_11F_REV type pack/unpack isn't implemented.
type = GL_FLOAT;
TexImageParams params = {
.function_id = function_id,
.target = target,
.level = level,
.internalformat = internalformat,
.xoffset = xoffset,
.yoffset = yoffset,
.zoffset = zoffset,
.depth = depth,
.format = format,
.type = type,
};
if (source_image_rect) {
params.width = source_image_rect->width();
params.height = source_image_rect->height();
params.depth = depth;
}
Vector<uint8_t> data;

gfx::Rect sub_rect = source_image_rect.value_or(
// Fall back to the rect based on the size of the Image.
SafeGetImageSize(image));
GetCurrentUnpackState(params);
TexImageImpl(params, image, dom_source, source_has_flip_y);
}

bool selecting_sub_rectangle = false;
if (!ValidateTexImageSubRectangle(func_name, function_id, image, sub_rect,
depth, unpack_image_height,
&selecting_sub_rectangle)) {
return;
}
void WebGLRenderingContextBase::TexImageImpl(
TexImageParams params,
Image* image,
WebGLImageConversion::ImageHtmlDomSource dom_source,
bool image_has_flip_y) {
// All calling functions check isContextLost, so a duplicate check is not
// needed here.
const char* func_name = GetTexImageFunctionName(params.function_id);

// Adjust the source image rectangle if doing a y-flip.
gfx::Rect adjusted_source_image_rect = sub_rect;
if (flip_y) {
adjusted_source_image_rect.set_y(image->height() -
adjusted_source_image_rect.bottom());
if (!params.width || !params.height) {
auto image_size = SafeGetImageSize(image);
params.width = image_size.width();
params.height = image_size.height();
}
if (!params.depth)
params.depth = 1;

WebGLImageConversion::ImageExtractor image_extractor(
image, dom_source, premultiply_alpha,
image, dom_source, params.unpack_premultiply_alpha,
unpack_colorspace_conversion_ == GL_NONE);
if (!image_extractor.ImagePixelData()) {
const SkPixmap* const image_extractor_pixmap = image_extractor.GetSkPixmap();
if (!image_extractor_pixmap) {
SynthesizeGLError(GL_INVALID_VALUE, func_name, "bad image data");
return;
}
DCHECK_EQ(image_extractor_pixmap->width(), image->width());
DCHECK_EQ(image_extractor_pixmap->height(), image->height());

WebGLImageConversion::DataFormat source_data_format =
image_extractor.ImageSourceFormat();
// ImageExtractor indicates a specific AlphaOp. Ensure that `params` and
// `pixmap`'s alpha type are configured to ensure operation be performed.
SkPixmap pixmap = *image_extractor_pixmap;
WebGLImageConversion::AlphaOp alpha_op = image_extractor.ImageAlphaOp();
const void* image_pixel_data = image_extractor.ImagePixelData();

bool need_conversion = true;
if (type == GL_UNSIGNED_BYTE &&
source_data_format == WebGLImageConversion::kDataFormatRGBA8 &&
format == GL_RGBA && alpha_op == WebGLImageConversion::kAlphaDoNothing &&
!flip_y && !selecting_sub_rectangle && depth == 1) {
need_conversion = false;
} else {
if (!WebGLImageConversion::PackImageData(
image, image_pixel_data, format, type, flip_y, alpha_op,
source_data_format, image_extractor.ImageWidth(),
image_extractor.ImageHeight(), adjusted_source_image_rect, depth,
image_extractor.ImageSourceUnpackAlignment(), unpack_image_height,
data)) {
SynthesizeGLError(GL_INVALID_VALUE, func_name, "packImage error");
return;
}
switch (alpha_op) {
case WebGLImageConversion::kAlphaDoNothing:
params.unpack_premultiply_alpha =
pixmap.alphaType() == kPremul_SkAlphaType;
break;
case WebGLImageConversion::kAlphaDoPremultiply:
params.unpack_premultiply_alpha = true;
pixmap.reset(pixmap.info().makeAlphaType(kUnpremul_SkAlphaType),
pixmap.addr(), pixmap.rowBytes());
break;
case WebGLImageConversion::kAlphaDoUnmultiply:
params.unpack_premultiply_alpha = false;
pixmap.reset(pixmap.info().makeAlphaType(kPremul_SkAlphaType),
pixmap.addr(), pixmap.rowBytes());
break;
}

ScopedUnpackParametersResetRestore temporary_reset_unpack(this);
if (function_id == kTexImage2D) {
TexImage2DBase(target, level, internalformat,
adjusted_source_image_rect.width(),
adjusted_source_image_rect.height(), 0, format, type,
need_conversion ? data.data() : image_pixel_data);
} else if (function_id == kTexSubImage2D) {
ContextGL()->TexSubImage2D(
target, level, xoffset, yoffset, adjusted_source_image_rect.width(),
adjusted_source_image_rect.height(), format, type,
need_conversion ? data.data() : image_pixel_data);
} else {
// 3D functions.
if (function_id == kTexImage3D) {
ContextGL()->TexImage3D(
target, level, internalformat, adjusted_source_image_rect.width(),
adjusted_source_image_rect.height(), depth, 0, format, type,
need_conversion ? data.data() : image_pixel_data);
} else {
DCHECK_EQ(function_id, kTexSubImage3D);
ContextGL()->TexSubImage3D(
target, level, xoffset, yoffset, zoffset,
adjusted_source_image_rect.width(),
adjusted_source_image_rect.height(), depth, format, type,
need_conversion ? data.data() : image_pixel_data);
}
}
TexImageSkPixmap(params, &pixmap, image_has_flip_y);
}

bool WebGLRenderingContextBase::ValidateTexFunc(
Expand Down Expand Up @@ -5654,9 +5639,8 @@ void WebGLRenderingContextBase::TexImageHelperHTMLImageElement(

TexImageImpl(function_id, target, level, internalformat, xoffset, yoffset,
zoffset, format, type, image_for_render.get(),
WebGLImageConversion::kHtmlDomImage, unpack_flip_y_,
unpack_premultiply_alpha_, source_image_rect, depth,
unpack_image_height);
WebGLImageConversion::kHtmlDomImage, /*source_has_flip_y=*/false,
source_image_rect, depth, unpack_image_height);
}

void WebGLRenderingContextBase::texImage2D(ExecutionContext* execution_context,
Expand Down Expand Up @@ -5932,15 +5916,11 @@ void WebGLRenderingContextBase::TexImageHelperCanvasRenderingContextHost(
// TODO(crbug.com/612542): Implement GPU-to-GPU copy path for more
// cases, like copying to layers of 3D textures, and elements of
// 2D texture arrays.
bool flip_y = unpack_flip_y_;
if (is_origin_top_left_ && is_webgl_canvas)
flip_y = !flip_y;

const bool source_has_flip_y = is_origin_top_left_ && is_webgl_canvas;
TexImageImpl(function_id, target, level, internalformat, xoffset, yoffset,
zoffset, format, type, image.get(),
WebGLImageConversion::kHtmlDomCanvas, flip_y,
unpack_premultiply_alpha_, source_sub_rectangle, depth,
unpack_image_height);
WebGLImageConversion::kHtmlDomCanvas, source_has_flip_y,
source_sub_rectangle, depth, unpack_image_height);
}
}

Expand Down Expand Up @@ -6089,8 +6069,8 @@ void WebGLRenderingContextBase::TexImageHelperVideoFrame(
TexImageImpl(function_id, target, level, adjusted_internalformat, xoffset,
yoffset, zoffset, format, type, image.get(),
// Note: kHtmlDomVideo means alpha won't be unmultiplied.
WebGLImageConversion::kHtmlDomVideo, unpack_flip_y_,
unpack_premultiply_alpha_, source_image_rect, depth,
WebGLImageConversion::kHtmlDomVideo,
/*source_has_flip_y=*/false, source_image_rect, depth,
unpack_image_height);
texture->UpdateLastUploadedFrame(metadata);
return;
Expand Down Expand Up @@ -6319,8 +6299,8 @@ void WebGLRenderingContextBase::TexImageHelperMediaVideoFrame(
TexImageImpl(function_id, target, level, adjusted_internalformat, xoffset,
yoffset, zoffset, format, type, image.get(),
// Note: kHtmlDomVideo means alpha won't be unmultiplied.
WebGLImageConversion::kHtmlDomVideo, unpack_flip_y_,
unpack_premultiply_alpha_, source_image_rect, depth,
WebGLImageConversion::kHtmlDomVideo,
/*source_has_flip_y=*/false, source_image_rect, depth,
unpack_image_height);
}

Expand Down
7 changes: 5 additions & 2 deletions third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.h
View file
Expand Up @@ -1217,6 +1217,10 @@ class MODULES_EXPORT WebGLRenderingContextBase : public CanvasRenderingContext,
GLenum format,
GLenum type,
const void* pixels);
void TexImageImpl(TexImageParams params,
Image* image,
WebGLImageConversion::ImageHtmlDomSource,
bool image_has_flip_y);
void TexImageImpl(TexImageFunctionID,
GLenum target,
GLint level,
Expand All @@ -1228,8 +1232,7 @@ class MODULES_EXPORT WebGLRenderingContextBase : public CanvasRenderingContext,
GLenum type,
Image*,
WebGLImageConversion::ImageHtmlDomSource,
bool flip_y,
bool premultiply_alpha,
bool source_has_flip_y,
const absl::optional<gfx::Rect>&,
GLsizei depth,
GLint unpack_image_height);
Expand Down
15 changes: 3 additions & 12 deletions third_party/blink/renderer/platform/graphics/gpu/webgl_image_conversion.cc
View file
Expand Up @@ -3807,23 +3807,14 @@ void WebGLImageConversion::ImageExtractor::ExtractImage(
if (!skia_image)
return;

#if SK_B32_SHIFT
image_source_format_ = kDataFormatRGBA8;
#else
image_source_format_ = kDataFormatBGRA8;
#endif
image_source_unpack_alignment_ =
0; // FIXME: this seems to always be zero - why use at all?

DCHECK(skia_image->width());
DCHECK(skia_image->height());
image_width_ = skia_image->width();
image_height_ = skia_image->height();

// Fail if the image was downsampled because of memory limits.
if (image_width_ != (unsigned)image_->width() ||
image_height_ != (unsigned)image_->height())
if (skia_image->width() != image_->width() ||
skia_image->height() != image_->height()) {
return;
}

image_pixel_locker_.emplace(std::move(skia_image), info.alphaType(),
kN32_SkColorType);
Expand Down
14 changes: 2 additions & 12 deletions third_party/blink/renderer/platform/graphics/gpu/webgl_image_conversion.h
View file
Expand Up @@ -141,16 +141,10 @@ class PLATFORM_EXPORT WebGLImageConversion final {
ImageExtractor(const ImageExtractor&) = delete;
ImageExtractor& operator=(const ImageExtractor&) = delete;

const void* ImagePixelData() {
return image_pixel_locker_ ? image_pixel_locker_->Pixels() : nullptr;
const SkPixmap* GetSkPixmap() {
return image_pixel_locker_ ? image_pixel_locker_->GetSkPixmap() : nullptr;
}
unsigned ImageWidth() { return image_width_; }
unsigned ImageHeight() { return image_height_; }
DataFormat ImageSourceFormat() { return image_source_format_; }
AlphaOp ImageAlphaOp() { return alpha_op_; }
unsigned ImageSourceUnpackAlignment() {
return image_source_unpack_alignment_;
}

private:
// Extracts the image and keeps track of its status, such as width, height,
Expand All @@ -161,11 +155,7 @@ class PLATFORM_EXPORT WebGLImageConversion final {
Image* image_;
absl::optional<ImagePixelLocker> image_pixel_locker_;
ImageHtmlDomSource image_html_dom_source_;
unsigned image_width_;
unsigned image_height_;
DataFormat image_source_format_;
AlphaOp alpha_op_;
unsigned image_source_unpack_alignment_;
};

// Convert an SkColorType to the most appropriate DataFormat.
Expand Down
18 changes: 8 additions & 10 deletions third_party/blink/renderer/platform/graphics/skia/image_pixel_locker.cc
View file
Expand Up @@ -33,16 +33,14 @@ ImagePixelLocker::ImagePixelLocker(sk_sp<const SkImage> image,
// If the image has in-RAM pixels and their format matches, use them directly.
// TODO(fmalita): All current clients expect packed pixel rows. Maybe we
// could update them to support arbitrary rowBytes, and relax the check below.
SkPixmap pixmap;
image_->peekPixels(&pixmap);
pixels_ = pixmap.addr();
if (pixels_ && InfoIsCompatible(pixmap.info(), alpha_type, color_type) &&
pixmap.rowBytes() == pixmap.info().minRowBytes()) {
image_->peekPixels(&pixmap_);
if (pixmap_.addr() &&
InfoIsCompatible(pixmap_.info(), alpha_type, color_type) &&
pixmap_.rowBytes() == pixmap_.info().minRowBytes()) {
pixmap_valid_ = true;
return;
}

pixels_ = nullptr;

// No luck, we need to read the pixels into our local buffer.
SkImageInfo info = SkImageInfo::Make(image_->width(), image_->height(),
color_type, alpha_type);
Expand All @@ -53,12 +51,12 @@ ImagePixelLocker::ImagePixelLocker(sk_sp<const SkImage> image,

// this will throw on failure
pixel_storage_.resize(base::checked_cast<wtf_size_t>(size));
pixmap.reset(info, pixel_storage_.data(), row_bytes);
pixmap_.reset(info, pixel_storage_.data(), row_bytes);

if (!image_->readPixels(pixmap, 0, 0))
if (!image_->readPixels(pixmap_, 0, 0))
return;

pixels_ = pixel_storage_.data();
pixmap_valid_ = true;
}

} // namespace blink
9 changes: 7 additions & 2 deletions third_party/blink/renderer/platform/graphics/skia/image_pixel_locker.h
View file
Expand Up @@ -10,6 +10,7 @@
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
#include "third_party/blink/renderer/platform/wtf/vector.h"
#include "third_party/skia/include/core/SkImageInfo.h"
#include "third_party/skia/include/core/SkPixmap.h"
#include "third_party/skia/include/core/SkRefCnt.h"

class SkImage;
Expand All @@ -24,12 +25,16 @@ class ImagePixelLocker final {
ImagePixelLocker(const ImagePixelLocker&) = delete;
ImagePixelLocker& operator=(const ImagePixelLocker&) = delete;

const void* Pixels() const { return pixels_; }
const SkPixmap* GetSkPixmap() const {
return pixmap_valid_ ? &pixmap_ : nullptr;
}

private:
const sk_sp<const SkImage> image_;
const void* pixels_;
Vector<char> pixel_storage_;
// `pixmap_` will either point to `image_`'s storage, or `pixel_storage_`.
bool pixmap_valid_ = false;
SkPixmap pixmap_;
};

} // namespace blink
Expand Down

0 comments on commit badb39a

Please sign in to comment.