Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clamp alpha during parse, other tweaks #119

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Clamp alpha during parse, other tweaks #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ This release has an [MSRV][] of 1.82.
### Fixed

* Colors in `XyzD65` are serialized as `xyz-d65` rather than `xyz`. ([#118][] by [@waywardmonkeys][])
* Alpha values are clamped at parse time. ([#119][] by [@waywardmonkeys][])

## [0.2.1][] (2024-12-27)

Expand Down Expand Up @@ -100,6 +101,7 @@ This is the initial release.
[#111]: https://github.com/linebender/color/pull/111
[#113]: https://github.com/linebender/color/pull/113
[#118]: https://github.com/linebender/color/pull/118
[#119]: https://github.com/linebender/color/pull/119

[Unreleased]: https://github.com/linebender/color/compare/v0.2.1...HEAD
[0.2.1]: https://github.com/linebender/color/releases/tag/v0.2.1
Expand Down
88 changes: 64 additions & 24 deletions color/src/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,22 @@ enum Value<'a> {
Dimension(f64, &'a str),
}

/// Whether or not we are parsing modern or legacy mode syntax.
#[derive(Clone, Copy, Debug, PartialEq)]
enum Mode {
Legacy,
Modern,
}

impl Mode {
waywardmonkeys marked this conversation as resolved.
Show resolved Hide resolved
fn alpha_separator(self) -> u8 {
match self {
Self::Legacy => b',',
Self::Modern => b'/',
}
}
}

#[expect(
clippy::cast_possible_truncation,
reason = "deliberate choice of f32 for colors"
Expand Down Expand Up @@ -317,11 +333,6 @@ impl<'a> Parser<'a> {
}
}

fn opacity_separator(&mut self, comma: bool) -> bool {
self.ws();
self.ch(if comma { b',' } else { b'/' })
}

fn rgb(&mut self) -> Result<DynamicColor, ParseError> {
if !self.raw_ch(b'(') {
return Err(ParseError::ExpectedArguments);
Expand All @@ -333,32 +344,42 @@ impl<'a> Parser<'a> {
.map(|x| x.clamp(0., 1.));
self.ws();
let comma = self.ch(b',');
let mode = if comma { Mode::Legacy } else { Mode::Modern };
let g = self
.scaled_component(1. / 255., 0.01)?
.map(|x| x.clamp(0., 1.));
self.optional_comma(comma)?;
let b = self
.scaled_component(1. / 255., 0.01)?
.map(|x| x.clamp(0., 1.));
let mut alpha = Some(1.0);
if self.opacity_separator(comma) {
alpha = self.scaled_component(1., 0.01)?.map(|a| a.clamp(0., 1.));
}
let alpha = self.alpha(mode)?;
self.ws();
if !self.ch(b')') {
return Err(ParseError::ExpectedClosingParenthesis);
}
Ok(color_from_components([r, g, b, alpha], ColorSpaceTag::Srgb))
}

fn optional_alpha(&mut self) -> Result<Option<f64>, ParseError> {
let mut alpha = Some(1.0);
/// Read a slash separator and an alpha value.
///
/// The value may be either number or a percentage.
///
/// The alpha value defaults to `1.0` if not present. The value will be clamped
/// to the range [0, 1].
///
/// If the value is `"none"`, then `Ok(None)` will be returned.
///
/// The separator will be a `'/'` in modern mode and a `','` in legacy mode.
/// If no separator is present, then the default value will be returned.
///
/// Reference: § 4.2 of CSS Color 4 spec.
waywardmonkeys marked this conversation as resolved.
Show resolved Hide resolved
fn alpha(&mut self, mode: Mode) -> Result<Option<f64>, ParseError> {
self.ws();
if self.ch(b'/') {
alpha = self.scaled_component(1., 0.01)?;
if self.ch(mode.alpha_separator()) {
Ok(self.scaled_component(1., 0.01)?.map(|a| a.clamp(0., 1.)))
} else {
Ok(Some(1.0))
}
self.ws();
Ok(alpha)
}

fn lab(&mut self, lmax: f64, c: f64, tag: ColorSpaceTag) -> Result<DynamicColor, ParseError> {
Expand All @@ -370,7 +391,8 @@ impl<'a> Parser<'a> {
.map(|x| x.clamp(0., lmax));
let a = self.scaled_component(1., c)?;
let b = self.scaled_component(1., c)?;
let alpha = self.optional_alpha()?;
let alpha = self.alpha(Mode::Modern)?;
self.ws();
if !self.ch(b')') {
return Err(ParseError::ExpectedClosingParenthesis);
}
Expand All @@ -386,7 +408,8 @@ impl<'a> Parser<'a> {
.map(|x| x.clamp(0., lmax));
let c = self.scaled_component(1., c)?.map(|x| x.max(0.));
let h = self.angle()?;
let alpha = self.optional_alpha()?;
let alpha = self.alpha(Mode::Modern)?;
self.ws();
if !self.ch(b')') {
return Err(ParseError::ExpectedClosingParenthesis);
}
Expand All @@ -399,13 +422,11 @@ impl<'a> Parser<'a> {
}
let h = self.angle()?;
let comma = self.ch(b',');
let mode = if comma { Mode::Legacy } else { Mode::Modern };
let s = self.scaled_component(1., 1.)?.map(|x| x.max(0.));
self.optional_comma(comma)?;
let l = self.scaled_component(1., 1.)?;
let mut alpha = Some(1.0);
if self.opacity_separator(comma) {
alpha = self.scaled_component(1., 0.01)?.map(|a| a.clamp(0., 1.));
}
let alpha = self.alpha(mode)?;
self.ws();
if !self.ch(b')') {
return Err(ParseError::ExpectedClosingParenthesis);
Expand All @@ -420,7 +441,8 @@ impl<'a> Parser<'a> {
let h = self.angle()?;
let w = self.scaled_component(1., 1.)?;
let b = self.scaled_component(1., 1.)?;
let alpha = self.optional_alpha()?;
let alpha = self.alpha(Mode::Modern)?;
self.ws();
if !self.ch(b')') {
return Err(ParseError::ExpectedClosingParenthesis);
}
Expand Down Expand Up @@ -451,7 +473,8 @@ impl<'a> Parser<'a> {
let r = self.scaled_component(1., 0.01)?;
let g = self.scaled_component(1., 0.01)?;
let b = self.scaled_component(1., 0.01)?;
let alpha = self.optional_alpha()?;
let alpha = self.alpha(Mode::Modern)?;
self.ws();
if !self.ch(b')') {
return Err(ParseError::ExpectedClosingParenthesis);
}
Expand Down Expand Up @@ -678,7 +701,7 @@ fn make_lowercase<'a>(s: &'a str, buf: &'a mut [u8; LOWERCASE_BUF_SIZE]) -> &'a
mod tests {
use crate::DynamicColor;

use super::{parse_color, parse_color_prefix, ParseError, Parser};
use super::{parse_color, parse_color_prefix, Mode, ParseError, Parser};

fn assert_close_color(c1: DynamicColor, c2: DynamicColor) {
const EPSILON: f32 = 1e-4;
Expand Down Expand Up @@ -754,6 +777,23 @@ mod tests {
}
}

#[test]
fn alpha() {
for (alpha, expected, mode) in [
(", 10%", Ok(Some(0.1)), Mode::Legacy),
("/ 0.25", Ok(Some(0.25)), Mode::Modern),
("/ -0.3", Ok(Some(0.)), Mode::Modern),
("/ 110%", Ok(Some(1.)), Mode::Modern),
("", Ok(Some(1.)), Mode::Legacy),
("/ none", Ok(None), Mode::Modern),
] {
let mut parser = Parser::new(alpha);
let result = parser.alpha(mode);
assert_eq!(result, expected,
"Failed parsing specified alpha `{alpha}`. Expected: `{expected:?}`. Got: `{result:?}`.");
}
}

#[test]
fn angles() {
for (angle, expected) in [
Expand Down
Loading