Skip to content

Commit 2d01ae8

Browse files
nex3nex3
committed
[Color 4] Update behavior to cover more missing channel edge cases
1 parent 32530c3 commit 2d01ae8

File tree

3 files changed

+160
-95
lines changed

3 files changed

+160
-95
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
is now interpreted as a percentage, instead of ignoring the unit. For example,
66
`color.change(red, $alpha: 50%)` now returns `rgb(255 0 0 / 0.5)`.
77

8+
* **Potentially breaking compatibility fix**: Sass no longer rounds RGB channels
9+
to the nearest integer. This means that, for example, `rgb(0 0 1) != rgb(0 0
10+
0.6)`. This matches the latest version of the CSS spec and browser behavior.
11+
812
* **Potentially breaking compatibility fix**: Passing large positive or negative
913
values to `color.adjust()` can now cause a color's channels to go outside that
1014
color's gamut. In most cases this will currently be clipped by the browser and

‎lib/src/functions/color.dart

Lines changed: 132 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -457,21 +457,14 @@ final module = BuiltInModule("color", functions: <Callable>[
457457
(arguments) => SassString(arguments.first.assertColor("color").space.name,
458458
quotes: false)),
459459

460-
_function("to-space", r"$color, $space", (arguments) {
461-
var converted = _colorInSpace(arguments[0], arguments[1]);
462-
// `color.to-space()` never returns missing channels for legacy color
463-
// spaces because they're less compatible and users are probably using a
464-
// legacy space because they want a highly compatible color.
465-
return converted.isLegacy &&
466-
(converted.isChannel0Missing ||
467-
converted.isChannel1Missing ||
468-
converted.isChannel2Missing ||
469-
converted.isAlphaMissing) &&
470-
converted.space != (arguments[0] as SassColor).space
471-
? SassColor.forSpaceInternal(converted.space, converted.channel0,
472-
converted.channel1, converted.channel2, converted.alpha)
473-
: converted;
474-
}),
460+
// `color.to-space()` never returns missing channels for legacy color spaces
461+
// because they're less compatible and users are probably using a legacy space
462+
// because they want a highly compatible color.
463+
_function(
464+
"to-space",
465+
r"$color, $space",
466+
(arguments) =>
467+
_colorInSpace(arguments[0], arguments[1], legacyMissing: false)),
475468

476469
_function("is-legacy", r"$color",
477470
(arguments) => SassBoolean(arguments[0].assertColor("color").isLegacy)),
@@ -508,7 +501,10 @@ final module = BuiltInModule("color", functions: <Callable>[
508501
(arguments[2].assertString("method")..assertUnquoted("method")).text);
509502
if (!space.isBounded) return color;
510503

511-
return color.toSpace(space).toGamut(method).toSpace(color.space);
504+
return color
505+
.toSpace(space)
506+
.toGamut(method)
507+
.toSpace(color.space, legacyMissing: false);
512508
}),
513509

514510
_function("channel", r"$color, $channel, $space: null", (arguments) {
@@ -595,7 +591,8 @@ final _mix = _function("mix", r"$color1, $color2, $weight: 50%, $method: null",
595591
if (arguments[3] != sassNull) {
596592
return color1.interpolate(
597593
color2, InterpolationMethod.fromValue(arguments[3], "method"),
598-
weight: weight.valueInRangeWithUnit(0, 100, "weight", "%") / 100);
594+
weight: weight.valueInRangeWithUnit(0, 100, "weight", "%") / 100,
595+
legacyMissing: false);
599596
}
600597

601598
_checkPercent(weight, "weight");
@@ -630,9 +627,24 @@ final _complement =
630627
"Color space $space doesn't have a hue channel.", 'space');
631628
}
632629

633-
var inSpace = color.toSpace(space);
634-
return inSpace.changeChannels({'hue': inSpace.channel('hue') + 180}).toSpace(
635-
color.space);
630+
var colorInSpace =
631+
color.toSpace(space, legacyMissing: arguments[1] != sassNull);
632+
return (space.isLegacy
633+
? SassColor.forSpaceInternal(
634+
space,
635+
_adjustChannel(colorInSpace, space.channels[0],
636+
colorInSpace.channel0OrNull, SassNumber(180)),
637+
colorInSpace.channel1OrNull,
638+
colorInSpace.channel2OrNull,
639+
colorInSpace.alphaOrNull)
640+
: SassColor.forSpaceInternal(
641+
space,
642+
colorInSpace.channel0OrNull,
643+
colorInSpace.channel1OrNull,
644+
_adjustChannel(colorInSpace, space.channels[2],
645+
colorInSpace.channel2OrNull, SassNumber(180)),
646+
colorInSpace.alphaOrNull))
647+
.toSpace(color.space, legacyMissing: false);
636648
});
637649

638650
/// The implementation of the `invert()` function.
@@ -662,9 +674,13 @@ Value _invert(List<Value> arguments, {bool global = false}) {
662674

663675
_checkPercent(weightNumber, "weight");
664676
var rgb = color.toSpace(ColorSpace.rgb);
677+
var [channel0, channel1, channel2] = ColorSpace.rgb.channels;
665678
return _mixLegacy(
666-
SassColor.rgb(255.0 - rgb.channel0, 255.0 - rgb.channel1,
667-
255.0 - rgb.channel2, color.alphaOrNull),
679+
SassColor.rgb(
680+
_invertChannel(rgb, channel0, rgb.channel0OrNull),
681+
_invertChannel(rgb, channel1, rgb.channel1OrNull),
682+
_invertChannel(rgb, channel2, rgb.channel2OrNull),
683+
color.alphaOrNull),
668684
color,
669685
weightNumber)
670686
.toSpace(color.space);
@@ -678,38 +694,46 @@ Value _invert(List<Value> arguments, {bool global = false}) {
678694

679695
var inSpace = color.toSpace(space);
680696
var inverted = switch (space) {
681-
ColorSpace.hwb => SassColor.hwb((inSpace.channel0 + 180) % 360,
682-
inSpace.channel2, inSpace.channel1, inSpace.alpha),
683-
ColorSpace.hsl => SassColor.hsl((inSpace.channel0 + 180) % 360,
684-
inSpace.channel1, 100 - inSpace.channel2, inSpace.alpha),
685-
ColorSpace.lch => SassColor.lch(100 - inSpace.channel0, inSpace.channel1,
686-
(inSpace.channel2 + 180) % 360, inSpace.alpha),
687-
ColorSpace.oklch => SassColor.oklch(1 - inSpace.channel0, inSpace.channel1,
688-
(inSpace.channel2 + 180) % 360, inSpace.alpha),
689-
ColorSpace(
690-
channels: [
691-
LinearChannel channel0,
692-
LinearChannel channel1,
693-
LinearChannel channel2
694-
]
695-
) =>
697+
ColorSpace.hwb => SassColor.hwb(
698+
_invertChannel(inSpace, space.channels[0], inSpace.channel0OrNull),
699+
inSpace.channel2OrNull,
700+
inSpace.channel1OrNull,
701+
inSpace.alpha),
702+
ColorSpace.hsl ||
703+
ColorSpace.lch ||
704+
ColorSpace.oklch =>
705+
SassColor.forSpaceInternal(
706+
space,
707+
_invertChannel(inSpace, space.channels[0], inSpace.channel0OrNull),
708+
inSpace.channel1OrNull,
709+
_invertChannel(inSpace, space.channels[2], inSpace.channel2OrNull),
710+
inSpace.alpha),
711+
ColorSpace(channels: [var channel0, var channel1, var channel2]) =>
696712
SassColor.forSpaceInternal(
697713
space,
698-
_invertChannel(channel0, inSpace.channel0),
699-
_invertChannel(channel1, inSpace.channel1),
700-
_invertChannel(channel2, inSpace.channel2),
714+
_invertChannel(inSpace, channel0, inSpace.channel0OrNull),
715+
_invertChannel(inSpace, channel1, inSpace.channel1OrNull),
716+
_invertChannel(inSpace, channel2, inSpace.channel2OrNull),
701717
inSpace.alpha),
702718
_ => throw UnsupportedError("Unknown color space $space.")
703719
};
704720

705-
if (fuzzyEquals(weight, 1)) return inverted.toSpace(color.space);
706-
return color.interpolate(inverted, InterpolationMethod(space),
707-
weight: 1 - weight);
721+
return fuzzyEquals(weight, 1)
722+
? inverted.toSpace(color.space, legacyMissing: false)
723+
: color.interpolate(inverted, InterpolationMethod(space),
724+
weight: 1 - weight, legacyMissing: false);
708725
}
709726

710727
/// Returns the inverse of the given [value] in a linear color channel.
711-
double _invertChannel(LinearChannel channel, double value) =>
712-
channel.min < 0 ? -value : channel.max - value;
728+
double _invertChannel(SassColor color, ColorChannel channel, double? value) {
729+
if (value == null) _missingChannelError(color, channel.name);
730+
return switch (channel) {
731+
LinearChannel(min: < 0) => -value,
732+
LinearChannel(min: 0, :var max) => max - value,
733+
ColorChannel(isPolarAngle: true) => (value + 180) % 360,
734+
_ => throw UnsupportedError("Unknown channel $channel.")
735+
};
736+
}
713737

714738
/// The implementation of the `grayscale()` function, without any logic for the
715739
/// plain-CSS `grayscale()` syntax.
@@ -718,11 +742,12 @@ Value _grayscale(Value colorArg) {
718742

719743
if (color.isLegacy) {
720744
var hsl = color.toSpace(ColorSpace.hsl);
721-
return SassColor.hsl(hsl.channel0, 0, hsl.channel2, hsl.alpha)
722-
.toSpace(color.space);
745+
return SassColor.hsl(hsl.channel0OrNull, 0, hsl.channel2OrNull, hsl.alpha)
746+
.toSpace(color.space, legacyMissing: false);
723747
} else {
724748
var oklch = color.toSpace(ColorSpace.oklch);
725-
return SassColor.oklch(oklch.channel0, 0, oklch.channel2, oklch.alpha)
749+
return SassColor.oklch(
750+
oklch.channel0OrNull, 0, oklch.channel2OrNull, oklch.alpha)
726751
.toSpace(color.space);
727752
}
728753
}
@@ -774,12 +799,14 @@ SassColor _updateComponents(List<Value> arguments,
774799
var alphaArg = keywords.remove('alpha');
775800

776801
// For backwards-compatibility, we allow legacy colors to modify channels in
777-
// any legacy color space.
778-
var color =
779-
spaceKeyword == null && originalColor.isLegacy && keywords.isNotEmpty
780-
? _sniffLegacyColorSpace(keywords).andThen(originalColor.toSpace) ??
781-
originalColor
782-
: _colorInSpace(originalColor, spaceKeyword ?? sassNull);
802+
// any legacy color space and we their powerless channels as 0.
803+
var color = spaceKeyword == null &&
804+
originalColor.isLegacy &&
805+
keywords.isNotEmpty
806+
? _sniffLegacyColorSpace(keywords).andThen(
807+
(space) => originalColor.toSpace(space, legacyMissing: false)) ??
808+
originalColor
809+
: _colorInSpace(originalColor, spaceKeyword ?? sassNull);
783810

784811
var oldChannels = color.channels;
785812
var channelArgs = List<Value?>.filled(oldChannels.length, null);
@@ -809,7 +836,7 @@ SassColor _updateComponents(List<Value> arguments,
809836
: _adjustColor(color, channelNumbers, alphaNumber);
810837
}
811838

812-
return result.toSpace(originalColor.space);
839+
return result.toSpace(originalColor.space, legacyMissing: false);
813840
}
814841

815842
/// Returns a copy of [color] with its channel values replaced by those in
@@ -872,20 +899,25 @@ SassColor _scaleColor(
872899
SassColor color, List<SassNumber?> channelArgs, SassNumber? alphaArg) =>
873900
SassColor.forSpaceInternal(
874901
color.space,
875-
_scaleChannel(color.space.channels[0], color.channel0, channelArgs[0]),
876-
_scaleChannel(color.space.channels[1], color.channel1, channelArgs[1]),
877-
_scaleChannel(color.space.channels[2], color.channel2, channelArgs[2]),
878-
_scaleChannel(ColorChannel.alpha, color.alpha, alphaArg));
902+
_scaleChannel(color, color.space.channels[0], color.channel0OrNull,
903+
channelArgs[0]),
904+
_scaleChannel(color, color.space.channels[1], color.channel1OrNull,
905+
channelArgs[1]),
906+
_scaleChannel(color, color.space.channels[2], color.channel2OrNull,
907+
channelArgs[2]),
908+
_scaleChannel(color, ColorChannel.alpha, color.alphaOrNull, alphaArg));
879909

880910
/// Returns [oldValue] scaled by [factorArg] according to the definition in
881911
/// [channel].
882-
double _scaleChannel(
883-
ColorChannel channel, double oldValue, SassNumber? factorArg) {
912+
double? _scaleChannel(SassColor color, ColorChannel channel, double? oldValue,
913+
SassNumber? factorArg) {
884914
if (factorArg == null) return oldValue;
885915
if (channel is! LinearChannel) {
886916
throw SassScriptException("Channel isn't scalable.", channel.name);
887917
}
888918

919+
if (oldValue == null) _missingChannelError(color, channel.name);
920+
889921
var factor = (factorArg..assertUnit('%', channel.name))
890922
.valueInRangeWithUnit(-100, 100, channel.name, '%') /
891923
100;
@@ -906,32 +938,33 @@ SassColor _adjustColor(
906938
SassColor color, List<SassNumber?> channelArgs, SassNumber? alphaArg) =>
907939
SassColor.forSpaceInternal(
908940
color.space,
909-
_adjustChannel(color.space, color.space.channels[0], color.channel0,
941+
_adjustChannel(color, color.space.channels[0], color.channel0OrNull,
910942
channelArgs[0]),
911-
_adjustChannel(color.space, color.space.channels[1], color.channel1,
943+
_adjustChannel(color, color.space.channels[1], color.channel1OrNull,
912944
channelArgs[1]),
913-
_adjustChannel(color.space, color.space.channels[2], color.channel2,
945+
_adjustChannel(color, color.space.channels[2], color.channel2OrNull,
914946
channelArgs[2]),
915947
// The color space doesn't matter for alpha, as long as it's not
916948
// strictly bounded.
917-
_adjustChannel(
918-
ColorSpace.lab, ColorChannel.alpha, color.alpha, alphaArg)
919-
.clamp(0, 1));
949+
_adjustChannel(color, ColorChannel.alpha, color.alphaOrNull, alphaArg)
950+
?.clamp(0, 1));
920951

921952
/// Returns [oldValue] adjusted by [adjustmentArg] according to the definition
922-
/// in [space]'s [channel].
923-
double _adjustChannel(ColorSpace space, ColorChannel channel, double oldValue,
953+
/// in [color]'s space's [channel].
954+
double? _adjustChannel(SassColor color, ColorChannel channel, double? oldValue,
924955
SassNumber? adjustmentArg) {
925956
if (adjustmentArg == null) return oldValue;
926957

927-
switch ((space, channel)) {
928-
case (ColorSpace.hsl || ColorSpace.hwb, _) when channel is! LinearChannel:
958+
if (oldValue == null) _missingChannelError(color, channel.name);
959+
960+
switch ((color.space, channel)) {
961+
case (ColorSpace.hsl || ColorSpace.hwb, ColorChannel(isPolarAngle: true)):
929962
// `_channelFromValue` expects all hue values to be compatible with `deg`,
930963
// but we're still in the deprecation period where we allow non-`deg`
931964
// values for HSL and HWB so we have to handle that ahead-of-time.
932965
adjustmentArg = SassNumber(_angleValue(adjustmentArg, 'hue'));
933966

934-
case (ColorSpace.hsl, LinearChannel()):
967+
case (ColorSpace.hsl, LinearChannel(name: 'saturation' || 'lightness')):
935968
// `_channelFromValue` expects lightness/saturation to be `%`, but we're
936969
// still in the deprecation period where we allow non-`%` values so we
937970
// have to handle that ahead-of-time.
@@ -1047,6 +1080,14 @@ Value _rgbTwoArg(String name, List<Value> arguments) {
10471080
}
10481081

10491082
var color = first.assertColor("color");
1083+
if (!color.isLegacy) {
1084+
throw SassScriptException(
1085+
'Expected $color to be in the legacy RGB, HSL, or HWB color space.\n'
1086+
'\n'
1087+
'Recommendation: color.change($color, \$alpha: $second)',
1088+
name);
1089+
}
1090+
10501091
color.assertLegacy("color");
10511092
color = color.toSpace(ColorSpace.rgb);
10521093
if (second.isSpecialNumber) {
@@ -1233,17 +1274,22 @@ SassColor _transparentize(String name, List<Value> arguments) {
12331274
/// Returns the [colorUntyped] as a [SassColor] in the color space specified by
12341275
/// [spaceUntyped].
12351276
///
1277+
/// If [legacyMissing] is false, this will convert missing channels in legacy
1278+
/// color spaces to zero if a conversion occurs.
1279+
///
12361280
/// Throws a [SassScriptException] if either argument isn't the expected type or
12371281
/// if [spaceUntyped] isn't the name of a color space. If [spaceUntyped] is
12381282
/// `sassNull`, it defaults to the color's existing space.
1239-
SassColor _colorInSpace(Value colorUntyped, Value spaceUntyped) {
1283+
SassColor _colorInSpace(Value colorUntyped, Value spaceUntyped,
1284+
{bool legacyMissing = true}) {
12401285
var color = colorUntyped.assertColor("color");
12411286
if (spaceUntyped == sassNull) return color;
12421287

1243-
var space = ColorSpace.fromName(
1244-
(spaceUntyped.assertString("space")..assertUnquoted("space")).text,
1245-
"space");
1246-
return color.space == space ? color : color.toSpace(space);
1288+
return color.toSpace(
1289+
ColorSpace.fromName(
1290+
(spaceUntyped.assertString("space")..assertUnquoted("space")).text,
1291+
"space"),
1292+
legacyMissing: legacyMissing);
12471293
}
12481294

12491295
/// Returns the color space named by [space], or throws a [SassScriptException]
@@ -1584,6 +1630,15 @@ String _suggestScaleAndAdjust(
15841630
return suggestion + "color.adjust(\$color, \$$channelName: $difference)";
15851631
}
15861632

1633+
/// Throws an error indicating that a missing channel named [name] can't be
1634+
/// modified.
1635+
Never _missingChannelError(SassColor color, String channel) =>
1636+
throw SassScriptException(
1637+
"Because the CSS working group is still deciding on the best behavior, "
1638+
"Sass doesn't currently support modifying missing channels (color: "
1639+
"$color).",
1640+
channel);
1641+
15871642
/// Asserts that `value` is an unquoted string and throws an error if it's not.
15881643
///
15891644
/// Assumes that `value` comes from a parameter named `$channel`.

0 commit comments

Comments
 (0)