Commit 32ee32c4 authored by Khushal's avatar Khushal Committed by Commit Bot

blink/images: Ensure cached PaintImage invalidated on alpha change.

BitmapImage caches the last generated PaintImage which is updated only
if more data for the image is loaded. The PaintImage stores the alpha
state for the image which may change once it has been decoded. Ensure
that this cached PaintImage is invalidated if there is an alpha change.

R=wangxianzhu@chromium.org

Bug: 1063679,739339
Change-Id: Icbe2eda8861d71b6cf200e939982293635e520a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2114582
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Auto-Submit: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759832}
parent b911edcf
...@@ -278,6 +278,14 @@ SkColorType PaintImage::GetColorType() const { ...@@ -278,6 +278,14 @@ SkColorType PaintImage::GetColorType() const {
return kUnknown_SkColorType; return kUnknown_SkColorType;
} }
SkAlphaType PaintImage::GetAlphaType() const {
if (paint_image_generator_)
return paint_image_generator_->GetSkImageInfo().alphaType();
if (GetSkImage())
return GetSkImage()->alphaType();
return kUnknown_SkAlphaType;
}
int PaintImage::width() const { int PaintImage::width() const {
return paint_worklet_input_ return paint_worklet_input_
? static_cast<int>(paint_worklet_input_->GetSize().width()) ? static_cast<int>(paint_worklet_input_->GetSize().width())
......
...@@ -271,8 +271,9 @@ class CC_PAINT_EXPORT PaintImage { ...@@ -271,8 +271,9 @@ class CC_PAINT_EXPORT PaintImage {
SkYUVAIndex* plane_indices = nullptr, SkYUVAIndex* plane_indices = nullptr,
SkYUVColorSpace* yuv_color_space = nullptr) const; SkYUVColorSpace* yuv_color_space = nullptr) const;
// Returns the color type of this image. // Get metadata associated with this image.
SkColorType GetColorType() const; SkColorType GetColorType() const;
SkAlphaType GetAlphaType() const;
// Returns general information about the underlying image. Returns nullptr if // Returns general information about the underlying image. Returns nullptr if
// there is no available |paint_image_generator_|. // there is no available |paint_image_generator_|.
......
...@@ -343,7 +343,8 @@ bool BitmapImage::IsSizeAvailable() { ...@@ -343,7 +343,8 @@ bool BitmapImage::IsSizeAvailable() {
} }
PaintImage BitmapImage::PaintImageForCurrentFrame() { PaintImage BitmapImage::PaintImageForCurrentFrame() {
if (cached_frame_) auto alpha_type = decoder_ ? decoder_->AlphaType() : kUnknown_SkAlphaType;
if (cached_frame_ && cached_frame_.GetAlphaType() == alpha_type)
return cached_frame_; return cached_frame_;
cached_frame_ = CreatePaintImage(); cached_frame_ = CreatePaintImage();
...@@ -380,20 +381,7 @@ scoped_refptr<Image> BitmapImage::ImageForDefaultFrame() { ...@@ -380,20 +381,7 @@ scoped_refptr<Image> BitmapImage::ImageForDefaultFrame() {
} }
bool BitmapImage::CurrentFrameKnownToBeOpaque() { bool BitmapImage::CurrentFrameKnownToBeOpaque() {
// If the image is animated, it is being animated by the compositor and we return decoder_ ? decoder_->AlphaType() == kOpaque_SkAlphaType : false;
// don't know what the current frame is.
// TODO(khushalsagar): We could say the image is opaque if none of the frames
// have alpha.
if (MaybeAnimated())
return false;
// We ask the decoder whether the image has alpha because in some cases the
// the correct value is known after decoding. The DeferredImageDecoder caches
// the accurate value from the decoded result.
const bool frame_has_alpha =
decoder_ ? decoder_->FrameHasAlphaAtIndex(PaintImage::kDefaultFrameIndex)
: true;
return !frame_has_alpha;
} }
bool BitmapImage::CurrentFrameIsComplete() { bool BitmapImage::CurrentFrameIsComplete() {
......
...@@ -191,20 +191,9 @@ sk_sp<PaintImageGenerator> DeferredImageDecoder::CreateGenerator() { ...@@ -191,20 +191,9 @@ sk_sp<PaintImageGenerator> DeferredImageDecoder::CreateGenerator() {
scoped_refptr<SegmentReader> segment_reader = scoped_refptr<SegmentReader> segment_reader =
SegmentReader::CreateFromSkROBuffer(std::move(ro_buffer)); SegmentReader::CreateFromSkROBuffer(std::move(ro_buffer));
// ImageFrameGenerator has the latest known alpha state. There will be a
// performance boost if the image is opaque since we can avoid painting
// the background in this case.
// For multi-frame images, these maybe animated on the compositor thread.
// So we can not mark them as opaque unless all frames are opaque.
// TODO(khushalsagar): Check whether all frames being added to the
// generator are opaque when populating FrameMetadata below.
SkAlphaType alpha_type = kPremul_SkAlphaType;
if (frame_data_.size() == 1u && !frame_generator_->HasAlpha(0u))
alpha_type = kOpaque_SkAlphaType;
SkImageInfo info = SkImageInfo info =
SkImageInfo::MakeN32(decoded_size.width(), decoded_size.height(), SkImageInfo::MakeN32(decoded_size.width(), decoded_size.height(),
alpha_type, color_space_for_sk_images_); AlphaType(), color_space_for_sk_images_);
if (image_is_high_bit_depth_) if (image_is_high_bit_depth_)
info = info.makeColorType(kRGBA_F16_SkColorType); info = info.makeColorType(kRGBA_F16_SkColorType);
...@@ -327,12 +316,18 @@ int DeferredImageDecoder::RepetitionCount() const { ...@@ -327,12 +316,18 @@ int DeferredImageDecoder::RepetitionCount() const {
: repetition_count_; : repetition_count_;
} }
bool DeferredImageDecoder::FrameHasAlphaAtIndex(size_t index) const { SkAlphaType DeferredImageDecoder::AlphaType() const {
if (metadata_decoder_) // ImageFrameGenerator has the latest known alpha state. There will be a
return metadata_decoder_->FrameHasAlphaAtIndex(index); // performance boost if the image is opaque since we can avoid painting
if (!frame_generator_->IsMultiFrame()) // the background in this case.
return frame_generator_->HasAlpha(index); // For multi-frame images, these maybe animated on the compositor thread.
return true; // So we can not mark them as opaque unless all frames are opaque.
// TODO(khushalsagar): Check whether all frames being added to the
// generator are opaque when populating FrameMetadata below.
SkAlphaType alpha_type = kPremul_SkAlphaType;
if (frame_data_.size() == 1u && !frame_generator_->HasAlpha(0u))
alpha_type = kOpaque_SkAlphaType;
return alpha_type;
} }
bool DeferredImageDecoder::FrameIsReceivedAtIndex(size_t index) const { bool DeferredImageDecoder::FrameIsReceivedAtIndex(size_t index) const {
......
...@@ -74,11 +74,11 @@ class PLATFORM_EXPORT DeferredImageDecoder final { ...@@ -74,11 +74,11 @@ class PLATFORM_EXPORT DeferredImageDecoder final {
size_t FrameCount(); size_t FrameCount();
bool ImageIsHighBitDepth() const { return image_is_high_bit_depth_; } bool ImageIsHighBitDepth() const { return image_is_high_bit_depth_; }
int RepetitionCount() const; int RepetitionCount() const;
bool FrameHasAlphaAtIndex(size_t index) const;
bool FrameIsReceivedAtIndex(size_t index) const; bool FrameIsReceivedAtIndex(size_t index) const;
base::TimeDelta FrameDurationAtIndex(size_t index) const; base::TimeDelta FrameDurationAtIndex(size_t index) const;
ImageOrientation OrientationAtIndex(size_t index) const; ImageOrientation OrientationAtIndex(size_t index) const;
bool HotSpot(IntPoint&) const; bool HotSpot(IntPoint&) const;
SkAlphaType AlphaType() const;
// A less expensive method for getting the number of bytes thus far received // A less expensive method for getting the number of bytes thus far received
// for the image. Checking Data()->size() involves copying bytes to a // for the image. Checking Data()->size() involves copying bytes to a
......
...@@ -399,13 +399,13 @@ TEST_F(DeferredImageDecoderTest, frameOpacity) { ...@@ -399,13 +399,13 @@ TEST_F(DeferredImageDecoderTest, frameOpacity) {
sk_sp<SkImage> frame = CreatePaintImage(decoder.get()).GetSkImage(); sk_sp<SkImage> frame = CreatePaintImage(decoder.get()).GetSkImage();
ASSERT_TRUE(frame); ASSERT_TRUE(frame);
EXPECT_FALSE(frame->isOpaque()); EXPECT_FALSE(frame->isOpaque());
EXPECT_TRUE(decoder->FrameHasAlphaAtIndex(0)); EXPECT_EQ(decoder->AlphaType(), kPremul_SkAlphaType);
// Force a lazy decode by reading pixels. // Force a lazy decode by reading pixels.
EXPECT_TRUE(frame->readPixels(pixmap, 0, 0)); EXPECT_TRUE(frame->readPixels(pixmap, 0, 0));
// After decoding, the frame is known to be opaque. // After decoding, the frame is known to be opaque.
EXPECT_FALSE(decoder->FrameHasAlphaAtIndex(0)); EXPECT_EQ(decoder->AlphaType(), kOpaque_SkAlphaType);
frame = CreatePaintImage(decoder.get()).GetSkImage(); frame = CreatePaintImage(decoder.get()).GetSkImage();
ASSERT_TRUE(frame); ASSERT_TRUE(frame);
EXPECT_TRUE(frame->isOpaque()); EXPECT_TRUE(frame->isOpaque());
......
...@@ -2814,8 +2814,12 @@ void WebGLImageConversion::ImageExtractor::ExtractImage( ...@@ -2814,8 +2814,12 @@ void WebGLImageConversion::ImageExtractor::ExtractImage(
alpha_op_ = kAlphaDoNothing; alpha_op_ = kAlphaDoNothing;
bool has_alpha = skia_image ? !skia_image->isOpaque() : true; bool has_alpha = skia_image ? !skia_image->isOpaque() : true;
if ((!skia_image || ignore_color_space || bool need_unpremultiplied = has_alpha && !premultiply_alpha;
(has_alpha && !premultiply_alpha)) && bool need_color_conversion = !ignore_color_space &&
skia_image->colorSpace() &&
!skia_image->colorSpace()->isSRGB();
if ((!skia_image || ignore_color_space || need_unpremultiplied ||
need_color_conversion) &&
image_->Data()) { image_->Data()) {
// Attempt to get raw unpremultiplied image data. // Attempt to get raw unpremultiplied image data.
const bool data_complete = true; const bool data_complete = true;
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
"name": "LayoutNGBlockFlow (positioned) DIV class='test image'", "name": "LayoutNGBlockFlow (positioned) DIV class='test image'",
"position": [8, 8], "position": [8, 8],
"bounds": [110, 44], "bounds": [110, 44],
"contentsOpaque": true,
"invalidations": [ "invalidations": [
[50, 0, 60, 44], [50, 0, 60, 44],
[0, 40, 60, 4] [0, 40, 60, 4]
...@@ -19,6 +20,7 @@ ...@@ -19,6 +20,7 @@
"name": "LayoutNGBlockFlow (positioned) DIV class='test image size-contain'", "name": "LayoutNGBlockFlow (positioned) DIV class='test image size-contain'",
"position": [108, 8], "position": [108, 8],
"bounds": [110, 44], "bounds": [110, 44],
"contentsOpaque": true,
"invalidations": [ "invalidations": [
[0, 0, 60, 44], [0, 0, 60, 44],
[50, 40, 60, 4] [50, 40, 60, 4]
...@@ -28,6 +30,7 @@ ...@@ -28,6 +30,7 @@
"name": "LayoutNGBlockFlow (positioned) DIV class='test image percent-height'", "name": "LayoutNGBlockFlow (positioned) DIV class='test image percent-height'",
"position": [208, 8], "position": [208, 8],
"bounds": [110, 44], "bounds": [110, 44],
"contentsOpaque": true,
"invalidations": [ "invalidations": [
[0, 0, 60, 44], [0, 0, 60, 44],
[50, 40, 60, 4] [50, 40, 60, 4]
...@@ -37,6 +40,7 @@ ...@@ -37,6 +40,7 @@
"name": "LayoutNGBlockFlow (positioned) DIV class='test image bottom'", "name": "LayoutNGBlockFlow (positioned) DIV class='test image bottom'",
"position": [308, 8], "position": [308, 8],
"bounds": [110, 44], "bounds": [110, 44],
"contentsOpaque": true,
"invalidations": [ "invalidations": [
[50, 0, 60, 44], [50, 0, 60, 44],
[0, 0, 60, 44] [0, 0, 60, 44]
...@@ -54,6 +58,7 @@ ...@@ -54,6 +58,7 @@
{ {
"name": "LayoutNGBlockFlow (positioned) DIV class='test image repeat-round'", "name": "LayoutNGBlockFlow (positioned) DIV class='test image repeat-round'",
"bounds": [60, 44], "bounds": [60, 44],
"contentsOpaque": true,
"backfaceVisibility": "hidden", "backfaceVisibility": "hidden",
"invalidations": [ "invalidations": [
[0, 0, 60, 44] [0, 0, 60, 44]
......
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