0

[Blink] Have DrawingBuffer set TransferableResource::alpha_type directly

This CL changes DrawingBuffer to directly set the `alpha_type` field on
the TransferableResource that it prepares rather than indirecting
through TextureLayer to do so. It also eliminates
TextureLayer::SetPremultipliedAlpha() [there were no other production
callsites] to ensure that the field doesn't get overwritten.

A few notes:

* The TransferableResource that the TextureLayer created by
  DrawingBuffer ends up with is the one created via
  PrepareTransferableResource [1]
* The TransferableResource defaults to premultiplied and is created
  fresh on every invocation to `PrepareTransferableResource()`, so we
  need to set `alpha_type` explicitly only if setting it to
  unpremultiplied
* The TextureLayer that DrawingBuffer creates will have SetNeedsCommit()
  called on it independently of the current call to
  SetPremultipliedAlpha() via the call to SetHitTestable() [2][3][4]

[1] https://source.chromium.org/chromium/chromium/src/+/main:cc/layers/texture_layer.cc;l=171-172?q=TextureLayer::Update&ss=chromium
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/graphics/gpu/drawing_buffer.cc;l=1?q=drawing_buffer.cc&sq=&ss=chromium
[3] https://source.chromium.org/chromium/chromium/src/+/main:cc/layers/layer.cc;l=880;drc=02446d66622a0a811448be7bb4ac8939c5b00aa9
[4] https://source.chromium.org/chromium/chromium/src/+/main:cc/layers/layer.cc;l=870-876;drc=02446d66622a0a811448be7bb4ac8939c5b00aa9
[5] https://source.chromium.org/chromium/chromium/src/+/main:cc/layers/layer.h;drc=02446d66622a0a811448be7bb4ac8939c5b00aa9;l=1054

Bug: 410591523
Change-Id: Ia14e8d8892ec040f0f7c3d251686b295cc231d19
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6549640
Reviewed-by: Antonio Sartori <antoniosartori@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1461192}
This commit is contained in:
Colin Blundell
2025-05-15 23:51:06 -07:00
committed by Chromium LUCI CQ
parent c6bdc3e2e9
commit 6fc8709e5f
9 changed files with 8 additions and 37 deletions
cc
components/viz/service/layers
services/viz/public/mojom/compositing
third_party/blink/renderer/platform/graphics/gpu

@ -30,7 +30,6 @@ scoped_refptr<TextureLayer> TextureLayer::Create(TextureLayerClient* client) {
TextureLayer::TextureLayer(TextureLayerClient* client)
: client_(client),
uv_bottom_right_(1.f, 1.f),
premultiplied_alpha_(true),
blend_background_color_(false),
force_texture_to_opaque_(false),
needs_set_resource_(false) {}
@ -62,13 +61,6 @@ void TextureLayer::SetUV(const gfx::PointF& top_left,
SetNeedsCommit();
}
void TextureLayer::SetPremultipliedAlpha(bool premultiplied_alpha) {
if (premultiplied_alpha_.Read(*this) == premultiplied_alpha)
return;
premultiplied_alpha_.Write(*this) = premultiplied_alpha;
SetNeedsCommit();
}
void TextureLayer::SetBlendBackgroundColor(bool blend) {
if (blend_background_color_.Read(*this) == blend)
return;
@ -206,7 +198,6 @@ void TextureLayer::PushDirtyPropertiesTo(
TextureLayerImpl* texture_layer = static_cast<TextureLayerImpl*>(layer);
texture_layer->SetUVTopLeft(uv_top_left_.Read(*this));
texture_layer->SetUVBottomRight(uv_bottom_right_.Read(*this));
texture_layer->SetPremultipliedAlpha(premultiplied_alpha_.Read(*this));
texture_layer->SetBlendBackgroundColor(blend_background_color_.Read(*this));
texture_layer->SetForceTextureToOpaque(
force_texture_to_opaque_.Read(*this));

@ -97,10 +97,6 @@ class CC_EXPORT TextureLayer : public Layer {
// Sets a UV transform to be used at draw time. Defaults to (0, 0) and (1, 1).
void SetUV(const gfx::PointF& top_left, const gfx::PointF& bottom_right);
// Sets whether the alpha channel is premultiplied or unpremultiplied.
// Defaults to true.
void SetPremultipliedAlpha(bool premultiplied_alpha);
// Sets whether the texture should be blended with the background color
// at draw time. Defaults to false.
void SetBlendBackgroundColor(bool blend);
@ -153,7 +149,6 @@ class CC_EXPORT TextureLayer : public Layer {
ProtectedSequenceReadable<gfx::PointF> uv_top_left_;
ProtectedSequenceReadable<gfx::PointF> uv_bottom_right_;
// [bottom left, top left, top right, bottom right]
ProtectedSequenceReadable<bool> premultiplied_alpha_;
ProtectedSequenceReadable<bool> blend_background_color_;
ProtectedSequenceReadable<bool> force_texture_to_opaque_;

@ -53,7 +53,6 @@ void TextureLayerImpl::PushPropertiesTo(LayerImpl* layer) {
TextureLayerImpl* texture_layer = static_cast<TextureLayerImpl*>(layer);
texture_layer->SetUVTopLeft(uv_top_left_);
texture_layer->SetUVBottomRight(uv_bottom_right_);
texture_layer->SetPremultipliedAlpha(premultiplied_alpha_);
texture_layer->SetBlendBackgroundColor(blend_background_color_);
texture_layer->SetForceTextureToOpaque(force_texture_to_opaque_);
if (needs_set_resource_push_) {
@ -101,11 +100,6 @@ bool TextureLayerImpl::WillDraw(
transferable_resource_.resource_source));
}
// NOTE: We must perform any updates to `transferable_resource_` *before*
// exporting it to the resource provider, as that copies the resource into
// a new instance that is what is transferred across processes.
transferable_resource_.alpha_type =
premultiplied_alpha_ ? kPremul_SkAlphaType : kUnpremul_SkAlphaType;
resource_id_ = resource_provider->ImportResource(
transferable_resource_,
/* impl_thread_release_callback= */ viz::ReleaseCallback(),
@ -194,10 +188,6 @@ gfx::ContentColorUsage TextureLayerImpl::GetContentColorUsage() const {
return transferable_resource_.color_space.GetContentColorUsage();
}
void TextureLayerImpl::SetPremultipliedAlpha(bool premultiplied_alpha) {
premultiplied_alpha_ = premultiplied_alpha;
}
void TextureLayerImpl::SetBlendBackgroundColor(bool blend) {
blend_background_color_ = blend;
}

@ -52,7 +52,6 @@ class CC_EXPORT TextureLayerImpl : public LayerImpl {
// must explicitly invalidate if they intend to cause a visible change in the
// layer's output.
void SetTextureId(unsigned id);
void SetPremultipliedAlpha(bool premultiplied_alpha);
void SetBlendBackgroundColor(bool blend);
void SetForceTextureToOpaque(bool opaque);
void SetUVTopLeft(const gfx::PointF& top_left);
@ -70,7 +69,6 @@ class CC_EXPORT TextureLayerImpl : public LayerImpl {
static bool MayEvictResourceInBackground(
viz::TransferableResource::ResourceSource source);
bool premultiplied_alpha() const { return premultiplied_alpha_; }
bool blend_background_color() const { return blend_background_color_; }
bool force_texture_to_opaque() const { return force_texture_to_opaque_; }
bool needs_set_resource_push() const { return needs_set_resource_push_; }
@ -89,7 +87,6 @@ class CC_EXPORT TextureLayerImpl : public LayerImpl {
void FreeTransferableResource();
void OnResourceEvicted();
bool premultiplied_alpha_ = true;
bool blend_background_color_ = false;
bool force_texture_to_opaque_ = false;

@ -259,7 +259,6 @@ TEST_F(TextureLayerTest, CheckPropertyChangeCausesCorrectBehavior) {
));
EXPECT_SET_NEEDS_COMMIT(1, test_layer->SetUV(gfx::PointF(0.25f, 0.25f),
gfx::PointF(0.75f, 0.75f)));
EXPECT_SET_NEEDS_COMMIT(1, test_layer->SetPremultipliedAlpha(false));
EXPECT_SET_NEEDS_COMMIT(1, test_layer->SetBlendBackgroundColor(true));
}

@ -555,7 +555,6 @@ void SerializeTextureLayerExtra(TextureLayerImpl& layer,
viz::mojom::TextureLayerExtraPtr& extra,
viz::ClientResourceProvider& resource_provider,
viz::RasterContextProvider& context_provider) {
extra->premultiplied_alpha = layer.premultiplied_alpha();
extra->blend_background_color = layer.blend_background_color();
extra->force_texture_to_opaque = layer.force_texture_to_opaque();
extra->uv_top_left = layer.uv_top_left();

@ -436,7 +436,6 @@ void UpdateMirrorLayerExtra(const mojom::MirrorLayerExtraPtr& extra,
void UpdateTextureLayerExtra(const mojom::TextureLayerExtraPtr& extra,
cc::TextureLayerImpl& layer) {
layer.SetPremultipliedAlpha(extra->premultiplied_alpha);
layer.SetBlendBackgroundColor(extra->blend_background_color);
layer.SetForceTextureToOpaque(extra->force_texture_to_opaque);
layer.SetUVTopLeft(extra->uv_top_left);

@ -85,7 +85,6 @@ struct SurfaceLayerExtra {
// Extra fields in a cc::TextureLayerImpl that has been added to a tree or
// modified in some interesting way since a prior tree update.
struct TextureLayerExtra {
bool premultiplied_alpha;
bool blend_background_color;
bool force_texture_to_opaque;
gfx.mojom.PointF uv_top_left;

@ -481,6 +481,14 @@ bool DrawingBuffer::PrepareTransferableResource(
}
}
// If staging_texture_ exists, then premultiplication
// has already been handled via CopySubTextureCHROMIUM.
// TODO(crbug.com/410591523): Always set this field when not using
// `staging_texture_` under a killswitch.
if (!staging_texture_ && requested_alpha_type_ == kUnpremul_SkAlphaType) {
out_resource->alpha_type = requested_alpha_type_;
}
return true;
}
@ -1213,12 +1221,6 @@ cc::Layer* DrawingBuffer::CcLayer() {
layer_->SetContentsOpaque(requested_alpha_type_ == kOpaque_SkAlphaType);
layer_->SetBlendBackgroundColor(requested_alpha_type_ !=
kOpaque_SkAlphaType);
// If staging_texture_ exists, then premultiplication
// has already been handled via CopySubTextureCHROMIUM.
if (!staging_texture_ && requested_alpha_type_ == kUnpremul_SkAlphaType) {
layer_->SetPremultipliedAlpha(false);
}
}
return layer_.get();