Auto merge of #18517 - BorisChiou:stylo/filter/clamp, r=emilio

style: Clamp some filter functions to one if the values over 100%

For grayscale, invert, opacity, and sepia, "values of amount over 100%
are allowed but UAs must clamp the values to 1" [1]-[4], so we clamp its value
in the parser.

[1] https://drafts.fxtf.org/filter-effects/#funcdef-filter-grayscale
[2] https://drafts.fxtf.org/filter-effects/#funcdef-filter-invert
[3] https://drafts.fxtf.org/filter-effects/#funcdef-filter-opacity
[4] https://drafts.fxtf.org/filter-effects/#funcdef-filter-sepia

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1399799](https://bugzilla.mozilla.org/show_bug.cgi?id=1399799).
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18517)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-09-18 05:23:36 -05:00 committed by GitHub
commit cff9b427fc
4 changed files with 62 additions and 7 deletions

View file

@ -40,6 +40,30 @@ pub type Filter = GenericFilter<Angle, Factor, NonNegativeLength, Impossible>;
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct Factor(NumberOrPercentage);
impl Factor {
/// Parse this factor but clamp to one if the value is over 100%.
#[inline]
pub fn parse_with_clamping_to_one<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>
) -> Result<Self, ParseError<'i>> {
Factor::parse(context, input).map(|v| v.clamp_to_one())
}
/// Clamp the value to 1 if the value is over 100%.
#[inline]
fn clamp_to_one(self) -> Self {
match self.0 {
NumberOrPercentage::Percentage(percent) => {
Factor(NumberOrPercentage::Percentage(percent.clamp_to_hundred()))
},
NumberOrPercentage::Number(number) => {
Factor(NumberOrPercentage::Number(number.clamp_to_one()))
}
}
}
}
impl Parse for Factor {
#[inline]
fn parse<'i, 't>(
@ -173,12 +197,28 @@ impl Parse for Filter {
"blur" => Ok(GenericFilter::Blur((Length::parse_non_negative(context, i)?).into())),
"brightness" => Ok(GenericFilter::Brightness(Factor::parse(context, i)?)),
"contrast" => Ok(GenericFilter::Contrast(Factor::parse(context, i)?)),
"grayscale" => Ok(GenericFilter::Grayscale(Factor::parse(context, i)?)),
"grayscale" => {
// Values of amount over 100% are allowed but UAs must clamp the values to 1.
// https://drafts.fxtf.org/filter-effects/#funcdef-filter-grayscale
Ok(GenericFilter::Grayscale(Factor::parse_with_clamping_to_one(context, i)?))
},
"hue-rotate" => Ok(GenericFilter::HueRotate(Angle::parse(context, i)?)),
"invert" => Ok(GenericFilter::Invert(Factor::parse(context, i)?)),
"opacity" => Ok(GenericFilter::Opacity(Factor::parse(context, i)?)),
"invert" => {
// Values of amount over 100% are allowed but UAs must clamp the values to 1.
// https://drafts.fxtf.org/filter-effects/#funcdef-filter-invert
Ok(GenericFilter::Invert(Factor::parse_with_clamping_to_one(context, i)?))
},
"opacity" => {
// Values of amount over 100% are allowed but UAs must clamp the values to 1.
// https://drafts.fxtf.org/filter-effects/#funcdef-filter-opacity
Ok(GenericFilter::Opacity(Factor::parse_with_clamping_to_one(context, i)?))
},
"saturate" => Ok(GenericFilter::Saturate(Factor::parse(context, i)?)),
"sepia" => Ok(GenericFilter::Sepia(Factor::parse(context, i)?)),
"sepia" => {
// Values of amount over 100% are allowed but UAs must clamp the values to 1.
// https://drafts.fxtf.org/filter-effects/#funcdef-filter-sepia
Ok(GenericFilter::Sepia(Factor::parse_with_clamping_to_one(context, i)?))
},
"drop-shadow" => Ok(GenericFilter::DropShadow(Parse::parse(context, i)?)),
_ => Err(ValueParseError::InvalidFilter(Token::Function(function.clone())).into()),
}

View file

@ -217,6 +217,15 @@ impl Number {
-> Result<Number, ParseError<'i>> {
parse_number_with_clamping_mode(context, input, AllowedNumericType::AtLeastOne)
}
/// Clamp to 1.0 if the value is over 1.0.
#[inline]
pub fn clamp_to_one(self) -> Self {
Number {
value: self.value.min(1.),
calc_clamping_mode: self.calc_clamping_mode,
}
}
}
impl ToComputedValue for Number {

View file

@ -125,6 +125,15 @@ impl Percentage {
) -> Result<Self, ParseError<'i>> {
Self::parse_with_clamping_mode(context, input, AllowedNumericType::NonNegative)
}
/// Clamp to 100% if the value is over 100%.
#[inline]
pub fn clamp_to_hundred(self) -> Self {
Percentage {
value: self.value.min(1.),
calc_clamping_mode: self.calc_clamping_mode,
}
}
}
impl Parse for Percentage {