style: Cleanup and fix interpolation of SVG lengths.

Instead of storing them as LengthPercentage | Number, always store as
LengthPercentage, and use the unitless length quirk to parse numbers instead.

Further cleanups to use the rust representation can happen as a followup, which
will also get rid of the boolean argument (since we can poke at the rust length
itself). That's why I didn't bother to convert it to an enum class yet.

Differential Revision: https://phabricator.services.mozilla.com/D21804
This commit is contained in:
Emilio Cobos Álvarez 2019-03-01 23:17:12 +01:00
parent 9e0d38a64f
commit f1b5d5c06a
7 changed files with 94 additions and 280 deletions

View file

@ -463,27 +463,22 @@ def set_gecko_property(ffi_name, expr):
// set on mContextFlags, and the length field is set to the initial value.
pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
use crate::values::generics::svg::{SVGLength, SvgLengthPercentageOrNumber};
use crate::values::generics::svg::SVGLength;
use crate::gecko_bindings::structs::nsStyleSVG_${ident.upper()}_CONTEXT as CONTEXT_VALUE;
let length = match v {
SVGLength::Length(length) => {
SVGLength::LengthPercentage(length) => {
self.gecko.mContextFlags &= !CONTEXT_VALUE;
length
}
SVGLength::ContextValue => {
self.gecko.mContextFlags |= CONTEXT_VALUE;
match longhands::${ident}::get_initial_value() {
SVGLength::Length(length) => length,
SVGLength::LengthPercentage(length) => length,
_ => unreachable!("Initial value should not be context-value"),
}
}
};
match length {
SvgLengthPercentageOrNumber::LengthPercentage(lp) =>
self.gecko.${gecko_ffi_name}.set(lp),
SvgLengthPercentageOrNumber::Number(num) =>
self.gecko.${gecko_ffi_name}.set_value(CoordDataValue::Factor(num.into())),
}
self.gecko.${gecko_ffi_name}.set(length)
}
pub fn copy_${ident}_from(&mut self, other: &Self) {
@ -499,32 +494,16 @@ def set_gecko_property(ffi_name, expr):
}
pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
use crate::values::generics::svg::{SVGLength, SvgLengthPercentageOrNumber};
use crate::values::generics::svg::SVGLength;
use crate::values::computed::LengthPercentage;
use crate::gecko_bindings::structs::nsStyleSVG_${ident.upper()}_CONTEXT as CONTEXT_VALUE;
if (self.gecko.mContextFlags & CONTEXT_VALUE) != 0 {
return SVGLength::ContextValue;
}
let length = match self.gecko.${gecko_ffi_name}.as_value() {
CoordDataValue::Factor(number) => {
SvgLengthPercentageOrNumber::Number(number)
},
CoordDataValue::Coord(coord) => {
SvgLengthPercentageOrNumber::LengthPercentage(
LengthPercentage::new(Au(coord).into(), None)
)
},
CoordDataValue::Percent(p) => {
SvgLengthPercentageOrNumber::LengthPercentage(
LengthPercentage::new(Au(0).into(), Some(Percentage(p)))
)
},
CoordDataValue::Calc(calc) => {
SvgLengthPercentageOrNumber::LengthPercentage(calc.into())
},
_ => unreachable!("Unexpected coordinate in ${ident}"),
};
SVGLength::Length(length.into())
// TODO(emilio): Use the Rust representation instead.
let length =
LengthPercentage::from_gecko_style_coord(&self.gecko.${gecko_ffi_name}).unwrap();
SVGLength::LengthPercentage(length.into())
}
</%def>
@ -4718,7 +4697,7 @@ clip-path
pub fn set_stroke_dasharray(&mut self, v: longhands::stroke_dasharray::computed_value::T) {
use crate::gecko_bindings::structs::nsStyleSVG_STROKE_DASHARRAY_CONTEXT as CONTEXT_VALUE;
use crate::values::generics::svg::{SVGStrokeDashArray, SvgLengthPercentageOrNumber};
use crate::values::generics::svg::SVGStrokeDashArray;
match v {
SVGStrokeDashArray::Values(v) => {
@ -4728,12 +4707,7 @@ clip-path
bindings::Gecko_nsStyleSVG_SetDashArrayLength(&mut self.gecko, v.len() as u32);
}
for (gecko, servo) in self.gecko.mStrokeDasharray.iter_mut().zip(v) {
match servo {
SvgLengthPercentageOrNumber::LengthPercentage(lp) =>
gecko.set(lp),
SvgLengthPercentageOrNumber::Number(num) =>
gecko.set_value(CoordDataValue::Factor(num.into())),
}
gecko.set(servo)
}
}
SVGStrokeDashArray::ContextValue => {
@ -4761,30 +4735,17 @@ clip-path
pub fn clone_stroke_dasharray(&self) -> longhands::stroke_dasharray::computed_value::T {
use crate::gecko_bindings::structs::nsStyleSVG_STROKE_DASHARRAY_CONTEXT as CONTEXT_VALUE;
use crate::values::computed::LengthPercentage;
use crate::values::generics::NonNegative;
use crate::values::generics::svg::{SVGStrokeDashArray, SvgLengthPercentageOrNumber};
use crate::values::computed::NonNegativeLengthPercentage;
use crate::values::generics::svg::SVGStrokeDashArray;
if self.gecko.mContextFlags & CONTEXT_VALUE != 0 {
debug_assert_eq!(self.gecko.mStrokeDasharray.len(), 0);
return SVGStrokeDashArray::ContextValue;
}
// TODO(emilio): Use the rust representation instead.
let mut vec = vec![];
for gecko in self.gecko.mStrokeDasharray.iter() {
match gecko.as_value() {
CoordDataValue::Factor(number) =>
vec.push(SvgLengthPercentageOrNumber::Number(number.into())),
CoordDataValue::Coord(coord) =>
vec.push(SvgLengthPercentageOrNumber::LengthPercentage(
NonNegative(LengthPercentage::new(Au(coord).into(), None).into()))),
CoordDataValue::Percent(p) =>
vec.push(SvgLengthPercentageOrNumber::LengthPercentage(
NonNegative(LengthPercentage::new_percent(Percentage(p)).into()))),
CoordDataValue::Calc(calc) =>
vec.push(SvgLengthPercentageOrNumber::LengthPercentage(
NonNegative(LengthPercentage::from(calc).clamp_to_non_negative()))),
_ => unreachable!(),
}
vec.push(NonNegativeLengthPercentage::from_gecko_style_coord(gecko).unwrap());
}
SVGStrokeDashArray::Values(vec)
}

View file

@ -16,7 +16,7 @@ use crate::values::computed::Image;
use crate::values::specified::SVGPathData;
use crate::values::CSSFloat;
use app_units::Au;
use euclid::{Point2D, Size2D};
use euclid::Point2D;
use smallvec::SmallVec;
use std::cmp;
@ -241,19 +241,6 @@ impl Animate for Au {
}
}
impl<T> Animate for Size2D<T>
where
T: Animate,
{
#[inline]
fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
Ok(Size2D::new(
self.width.animate(&other.width, procedure)?,
self.height.animate(&other.height, procedure)?,
))
}
}
impl<T> Animate for Point2D<T>
where
T: Animate,
@ -397,16 +384,13 @@ where
}
}
impl<T> ToAnimatedZero for Size2D<T>
impl<T> ToAnimatedZero for Vec<T>
where
T: ToAnimatedZero,
{
#[inline]
fn to_animated_zero(&self) -> Result<Self, ()> {
Ok(Size2D::new(
self.width.to_animated_zero()?,
self.height.to_animated_zero()?,
))
self.iter().map(|v| v.to_animated_zero()).collect()
}
}

View file

@ -8,10 +8,8 @@ use super::{Animate, Procedure, ToAnimatedZero};
use crate::properties::animated_properties::ListAnimation;
use crate::values::animated::color::Color as AnimatedColor;
use crate::values::computed::url::ComputedUrl;
use crate::values::computed::{LengthPercentage, Number, NumberOrPercentage};
use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
use crate::values::generics::svg::{SVGLength, SVGPaint, SvgLengthPercentageOrNumber};
use crate::values::generics::svg::{SVGOpacity, SVGStrokeDashArray};
use crate::values::distance::{SquaredDistance, ComputeSquaredDistance};
use crate::values::generics::svg::{SVGPaint, SVGStrokeDashArray};
/// Animated SVGPaint.
pub type IntermediateSVGPaint = SVGPaint<AnimatedColor, ComputedUrl>;
@ -26,67 +24,6 @@ impl ToAnimatedZero for IntermediateSVGPaint {
}
}
// FIXME: We need to handle calc here properly, see
// https://bugzilla.mozilla.org/show_bug.cgi?id=1386967
fn to_number_or_percentage(
value: &SvgLengthPercentageOrNumber<LengthPercentage, Number>,
) -> Result<NumberOrPercentage, ()> {
Ok(match *value {
SvgLengthPercentageOrNumber::LengthPercentage(ref l) => match l.specified_percentage() {
Some(p) => {
if l.unclamped_length().px() != 0. {
return Err(());
}
NumberOrPercentage::Percentage(p)
},
None => NumberOrPercentage::Number(l.length().px()),
},
SvgLengthPercentageOrNumber::Number(ref n) => NumberOrPercentage::Number(*n),
})
}
impl Animate for SvgLengthPercentageOrNumber<LengthPercentage, Number> {
#[inline]
fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
let this = to_number_or_percentage(self)?;
let other = to_number_or_percentage(other)?;
match (this, other) {
(NumberOrPercentage::Number(ref this), NumberOrPercentage::Number(ref other)) => Ok(
SvgLengthPercentageOrNumber::Number(this.animate(other, procedure)?),
),
(
NumberOrPercentage::Percentage(ref this),
NumberOrPercentage::Percentage(ref other),
) => Ok(SvgLengthPercentageOrNumber::LengthPercentage(
LengthPercentage::new_percent(this.animate(other, procedure)?),
)),
_ => Err(()),
}
}
}
impl ComputeSquaredDistance for SvgLengthPercentageOrNumber<LengthPercentage, Number> {
fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
to_number_or_percentage(self)?.compute_squared_distance(&to_number_or_percentage(other)?)
}
}
impl<L> Animate for SVGLength<L>
where
L: Animate + Clone,
{
#[inline]
fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
match (self, other) {
(&SVGLength::Length(ref this), &SVGLength::Length(ref other)) => {
Ok(SVGLength::Length(this.animate(other, procedure)?))
},
_ => Err(()),
}
}
}
/// <https://www.w3.org/TR/SVG11/painting.html#StrokeDasharrayProperty>
impl<L> Animate for SVGStrokeDashArray<L>
where
@ -121,36 +58,3 @@ where
}
}
}
impl<L> ToAnimatedZero for SVGStrokeDashArray<L>
where
L: ToAnimatedZero,
{
#[inline]
fn to_animated_zero(&self) -> Result<Self, ()> {
match *self {
SVGStrokeDashArray::Values(ref values) => Ok(SVGStrokeDashArray::Values(
values
.iter()
.map(ToAnimatedZero::to_animated_zero)
.collect::<Result<Vec<_>, _>>()?,
)),
SVGStrokeDashArray::ContextValue => Ok(SVGStrokeDashArray::ContextValue),
}
}
}
impl<O> Animate for SVGOpacity<O>
where
O: Animate + Clone,
{
#[inline]
fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
match (self, other) {
(&SVGOpacity::Opacity(ref this), &SVGOpacity::Opacity(ref other)) => {
Ok(SVGOpacity::Opacity(this.animate(other, procedure)?))
},
_ => Err(()),
}
}
}

View file

@ -6,8 +6,7 @@
use crate::values::computed::color::Color;
use crate::values::computed::url::ComputedUrl;
use crate::values::computed::{LengthPercentage, NonNegativeLengthPercentage};
use crate::values::computed::{NonNegativeNumber, Number, Opacity};
use crate::values::computed::{LengthPercentage, NonNegativeLengthPercentage, Opacity};
use crate::values::generics::svg as generic;
use crate::values::RGBA;
use crate::Zero;
@ -41,58 +40,31 @@ impl SVGPaint {
}
}
/// A value of <length> | <percentage> | <number> for stroke-dashoffset.
/// <https://www.w3.org/TR/SVG11/painting.html#StrokeProperties>
pub type SvgLengthPercentageOrNumber =
generic::SvgLengthPercentageOrNumber<LengthPercentage, Number>;
/// <length> | <percentage> | <number> | context-value
pub type SVGLength = generic::SVGLength<SvgLengthPercentageOrNumber>;
pub type SVGLength = generic::SVGLength<LengthPercentage>;
impl SVGLength {
/// `0px`
pub fn zero() -> Self {
generic::SVGLength::Length(generic::SvgLengthPercentageOrNumber::LengthPercentage(
LengthPercentage::zero(),
))
}
}
/// A value of <length> | <percentage> | <number> for stroke-width/stroke-dasharray.
/// <https://www.w3.org/TR/SVG11/painting.html#StrokeProperties>
pub type NonNegativeSvgLengthPercentageOrNumber =
generic::SvgLengthPercentageOrNumber<NonNegativeLengthPercentage, NonNegativeNumber>;
// FIXME(emilio): This is really hacky, and can go away with a bit of work on
// the clone_stroke_width code in gecko.mako.rs.
impl Into<NonNegativeSvgLengthPercentageOrNumber> for SvgLengthPercentageOrNumber {
fn into(self) -> NonNegativeSvgLengthPercentageOrNumber {
match self {
generic::SvgLengthPercentageOrNumber::LengthPercentage(lop) => {
generic::SvgLengthPercentageOrNumber::LengthPercentage(lop.into())
},
generic::SvgLengthPercentageOrNumber::Number(num) => {
generic::SvgLengthPercentageOrNumber::Number(num.into())
},
}
generic::SVGLength::LengthPercentage(LengthPercentage::zero())
}
}
/// An non-negative wrapper of SVGLength.
pub type SVGWidth = generic::SVGLength<NonNegativeSvgLengthPercentageOrNumber>;
pub type SVGWidth = generic::SVGLength<NonNegativeLengthPercentage>;
impl SVGWidth {
/// `1px`.
pub fn one() -> Self {
use crate::values::generics::NonNegative;
generic::SVGLength::Length(generic::SvgLengthPercentageOrNumber::LengthPercentage(
generic::SVGLength::LengthPercentage(
NonNegative(LengthPercentage::one()),
))
)
}
}
/// [ <length> | <percentage> | <number> ]# | context-value
pub type SVGStrokeDashArray = generic::SVGStrokeDashArray<NonNegativeSvgLengthPercentageOrNumber>;
pub type SVGStrokeDashArray = generic::SVGStrokeDashArray<NonNegativeLengthPercentage>;
impl Default for SVGStrokeDashArray {
fn default() -> Self {

View file

@ -128,64 +128,26 @@ impl<ColorType: Parse, UrlPaintServer: Parse> Parse for SVGPaint<ColorType, UrlP
}
}
/// A value of <length> | <percentage> | <number> for svg which allow unitless length.
/// <https://www.w3.org/TR/SVG11/painting.html#StrokeProperties>
#[derive(
Clone,
Copy,
Debug,
MallocSizeOf,
PartialEq,
Parse,
SpecifiedValueInfo,
ToAnimatedValue,
ToAnimatedZero,
ToComputedValue,
ToCss,
)]
pub enum SvgLengthPercentageOrNumber<LengthPercentage, Number> {
/// <number>
///
/// Note that this needs to be before, so it gets parsed before the length,
/// to handle `0` correctly as a number instead of a `<length>`.
Number(Number),
/// <length> | <percentage>
LengthPercentage(LengthPercentage),
}
/// Whether the `context-value` value is enabled.
#[cfg(feature = "gecko")]
pub fn is_context_value_enabled(_: &ParserContext) -> bool {
use crate::gecko_bindings::structs::mozilla;
unsafe { mozilla::StaticPrefs_sVarCache_gfx_font_rendering_opentype_svg_enabled }
}
/// Whether the `context-value` value is enabled.
#[cfg(not(feature = "gecko"))]
pub fn is_context_value_enabled(_: &ParserContext) -> bool {
false
}
/// An SVG length value supports `context-value` in addition to length.
#[derive(
Animate,
Clone,
ComputeSquaredDistance,
Copy,
Debug,
MallocSizeOf,
PartialEq,
Parse,
SpecifiedValueInfo,
ToAnimatedValue,
ToAnimatedZero,
ToComputedValue,
ToCss,
)]
pub enum SVGLength<LengthType> {
pub enum SVGLength<L> {
/// `<length> | <percentage> | <number>`
Length(LengthType),
LengthPercentage(L),
/// `context-value`
#[parse(condition = "is_context_value_enabled")]
#[animation(error)]
ContextValue,
}
@ -197,13 +159,14 @@ pub enum SVGLength<LengthType> {
PartialEq,
SpecifiedValueInfo,
ToAnimatedValue,
ToAnimatedZero,
ToComputedValue,
ToCss,
)]
pub enum SVGStrokeDashArray<LengthType> {
pub enum SVGStrokeDashArray<L> {
/// `[ <length> | <percentage> | <number> ]#`
#[css(comma)]
Values(#[css(if_empty = "none", iterable)] Vec<LengthType>),
Values(#[css(if_empty = "none", iterable)] Vec<L>),
/// `context-value`
ContextValue,
}
@ -211,6 +174,7 @@ pub enum SVGStrokeDashArray<LengthType> {
/// An SVG opacity value accepts `context-{fill,stroke}-opacity` in
/// addition to opacity value.
#[derive(
Animate,
Clone,
ComputeSquaredDistance,
Copy,
@ -227,7 +191,9 @@ pub enum SVGOpacity<OpacityType> {
/// `<opacity-value>`
Opacity(OpacityType),
/// `context-fill-opacity`
#[animation(error)]
ContextFillOpacity,
/// `context-stroke-opacity`
#[animation(error)]
ContextStrokeOpacity,
}

View file

@ -782,16 +782,22 @@ impl ClipRectOrAuto {
/// Whether quirks are allowed in this context.
#[derive(Clone, Copy, PartialEq)]
pub enum AllowQuirks {
/// Quirks are allowed.
Yes,
/// Quirks are not allowed.
No,
/// Quirks are allowed, in quirks mode.
Yes,
/// Quirks are always allowed, used for SVG lengths.
Always,
}
impl AllowQuirks {
/// Returns `true` if quirks are allowed in this context.
pub fn allowed(self, quirks_mode: QuirksMode) -> bool {
self == AllowQuirks::Yes && quirks_mode == QuirksMode::Quirks
match self {
AllowQuirks::Always => true,
AllowQuirks::No => false,
AllowQuirks::Yes => quirks_mode == QuirksMode::Quirks,
}
}
}

View file

@ -6,11 +6,11 @@
use crate::parser::{Parse, ParserContext};
use crate::values::generics::svg as generic;
use crate::values::specified::AllowQuirks;
use crate::values::specified::color::Color;
use crate::values::specified::url::SpecifiedUrl;
use crate::values::specified::LengthPercentage;
use crate::values::specified::{NonNegativeLengthPercentage, NonNegativeNumber};
use crate::values::specified::{Number, Opacity};
use crate::values::specified::{NonNegativeLengthPercentage, Opacity};
use crate::values::CustomIdent;
use cssparser::Parser;
use std::fmt::{self, Write};
@ -23,36 +23,53 @@ pub type SVGPaint = generic::SVGPaint<Color, SpecifiedUrl>;
/// Specified SVG Paint Kind value
pub type SVGPaintKind = generic::SVGPaintKind<Color, SpecifiedUrl>;
/// A value of <length> | <percentage> | <number> for stroke-dashoffset.
/// <https://www.w3.org/TR/SVG11/painting.html#StrokeProperties>
pub type SvgLengthPercentageOrNumber =
generic::SvgLengthPercentageOrNumber<LengthPercentage, Number>;
/// <length> | <percentage> | <number> | context-value
pub type SVGLength = generic::SVGLength<SvgLengthPercentageOrNumber>;
impl From<SvgLengthPercentageOrNumber> for SVGLength {
fn from(length: SvgLengthPercentageOrNumber) -> Self {
generic::SVGLength::Length(length)
}
}
/// A value of <length> | <percentage> | <number> for stroke-width/stroke-dasharray.
/// <https://www.w3.org/TR/SVG11/painting.html#StrokeProperties>
pub type NonNegativeSvgLengthPercentageOrNumber =
generic::SvgLengthPercentageOrNumber<NonNegativeLengthPercentage, NonNegativeNumber>;
pub type SVGLength = generic::SVGLength<LengthPercentage>;
/// A non-negative version of SVGLength.
pub type SVGWidth = generic::SVGLength<NonNegativeSvgLengthPercentageOrNumber>;
pub type SVGWidth = generic::SVGLength<NonNegativeLengthPercentage>;
impl From<NonNegativeSvgLengthPercentageOrNumber> for SVGWidth {
fn from(length: NonNegativeSvgLengthPercentageOrNumber) -> Self {
generic::SVGLength::Length(length)
/// [ <length> | <percentage> | <number> ]# | context-value
pub type SVGStrokeDashArray = generic::SVGStrokeDashArray<NonNegativeLengthPercentage>;
/// Whether the `context-value` value is enabled.
#[cfg(feature = "gecko")]
pub fn is_context_value_enabled() -> bool {
use crate::gecko_bindings::structs::mozilla;
unsafe { mozilla::StaticPrefs_sVarCache_gfx_font_rendering_opentype_svg_enabled }
}
/// Whether the `context-value` value is enabled.
#[cfg(not(feature = "gecko"))]
pub fn is_context_value_enabled() -> bool {
false
}
macro_rules! parse_svg_length {
($ty:ty, $lp:ty) => {
impl Parse for $ty {
fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
if let Ok(lp) = input.try(|i| {
<$lp>::parse_quirky(context, i, AllowQuirks::Always)
}) {
return Ok(generic::SVGLength::LengthPercentage(lp))
}
try_match_ident_ignore_ascii_case! { input,
"context-value" if is_context_value_enabled() => {
Ok(generic::SVGLength::ContextValue)
},
}
}
}
}
}
/// [ <length> | <percentage> | <number> ]# | context-value
pub type SVGStrokeDashArray = generic::SVGStrokeDashArray<NonNegativeSvgLengthPercentageOrNumber>;
parse_svg_length!(SVGLength, LengthPercentage);
parse_svg_length!(SVGWidth, NonNegativeLengthPercentage);
impl Parse for SVGStrokeDashArray {
fn parse<'i, 't>(
@ -61,14 +78,18 @@ impl Parse for SVGStrokeDashArray {
) -> Result<Self, ParseError<'i>> {
if let Ok(values) = input.try(|i| {
CommaWithSpace::parse(i, |i| {
NonNegativeSvgLengthPercentageOrNumber::parse(context, i)
NonNegativeLengthPercentage::parse_quirky(
context,
i,
AllowQuirks::Always,
)
})
}) {
return Ok(generic::SVGStrokeDashArray::Values(values));
}
try_match_ident_ignore_ascii_case! { input,
"context-value" if generic::is_context_value_enabled(context) => {
"context-value" if is_context_value_enabled() => {
Ok(generic::SVGStrokeDashArray::ContextValue)
},
"none" => Ok(generic::SVGStrokeDashArray::Values(vec![])),