Skip to content

Commit

Permalink
Make cc/trees/* use Skcolor4f
Browse files Browse the repository at this point in the history
This CL also connects the newly created SkColor4f Mojom to cc/mojom.

This is part of a long effort of increasing the color precision in
Chrome.

Bug: 1308932
Change-Id: I0b48e98dfd3020f105880a667d0a284f0395a678
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688042
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Juanmi Huertas <juanmihd@chromium.org>
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: ccameron chromium <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1015395}
  • Loading branch information
Juanmihd authored and Chromium LUCI CQ committed Jun 17, 2022
1 parent 038df71 commit f7335ba
Show file tree
Hide file tree
Showing 29 changed files with 76 additions and 62 deletions.
12 changes: 4 additions & 8 deletions cc/layers/layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -576,12 +576,9 @@ SkColor4f Layer::SafeOpaqueBackgroundColor(
}

SkColor4f Layer::SafeOpaqueBackgroundColor() const {
// TODO(crbug/1308932): Remove FromColor and make all SkColor4f.
SkColor4f host_background_color =
IsAttached()
? SkColor4f::FromColor(
layer_tree_host()->pending_commit_state()->background_color)
: layer_tree_inputs()->safe_opaque_background_color;
IsAttached() ? layer_tree_host()->pending_commit_state()->background_color
: layer_tree_inputs()->safe_opaque_background_color;
return SafeOpaqueBackgroundColor(host_background_color);
}

Expand Down Expand Up @@ -1469,9 +1466,8 @@ void Layer::PushPropertiesTo(LayerImpl* layer,
layer->SetElementId(inputs.element_id);
layer->SetHasTransformNode(has_transform_node());
layer->SetBackgroundColor(inputs.background_color);
// TODO(crbug/1308932): Remove FromColor and make all SkColor4f.
layer->SetSafeOpaqueBackgroundColor(SafeOpaqueBackgroundColor(
SkColor4f::FromColor(commit_state.background_color)));
layer->SetSafeOpaqueBackgroundColor(
SafeOpaqueBackgroundColor(commit_state.background_color));
layer->SetBounds(inputs.bounds);
layer->SetTransformTreeIndex(transform_tree_index(property_trees));
layer->SetEffectTreeIndex(effect_tree_index(property_trees));
Expand Down
2 changes: 1 addition & 1 deletion cc/layers/layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1402,7 +1402,7 @@ TEST_F(LayerTest, SafeOpaqueBackgroundColor) {
layer->SetBackgroundColor(layer_opaque ? SkColors::kRed
: SkColors::kTransparent);
layer_tree_host->set_background_color(
host_opaque ? SK_ColorRED : SK_ColorTRANSPARENT);
host_opaque ? SkColors::kRed : SkColors::kTransparent);

layer_tree_host->property_trees()->set_needs_rebuild(true);
layer_tree_host->BuildPropertyTreesForTesting();
Expand Down
4 changes: 1 addition & 3 deletions cc/layers/solid_color_scrollbar_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ SolidColorScrollbarLayerImpl::SolidColorScrollbarLayerImpl(
/*is_overlay*/ true),
thumb_thickness_(thumb_thickness),
track_start_(track_start),
// TODO(crbug/1308932): Remove FromColor and make all SkColor4f.
color_(SkColor4f::FromColor(
tree_impl->settings().solid_color_scrollbar_color)) {}
color_(tree_impl->settings().solid_color_scrollbar_color) {}

void SolidColorScrollbarLayerImpl::PushPropertiesTo(LayerImpl* layer) {
ScrollbarLayerImplBase::PushPropertiesTo(layer);
Expand Down
1 change: 1 addition & 0 deletions cc/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ mojom("mojom") {
public_deps = [
"//mojo/public/mojom/base",
"//services/viz/public/mojom",
"//skia/public/mojom",
"//ui/gfx/geometry/mojom",
]

Expand Down
1 change: 1 addition & 0 deletions cc/mojom/DEPS
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
include_rules = [
"+mojo/public/cpp/base",
"+services/viz/public/cpp/compositing",
"+skia/public/mojom",
]
3 changes: 2 additions & 1 deletion cc/mojom/render_frame_metadata.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import "mojo/public/mojom/base/time.mojom";
import "services/viz/public/mojom/compositing/local_surface_id.mojom";
import "services/viz/public/mojom/compositing/selection.mojom";
import "services/viz/public/mojom/compositing/vertical_scroll_direction.mojom";
import "skia/public/mojom/skcolor4f.mojom";
import "ui/gfx/geometry/mojom/geometry.mojom";

// Contains information to assist in making a decision about forwarding
Expand All @@ -27,7 +28,7 @@ struct RenderFrameMetadata {
// The background color of a CompositorFrame. It can be used for filling the
// content area if the primary surface is unavailable and fallback is not
// specified.
uint32 root_background_color;
skia.mojom.SkColor4f root_background_color;

// Scroll offset of the root layer. This optional parameter is only sent
// during tests.
Expand Down
5 changes: 3 additions & 2 deletions cc/mojom/render_frame_metadata_mojom_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "mojo/public/cpp/base/time_mojom_traits.h"
#include "services/viz/public/cpp/compositing/selection_mojom_traits.h"
#include "services/viz/public/cpp/compositing/vertical_scroll_direction_mojom_traits.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/gfx/geometry/mojom/geometry_mojom_traits.h"
#include "ui/gfx/mojom/selection_bound_mojom_traits.h"

Expand All @@ -27,7 +28,6 @@ bool StructTraits<
cc::mojom::RenderFrameMetadataDataView,
cc::RenderFrameMetadata>::Read(cc::mojom::RenderFrameMetadataDataView data,
cc::RenderFrameMetadata* out) {
out->root_background_color = data.root_background_color();
out->is_scroll_offset_at_top = data.is_scroll_offset_at_top();
out->is_mobile_optimized = data.is_mobile_optimized();
out->device_scale_factor = data.device_scale_factor();
Expand Down Expand Up @@ -60,7 +60,8 @@ bool StructTraits<
data.ReadPreviousSurfacesVisualUpdateDuration(
&out->previous_surfaces_visual_update_duration) &&
data.ReadCurrentSurfaceVisualUpdateDuration(
&out->current_surface_visual_update_duration);
&out->current_surface_visual_update_duration) &&
data.ReadRootBackgroundColor(&out->root_background_color);
}

} // namespace mojo
4 changes: 3 additions & 1 deletion cc/mojom/render_frame_metadata_mojom_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
#include "cc/mojom/render_frame_metadata.mojom-shared.h"
#include "cc/trees/render_frame_metadata.h"
#include "services/viz/public/cpp/compositing/local_surface_id_mojom_traits.h"
#include "skia/public/mojom/skcolor4f_mojom_traits.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/skia/include/core/SkColor.h"

namespace mojo {

Expand All @@ -31,7 +33,7 @@ template <>
struct COMPONENT_EXPORT(CC_SHARED_MOJOM_TRAITS)
StructTraits<cc::mojom::RenderFrameMetadataDataView,
cc::RenderFrameMetadata> {
static SkColor root_background_color(
static SkColor4f root_background_color(
const cc::RenderFrameMetadata& metadata) {
return metadata.root_background_color;
}
Expand Down
2 changes: 1 addition & 1 deletion cc/trees/commit_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ struct CC_EXPORT CommitState {
LayerSelection selection;
LayerTreeDebugState debug_state;
OverscrollBehavior overscroll_behavior;
SkColor background_color = SK_ColorWHITE;
SkColor4f background_color = SkColors::kWhite;
ViewportPropertyIds viewport_property_ids;
viz::LocalSurfaceId local_surface_id_from_parent;
base::TimeDelta previous_surfaces_visual_update_duration;
Expand Down
4 changes: 2 additions & 2 deletions cc/trees/layer_tree_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,10 @@ class CC_EXPORT LayerTreeHost : public MutatorHostClient {
return pending_commit_state()->max_page_scale_factor;
}

void set_background_color(SkColor color) {
void set_background_color(SkColor4f color) {
pending_commit_state()->background_color = color;
}
SkColor background_color() const {
SkColor4f background_color() const {
return pending_commit_state()->background_color;
}

Expand Down
13 changes: 8 additions & 5 deletions cc/trees/layer_tree_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,7 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(FrameData* frame) {
active_tree_->GetDeviceViewport().origin());
#endif
bool has_transparent_background =
SkColorGetA(active_tree_->background_color()) != SK_AlphaOPAQUE;
!active_tree_->background_color().isOpaque();
auto* root_render_surface = active_tree_->RootRenderSurface();
if (root_render_surface && !has_transparent_background) {
frame->render_passes.back()->has_transparent_background = false;
Expand All @@ -1454,9 +1454,10 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(FrameData* frame) {
if (num_missing_tiles > 0)
fill_region = root_render_surface->content_rect();

AppendQuadsToFillScreen(frame->render_passes.back().get(),
root_render_surface,
active_tree_->background_color(), fill_region);
// TODO(crbug/1308932): Remove toSkColor and make all SkColor4f.
AppendQuadsToFillScreen(
frame->render_passes.back().get(), root_render_surface,
active_tree_->background_color().toSkColor(), fill_region);
}

RemoveRenderPasses(frame);
Expand Down Expand Up @@ -2258,7 +2259,9 @@ viz::CompositorFrameMetadata LayerTreeHostImpl::MakeCompositorFrameMetadata() {

metadata.page_scale_factor = active_tree_->current_page_scale_factor();
metadata.scrollable_viewport_size = active_tree_->ScrollableViewportSize();
metadata.root_background_color = active_tree_->background_color();

// TODO(crbug/1308932): Remove toSkColor and make all SkColor4f.
metadata.root_background_color = active_tree_->background_color().toSkColor();
metadata.may_throttle_if_undrawn_frames = may_throttle_if_undrawn_frames_;

if (active_tree_->has_presentation_callbacks()) {
Expand Down
11 changes: 6 additions & 5 deletions cc/trees/layer_tree_host_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6281,7 +6281,8 @@ TEST_P(ScrollUnifiedLayerTreeHostImplTest,
// The background is default to transparent. If the background is opaque, we
// would fill the frame with background colour when no layers are contributing
// quads. This means we would end up with 0 quad.
EXPECT_EQ(host_impl_->active_tree()->background_color(), SK_ColorTRANSPARENT);
EXPECT_EQ(host_impl_->active_tree()->background_color(),
SkColors::kTransparent);

{
TestFrameData frame;
Expand Down Expand Up @@ -10724,7 +10725,7 @@ class LayerTreeHostImplViewportCoveredTest : public LayerTreeHostImplTest {
}

void SetupActiveTreeLayers() {
host_impl_->active_tree()->set_background_color(SK_ColorGRAY);
host_impl_->active_tree()->set_background_color(SkColors::kGray);
LayerImpl* root = SetupDefaultRootLayer(viewport_size_);
child_ = AddLayer<BlendStateCheckLayer>(host_impl_->active_tree(),
host_impl_->resource_provider());
Expand Down Expand Up @@ -11146,7 +11147,7 @@ TEST_P(ScrollUnifiedLayerTreeHostImplTest, LayersFreeTextures) {

TEST_P(ScrollUnifiedLayerTreeHostImplTest, HasTransparentBackground) {
SetupDefaultRootLayer(gfx::Size(10, 10));
host_impl_->active_tree()->set_background_color(SK_ColorWHITE);
host_impl_->active_tree()->set_background_color(SkColors::kWhite);
UpdateDrawProperties(host_impl_->active_tree());

// Verify one quad is drawn when transparent background set is not set.
Expand All @@ -11170,7 +11171,7 @@ TEST_P(ScrollUnifiedLayerTreeHostImplTest, HasTransparentBackground) {
host_impl_->SetFullViewportDamage();

// Verify no quads are drawn when transparent background is set.
host_impl_->active_tree()->set_background_color(SK_ColorTRANSPARENT);
host_impl_->active_tree()->set_background_color(SkColors::kTransparent);
host_impl_->SetFullViewportDamage();
args = viz::CreateBeginFrameArgsForTesting(
BEGINFRAME_FROM_HERE, viz::BeginFrameArgs::kManualSourceId, 1,
Expand All @@ -11189,7 +11190,7 @@ TEST_P(ScrollUnifiedLayerTreeHostImplTest, HasTransparentBackground) {
host_impl_->SetFullViewportDamage();

// Verify no quads are drawn when semi-transparent background is set.
host_impl_->active_tree()->set_background_color(SkColorSetARGB(5, 255, 0, 0));
host_impl_->active_tree()->set_background_color({1.0f, 0.0f, 0.0f, 0.1f});
host_impl_->SetFullViewportDamage();
host_impl_->WillBeginImplFrame(viz::CreateBeginFrameArgsForTesting(
BEGINFRAME_FROM_HERE, viz::BeginFrameArgs::kManualSourceId, 1,
Expand Down
8 changes: 4 additions & 4 deletions cc/trees/layer_tree_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3208,7 +3208,7 @@ class LayerTreeHostTestCommit : public LayerTreeHostTest {
void BeginTest() override {
layer_tree_host()->SetViewportRectAndScale(gfx::Rect(20, 20), 1.f,
viz::LocalSurfaceId());
layer_tree_host()->set_background_color(SK_ColorGRAY);
layer_tree_host()->set_background_color(SkColors::kGray);
layer_tree_host()->SetEventListenerProperties(
EventListenerClass::kMouseWheel, EventListenerProperties::kPassive);
layer_tree_host()->SetEventListenerProperties(
Expand All @@ -3224,7 +3224,7 @@ class LayerTreeHostTestCommit : public LayerTreeHostTest {

void DidActivateTreeOnThread(LayerTreeHostImpl* impl) override {
EXPECT_EQ(gfx::Rect(20, 20), impl->active_tree()->GetDeviceViewport());
EXPECT_EQ(SK_ColorGRAY, impl->active_tree()->background_color());
EXPECT_EQ(SkColors::kGray, impl->active_tree()->background_color());
EXPECT_EQ(EventListenerProperties::kPassive,
impl->active_tree()->event_listener_properties(
EventListenerClass::kMouseWheel));
Expand Down Expand Up @@ -3254,7 +3254,7 @@ class LayerTreeHostTestFrameTimeUpdatesAfterActivationFails
void BeginTest() override {
layer_tree_host()->SetViewportRectAndScale(gfx::Rect(20, 20), 1.f,
viz::LocalSurfaceId());
layer_tree_host()->set_background_color(SK_ColorGRAY);
layer_tree_host()->set_background_color(SkColors::kGray);

PostSetNeedsCommitToMainThread();
}
Expand Down Expand Up @@ -3307,7 +3307,7 @@ class LayerTreeHostTestFrameTimeUpdatesAfterDraw : public LayerTreeHostTest {
void BeginTest() override {
layer_tree_host()->SetViewportRectAndScale(gfx::Rect(20, 20), 1.f,
viz::LocalSurfaceId());
layer_tree_host()->set_background_color(SK_ColorGRAY);
layer_tree_host()->set_background_color(SkColors::kGray);

PostSetNeedsCommitToMainThread();
}
Expand Down
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ LayerTreeImpl::LayerTreeImpl(
is_first_frame_after_commit_tracker_(-1),
hud_layer_(nullptr),
property_trees_(host_impl),
background_color_(0),
background_color_(SkColors::kTransparent),
last_scrolled_scroll_node_index_(kInvalidPropertyNodeId),
page_scale_factor_(page_scale_factor),
min_page_scale_factor_(0),
Expand Down
6 changes: 3 additions & 3 deletions cc/trees/layer_tree_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,8 @@ class CC_EXPORT LayerTreeImpl {

void ApplySentScrollAndScaleDeltasFromAbortedCommit();

SkColor background_color() const { return background_color_; }
void set_background_color(SkColor color) { background_color_ = color; }
SkColor4f background_color() const { return background_color_; }
void set_background_color(SkColor4f color) { background_color_ = color; }

gfx::OverlayTransform display_transform_hint() const {
return display_transform_hint_;
Expand Down Expand Up @@ -831,7 +831,7 @@ class CC_EXPORT LayerTreeImpl {
int is_first_frame_after_commit_tracker_;
raw_ptr<HeadsUpDisplayLayerImpl> hud_layer_;
PropertyTrees property_trees_;
SkColor background_color_;
SkColor4f background_color_;

int last_scrolled_scroll_node_index_;

Expand Down
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class CC_EXPORT LayerTreeSettings {
base::TimeDelta scrollbar_fade_duration;
base::TimeDelta scrollbar_thinning_duration;
bool scrollbar_flash_after_any_scroll_update = false;
SkColor solid_color_scrollbar_color = SK_ColorWHITE;
SkColor4f solid_color_scrollbar_color = SkColors::kWhite;
base::TimeDelta scroll_animation_duration_for_testing;
bool timeout_and_draw_when_animation_checkerboards = true;
bool layers_always_allowed_lcd_text = false;
Expand Down
10 changes: 6 additions & 4 deletions cc/trees/property_tree_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -806,10 +806,12 @@ void PropertyTreeBuilderContext::BuildPropertyTrees() {
data_for_recursion.animation_axis_aligned_since_render_target = true;
data_for_recursion.not_axis_aligned_since_last_clip = false;

SkColor root_background_color = layer_tree_host_->background_color();
if (SkColorGetA(root_background_color) != 255)
root_background_color = SkColorSetA(root_background_color, 255);
data_for_recursion.safe_opaque_background_color = root_background_color;
SkColor4f root_background_color =
layer_tree_host_->background_color().isOpaque()
? layer_tree_host_->background_color()
: layer_tree_host_->background_color().makeOpaque();
data_for_recursion.safe_opaque_background_color =
root_background_color.toSkColor();

property_trees_.clear();
transform_tree_.set_device_scale_factor(
Expand Down
2 changes: 1 addition & 1 deletion cc/trees/render_frame_metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class CC_EXPORT RenderFrameMetadata {
// The background color of a CompositorFrame. It can be used for filling the
// content area if the primary surface is unavailable and fallback is not
// specified.
SkColor root_background_color = SK_ColorWHITE;
SkColor4f root_background_color = SkColors::kWhite;

// Scroll offset of the root layer.
absl::optional<gfx::PointF> root_scroll_offset;
Expand Down
3 changes: 2 additions & 1 deletion content/browser/renderer_host/compositor_impl_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
#include "services/viz/public/mojom/compositing/compositor_frame_sink.mojom.h"
#include "third_party/khronos/GLES2/gl2.h"
#include "third_party/khronos/GLES2/gl2ext.h"
#include "third_party/skia/include/core/SkColor.h"
#include "third_party/skia/include/core/SkMallocPixelRef.h"
#include "ui/android/window_android.h"
#include "ui/display/display.h"
Expand Down Expand Up @@ -468,7 +469,7 @@ void CompositorImpl::SetSurface(const base::android::JavaRef<jobject>& surface,

void CompositorImpl::SetBackgroundColor(int color) {
DCHECK(host_);
host_->set_background_color(color);
host_->set_background_color(SkColor4f::FromColor(color));
}

void CompositorImpl::CreateLayerTreeHost() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,6 @@ void RenderWidgetHostViewAndroid::LostFocus() {
void RenderWidgetHostViewAndroid::OnRenderFrameMetadataChangedBeforeActivation(
const cc::RenderFrameMetadata& metadata) {
bool is_transparent = metadata.has_transparent_background;
SkColor root_background_color = metadata.root_background_color;

if (!using_browser_compositor_) {
// DevTools ScreenCast support for Android WebView.
Expand Down Expand Up @@ -480,8 +479,10 @@ void RenderWidgetHostViewAndroid::OnRenderFrameMetadataChangedBeforeActivation(
metadata.bottom_controls_shown_ratio,
metadata.bottom_controls_min_height_offset);

SetContentBackgroundColor(is_transparent ? SK_ColorTRANSPARENT
: root_background_color);
// TODO(crbug/1308932): Remove toSkColor and make all SkColor4f.
SetContentBackgroundColor(is_transparent
? SK_ColorTRANSPARENT
: metadata.root_background_color.toSkColor());

if (overscroll_controller_) {
overscroll_controller_->OnFrameMetadataUpdated(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2133,7 +2133,9 @@ void RenderWidgetHostViewAura::OnRenderFrameMetadataChangedAfterActivation(
base::TimeTicks activation_time) {
const cc::RenderFrameMetadata& metadata =
host()->render_frame_metadata_provider()->LastRenderFrameMetadata();
SetContentBackgroundColor(metadata.root_background_color);

// TODO(crbug/1308932): Remove toSkColor and make all SkColor4f.
SetContentBackgroundColor(metadata.root_background_color.toSkColor());
if (inset_surface_id_.is_valid() && metadata.local_surface_id &&
metadata.local_surface_id.value().is_valid() &&
metadata.local_surface_id.value().IsSameOrNewerThan(inset_surface_id_)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3113,7 +3113,7 @@ TEST_F(RenderWidgetHostViewAuraTest, BackgroundColorMatchesCompositorFrame) {
view_->SetSize(frame_size);
view_->Show();
cc::RenderFrameMetadata metadata;
metadata.root_background_color = SK_ColorRED;
metadata.root_background_color = SkColors::kRed;
view_->SetRenderFrameMetadata(metadata);
view_->OnRenderFrameMetadataChangedAfterActivation(base::TimeTicks::Now());
ui::Layer* parent_layer = view_->GetNativeView()->layer();
Expand All @@ -3134,7 +3134,7 @@ TEST_F(RenderWidgetHostViewAuraTest, BackgroundColorOrder) {
// If the content background color is available, ignore the default background
// color setting.
cc::RenderFrameMetadata metadata;
metadata.root_background_color = SK_ColorWHITE;
metadata.root_background_color = SkColors::kWhite;
view_->SetRenderFrameMetadata(metadata);
view_->OnRenderFrameMetadataChangedAfterActivation(base::TimeTicks::Now());
ASSERT_TRUE(view_->GetBackgroundColor());
Expand Down
Loading

0 comments on commit f7335ba

Please sign in to comment.