color: Replace more instances of ICCProfile with ColorSpace (issue 2954793002 by ccameron@chromium.org)

0 views
Skip to first unread message

ccam...@chromium.org

unread,
Jun 24, 2017, 4:09:17 AM6/24/17
to hu...@chromium.org, chromium...@chromium.org, mlamouri+wa...@chromium.org, dongseo...@intel.com, creis...@chromium.org, blink-reviews-p...@chromium.org, nasko+c...@chromium.org, j...@chromium.org, pdr+graphi...@chromium.org, fmalit...@chromium.org, blink-re...@chromium.org, dsch...@chromium.org, dglazko...@chromium.org, caba...@adobe.com, dari...@chromium.org, ju...@chromium.org, mac-r...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, kinuko...@chromium.org, sche...@chromium.org, rob....@samsung.com
Reviewers: hubbe
CL: https://codereview.chromium.org/2954793002/

Message:
This covers some of the suggestions you had from the previous patch .. turns out
they weren't very complicated, and I should have rolled them in there.

Description:
color: Replace more instances of ICCProfile with ColorSpace

The gfx::ColorSpace representation is more compact and can communicate
information such as HDR status.

TBR=hubbe
BUG=735613

Affected files (+22, -26 lines):
M content/browser/renderer_host/render_widget_host_impl.cc
M content/browser/web_contents/web_contents_view_aura.cc
M content/browser/web_contents/web_contents_view_mac.mm
M content/common/view_messages.h
M content/public/common/screen_info.h
M content/public/common/screen_info.cc
M content/renderer/render_view_impl.cc
M content/renderer/render_widget.cc
M third_party/WebKit/Source/platform/graphics/ColorSpaceGamut.cpp
M third_party/WebKit/public/platform/WebScreenInfo.h


Index: content/browser/renderer_host/render_widget_host_impl.cc
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 2d02a4f5b06820523b396f4892a2b8d9d584f130..9ae1ad4b019cb2d0513438144ccb8248a3fa3c42 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -680,10 +680,8 @@ bool RenderWidgetHostImpl::GetResizeParams(ResizeParams* resize_params) {
// coloring.
// TODO(ccameron): Disable this once color correct rasterization is functional
// https://crbug.com/701942
- if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableHDR)) {
- gfx::ColorSpace::CreateSRGB().GetICCProfile(
- &resize_params->screen_info.icc_profile);
- }
+ if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableHDR))
+ resize_params->screen_info.color_space = gfx::ColorSpace::CreateSRGB();

if (delegate_) {
resize_params->is_fullscreen_granted =
Index: content/browser/web_contents/web_contents_view_aura.cc
diff --git a/content/browser/web_contents/web_contents_view_aura.cc b/content/browser/web_contents/web_contents_view_aura.cc
index 58b55db1bcfcbf53ca99f99ea4ec2bf54aad9617..870aa442d919ac4aaab9ae62ad49d3f7ff622dbe 100644
--- a/content/browser/web_contents/web_contents_view_aura.cc
+++ b/content/browser/web_contents/web_contents_view_aura.cc
@@ -711,7 +711,7 @@ void GetScreenInfoForWindow(ScreenInfo* results,
results->depth_per_component = display.depth_per_component();
results->is_monochrome = display.is_monochrome();
results->device_scale_factor = display.device_scale_factor();
- display.color_space().GetICCProfile(&results->icc_profile);
+ results->color_space = display.color_space();

// The Display rotation and the ScreenInfo orientation are not the same
// angle. The former is the physical display rotation while the later is the
Index: content/browser/web_contents/web_contents_view_mac.mm
diff --git a/content/browser/web_contents/web_contents_view_mac.mm b/content/browser/web_contents/web_contents_view_mac.mm
index f046623287bd6dfde3317038ffd33d2dd7f86446..1062bb51622336d1a9060746c2f286543a75d98f 100644
--- a/content/browser/web_contents/web_contents_view_mac.mm
+++ b/content/browser/web_contents/web_contents_view_mac.mm
@@ -89,7 +89,7 @@ content::ScreenInfo GetNSViewScreenInfo(NSView* view) {

content::ScreenInfo results;
results.device_scale_factor = static_cast<int>(display.device_scale_factor());
- display.color_space().GetICCProfile(&results.icc_profile);
+ results.color_space = display.color_space();
results.depth = display.color_depth();
results.depth_per_component = display.depth_per_component();
results.is_monochrome = display.is_monochrome();
Index: content/common/view_messages.h
diff --git a/content/common/view_messages.h b/content/common/view_messages.h
index fc2ad5b07ec38d6a20d9d23fc6529fa10a538e15..d52258a75b77fdd2f3c1e309c98c34f08153fd2e 100644
--- a/content/common/view_messages.h
+++ b/content/common/view_messages.h
@@ -56,12 +56,12 @@
#include "ui/base/ime/text_input_mode.h"
#include "ui/base/ime/text_input_type.h"
#include "ui/base/ui_base_types.h"
+#include "ui/gfx/color_space.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/rect_f.h"
#include "ui/gfx/geometry/vector2d.h"
#include "ui/gfx/geometry/vector2d_f.h"
-#include "ui/gfx/icc_profile.h"
#include "ui/gfx/ipc/color/gfx_param_traits.h"
#include "ui/gfx/ipc/gfx_param_traits.h"
#include "ui/gfx/ipc/skia/gfx_skia_param_traits.h"
@@ -164,7 +164,7 @@ IPC_STRUCT_TRAITS_END()

IPC_STRUCT_TRAITS_BEGIN(content::ScreenInfo)
IPC_STRUCT_TRAITS_MEMBER(device_scale_factor)
- IPC_STRUCT_TRAITS_MEMBER(icc_profile)
+ IPC_STRUCT_TRAITS_MEMBER(color_space)
IPC_STRUCT_TRAITS_MEMBER(depth)
IPC_STRUCT_TRAITS_MEMBER(depth_per_component)
IPC_STRUCT_TRAITS_MEMBER(is_monochrome)
Index: content/public/common/screen_info.cc
diff --git a/content/public/common/screen_info.cc b/content/public/common/screen_info.cc
index acd02a042b1a904d057364ffad1c727beecb963a..488e3681027573e8fa7602ff7d3ba87e37103670 100644
--- a/content/public/common/screen_info.cc
+++ b/content/public/common/screen_info.cc
@@ -12,11 +12,9 @@ ScreenInfo::~ScreenInfo() = default;

bool ScreenInfo::operator==(const ScreenInfo& other) const {
return device_scale_factor == other.device_scale_factor &&
- icc_profile == other.icc_profile &&
- depth == other.depth &&
+ color_space == other.color_space && depth == other.depth &&
depth_per_component == other.depth_per_component &&
- is_monochrome == other.is_monochrome &&
- rect == other.rect &&
+ is_monochrome == other.is_monochrome && rect == other.rect &&
available_rect == other.available_rect &&
orientation_type == other.orientation_type &&
orientation_angle == other.orientation_angle;
Index: content/public/common/screen_info.h
diff --git a/content/public/common/screen_info.h b/content/public/common/screen_info.h
index b758fed561b4c0562967ab437ab7a3655ebe1f3f..0a2afbb0e3205615680f759400f6b7a2bd8e69d4 100644
--- a/content/public/common/screen_info.h
+++ b/content/public/common/screen_info.h
@@ -7,8 +7,8 @@

#include "content/common/content_export.h"
#include "content/public/common/screen_orientation_values.h"
+#include "ui/gfx/color_space.h"
#include "ui/gfx/geometry/rect.h"
-#include "ui/gfx/icc_profile.h"

namespace content {

@@ -23,8 +23,8 @@ struct CONTENT_EXPORT ScreenInfo {
// pixels.
float device_scale_factor = 1.f;

- // The ICC profile of the output display.
- gfx::ICCProfile icc_profile;
+ // The color space of the output display.
+ gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();

// The screen depth in bits per pixel
uint32_t depth = 0;
Index: content/renderer/render_view_impl.cc
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 506a5a1a7b604a8499c0a0cbeb4de2f473e35771..743694032c9eabf354953d05c900642d868dd72d 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -2481,7 +2481,7 @@ void RenderViewImpl::SetDeviceColorProfileForTesting(
const gfx::ICCProfile& icc_profile) {
ResizeParams params;
params.screen_info = screen_info_;
- params.screen_info.icc_profile = icc_profile;
+ params.screen_info.color_space = icc_profile.GetColorSpace();
params.new_size = size();
params.visible_viewport_size = visible_viewport_size_;
params.physical_backing_size = physical_backing_size_;
Index: content/renderer/render_widget.cc
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index ed2476d7dab878d97a497c571d17a60e08d01746..0519a5a031b62e770653806cf7d82718353dcc6a 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -1219,7 +1219,7 @@ void RenderWidget::Resize(const ResizeParams& params) {
compositor_->SetViewportSize(params.physical_backing_size);
compositor_->setBottomControlsHeight(params.bottom_controls_height);
compositor_->SetRasterColorSpace(
- screen_info_.icc_profile.GetParametricColorSpace());
+ screen_info_.color_space.GetParametricApproximation());
// If surface synchronization is enable, then this will use the provided
// |local_surface_id_| to submit the next generated CompositorFrame.
// If the ID is not valid, then the compositor will defer commits until
@@ -1317,7 +1317,7 @@ blink::WebLayerTreeView* RenderWidget::InitializeLayerTreeView() {
compositor_->SetViewportSize(physical_backing_size_);
OnDeviceScaleFactorChanged();
compositor_->SetRasterColorSpace(
- screen_info_.icc_profile.GetParametricColorSpace());
+ screen_info_.color_space.GetParametricApproximation());
compositor_->SetContentSourceId(current_content_source_id_);
compositor_->SetLocalSurfaceId(local_surface_id_);
// For background pages and certain tests, we don't want to trigger
@@ -2168,7 +2168,7 @@ bool RenderWidget::CanComposeInline() {
blink::WebScreenInfo RenderWidget::GetScreenInfo() {
blink::WebScreenInfo web_screen_info;
web_screen_info.device_scale_factor = screen_info_.device_scale_factor;
- web_screen_info.icc_profile = screen_info_.icc_profile;
+ web_screen_info.color_space = screen_info_.color_space;
web_screen_info.depth = screen_info_.depth;
web_screen_info.depth_per_component = screen_info_.depth_per_component;
web_screen_info.is_monochrome = screen_info_.is_monochrome;
Index: third_party/WebKit/Source/platform/graphics/ColorSpaceGamut.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/ColorSpaceGamut.cpp b/third_party/WebKit/Source/platform/graphics/ColorSpaceGamut.cpp
index 452a1979661fac0a0e8b80a335fc78a17f012e02..d83dabfca1c8f1df86464085028164f95a9e131e 100644
--- a/third_party/WebKit/Source/platform/graphics/ColorSpaceGamut.cpp
+++ b/third_party/WebKit/Source/platform/graphics/ColorSpaceGamut.cpp
@@ -12,12 +12,12 @@ namespace blink {
namespace ColorSpaceUtilities {

ColorSpaceGamut GetColorSpaceGamut(const WebScreenInfo& screen_info) {
- const gfx::ICCProfile& profile = screen_info.icc_profile;
- if (profile == gfx::ICCProfile())
+ const gfx::ColorSpace& color_space = screen_info.color_space;
+ if (!color_space.IsValid())
return ColorSpaceGamut::kUnknown;

return ColorSpaceUtilities::GetColorSpaceGamut(
- profile.GetColorSpace().ToSkColorSpace().get());
+ color_space.ToSkColorSpace().get());
}

ColorSpaceGamut GetColorSpaceGamut(SkColorSpace* color_space) {
Index: third_party/WebKit/public/platform/WebScreenInfo.h
diff --git a/third_party/WebKit/public/platform/WebScreenInfo.h b/third_party/WebKit/public/platform/WebScreenInfo.h
index 857918d5ae0b89b14d57c7894b0e2da462e239ce..06ecf1eb3a8e2e308c753b9b1fa5b01eacccc080 100644
--- a/third_party/WebKit/public/platform/WebScreenInfo.h
+++ b/third_party/WebKit/public/platform/WebScreenInfo.h
@@ -34,7 +34,7 @@
#include "WebRect.h"
#include "public/platform/ShapeProperties.h"
#include "public/platform/modules/screen_orientation/WebScreenOrientationType.h"
-#include "ui/gfx/icc_profile.h"
+#include "ui/gfx/color_space.h"

namespace blink {

@@ -43,8 +43,8 @@ struct WebScreenInfo {
// pixels.
float device_scale_factor = 1.f;

- // The ICC profile of the output display.
- gfx::ICCProfile icc_profile;
+ // The color space of the output display.
+ gfx::ColorSpace color_space;

// The screen depth in bits per pixel
int depth = 0;
@@ -89,7 +89,7 @@ struct WebScreenInfo {

bool operator==(const WebScreenInfo& other) const {
return this->device_scale_factor == other.device_scale_factor &&
- this->icc_profile == other.icc_profile &&
+ this->color_space == other.color_space &&
this->depth == other.depth &&
this->depth_per_component == other.depth_per_component &&
this->is_monochrome == other.is_monochrome &&


commit-bot@chromium.org via codereview.chromium.org

unread,
Jun 24, 2017, 4:35:12 AM6/24/17
to ccamero...@chromium.org, hu...@chromium.org, commi...@chromium.org, chromium...@chromium.org, mlamouri+wa...@chromium.org, dongseo...@intel.com, creis...@chromium.org, blink-reviews-p...@chromium.org, nasko+c...@chromium.org, j...@chromium.org, pdr+graphi...@chromium.org, fmalit...@chromium.org, blink-re...@chromium.org, dsch...@chromium.org, dglazko...@chromium.org, caba...@adobe.com, dari...@chromium.org, ju...@chromium.org, mac-r...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, kinuko...@chromium.org, sche...@chromium.org, rob....@samsung.com

commit-bot@chromium.org via codereview.chromium.org

unread,
Jun 24, 2017, 1:22:09 PM6/24/17
to ccamero...@chromium.org, hu...@chromium.org, commi...@chromium.org, chromium...@chromium.org, mlamouri+wa...@chromium.org, dongseo...@intel.com, creis...@chromium.org, blink-reviews-p...@chromium.org, nasko+c...@chromium.org, j...@chromium.org, pdr+graphi...@chromium.org, fmalit...@chromium.org, blink-re...@chromium.org, dsch...@chromium.org, dglazko...@chromium.org, caba...@adobe.com, dari...@chromium.org, ju...@chromium.org, mac-r...@chromium.org, blink-...@chromium.org, drott+bl...@chromium.org, kinuko...@chromium.org, sche...@chromium.org, rob....@samsung.com
Reply all
Reply to author
Forward
0 new messages