Commit 7354503d authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

Communicate color space to ui::Compositor

We don't send the color space to the browser's ui::Compositor on macOS.
This has been broken on macOS for an unknown period of time. It didn't
actually make a difference because
- that only set the raster space for browser UI, which is sRGB anyway
- the renderer got the correct color space for raster via
  RenderWidgetHostImpl::GetScreenInfo
- we would draw using CoreAnimation (not GLRenderer) >99.9% of the
  time, so the ui::Compositor's GLRenderer's color space didn't matter
Because of rounded corner rects, we hit the GLRenderer more often,
causing this bug be visible.

Change RecyclableCompositorMac::UpdateSurface to take a required
color space argument, ensuring that all callers specify a value.

Bug: 1038723
Change-Id: Icde934718371802356c5c26277a628361705009f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985127Reviewed-by: default avatarEric Karl <ericrk@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728067}
parent 75f74662
...@@ -152,10 +152,9 @@ bool BrowserCompositorMac::UpdateSurfaceFromNSView( ...@@ -152,10 +152,9 @@ bool BrowserCompositorMac::UpdateSurfaceFromNSView(
} }
if (recyclable_compositor_) { if (recyclable_compositor_) {
recyclable_compositor_->compositor()->SetDisplayColorSpace(
dfh_display_.color_space());
recyclable_compositor_->UpdateSurface(dfh_size_pixels_, recyclable_compositor_->UpdateSurface(dfh_size_pixels_,
dfh_display_.device_scale_factor()); dfh_display_.device_scale_factor(),
dfh_display_.color_space());
} }
return true; return true;
...@@ -174,7 +173,8 @@ void BrowserCompositorMac::UpdateSurfaceFromChild( ...@@ -174,7 +173,8 @@ void BrowserCompositorMac::UpdateSurfaceFromChild(
root_layer_->SetBounds(gfx::Rect(dfh_size_dip_)); root_layer_->SetBounds(gfx::Rect(dfh_size_dip_));
if (recyclable_compositor_) { if (recyclable_compositor_) {
recyclable_compositor_->UpdateSurface(dfh_size_pixels_, recyclable_compositor_->UpdateSurface(dfh_size_pixels_,
dfh_display_.device_scale_factor()); dfh_display_.device_scale_factor(),
dfh_display_.color_space());
} }
delegated_frame_host_->EmbedSurface( delegated_frame_host_->EmbedSurface(
dfh_local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation() dfh_local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation()
...@@ -278,11 +278,10 @@ void BrowserCompositorMac::TransitionToState(State new_state) { ...@@ -278,11 +278,10 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
ui::RecyclableCompositorMacFactory::Get()->CreateCompositor( ui::RecyclableCompositorMacFactory::Get()->CreateCompositor(
content::GetContextFactory(), content::GetContextFactoryPrivate()); content::GetContextFactory(), content::GetContextFactoryPrivate());
recyclable_compositor_->UpdateSurface(dfh_size_pixels_, recyclable_compositor_->UpdateSurface(dfh_size_pixels_,
dfh_display_.device_scale_factor()); dfh_display_.device_scale_factor(),
dfh_display_.color_space());
recyclable_compositor_->compositor()->SetRootLayer(root_layer_.get()); recyclable_compositor_->compositor()->SetRootLayer(root_layer_.get());
recyclable_compositor_->compositor()->SetBackgroundColor(background_color_); recyclable_compositor_->compositor()->SetBackgroundColor(background_color_);
recyclable_compositor_->compositor()->SetDisplayColorSpace(
dfh_display_.color_space());
recyclable_compositor_->widget()->SetNSView( recyclable_compositor_->widget()->SetNSView(
accelerated_widget_mac_ns_view_); accelerated_widget_mac_ns_view_);
recyclable_compositor_->Unsuspend(); recyclable_compositor_->Unsuspend();
......
...@@ -67,8 +67,10 @@ void RecyclableCompositorMac::Unsuspend() { ...@@ -67,8 +67,10 @@ void RecyclableCompositorMac::Unsuspend() {
compositor_suspended_lock_ = nullptr; compositor_suspended_lock_ = nullptr;
} }
void RecyclableCompositorMac::UpdateSurface(const gfx::Size& size_pixels, void RecyclableCompositorMac::UpdateSurface(
float scale_factor) { const gfx::Size& size_pixels,
float scale_factor,
const gfx::ColorSpace& color_space) {
if (size_pixels != size_pixels_ || scale_factor != scale_factor_) { if (size_pixels != size_pixels_ || scale_factor != scale_factor_) {
size_pixels_ = size_pixels; size_pixels_ = size_pixels;
scale_factor_ = scale_factor; scale_factor_ = scale_factor;
...@@ -78,15 +80,21 @@ void RecyclableCompositorMac::UpdateSurface(const gfx::Size& size_pixels, ...@@ -78,15 +80,21 @@ void RecyclableCompositorMac::UpdateSurface(const gfx::Size& size_pixels,
compositor()->SetScaleAndSize(scale_factor_, size_pixels_, compositor()->SetScaleAndSize(scale_factor_, size_pixels_,
local_surface_id_allocation); local_surface_id_allocation);
} }
if (color_space != color_space_) {
color_space_ = color_space;
compositor()->SetDisplayColorSpace(color_space_);
}
} }
void RecyclableCompositorMac::InvalidateSurface() { void RecyclableCompositorMac::InvalidateSurface() {
size_pixels_ = gfx::Size(); size_pixels_ = gfx::Size();
scale_factor_ = 1.f; scale_factor_ = 1.f;
local_surface_id_allocator_.Invalidate(); local_surface_id_allocator_.Invalidate();
color_space_ = gfx::ColorSpace();
compositor()->SetScaleAndSize( compositor()->SetScaleAndSize(
scale_factor_, size_pixels_, scale_factor_, size_pixels_,
local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation()); local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation());
compositor()->SetDisplayColorSpace(color_space_);
} }
void RecyclableCompositorMac::OnCompositingDidCommit( void RecyclableCompositorMac::OnCompositingDidCommit(
......
...@@ -41,22 +41,25 @@ class COMPOSITOR_EXPORT RecyclableCompositorMac ...@@ -41,22 +41,25 @@ class COMPOSITOR_EXPORT RecyclableCompositorMac
void Unsuspend(); void Unsuspend();
// Update the compositor's surface information, if needed. // Update the compositor's surface information, if needed.
void UpdateSurface(const gfx::Size& size_pixels, float scale_factor); void UpdateSurface(const gfx::Size& size_pixels,
float scale_factor,
const gfx::ColorSpace& color_space);
// Invalidate the compositor's surface information. // Invalidate the compositor's surface information.
void InvalidateSurface(); void InvalidateSurface();
// The viz::ParentLocalSurfaceIdAllocator for the ui::Compositor dispenses
// viz::LocalSurfaceIds that are renderered into by the ui::Compositor.
viz::ParentLocalSurfaceIdAllocator local_surface_id_allocator_;
gfx::Size size_pixels_;
float scale_factor_ = 1.f;
private: private:
friend class RecyclableCompositorMacFactory; friend class RecyclableCompositorMacFactory;
// ui::CompositorObserver implementation: // ui::CompositorObserver implementation:
void OnCompositingDidCommit(ui::Compositor* compositor) override; void OnCompositingDidCommit(ui::Compositor* compositor) override;
// The viz::ParentLocalSurfaceIdAllocator for the ui::Compositor dispenses
// viz::LocalSurfaceIds that are renderered into by the ui::Compositor.
viz::ParentLocalSurfaceIdAllocator local_surface_id_allocator_;
gfx::Size size_pixels_;
float scale_factor_ = 1.f;
gfx::ColorSpace color_space_;
std::unique_ptr<ui::AcceleratedWidgetMac> accelerated_widget_mac_; std::unique_ptr<ui::AcceleratedWidgetMac> accelerated_widget_mac_;
ui::Compositor compositor_; ui::Compositor compositor_;
std::unique_ptr<ui::CompositorLock> compositor_suspended_lock_; std::unique_ptr<ui::CompositorLock> compositor_suspended_lock_;
......
...@@ -529,7 +529,7 @@ void NativeWidgetMacNSWindowHost::UpdateCompositorProperties() { ...@@ -529,7 +529,7 @@ void NativeWidgetMacNSWindowHost::UpdateCompositorProperties() {
layer()->SetBounds(gfx::Rect(surface_size_in_dip)); layer()->SetBounds(gfx::Rect(surface_size_in_dip));
compositor_->UpdateSurface( compositor_->UpdateSurface(
ConvertSizeToPixel(display_.device_scale_factor(), surface_size_in_dip), ConvertSizeToPixel(display_.device_scale_factor(), surface_size_in_dip),
display_.device_scale_factor()); display_.device_scale_factor(), display_.color_space());
} }
void NativeWidgetMacNSWindowHost::DestroyCompositor() { void NativeWidgetMacNSWindowHost::DestroyCompositor() {
...@@ -989,7 +989,7 @@ void NativeWidgetMacNSWindowHost::OnWindowDisplayChanged( ...@@ -989,7 +989,7 @@ void NativeWidgetMacNSWindowHost::OnWindowDisplayChanged(
compositor_->UpdateSurface( compositor_->UpdateSurface(
ConvertSizeToPixel(display_.device_scale_factor(), ConvertSizeToPixel(display_.device_scale_factor(),
content_bounds_in_screen_.size()), content_bounds_in_screen_.size()),
display_.device_scale_factor()); display_.device_scale_factor(), display_.color_space());
} }
if (display_id_changed) { if (display_id_changed) {
display_link_ = ui::DisplayLinkMac::GetForDisplay(display_.id()); display_link_ = ui::DisplayLinkMac::GetForDisplay(display_.id());
......
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