Skip to content

Commit

Permalink
Make color hint interpolation match the gradient
Browse files Browse the repository at this point in the history 
Color hints were previously being generated by a Blend() function that
produced sRGBLegacy colors, no matter what gradient interpolation was
set. This CL changes gradient hints to use the new
Color::InterpolateColors function that can handle different color
spaces. graphics/color_blend.h is no longer used and deleted.

In the spec, when no interpolation space is specified, "legacy" colors
are interpolated in the sRGB color space while non-legacy colors are
interpolated in OKLAB. In order to support this a "kSRGBLegacy" color
interpolation space is added to Color::ColorInterpolation space to
match the Color::ColorSpace of the same name. This space interpolates
exactly the same way as "kSRGB", but produces a legacy color for
supporting legacy interpolation between points.

Also, Color::ColorSpace::kRGBLegacy is renamed to kSRGBLegacy to match
convention elsewhere (it is in the "sRGB" color space, after all).

Some gradient expectations are rebaselined to match the results of using
higher resolution colors to generate them.

Bug: 1416273, 1411351
Change-Id: I24446988e981291c6b0c24ea3b5d8afd39aaceca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4250939
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1118133}
  • Loading branch information
@mysteryDate
mysteryDate authored and Chromium LUCI CQ committed Mar 16, 2023
1 parent 4d813ee commit b7878a7
Show file tree
Hide file tree
Showing 43 changed files with 86 additions and 120 deletions.
50 changes: 40 additions & 10 deletions third_party/blink/renderer/core/css/css_gradient_value.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
#include "third_party/blink/renderer/core/css_value_keywords.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/dom/text_link_colors.h"
#include "third_party/blink/renderer/platform/graphics/color_blend.h"
#include "third_party/blink/renderer/platform/graphics/color.h"
#include "third_party/blink/renderer/platform/graphics/gradient.h"
#include "third_party/blink/renderer/platform/graphics/gradient_generated_image.h"
#include "third_party/blink/renderer/platform/graphics/image.h"
Expand Down Expand Up @@ -218,7 +218,9 @@ struct CSSGradientValue::GradientDesc {

static void ReplaceColorHintsWithColorStops(
Vector<GradientStop>& stops,
const HeapVector<CSSGradientColorStop, 2>& css_gradient_stops) {
const HeapVector<CSSGradientColorStop, 2>& css_gradient_stops,
Color::ColorInterpolationSpace color_interpolation_space,
Color::HueInterpolationMethod hue_interpolation_method) {
// This algorithm will replace each color interpolation hint with 9 regular
// color stops. The color values for the new color stops will be calculated
// using the color weighting formula defined in the spec. The new color
Expand All @@ -234,6 +236,12 @@ static void ReplaceColorHintsWithColorStops(
// the algorithm, or adding support for color interpolation hints to skia
// shaders.

// Support legacy gradients with color hints when no interpolation space is
// specified.
if (color_interpolation_space == Color::ColorInterpolationSpace::kNone) {
color_interpolation_space = Color::ColorInterpolationSpace::kSRGBLegacy;
}

int index_offset = 0;

// The first and the last color stops cannot be color hints.
Expand Down Expand Up @@ -310,12 +318,23 @@ static void ReplaceColorHintsWithColorStops(
// The color weighting for the new color stops will be
// pointRelativeOffset^(ln(0.5)/ln(hintRelativeOffset)).
float hint_relative_offset = left_dist / total_dist;
for (size_t y = 0; y < 9; ++y) {
for (auto& new_stop : new_stops) {
float point_relative_offset =
(new_stops[y].offset - offset_left) / total_dist;
(new_stop.offset - offset_left) / total_dist;
float weighting =
powf(point_relative_offset, logf(.5f) / logf(hint_relative_offset));
new_stops[y].color = Blend(left_color, right_color, weighting);
// Prevent crashes from huge gradient stops. See:
// wpt/css/css-images/radial-gradient-transition-hint-crash.html
if (std::isinf(weighting) || std::isnan(weighting)) {
continue;
}
// TODO(crbug.com/1416273): Testing that color hints are using the
// correct interpolation space is challenging in CSS. Once Canvas2D
// implements colorspaces for gradients we can use GetImageData() to
// test this.
new_stop.color = Color::InterpolateColors(
color_interpolation_space, hue_interpolation_method, left_color,
right_color, weighting);
}

// Replace the color hint with the new color stops.
Expand Down Expand Up @@ -463,7 +482,15 @@ bool NormalizeAndAddStops(const Vector<GradientStop>& stops,

// Collapse all negative-offset stops to 0 and compute an interpolated color
// value for that point.
void ClampNegativeOffsets(Vector<GradientStop>& stops) {
void ClampNegativeOffsets(
Vector<GradientStop>& stops,
Color::ColorInterpolationSpace color_interpolation_space,
Color::HueInterpolationMethod hue_interpolation_method) {
// Support legacy gradients with color hints when no interpolation space is
// specified.
if (color_interpolation_space == Color::ColorInterpolationSpace::kNone) {
color_interpolation_space = Color::ColorInterpolationSpace::kSRGBLegacy;
}
float last_negative_offset = 0;

for (wtf_size_t i = 0; i < stops.size(); ++i) {
Expand All @@ -475,8 +502,9 @@ void ClampNegativeOffsets(Vector<GradientStop>& stops) {
DCHECK_LT(last_negative_offset, 0);
float lerp_ratio =
-last_negative_offset / (current_offset - last_negative_offset);
stops[i - 1].color =
Blend(stops[i - 1].color, stops[i].color, lerp_ratio);
stops[i - 1].color = Color::InterpolateColors(
color_interpolation_space, hue_interpolation_method,
stops[i - 1].color, stops[i].color, lerp_ratio);
}

break;
Expand Down Expand Up @@ -675,7 +703,8 @@ void CSSGradientValue::AddStops(

DCHECK_EQ(stops.size(), stops_.size());
if (has_hints) {
ReplaceColorHintsWithColorStops(stops, stops_);
ReplaceColorHintsWithColorStops(stops, stops_, color_interpolation_space_,
hue_interpolation_method_);
}

// At this point we have a fully resolved set of stops. Time to perform
Expand All @@ -701,7 +730,8 @@ void CSSGradientValue::AddStops(
// repeating radial gradients we shift the radii into equivalent positive
// values.
if (!repeating_) {
ClampNegativeOffsets(stops);
ClampNegativeOffsets(stops, color_interpolation_space_,
hue_interpolation_method_);
}

if (NormalizeAndAddStops(stops, desc)) {
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/renderer/core/style/shadow_data.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

#include "third_party/blink/renderer/core/style/shadow_data.h"

#include "third_party/blink/renderer/platform/graphics/color_blend.h"
#include "third_party/blink/renderer/platform/graphics/skia/skia_utils.h"

namespace blink {
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/renderer/core/svg/color_distance.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "third_party/blink/renderer/core/svg/color_distance.h"

#include "third_party/blink/renderer/platform/graphics/color.h"
#include "third_party/blink/renderer/platform/graphics/color_blend.h"

#include <math.h>

Expand Down
1 change: 0 additions & 1 deletion third_party/blink/renderer/platform/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,6 @@ component("platform") {
"graphics/color.h",
"graphics/color_behavior.cc",
"graphics/color_behavior.h",
"graphics/color_blend.h",
"graphics/color_space_gamut.cc",
"graphics/color_space_gamut.h",
"graphics/color_space_profile_data.cc",
Expand Down
58 changes: 24 additions & 34 deletions third_party/blink/renderer/platform/graphics/color.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ Color Color::FromRGBALegacy(absl::optional<int> r,
result.param1_is_none_ = !g;
result.param2_is_none_ = !b;
result.alpha_is_none_ = !a;
result.color_space_ = ColorSpace::kRGBLegacy;
result.color_space_ = ColorSpace::kSRGBLegacy;
return result;
}

Expand Down Expand Up @@ -298,7 +298,7 @@ void Color::CarryForwardAnalogousMissingComponents(
color_space == Color::ColorSpace::kRec2020 ||
color_space == Color::ColorSpace::kXYZD50 ||
color_space == Color::ColorSpace::kXYZD65 ||
color_space == Color::ColorSpace::kRGBLegacy;
color_space == Color::ColorSpace::kSRGBLegacy;
};

auto IsLightnessFirstComponent = [](Color::ColorSpace color_space) {
Expand Down Expand Up @@ -362,6 +362,8 @@ Color::ColorSpace Color::ColorInterpolationSpaceToColorSpace(
return ColorSpace::kHSL;
case (ColorInterpolationSpace::kHWB):
return ColorSpace::kHWB;
case (ColorInterpolationSpace::kSRGBLegacy):
return ColorSpace::kSRGBLegacy;
case (ColorInterpolationSpace::kSRGB):
case (ColorInterpolationSpace::kNone):
return ColorSpace::kSRGB;
Expand Down Expand Up @@ -466,7 +468,7 @@ Color Color::InterpolateColors(

std::tuple<float, float, float> Color::ExportAsXYZD50Floats() const {
switch (color_space_) {
case ColorSpace::kRGBLegacy:
case ColorSpace::kSRGBLegacy:
case ColorSpace::kSRGB:
return gfx::SRGBToXYZD50(param0_, param1_, param2_);
case ColorSpace::kSRGBLinear:
Expand Down Expand Up @@ -507,10 +509,13 @@ std::tuple<float, float, float> Color::ExportAsXYZD50Floats() const {

void Color::ConvertToColorInterpolationSpace(
Color::ColorInterpolationSpace interpolation_space) {
if (color_space_ ==
ColorInterpolationSpaceToColorSpace(interpolation_space)) {
return;
}

switch (interpolation_space) {
case ColorInterpolationSpace::kXYZD65: {
if (color_space_ == ColorSpace::kXYZD65)
return;
if (color_space_ == ColorSpace::kOklab) {
std::tie(param0_, param1_, param2_) =
gfx::OklabToXYZD65(param0_, param1_, param2_);
Expand All @@ -522,24 +527,17 @@ void Color::ConvertToColorInterpolationSpace(
return;
}
case ColorInterpolationSpace::kXYZD50: {
if (color_space_ == ColorSpace::kXYZD50)
return;
std::tie(param0_, param1_, param2_) = ExportAsXYZD50Floats();
color_space_ = ColorSpace::kXYZD50;
return;
}
case ColorInterpolationSpace::kSRGBLinear: {
if (color_space_ == ColorSpace::kSRGBLinear)
return;
auto [x, y, z] = ExportAsXYZD50Floats();
std::tie(param0_, param1_, param2_) = gfx::XYZD50TosRGBLinear(x, y, z);
color_space_ = ColorSpace::kSRGBLinear;
return;
}
case ColorInterpolationSpace::kLab: {
if (color_space_ == ColorSpace::kLab) {
return;
}
if (color_space_ == ColorSpace::kLch) {
std::tie(param0_, param1_, param2_) =
gfx::LchToLab(param0_, param1_, param2_);
Expand Down Expand Up @@ -580,9 +578,6 @@ void Color::ConvertToColorInterpolationSpace(
return;
}
case ColorInterpolationSpace::kLch: {
if (color_space_ == ColorSpace::kLch) {
return;
}
// Conversion to lch is done through lab.
auto [l, a, b] = [&]() {
if (color_space_ == ColorSpace::kLab) {
Expand All @@ -599,9 +594,6 @@ void Color::ConvertToColorInterpolationSpace(
return;
}
case ColorInterpolationSpace::kOklch: {
if (color_space_ == ColorSpace::kOklch) {
return;
}
if (color_space_ == ColorSpace::kOklab) {
std::tie(param0_, param1_, param2_) =
gfx::LabToLch(param0_, param1_, param2_);
Expand All @@ -625,28 +617,25 @@ void Color::ConvertToColorInterpolationSpace(
color_space_ = ColorSpace::kOklch;
return;
}
case ColorInterpolationSpace::kSRGB: {
if (color_space_ == ColorSpace::kSRGB)
return;
case ColorInterpolationSpace::kSRGB:
case ColorInterpolationSpace::kSRGBLegacy: {
SkColor4f sRGB_color = toSkColor4f();
param0_ = sRGB_color.fR;
param1_ = sRGB_color.fG;
param2_ = sRGB_color.fB;
color_space_ = ColorSpace::kSRGB;
color_space_ = (interpolation_space == ColorInterpolationSpace::kSRGB)
? ColorSpace::kSRGB
: ColorSpace::kSRGBLegacy;
return;
}
case ColorInterpolationSpace::kHSL: {
if (color_space_ == ColorSpace::kHSL)
return;
SkColor4f sRGB_color = toSkColor4f();
std::tie(param0_, param1_, param2_) =
gfx::SRGBToHSL(sRGB_color.fR, sRGB_color.fG, sRGB_color.fB);
color_space_ = ColorSpace::kHSL;
return;
}
case ColorInterpolationSpace::kHWB: {
if (color_space_ == ColorSpace::kHWB)
return;
SkColor4f sRGB_color = toSkColor4f();
std::tie(param0_, param1_, param2_) =
gfx::SRGBToHWB(sRGB_color.fR, sRGB_color.fG, sRGB_color.fB);
Expand Down Expand Up @@ -692,7 +681,7 @@ SkColor4f Color::toSkColor4f() const {
return gfx::HSLToSkColor4f(param0_, param1_, param2_, alpha_);
case ColorSpace::kHWB:
return gfx::HWBToSkColor4f(param0_, param1_, param2_, alpha_);
case ColorSpace::kRGBLegacy:
case ColorSpace::kSRGBLegacy:
return SkColor4f{param0_, param1_, param2_, alpha_};
default:
NOTIMPLEMENTED();
Expand Down Expand Up @@ -834,7 +823,7 @@ String Color::ColorSpaceToString(Color::ColorSpace color_space) {
return "lch";
case Color::ColorSpace::kOklch:
return "oklab";
case Color::ColorSpace::kRGBLegacy:
case Color::ColorSpace::kSRGBLegacy:
return "RGB Legacy";
case Color::ColorSpace::kHSL:
return "HSL";
Expand All @@ -844,10 +833,10 @@ String Color::ColorSpaceToString(Color::ColorSpace color_space) {
}

String Color::SerializeAsCanvasColor() const {
if ((color_space_ == ColorSpace::kRGBLegacy ||
color_space_ == ColorSpace::kHSL || color_space_ == ColorSpace::kHWB) &&
!HasAlpha())
// Opaque legacy colors will serialize in a hex format.
if (!HasAlpha() && IsLegacyColor()) {
return String::Format("#%02x%02x%02x", Red(), Green(), Blue());
}

return SerializeAsCSSColor();
}
Expand All @@ -857,7 +846,7 @@ String Color::SerializeAsCSSColor() const {
result.ReserveCapacity(28);

switch (color_space_) {
case ColorSpace::kRGBLegacy:
case ColorSpace::kSRGBLegacy:
case ColorSpace::kHSL:
case ColorSpace::kHWB:
if (HasAlpha())
Expand Down Expand Up @@ -980,7 +969,7 @@ String Color::SerializeAsCSSColor() const {
}

String Color::NameForLayoutTreeAsText() const {
if (color_space_ != ColorSpace::kRGBLegacy &&
if (color_space_ != ColorSpace::kSRGBLegacy &&
color_space_ != ColorSpace::kHSL && color_space_ != ColorSpace::kHWB) {
// TODO(https://crbug.com/1333988): Determine if CSS Color Level 4 colors
// should use this representation here.
Expand Down Expand Up @@ -1189,7 +1178,7 @@ RGBA32 PremultipliedARGBFromColor(const Color& color) {

// https://www.w3.org/TR/css-color-4/#legacy-color-syntax
bool Color::IsLegacyColor() const {
return (color_space_ == ColorSpace::kRGBLegacy ||
return (color_space_ == ColorSpace::kSRGBLegacy ||
color_space_ == ColorSpace::kHSL || color_space_ == ColorSpace::kHWB);
}

Expand Down Expand Up @@ -1243,6 +1232,7 @@ String Color::ColorInterpolationSpaceToString(
result.Append("hwb");
break;
case Color::ColorInterpolationSpace::kNone:
case Color::ColorInterpolationSpace::kSRGBLegacy:
result.Append("none");
break;
}
Expand Down

0 comments on commit b7878a7

Please sign in to comment.