[//cc] Move alpha type from TextureDrawQuad to TransferableResource
* Adds `alpha_type` to TransferableResource * Removes `premultiplied_alpha` from TextureDrawQuad * Has all sites that set the latter instead correspondingly set the former [*] * Changes SkiaRenderer to read the former instead of the latter [*] The only subtle change is in video_resource_updater.cc. The reasoning for the change is as follows: * The texture quad has its resource id set to `frame_resource_id_`. * That resource id is what identifies the TransferableResource associated with that texture quad. * `frame_resource_id_` is assigned at [1]. * Hence, we need to set `alpha_type` on the TransferableResource that was linked to `frame_resource_id_` in [1]. * That TransferableResource is created here [2], which is where I've added the setting of `alpha_type`. [1] https://source.chromium.org/chromium/chromium/src/+/main:media/renderers/video_resource_updater.cc;l=532-534?q=video_resource_updater.cc&ss=chromium [2] https://source.chromium.org/chromium/chromium/src/+/main:media/renderers/video_resource_updater.cc;l=522-523?q=video_resource_updater.cc&ss=chromium Bug: 410591523 Change-Id: Id97743a4d49a8aa1ed129a4a769b17916435ee28 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6544948 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Antonio Sartori <antoniosartori@chromium.org> Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org> Cr-Commit-Position: refs/heads/main@{#1460544}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
e71e7f22c9
commit
bbe2e76293
cc/layers
components/viz
common
service
media/renderers
services/viz/public
cpp
compositing
mojom
compositing
third_party/blink/renderer/platform/graphics
@ -100,6 +100,12 @@ bool TextureLayerImpl::WillDraw(
|
||||
DCHECK(MayEvictResourceInBackground(
|
||||
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(),
|
||||
@ -149,7 +155,6 @@ void TextureLayerImpl::AppendQuads(const AppendQuadsContext& context,
|
||||
resource_id_, uv_top_left_, uv_bottom_right_, bg_color,
|
||||
nearest_neighbor,
|
||||
/*secure_output=*/false, gfx::ProtectedVideoType::kClear);
|
||||
quad->premultiplied_alpha = premultiplied_alpha_;
|
||||
quad->dynamic_range_limit = GetDynamicRangeLimit();
|
||||
ValidateQuadResources(quad);
|
||||
}
|
||||
|
@ -1251,7 +1251,9 @@ void TextureDrawQuadToDict(const TextureDrawQuad* draw_quad,
|
||||
base::Value::Dict* dict) {
|
||||
DCHECK(draw_quad);
|
||||
DCHECK(dict);
|
||||
dict->Set("premultiplied_alpha", draw_quad->premultiplied_alpha);
|
||||
// Set premultiplied_alpha to not break backwards-compatibility with unit test
|
||||
// data.
|
||||
dict->Set("premultiplied_alpha", true);
|
||||
dict->Set("uv_top_left", PointFToDict(draw_quad->uv_top_left));
|
||||
dict->Set("uv_bottom_right", PointFToDict(draw_quad->uv_bottom_right));
|
||||
dict->Set("background_color", SkColor4fToDict(draw_quad->background_color));
|
||||
@ -1437,8 +1439,6 @@ bool TextureDrawQuadFromDict(const base::Value::Dict& dict,
|
||||
TextureDrawQuad* draw_quad) {
|
||||
DCHECK(draw_quad);
|
||||
|
||||
std::optional<bool> premultiplied_alpha =
|
||||
dict.FindBool("premultiplied_alpha");
|
||||
const base::Value::Dict* uv_top_left = dict.FindDict("uv_top_left");
|
||||
const base::Value::Dict* uv_bottom_right = dict.FindDict("uv_bottom_right");
|
||||
// TODO(crbug.com/40942150): Update
|
||||
@ -1451,9 +1451,8 @@ bool TextureDrawQuadFromDict(const base::Value::Dict& dict,
|
||||
const std::string* protected_video_type =
|
||||
dict.FindString("protected_video_type");
|
||||
|
||||
if (!premultiplied_alpha || !uv_top_left || !uv_bottom_right ||
|
||||
!vertex_opacity || !nearest_neighbor || !secure_output_only ||
|
||||
!protected_video_type) {
|
||||
if (!uv_top_left || !uv_bottom_right || !vertex_opacity ||
|
||||
!nearest_neighbor || !secure_output_only || !protected_video_type) {
|
||||
return false;
|
||||
}
|
||||
int protected_video_type_index =
|
||||
@ -1474,7 +1473,6 @@ bool TextureDrawQuadFromDict(const base::Value::Dict& dict,
|
||||
common.needs_blending, resource_id, t_uv_top_left, t_uv_bottom_right,
|
||||
t_background_color, nearest_neighbor.value(), secure_output_only.value(),
|
||||
static_cast<gfx::ProtectedVideoType>(protected_video_type_index));
|
||||
draw_quad->premultiplied_alpha = premultiplied_alpha.value();
|
||||
|
||||
gfx::Rect t_damage_rect;
|
||||
if (damage_rect && RectFromDict(*damage_rect, &t_damage_rect)) {
|
||||
|
@ -19,7 +19,6 @@ namespace viz {
|
||||
|
||||
TextureDrawQuad::TextureDrawQuad()
|
||||
: nearest_neighbor(false),
|
||||
premultiplied_alpha(true),
|
||||
secure_output_only(false),
|
||||
is_video_frame(false),
|
||||
protected_video_type(gfx::ProtectedVideoType::kClear) {
|
||||
@ -86,7 +85,6 @@ const TextureDrawQuad* TextureDrawQuad::MaterialCast(const DrawQuad* quad) {
|
||||
|
||||
void TextureDrawQuad::ExtendValue(base::trace_event::TracedValue* value) const {
|
||||
value->SetInteger("resource_id", resource_id.GetUnsafeValue());
|
||||
value->SetBoolean("premultiplied_alpha", premultiplied_alpha);
|
||||
|
||||
cc::MathUtil::AddToTracedValue("uv_top_left", uv_top_left, value);
|
||||
cc::MathUtil::AddToTracedValue("uv_bottom_right", uv_bottom_right, value);
|
||||
|
@ -59,7 +59,6 @@ class VIZ_COMMON_EXPORT TextureDrawQuad : public DrawQuad {
|
||||
SkColor4f background_color = SkColors::kTransparent;
|
||||
cc::PaintFlags::DynamicRangeLimitMixture dynamic_range_limit;
|
||||
bool nearest_neighbor : 1;
|
||||
bool premultiplied_alpha : 1;
|
||||
|
||||
// True if the quad must only be GPU composited if shown on secure outputs.
|
||||
bool secure_output_only : 1;
|
||||
|
@ -16,6 +16,7 @@
|
||||
#include "components/viz/common/viz_common_export.h"
|
||||
#include "gpu/command_buffer/common/mailbox_holder.h"
|
||||
#include "gpu/ipc/common/vulkan_ycbcr_info.h"
|
||||
#include "third_party/skia/include/core/SkAlphaType.h"
|
||||
#include "third_party/skia/include/gpu/ganesh/GrTypes.h"
|
||||
#include "ui/gfx/buffer_types.h"
|
||||
#include "ui/gfx/color_space.h"
|
||||
@ -236,6 +237,8 @@ struct VIZ_COMMON_EXPORT TransferableResource {
|
||||
// Origin of the underlying resource.
|
||||
GrSurfaceOrigin origin = kTopLeft_GrSurfaceOrigin;
|
||||
|
||||
SkAlphaType alpha_type = kPremul_SkAlphaType;
|
||||
|
||||
// The source that originally allocated this resource. For determining which
|
||||
// sources are maintaining lifetime after surface eviction.
|
||||
ResourceSource resource_source = ResourceSource::kUnknown;
|
||||
|
@ -208,6 +208,11 @@ GrSurfaceOrigin DisplayResourceProvider::GetOrigin(ResourceId id) const {
|
||||
return resource->transferable.origin;
|
||||
}
|
||||
|
||||
SkAlphaType DisplayResourceProvider::GetAlphaType(ResourceId id) const {
|
||||
const ChildResource* resource = GetResource(id);
|
||||
return resource->transferable.alpha_type;
|
||||
}
|
||||
|
||||
int DisplayResourceProvider::CreateChild(ReturnCallback return_callback,
|
||||
const SurfaceId& surface_id) {
|
||||
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
|
||||
|
@ -99,6 +99,7 @@ class VIZ_SERVICE_EXPORT DisplayResourceProvider
|
||||
const gfx::HDRMetadata& GetHDRMetadata(ResourceId id) const;
|
||||
|
||||
GrSurfaceOrigin GetOrigin(ResourceId id) const;
|
||||
SkAlphaType GetAlphaType(ResourceId id) const;
|
||||
|
||||
// Indicates if this resource may be used for a hardware overlay plane.
|
||||
bool IsOverlayCandidate(ResourceId id) const;
|
||||
|
@ -145,6 +145,7 @@ ResourceId CreateGpuResource(
|
||||
false /* is_overlay_candidate */);
|
||||
gl_resource.color_space = std::move(color_space);
|
||||
gl_resource.origin = origin;
|
||||
gl_resource.alpha_type = alpha_type;
|
||||
auto release_callback =
|
||||
base::BindOnce(&DeleteSharedImage, std::move(client_shared_image));
|
||||
return resource_provider->ImportResource(gl_resource,
|
||||
@ -351,7 +352,6 @@ void CreateTestTwoColoredTextureDrawQuad(
|
||||
quad->SetNew(shared_state, rect, rect, needs_blending, mapped_resource,
|
||||
uv_top_left, uv_bottom_right, background_color, nearest_neighbor,
|
||||
/*secure_output=*/false, gfx::ProtectedVideoType::kClear);
|
||||
quad->premultiplied_alpha = premultiplied_alpha;
|
||||
}
|
||||
|
||||
// TODO(crbug.com/40219248): Make this function use SkColor4f
|
||||
@ -421,7 +421,6 @@ void CreateTestTextureDrawQuad(
|
||||
quad->SetNew(shared_state, rect, rect, needs_blending, mapped_resource,
|
||||
uv_top_left, uv_bottom_right, background_color, nearest_neighbor,
|
||||
/*secure_output=*/false, gfx::ProtectedVideoType::kClear);
|
||||
quad->premultiplied_alpha = premultiplied_alpha;
|
||||
}
|
||||
|
||||
void CreateTestY16TextureDrawQuad_FromVideoFrame(
|
||||
@ -6409,7 +6408,6 @@ class ColorTransformPixelTest
|
||||
uv_top_left, uv_bottom_right, SkColors::kBlack,
|
||||
nearest_neighbor,
|
||||
/*secure_output=*/false, gfx::ProtectedVideoType::kClear);
|
||||
quad->premultiplied_alpha = this->premultiplied_alpha_;
|
||||
|
||||
auto* color_quad = pass->CreateAndAppendDrawQuad<SolidColorDrawQuad>();
|
||||
color_quad->SetNew(shared_state, rect, rect, SkColors::kBlack, false);
|
||||
|
@ -2709,8 +2709,8 @@ void SkiaRenderer::DrawTextureQuad(const TextureDrawQuad* quad,
|
||||
|
||||
ScopedSkImageBuilder builder(
|
||||
this, quad->resource_id, /*maybe_concurrent_reads=*/true,
|
||||
quad->premultiplied_alpha ? kPremul_SkAlphaType : kUnpremul_SkAlphaType,
|
||||
override_color_space, false, quad->force_rgbx);
|
||||
resource_provider_->GetAlphaType(quad->resource_id), override_color_space,
|
||||
false, quad->force_rgbx);
|
||||
const SkImage* image = builder.sk_image();
|
||||
if (!image)
|
||||
return;
|
||||
|
@ -529,6 +529,13 @@ void VideoResourceUpdater::ObtainFrameResource(
|
||||
return;
|
||||
}
|
||||
|
||||
// TODO(crbug.com/410591523): Move this to where the TransferableResources are
|
||||
// created. Note that it will be necessary to query `external_resource.type`
|
||||
// at that point as `frame_resource_type_` won't yet be assigned.
|
||||
external_resource.resource.alpha_type =
|
||||
(frame_resource_type_ == VideoFrameResourceType::RGBA_PREMULTIPLIED)
|
||||
? kPremul_SkAlphaType
|
||||
: kUnpremul_SkAlphaType;
|
||||
frame_resource_id_ = resource_provider_->ImportResource(
|
||||
external_resource.resource,
|
||||
std::move(external_resource.release_callback));
|
||||
@ -600,8 +607,6 @@ void VideoResourceUpdater::AppendQuad(
|
||||
needs_blending, frame_resource_id_, uv_top_left,
|
||||
uv_bottom_right, SkColors::kTransparent,
|
||||
nearest_neighbor, false, protected_video_type);
|
||||
texture_quad->premultiplied_alpha =
|
||||
frame_resource_type_ == VideoFrameResourceType::RGBA_PREMULTIPLIED;
|
||||
#if BUILDFLAG(IS_WIN)
|
||||
// Windows uses DComp surfaces to e.g. hold MediaFoundation videos, which
|
||||
// must be promoted to overlay to be composited correctly.
|
||||
|
@ -159,7 +159,6 @@ bool StructTraits<viz::mojom::TextureQuadStateDataView, viz::DrawQuad>::Read(
|
||||
return false;
|
||||
}
|
||||
|
||||
quad->premultiplied_alpha = data.premultiplied_alpha();
|
||||
gfx::ProtectedVideoType protected_video_type =
|
||||
gfx::ProtectedVideoType::kClear;
|
||||
viz::OverlayPriority overlay_priority_hint = viz::OverlayPriority::kLow;
|
||||
|
@ -416,12 +416,6 @@ struct StructTraits<viz::mojom::TextureQuadStateDataView, viz::DrawQuad> {
|
||||
return quad->rounded_display_masks_info;
|
||||
}
|
||||
|
||||
static bool premultiplied_alpha(const viz::DrawQuad& input) {
|
||||
const viz::TextureDrawQuad* quad =
|
||||
viz::TextureDrawQuad::MaterialCast(&input);
|
||||
return quad->premultiplied_alpha;
|
||||
}
|
||||
|
||||
static const gfx::PointF& uv_top_left(const viz::DrawQuad& input) {
|
||||
const viz::TextureDrawQuad* quad =
|
||||
viz::TextureDrawQuad::MaterialCast(&input);
|
||||
|
@ -168,7 +168,7 @@ bool StructTraits<viz::mojom::TransferableResourceDataView,
|
||||
!data.ReadHdrMetadata(&out->hdr_metadata) ||
|
||||
!data.ReadYcbcrInfo(&out->ycbcr_info) || !data.ReadId(&id) ||
|
||||
!data.ReadSynchronizationType(&out->synchronization_type) ||
|
||||
!data.ReadOrigin(&out->origin) ||
|
||||
!data.ReadOrigin(&out->origin) || !data.ReadAlphaType(&out->alpha_type) ||
|
||||
!data.ReadResourceSource(&out->resource_source)) {
|
||||
return false;
|
||||
}
|
||||
|
@ -14,6 +14,7 @@
|
||||
#include "gpu/ipc/common/vulkan_ycbcr_info_mojom_traits.h"
|
||||
#include "services/viz/public/cpp/compositing/shared_image_format_mojom_traits.h"
|
||||
#include "services/viz/public/mojom/compositing/transferable_resource.mojom-shared.h"
|
||||
#include "skia/public/mojom/image_info_mojom_traits.h"
|
||||
#include "skia/public/mojom/surface_origin_mojom_traits.h"
|
||||
#include "ui/gfx/ipc/color/gfx_param_traits.h"
|
||||
|
||||
@ -131,6 +132,10 @@ struct StructTraits<viz::mojom::TransferableResourceDataView,
|
||||
return resource.origin;
|
||||
}
|
||||
|
||||
static SkAlphaType alpha_type(const viz::TransferableResource& resource) {
|
||||
return resource.alpha_type;
|
||||
}
|
||||
|
||||
static viz::TransferableResource::ResourceSource resource_source(
|
||||
const viz::TransferableResource& resource) {
|
||||
return resource.resource_source;
|
||||
|
@ -88,7 +88,6 @@ struct SurfaceQuadState {
|
||||
|
||||
struct TextureQuadState {
|
||||
ResourceId resource_id;
|
||||
bool premultiplied_alpha;
|
||||
gfx.mojom.PointF uv_top_left;
|
||||
gfx.mojom.PointF uv_bottom_right;
|
||||
skia.mojom.SkColor4f background_color;
|
||||
|
@ -12,6 +12,7 @@ import "services/viz/public/mojom/compositing/resource_id.mojom";
|
||||
import "ui/gfx/geometry/mojom/geometry.mojom";
|
||||
import "ui/gfx/mojom/color_space.mojom";
|
||||
import "ui/gfx/mojom/hdr_metadata.mojom";
|
||||
import "skia/public/mojom/image_info.mojom";
|
||||
import "skia/public/mojom/surface_origin.mojom";
|
||||
|
||||
// See viz::TransferableResource::SynchronizationType in
|
||||
@ -70,5 +71,6 @@ struct TransferableResource {
|
||||
bool needs_detiling;
|
||||
gpu.mojom.VulkanYCbCrInfo? ycbcr_info;
|
||||
skia.mojom.SurfaceOrigin origin;
|
||||
skia.mojom.AlphaType alpha_type;
|
||||
ResourceSource resource_source;
|
||||
};
|
||||
|
@ -292,6 +292,9 @@ bool CanvasResourceDispatcher::PrepareFrame(
|
||||
|
||||
PostImageToPlaceholderIfNotBlocked(std::move(canvas_resource), resource_id);
|
||||
|
||||
// TODO(crbug.com/645993): this should be inherited from WebGL context's
|
||||
// creation settings.
|
||||
resource.alpha_type = kPremul_SkAlphaType;
|
||||
frame->resource_list.push_back(std::move(resource));
|
||||
|
||||
viz::TextureDrawQuad* quad =
|
||||
@ -304,10 +307,6 @@ bool CanvasResourceDispatcher::PrepareFrame(
|
||||
uv_bottom_right, SkColors::kTransparent, nearest_neighbor,
|
||||
/*secure_output=*/false, gfx::ProtectedVideoType::kClear);
|
||||
|
||||
// TODO(crbug.com/645993): this should be inherited from WebGL context's
|
||||
// creation settings.
|
||||
quad->premultiplied_alpha = true;
|
||||
|
||||
frame->render_pass_list.push_back(std::move(pass));
|
||||
|
||||
if (change_size_for_next_commit_ ||
|
||||
|
@ -431,7 +431,6 @@ TEST_P(CanvasResourceDispatcherTest, DispatchFrame) {
|
||||
|
||||
const auto* texture_quad =
|
||||
static_cast<const viz::TextureDrawQuad*>(quad);
|
||||
EXPECT_TRUE(texture_quad->premultiplied_alpha);
|
||||
EXPECT_EQ(texture_quad->uv_top_left, gfx::PointF(0.0f, 0.0f));
|
||||
EXPECT_EQ(texture_quad->uv_bottom_right, gfx::PointF(1.0f, 1.0f));
|
||||
|
||||
@ -439,6 +438,8 @@ TEST_P(CanvasResourceDispatcherTest, DispatchFrame) {
|
||||
// whose origin is top-left.
|
||||
EXPECT_EQ(frame->resource_list.front().origin,
|
||||
kTopLeft_GrSurfaceOrigin);
|
||||
EXPECT_EQ(frame->resource_list.front().alpha_type,
|
||||
kPremul_SkAlphaType);
|
||||
})));
|
||||
|
||||
constexpr SkIRect damage_rect = SkIRect::MakeWH(kDamageWidth, kDamageHeight);
|
||||
|
Reference in New Issue
Block a user