Skip to content

Commit

Permalink
fix: restore ability to disable color correct rendering (backport: 4-…
Browse files Browse the repository at this point in the history
…0-x) (#16020)

Backport of #15898

See that PR for details.

Notes: Add `--disable-color-correct-rendering` switch
  • Loading branch information
poiru authored and ckerr committed Dec 11, 2018
1 parent dd595a7 commit 388197d
Show file tree
Hide file tree
Showing 2 changed files with 369 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/common/chromium/.patches
Expand Up @@ -84,3 +84,4 @@ cross_site_document_resource_handler.patch
fix_trackpad_scrolling.patch
mac_fix_form_control_rendering_on_10_14_mojave.patch
ensure_cookie_store.patch
disable_color_correct_rendering.patch
368 changes: 368 additions & 0 deletions patches/common/chromium/disable_color_correct_rendering.patch
@@ -0,0 +1,368 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Birunthan Mohanathas <birunthan@mohanathas.com>
Date: Fri, 30 Nov 2018 12:44:12 +0200
Subject: Add --disable-color-correct-rendering switch

In Electron 2.0, `--disable-features=ColorCorrectRendering` could be
used to make the app use the display color space (e.g. P3 on Macs)
instead of color correcting to sRGB. Because color correct rendering is
always enabled on Chromium 62 and later and because
`--force-color-profile` has no effect on macOS, apps that need e.g. P3
colors are currently stuck on Electron 2.0.

This restores the functionality removed in
https://chromium-review.googlesource.com/698347 in the form of the
`--disable-color-correct-rendering` switch.

This can be removed once web content (including WebGL) learn how
to deal with color spaces. That is being tracked at
https://crbug.com/634542 and https://crbug.com/711107.

diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index f51e30c0d2a55f104ca0912e487484102e443c85..fbec9767bdefc3b90335ecc423ffd4ea31bf914d 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -1578,6 +1578,10 @@ void LayerTreeHostImpl::SetIsLikelyToRequireADraw(
}

RasterColorSpace LayerTreeHostImpl::GetRasterColorSpace() const {
+ if (!settings_.enable_color_correct_rendering) {
+ return {};
+ }
+
RasterColorSpace result;
// The pending tree will have the most recently updated color space, so
// prefer that.
diff --git a/cc/trees/layer_tree_settings.h b/cc/trees/layer_tree_settings.h
index e124035096b02e82d526d7dfd0e70d4cf73819be..4f182c826a8d08a8674e3ca2f0bf833aa49a1309 100644
--- a/cc/trees/layer_tree_settings.h
+++ b/cc/trees/layer_tree_settings.h
@@ -98,6 +98,8 @@ class CC_EXPORT LayerTreeSettings {

bool enable_mask_tiling = true;

+ bool enable_color_correct_rendering = true;
+
// If set to true, the compositor may selectively defer image decodes to the
// Image Decode Service and raster tiles without images until the decode is
// ready.
diff --git a/components/viz/common/display/renderer_settings.h b/components/viz/common/display/renderer_settings.h
index 1327e8607c5192f2440341a33fc210aecd47ced5..543d6937a908560270c6bba431a255436b522608 100644
--- a/components/viz/common/display/renderer_settings.h
+++ b/components/viz/common/display/renderer_settings.h
@@ -18,6 +18,7 @@ class VIZ_COMMON_EXPORT RendererSettings {
RendererSettings(const RendererSettings& other);
~RendererSettings();

+ bool enable_color_correct_rendering = true;
bool allow_antialiasing = true;
bool force_antialiasing = false;
bool force_blending_with_shaders = false;
diff --git a/components/viz/host/renderer_settings_creation.cc b/components/viz/host/renderer_settings_creation.cc
index eca6020535249e51b428de9dd6454273e6c9dd71..e190d2fb2e2cc41135c119485c2d447522134da1 100644
--- a/components/viz/host/renderer_settings_creation.cc
+++ b/components/viz/host/renderer_settings_creation.cc
@@ -11,6 +11,7 @@
#include "components/viz/common/display/renderer_settings.h"
#include "components/viz/common/features.h"
#include "ui/base/ui_base_switches.h"
+#include "ui/gfx/switches.h"

#if defined(OS_MACOSX)
#include "ui/base/cocoa/remote_layer_api.h"
@@ -43,6 +44,8 @@ bool GetSwitchValueAsInt(const base::CommandLine* command_line,
RendererSettings CreateRendererSettings() {
RendererSettings renderer_settings;
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+ renderer_settings.enable_color_correct_rendering =
+ !command_line->HasSwitch(switches::kDisableColorCorrectRendering);
renderer_settings.partial_swap_enabled =
!command_line->HasSwitch(switches::kUIDisablePartialSwap);
#if defined(OS_WIN)
diff --git a/components/viz/service/display/gl_renderer.cc b/components/viz/service/display/gl_renderer.cc
index a9ba5ca388b8d0979a1235a0d976b8cb3277f6d3..574deb6ad7d1b088eea43d87b8fe4637b980139b 100644
--- a/components/viz/service/display/gl_renderer.cc
+++ b/components/viz/service/display/gl_renderer.cc
@@ -77,6 +77,9 @@

using gpu::gles2::GLES2Interface;

+#define PATCH_CS(color_space) \
+ (settings_->enable_color_correct_rendering ? color_space : gfx::ColorSpace())
+
namespace viz {
namespace {

@@ -516,8 +519,9 @@ void GLRenderer::DoDrawQuad(const DrawQuad* quad,
void GLRenderer::DrawDebugBorderQuad(const DebugBorderDrawQuad* quad) {
SetBlendEnabled(quad->ShouldDrawWithBlending());

- SetUseProgram(ProgramKey::DebugBorder(), gfx::ColorSpace::CreateSRGB(),
- current_frame()->current_render_pass->color_space);
+ SetUseProgram(ProgramKey::DebugBorder(),
+ PATCH_CS(gfx::ColorSpace::CreateSRGB()),
+ PATCH_CS(current_frame()->current_render_pass->color_space));

// Use the full quad_rect for debug quads to not move the edges based on
// partial swaps.
@@ -1289,7 +1293,8 @@ void GLRenderer::ChooseRPDQProgram(DrawRenderPassDrawQuadParams* params,
tex_coord_precision, sampler_type, shader_blend_mode,
params->use_aa ? USE_AA : NO_AA, mask_mode, mask_for_background,
params->use_color_matrix, tint_gl_composited_content_),
- params->contents_and_bypass_color_space, target_color_space);
+ PATCH_CS(params->contents_and_bypass_color_space),
+ PATCH_CS(target_color_space));
}

void GLRenderer::UpdateRPDQUniforms(DrawRenderPassDrawQuadParams* params) {
@@ -1750,8 +1755,8 @@ void GLRenderer::DrawSolidColorQuad(const SolidColorDrawQuad* quad,
gfx::ColorSpace quad_color_space = gfx::ColorSpace::CreateSRGB();
SetUseProgram(ProgramKey::SolidColor(use_aa ? USE_AA : NO_AA,
tint_gl_composited_content_),
- quad_color_space,
- current_frame()->current_render_pass->color_space);
+ PATCH_CS(quad_color_space),
+ PATCH_CS(current_frame()->current_render_pass->color_space));
SetShaderColor(color, opacity);

if (current_program_->tint_color_matrix_location() != -1) {
@@ -1901,8 +1906,8 @@ void GLRenderer::DrawContentQuadAA(const ContentDrawQuadBase* quad,
quad->is_premultiplied ? PREMULTIPLIED_ALPHA
: NON_PREMULTIPLIED_ALPHA,
false, false, tint_gl_composited_content_),
- quad_resource_lock.color_space(),
- current_frame()->current_render_pass->color_space);
+ PATCH_CS(quad_resource_lock.color_space()),
+ PATCH_CS(current_frame()->current_render_pass->color_space));

if (current_program_->tint_color_matrix_location() != -1) {
auto matrix = cc::DebugColors::TintCompositedContentColorTransformMatrix();
@@ -1990,8 +1995,8 @@ void GLRenderer::DrawContentQuadNoAA(const ContentDrawQuadBase* quad,
: NON_PREMULTIPLIED_ALPHA,
!quad->ShouldDrawWithBlending(), has_tex_clamp_rect,
tint_gl_composited_content_),
- quad_resource_lock.color_space(),
- current_frame()->current_render_pass->color_space);
+ PATCH_CS(quad_resource_lock.color_space()),
+ PATCH_CS(current_frame()->current_render_pass->color_space));

if (current_program_->tint_color_matrix_location() != -1) {
auto matrix = cc::DebugColors::TintCompositedContentColorTransformMatrix();
@@ -2086,7 +2091,7 @@ void GLRenderer::DrawYUVVideoQuad(const YUVVideoDrawQuad* quad,
DCHECK_NE(src_color_space, src_color_space.GetAsFullRangeRGB());

gfx::ColorSpace dst_color_space =
- current_frame()->current_render_pass->color_space;
+ PATCH_CS(current_frame()->current_render_pass->color_space);
// Force sRGB output on Windows for overlay candidate video quads to match
// DirectComposition behavior in case these switch between overlays and
// compositing. See https://crbug.com/811118 for details.
@@ -2234,8 +2239,8 @@ void GLRenderer::DrawStreamVideoQuad(const StreamVideoDrawQuad* quad,
quad->resource_id());

SetUseProgram(ProgramKey::VideoStream(tex_coord_precision),
- lock.color_space(),
- current_frame()->current_render_pass->color_space);
+ PATCH_CS(lock.color_space()),
+ PATCH_CS(current_frame()->current_render_pass->color_space));

DCHECK_EQ(GL_TEXTURE0, GetActiveTextureUnit(gl_));
gl_->BindTexture(GL_TEXTURE_EXTERNAL_OES, lock.texture_id());
@@ -2287,8 +2292,8 @@ void GLRenderer::FlushTextureQuadCache(BoundGeometry flush_binding) {
draw_cache_.nearest_neighbor ? GL_NEAREST : GL_LINEAR);

// Bind the program to the GL state.
- SetUseProgram(draw_cache_.program_key, locked_quad.color_space(),
- current_frame()->current_render_pass->color_space);
+ SetUseProgram(draw_cache_.program_key, PATCH_CS(locked_quad.color_space()),
+ PATCH_CS(current_frame()->current_render_pass->color_space));

DCHECK_EQ(GL_TEXTURE0, GetActiveTextureUnit(gl_));
gl_->BindTexture(locked_quad.target(), locked_quad.texture_id());
@@ -2938,7 +2943,9 @@ void GLRenderer::PrepareGeometry(BoundGeometry binding) {
void GLRenderer::SetUseProgram(const ProgramKey& program_key_no_color,
const gfx::ColorSpace& src_color_space,
const gfx::ColorSpace& dst_color_space) {
- DCHECK(dst_color_space.IsValid());
+ if (settings_->enable_color_correct_rendering) {
+ DCHECK(dst_color_space.IsValid());
+ }

ProgramKey program_key = program_key_no_color;
const gfx::ColorTransform* color_transform =
@@ -3306,7 +3313,7 @@ void GLRenderer::CopyRenderPassDrawQuadToOverlayResource(

*overlay_texture = FindOrCreateOverlayTexture(
params.quad->render_pass_id, iosurface_width, iosurface_height,
- current_frame()->root_render_pass->color_space);
+ PATCH_CS(current_frame()->root_render_pass->color_space));
*new_bounds = gfx::RectF(updated_dst_rect.origin(),
gfx::SizeF((*overlay_texture)->texture.size()));

@@ -3511,8 +3518,9 @@ void GLRenderer::FlushOverdrawFeedback(const gfx::Rect& output_rect) {

PrepareGeometry(SHARED_BINDING);

- SetUseProgram(ProgramKey::DebugBorder(), gfx::ColorSpace::CreateSRGB(),
- current_frame()->root_render_pass->color_space);
+ SetUseProgram(ProgramKey::DebugBorder(),
+ PATCH_CS(gfx::ColorSpace::CreateSRGB()),
+ PATCH_CS(current_frame()->root_render_pass->color_space));

gfx::Transform render_matrix;
render_matrix.Translate(0.5 * output_rect.width() + output_rect.x(),
@@ -3671,3 +3679,5 @@ gfx::Size GLRenderer::GetRenderPassBackingPixelSize(
}

} // namespace viz
+
+#undef PATCH_CS
diff --git a/components/viz/service/display/skia_renderer.cc b/components/viz/service/display/skia_renderer.cc
index 0544d031b60d38279104a4ca9c6dc25126dea185..1acb3dfa55f39b8ecb9fccaa4627ac25cac81f1f 100644
--- a/components/viz/service/display/skia_renderer.cc
+++ b/components/viz/service/display/skia_renderer.cc
@@ -584,9 +584,11 @@ void SkiaRenderer::DrawPictureQuad(const PictureDrawQuad* quad) {

std::unique_ptr<SkCanvas> color_transform_canvas;
// TODO(enne): color transform needs to be replicated in gles2_cmd_decoder
- color_transform_canvas = SkCreateColorSpaceXformCanvas(
- current_canvas_, gfx::ColorSpace::CreateSRGB().ToSkColorSpace());
- raster_canvas = color_transform_canvas.get();
+ if (settings_->enable_color_correct_rendering) {
+ color_transform_canvas = SkCreateColorSpaceXformCanvas(
+ current_canvas_, gfx::ColorSpace::CreateSRGB().ToSkColorSpace());
+ raster_canvas = color_transform_canvas.get();
+ }

base::Optional<skia::OpacityFilterCanvas> opacity_canvas;
if (needs_transparency || disable_image_filtering) {
diff --git a/components/viz/service/display/software_renderer.cc b/components/viz/service/display/software_renderer.cc
index 5c41958ed259b1d3ae076312b97a802746897c98..4cb9801a859c12dae03e03073085ff82ea0e4a32 100644
--- a/components/viz/service/display/software_renderer.cc
+++ b/components/viz/service/display/software_renderer.cc
@@ -334,9 +334,11 @@ void SoftwareRenderer::DrawPictureQuad(const PictureDrawQuad* quad) {

std::unique_ptr<SkCanvas> color_transform_canvas;
// TODO(enne): color transform needs to be replicated in gles2_cmd_decoder
- color_transform_canvas = SkCreateColorSpaceXformCanvas(
- current_canvas_, gfx::ColorSpace::CreateSRGB().ToSkColorSpace());
- raster_canvas = color_transform_canvas.get();
+ if (settings_->enable_color_correct_rendering) {
+ color_transform_canvas = SkCreateColorSpaceXformCanvas(
+ current_canvas_, gfx::ColorSpace::CreateSRGB().ToSkColorSpace());
+ raster_canvas = color_transform_canvas.get();
+ }

base::Optional<skia::OpacityFilterCanvas> opacity_canvas;
if (needs_transparency || disable_image_filtering) {
diff --git a/content/browser/gpu/gpu_process_host.cc b/content/browser/gpu/gpu_process_host.cc
index 9511100e6077b6788abfead4b1962773a22ae40b..db2d8a4fc979b20810776cf2a31018d197cdba7d 100644
--- a/content/browser/gpu/gpu_process_host.cc
+++ b/content/browser/gpu/gpu_process_host.cc
@@ -193,6 +193,7 @@ GpuTerminationStatus ConvertToGpuTerminationStatus(

// Command-line switches to propagate to the GPU process.
static const char* const kSwitchNames[] = {
+ switches::kDisableColorCorrectRendering,
service_manager::switches::kDisableSeccompFilterSandbox,
service_manager::switches::kGpuSandboxAllowSysVShm,
service_manager::switches::kGpuSandboxFailuresFatal,
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 1f2897aabf94124f108a0f0449e4d687b084f1a4..4f88e4425ceada8af6b412b087ac134572222824 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -218,6 +218,7 @@
#include "ui/base/ui_base_switches.h"
#include "ui/base/ui_base_switches_util.h"
#include "ui/display/display_switches.h"
+#include "ui/gfx/switches.h"
#include "ui/gl/gl_switches.h"
#include "ui/gl/gpu_switching_manager.h"
#include "ui/native_theme/native_theme_features.h"
@@ -2931,6 +2932,7 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
// Propagate the following switches to the renderer command line (along
// with any associated values) if present in the browser command line.
static const char* const kSwitchNames[] = {
+ switches::kDisableColorCorrectRendering,
network::switches::kNoReferrers,
service_manager::switches::kDisableInProcessStackTraces,
service_manager::switches::kDisableSeccompFilterSandbox,
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 6d0580d5b0b76109bbaf0d058ce4201dc970adf0..f0271310ec6112ca7276c5af13ee6677d393109e 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -2565,6 +2565,9 @@ cc::LayerTreeSettings RenderWidget::GenerateLayerTreeSettings(
settings.main_frame_before_activation_enabled =
cmd.HasSwitch(cc::switches::kEnableMainFrameBeforeActivation);

+ settings.enable_color_correct_rendering =
+ !cmd.HasSwitch(switches::kDisableColorCorrectRendering);
+
// Checkerimaging is not supported for synchronous single-threaded mode, which
// is what the renderer uses if its not threaded.
settings.enable_checker_imaging =
diff --git a/ui/gfx/mac/io_surface.cc b/ui/gfx/mac/io_surface.cc
index 00ad38db090096645d9ef9e3220a48661b6401df..af50266b42abd514bfca3379191675e022002406 100644
--- a/ui/gfx/mac/io_surface.cc
+++ b/ui/gfx/mac/io_surface.cc
@@ -16,6 +16,7 @@
#include "base/trace_event/trace_event.h"
#include "ui/gfx/buffer_format_util.h"
#include "ui/gfx/icc_profile.h"
+#include "ui/gfx/switches.h"

namespace gfx {

@@ -199,6 +200,11 @@ IOSurfaceRef CreateIOSurface(const gfx::Size& size,

// Ensure that all IOSurfaces start as sRGB.
CGColorSpaceRef color_space = base::mac::GetSRGBColorSpace();
+ auto* cmd_line = base::CommandLine::ForCurrentProcess();
+ if (cmd_line->HasSwitch(switches::kDisableColorCorrectRendering)) {
+ color_space = base::mac::GetSystemColorSpace();
+ }
+
base::ScopedCFTypeRef<CFDataRef> color_space_icc(
CGColorSpaceCopyICCProfile(color_space));
IOSurfaceSetValue(surface, CFSTR("IOSurfaceColorSpace"), color_space_icc);
@@ -210,6 +216,14 @@ IOSurfaceRef CreateIOSurface(const gfx::Size& size,

void IOSurfaceSetColorSpace(IOSurfaceRef io_surface,
const ColorSpace& color_space) {
+ auto* cmd_line = base::CommandLine::ForCurrentProcess();
+ if (cmd_line->HasSwitch(switches::kDisableColorCorrectRendering)) {
+ base::ScopedCFTypeRef<CFDataRef> system_icc(
+ CGColorSpaceCopyICCProfile(base::mac::GetSystemColorSpace()));
+ IOSurfaceSetValue(io_surface, CFSTR("IOSurfaceColorSpace"), system_icc);
+ return;
+ }
+
// Special-case sRGB.
if (color_space == ColorSpace::CreateSRGB()) {
base::ScopedCFTypeRef<CFDataRef> srgb_icc(
diff --git a/ui/gfx/switches.cc b/ui/gfx/switches.cc
index e1943d5970ac37a19430b45f54a5608386433ef8..d4f56057b50fb8925371ad109727cc25f05c6d18 100644
--- a/ui/gfx/switches.cc
+++ b/ui/gfx/switches.cc
@@ -7,6 +7,8 @@

namespace switches {

+const char kDisableColorCorrectRendering[] = "disable-color-correct-rendering";
+
#if defined(OS_WIN)
// Disables DirectWrite font rendering for general UI elements.
const char kDisableDirectWriteForUI[] = "disable-directwrite-for-ui";
diff --git a/ui/gfx/switches.h b/ui/gfx/switches.h
index dc4921dbf0cc4a2c0454269bd03b79cef7f97b72..019e39bb16623c25fc173e215982b45035db4db3 100644
--- a/ui/gfx/switches.h
+++ b/ui/gfx/switches.h
@@ -11,6 +11,8 @@

namespace switches {

+GFX_SWITCHES_EXPORT extern const char kDisableColorCorrectRendering[];
+
#if defined(OS_WIN)
GFX_SWITCHES_EXPORT extern const char kDisableDirectWriteForUI[];
#endif

0 comments on commit 388197d

Please sign in to comment.