style: Add bindings for ArcSlice and ThinArc, and use them to reduce copies of SVG path data.

As I said over bug 1549593, the eventual goal is to use ArcSlice in all
inherited properties. But this seemed like a good first candidate that doesn't
require me to move around a lot more code, since we were already using cbindgen
for the path commands.

Differential Revision: https://phabricator.services.mozilla.com/D30134
This commit is contained in:
Emilio Cobos Álvarez 2019-05-09 12:43:19 +00:00
parent 0d5c4481b8
commit f429c28f23
5 changed files with 39 additions and 41 deletions

View file

@ -308,6 +308,9 @@ impl<T: ?Sized> Arc<T> {
impl<T: ?Sized> Clone for Arc<T> { impl<T: ?Sized> Clone for Arc<T> {
#[inline] #[inline]
fn clone(&self) -> Self { fn clone(&self) -> Self {
// NOTE(emilio): If you change anything here, make sure that the
// implementation in layout/style/ServoStyleConstsInlines.h matches!
//
// Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since
// `count` never changes between STATIC_REFCOUNT and other values. // `count` never changes between STATIC_REFCOUNT and other values.
if self.inner().count.load(Relaxed) != STATIC_REFCOUNT { if self.inner().count.load(Relaxed) != STATIC_REFCOUNT {
@ -416,6 +419,9 @@ impl<T: ?Sized> Arc<T> {
impl<T: ?Sized> Drop for Arc<T> { impl<T: ?Sized> Drop for Arc<T> {
#[inline] #[inline]
fn drop(&mut self) { fn drop(&mut self) {
// NOTE(emilio): If you change anything here, make sure that the
// implementation in layout/style/ServoStyleConstsInlines.h matches!
//
// Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since
// `count` never changes between STATIC_REFCOUNT and other values. // `count` never changes between STATIC_REFCOUNT and other values.
if self.inner().count.load(Relaxed) == STATIC_REFCOUNT { if self.inner().count.load(Relaxed) == STATIC_REFCOUNT {
@ -571,6 +577,7 @@ impl<T: Serialize> Serialize for Arc<T> {
/// Structure to allow Arc-managing some fixed-sized data and a variably-sized /// Structure to allow Arc-managing some fixed-sized data and a variably-sized
/// slice in a single allocation. /// slice in a single allocation.
#[derive(Debug, Eq, PartialEq, PartialOrd)] #[derive(Debug, Eq, PartialEq, PartialOrd)]
#[repr(C)]
pub struct HeaderSlice<H, T: ?Sized> { pub struct HeaderSlice<H, T: ?Sized> {
/// The fixed-sized data. /// The fixed-sized data.
pub header: H, pub header: H,
@ -743,6 +750,7 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
/// Header data with an inline length. Consumers that use HeaderWithLength as the /// Header data with an inline length. Consumers that use HeaderWithLength as the
/// Header type in HeaderSlice can take advantage of ThinArc. /// Header type in HeaderSlice can take advantage of ThinArc.
#[derive(Debug, Eq, PartialEq, PartialOrd)] #[derive(Debug, Eq, PartialEq, PartialOrd)]
#[repr(C)]
pub struct HeaderWithLength<H> { pub struct HeaderWithLength<H> {
/// The fixed-sized data. /// The fixed-sized data.
pub header: H, pub header: H,

View file

@ -577,12 +577,10 @@ pub mod basic_shape {
/// Generate a SVGPathData from StyleShapeSource if possible. /// Generate a SVGPathData from StyleShapeSource if possible.
fn to_svg_path(&self) -> Option<SVGPathData> { fn to_svg_path(&self) -> Option<SVGPathData> {
use crate::values::specified::svg_path::PathCommand;
match self.mType { match self.mType {
StyleShapeSourceType::Path => { StyleShapeSourceType::Path => {
let gecko_path = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }; let gecko_path = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr };
let result: Vec<PathCommand> = gecko_path.mPath.iter().cloned().collect(); Some(SVGPathData(gecko_path.mPath.clone()))
Some(SVGPathData::new(result.into_boxed_slice()))
}, },
_ => None, _ => None,
} }

View file

@ -2901,7 +2901,7 @@ fn static_assert() {
match v { match v {
OffsetPath::None => motion.mOffsetPath.mType = StyleShapeSourceType::None, OffsetPath::None => motion.mOffsetPath.mType = StyleShapeSourceType::None,
OffsetPath::Path(p) => { OffsetPath::Path(p) => {
set_style_svg_path(&mut motion.mOffsetPath, &p, FillRule::Nonzero) set_style_svg_path(&mut motion.mOffsetPath, p, FillRule::Nonzero)
}, },
} }
unsafe { Gecko_SetStyleMotion(&mut self.gecko.mMotion, motion) }; unsafe { Gecko_SetStyleMotion(&mut self.gecko.mMotion, motion) };
@ -4023,25 +4023,17 @@ fn static_assert() {
// Set SVGPathData to StyleShapeSource. // Set SVGPathData to StyleShapeSource.
fn set_style_svg_path( fn set_style_svg_path(
shape_source: &mut structs::mozilla::StyleShapeSource, shape_source: &mut structs::mozilla::StyleShapeSource,
servo_path: &values::specified::svg_path::SVGPathData, servo_path: values::specified::svg_path::SVGPathData,
fill: values::generics::basic_shape::FillRule, fill: values::generics::basic_shape::FillRule,
) { ) {
use crate::gecko_bindings::bindings::Gecko_NewStyleSVGPath;
use crate::gecko_bindings::structs::StyleShapeSourceType;
// Setup type.
shape_source.mType = StyleShapeSourceType::Path;
// Setup path. // Setup path.
let gecko_path = unsafe { unsafe {
Gecko_NewStyleSVGPath(shape_source); bindings::Gecko_SetToSVGPath(
&mut shape_source.__bindgen_anon_1.mSVGPath.as_mut().mPtr.as_mut().unwrap() shape_source,
}; servo_path.0.forget(),
fill,
gecko_path.mPath.assign_from_iter_pod(servo_path.commands().iter().cloned()); );
}
// Setup fill-rule.
gecko_path.mFillRule = fill;
} }
<%def name="impl_shape_source(ident, gecko_ffi_name)"> <%def name="impl_shape_source(ident, gecko_ffi_name)">
@ -4081,7 +4073,7 @@ fn set_style_svg_path(
${ident}.mReferenceBox = reference.into(); ${ident}.mReferenceBox = reference.into();
${ident}.mType = StyleShapeSourceType::Box; ${ident}.mType = StyleShapeSourceType::Box;
} }
ShapeSource::Path(p) => set_style_svg_path(${ident}, &p.path, p.fill), ShapeSource::Path(p) => set_style_svg_path(${ident}, p.path, p.fill),
ShapeSource::Shape(servo_shape, maybe_box) => { ShapeSource::Shape(servo_shape, maybe_box) => {
unsafe { unsafe {
${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr = ${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr =

View file

@ -88,7 +88,6 @@ ${helpers.predefined_type(
"generics::basic_shape::ShapeSource::None", "generics::basic_shape::ShapeSource::None",
products="gecko", products="gecko",
animation_value_type="basic_shape::ClippingShape", animation_value_type="basic_shape::ClippingShape",
boxed=True,
flags="CREATES_STACKING_CONTEXT", flags="CREATES_STACKING_CONTEXT",
spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path", spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path",
)} )}

View file

@ -29,16 +29,15 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
ToResolvedValue, ToResolvedValue,
ToShmem, ToShmem,
)] )]
pub struct SVGPathData(Box<[PathCommand]>); #[repr(C)]
pub struct SVGPathData(
// TODO(emilio): Should probably measure this somehow only from the
// specified values.
#[ignore_malloc_size_of = "Arc"]
pub crate::ArcSlice<PathCommand>
);
impl SVGPathData { impl SVGPathData {
/// Return SVGPathData by a slice of PathCommand.
#[inline]
pub fn new(cmd: Box<[PathCommand]>) -> Self {
debug_assert!(!cmd.is_empty());
SVGPathData(cmd)
}
/// Get the array of PathCommand. /// Get the array of PathCommand.
#[inline] #[inline]
pub fn commands(&self) -> &[PathCommand] { pub fn commands(&self) -> &[PathCommand] {
@ -46,9 +45,9 @@ impl SVGPathData {
&self.0 &self.0
} }
/// Create a normalized copy of this path by converting each relative command to an absolute /// Create a normalized copy of this path by converting each relative
/// command. /// command to an absolute command.
fn normalize(&self) -> Self { fn normalize(&self) -> Box<[PathCommand]> {
let mut state = PathTraversalState { let mut state = PathTraversalState {
subpath_start: CoordPair::new(0.0, 0.0), subpath_start: CoordPair::new(0.0, 0.0),
pos: CoordPair::new(0.0, 0.0), pos: CoordPair::new(0.0, 0.0),
@ -58,7 +57,7 @@ impl SVGPathData {
.iter() .iter()
.map(|seg| seg.normalize(&mut state)) .map(|seg| seg.normalize(&mut state))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
SVGPathData(result.into_boxed_slice()) result.into_boxed_slice()
} }
} }
@ -71,7 +70,7 @@ impl ToCss for SVGPathData {
dest.write_char('"')?; dest.write_char('"')?;
{ {
let mut writer = SequenceWriter::new(dest, " "); let mut writer = SequenceWriter::new(dest, " ");
for command in self.0.iter() { for command in self.commands() {
writer.item(command)?; writer.item(command)?;
} }
} }
@ -104,7 +103,7 @@ impl Parse for SVGPathData {
} }
} }
Ok(SVGPathData::new(path_parser.path.into_boxed_slice())) Ok(SVGPathData(crate::ArcSlice::from_iter(path_parser.path.into_iter())))
} }
} }
@ -114,14 +113,17 @@ impl Animate for SVGPathData {
return Err(()); return Err(());
} }
// FIXME(emilio): This allocates three copies of the path, that's not
// great! Specially, once we're normalized once, we don't need to
// re-normalize again.
let result = self let result = self
.normalize() .normalize()
.0
.iter() .iter()
.zip(other.normalize().0.iter()) .zip(other.normalize().iter())
.map(|(a, b)| a.animate(&b, procedure)) .map(|(a, b)| a.animate(&b, procedure))
.collect::<Result<Vec<_>, _>>()?; .collect::<Result<Vec<_>, _>>()?;
Ok(SVGPathData::new(result.into_boxed_slice()))
Ok(SVGPathData(crate::ArcSlice::from_iter(result.into_iter())))
} }
} }
@ -131,9 +133,8 @@ impl ComputeSquaredDistance for SVGPathData {
return Err(()); return Err(());
} }
self.normalize() self.normalize()
.0
.iter() .iter()
.zip(other.normalize().0.iter()) .zip(other.normalize().iter())
.map(|(this, other)| this.compute_squared_distance(&other)) .map(|(this, other)| this.compute_squared_distance(&other))
.sum() .sum()
} }