style: Use OwnedSlice in the specified and computed values of most vector properties.

This is just a refactor in the right direction. Eventual goal is:

 * All inherited properties use ArcSlice<>.
 * All reset properties use OwnedSlice<> (or ThinVec<>).

No conversion happens at all, so we can remove all that glue, and also
compute_iter and co.

Of course there's work to do, but this is a step towards that.

Differential Revision: https://phabricator.services.mozilla.com/D30127
This commit is contained in:
Emilio Cobos Álvarez 2019-05-16 23:21:37 +00:00
parent b61eb84d96
commit ab8776a144
9 changed files with 100 additions and 107 deletions

View file

@ -3714,42 +3714,41 @@ fn static_assert() {
use crate::gecko_bindings::structs::NS_STYLE_FILTER_DROP_SHADOW; use crate::gecko_bindings::structs::NS_STYLE_FILTER_DROP_SHADOW;
use crate::gecko_bindings::structs::NS_STYLE_FILTER_URL; use crate::gecko_bindings::structs::NS_STYLE_FILTER_URL;
let mut filters = Vec::new(); longhands::filter::computed_value::List(self.gecko.mFilters.iter().map(|filter| {
for filter in self.gecko.mFilters.iter(){
match filter.mType { match filter.mType {
% for func in FILTER_FUNCTIONS: % for func in FILTER_FUNCTIONS:
NS_STYLE_FILTER_${func.upper()} => { NS_STYLE_FILTER_${func.upper()} => {
filters.push(Filter::${func}( Filter::${func}(
GeckoStyleCoordConvertible::from_gecko_style_coord( GeckoStyleCoordConvertible::from_gecko_style_coord(
&filter.mFilterParameter).unwrap())); &filter.mFilterParameter
).unwrap()
)
}, },
% endfor % endfor
NS_STYLE_FILTER_BLUR => { NS_STYLE_FILTER_BLUR => {
filters.push(Filter::Blur(NonNegativeLength::from_gecko_style_coord( Filter::Blur(NonNegativeLength::from_gecko_style_coord(
&filter.mFilterParameter).unwrap())); &filter.mFilterParameter
).unwrap())
}, },
NS_STYLE_FILTER_HUE_ROTATE => { NS_STYLE_FILTER_HUE_ROTATE => {
filters.push(Filter::HueRotate( Filter::HueRotate(GeckoStyleCoordConvertible::from_gecko_style_coord(
GeckoStyleCoordConvertible::from_gecko_style_coord( &filter.mFilterParameter,
&filter.mFilterParameter).unwrap())); ).unwrap())
}, },
NS_STYLE_FILTER_DROP_SHADOW => { NS_STYLE_FILTER_DROP_SHADOW => {
filters.push(unsafe { Filter::DropShadow(unsafe {
Filter::DropShadow( (**filter.__bindgen_anon_1.mDropShadow.as_ref()).mArray[0].to_simple_shadow()
(**filter.__bindgen_anon_1.mDropShadow.as_ref()).mArray[0].to_simple_shadow(), })
)
});
}, },
NS_STYLE_FILTER_URL => { NS_STYLE_FILTER_URL => {
filters.push(Filter::Url(unsafe { Filter::Url(unsafe {
let url = RefPtr::new(*filter.__bindgen_anon_1.mURL.as_ref()); let url = RefPtr::new(*filter.__bindgen_anon_1.mURL.as_ref());
ComputedUrl::from_url_value(url) ComputedUrl::from_url_value(url)
})); })
} }
_ => {}, _ => unreachable!("Unknown filter function?"),
} }
} }).collect())
longhands::filter::computed_value::List(filters)
} }
</%self:impl_trait> </%self:impl_trait>
@ -3793,8 +3792,9 @@ fn static_assert() {
${impl_keyword('text_align', 'mTextAlign', text_align_keyword)} ${impl_keyword('text_align', 'mTextAlign', text_align_keyword)}
pub fn set_text_shadow<I>(&mut self, v: I) pub fn set_text_shadow<I>(&mut self, v: I)
where I: IntoIterator<Item = SimpleShadow>, where
I::IntoIter: ExactSizeIterator I: IntoIterator<Item = SimpleShadow>,
I::IntoIter: ExactSizeIterator
{ {
let v = v.into_iter(); let v = v.into_iter();
self.gecko.mTextShadow.replace_with_new(v.len() as u32); self.gecko.mTextShadow.replace_with_new(v.len() as u32);
@ -3812,8 +3812,8 @@ fn static_assert() {
} }
pub fn clone_text_shadow(&self) -> longhands::text_shadow::computed_value::T { pub fn clone_text_shadow(&self) -> longhands::text_shadow::computed_value::T {
let buf = self.gecko.mTextShadow.iter().map(|v| v.to_simple_shadow()).collect(); let buf = self.gecko.mTextShadow.iter().map(|v| v.to_simple_shadow()).collect::<Vec<_>>();
longhands::text_shadow::computed_value::List(buf) longhands::text_shadow::computed_value::List(buf.into())
} }
fn clear_text_emphasis_style_if_string(&mut self) { fn clear_text_emphasis_style_if_string(&mut self) {

View file

@ -113,10 +113,8 @@
pub mod computed_value { pub mod computed_value {
pub use super::single_value::computed_value as single_value; pub use super::single_value::computed_value as single_value;
pub use self::single_value::T as SingleComputedValue; pub use self::single_value::T as SingleComputedValue;
% if allow_empty and allow_empty != "NotInitial": % if not allow_empty or allow_empty == "NotInitial":
use std::vec::IntoIter; use smallvec::SmallVec;
% else:
use smallvec::{IntoIter, SmallVec};
% endif % endif
use crate::values::computed::ComputedVecIter; use crate::values::computed::ComputedVecIter;
@ -144,14 +142,16 @@
#[css(if_empty = "none", iterable)] #[css(if_empty = "none", iterable)]
% endif % endif
% if allow_empty and allow_empty != "NotInitial": % if allow_empty and allow_empty != "NotInitial":
pub Vec<T>, pub crate::OwnedSlice<T>,
% else: % else:
// FIXME(emilio): Add an OwnedNonEmptySlice type, and figure out
// something for transition-name, which is the only remaining
// user of NotInitial.
pub SmallVec<[T; 1]>, pub SmallVec<[T; 1]>,
% endif % endif
); );
/// The computed value, effectively a list of single values.
% if vector_animation_type: % if vector_animation_type:
% if not animation_value_type: % if not animation_value_type:
Sorry, this is stupid but needed for now. Sorry, this is stupid but needed for now.
@ -191,21 +191,10 @@
} }
% endif % endif
/// The computed value, effectively a list of single values.
pub type T = List<single_value::T>; pub type T = List<single_value::T>;
pub type Iter<'a, 'cx, 'cx_a> = ComputedVecIter<'a, 'cx, 'cx_a, super::single_value::SpecifiedValue>; pub type Iter<'a, 'cx, 'cx_a> = ComputedVecIter<'a, 'cx, 'cx_a, super::single_value::SpecifiedValue>;
impl IntoIterator for T {
type Item = single_value::T;
% if allow_empty and allow_empty != "NotInitial":
type IntoIter = IntoIter<single_value::T>;
% else:
type IntoIter = IntoIter<[single_value::T; 1]>;
% endif
fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
}
}
} }
/// The specified value of ${name}. /// The specified value of ${name}.
@ -219,12 +208,12 @@
% else: % else:
#[css(if_empty = "none", iterable)] #[css(if_empty = "none", iterable)]
% endif % endif
pub Vec<single_value::SpecifiedValue>, pub crate::OwnedSlice<single_value::SpecifiedValue>,
); );
pub fn get_initial_value() -> computed_value::T { pub fn get_initial_value() -> computed_value::T {
% if allow_empty and allow_empty != "NotInitial": % if allow_empty and allow_empty != "NotInitial":
computed_value::List(vec![]) computed_value::List(Default::default())
% else: % else:
let mut v = SmallVec::new(); let mut v = SmallVec::new();
v.push(single_value::get_initial_value()); v.push(single_value::get_initial_value());
@ -239,20 +228,21 @@
use style_traits::Separator; use style_traits::Separator;
% if allow_empty: % if allow_empty:
if input.try(|input| input.expect_ident_matching("none")).is_ok() { if input.try(|input| input.expect_ident_matching("none")).is_ok() {
return Ok(SpecifiedValue(Vec::new())) return Ok(SpecifiedValue(Default::default()))
} }
% endif % endif
style_traits::${separator}::parse(input, |parser| { let v = style_traits::${separator}::parse(input, |parser| {
single_value::parse(context, parser) single_value::parse(context, parser)
}).map(SpecifiedValue) })?;
Ok(SpecifiedValue(v.into()))
} }
pub use self::single_value::SpecifiedValue as SingleSpecifiedValue; pub use self::single_value::SpecifiedValue as SingleSpecifiedValue;
impl SpecifiedValue { impl SpecifiedValue {
pub fn compute_iter<'a, 'cx, 'cx_a>( fn compute_iter<'a, 'cx, 'cx_a>(
&'a self, &'a self,
context: &'cx Context<'cx_a>, context: &'cx Context<'cx_a>,
) -> computed_value::Iter<'a, 'cx, 'cx_a> { ) -> computed_value::Iter<'a, 'cx, 'cx_a> {
@ -265,14 +255,13 @@
#[inline] #[inline]
fn to_computed_value(&self, context: &Context) -> computed_value::T { fn to_computed_value(&self, context: &Context) -> computed_value::T {
computed_value::List(self.compute_iter(context).collect()) computed_value::List(self.0.iter().map(|i| i.to_computed_value(context)).collect())
} }
#[inline] #[inline]
fn from_computed_value(computed: &computed_value::T) -> Self { fn from_computed_value(computed: &computed_value::T) -> Self {
SpecifiedValue(computed.0.iter() let iter = computed.0.iter().map(ToComputedValue::from_computed_value);
.map(ToComputedValue::from_computed_value) SpecifiedValue(iter.collect())
.collect())
} }
} }
</%call> </%call>

View file

@ -773,6 +773,7 @@ macro_rules! animated_list_impl {
} }
} }
animated_list_impl!(<T> for crate::OwnedSlice<T>);
animated_list_impl!(<T> for SmallVec<[T; 1]>); animated_list_impl!(<T> for SmallVec<[T; 1]>);
animated_list_impl!(<T> for Vec<T>); animated_list_impl!(<T> for Vec<T>);

View file

@ -40,11 +40,11 @@
let mut background_color = None; let mut background_color = None;
% for name in "image position_x position_y repeat size attachment origin clip".split(): % for name in "image position_x position_y repeat size attachment origin clip".split():
// Vec grows from 0 to 4 by default on first push(). So allocate // Vec grows from 0 to 4 by default on first push(). So allocate with
// with capacity 1, so in the common case of only one item we don't // capacity 1, so in the common case of only one item we don't way
// way overallocate. Note that we always push at least one item if // overallocate, then shrink. Note that we always push at least one
// parsing succeeds. // item if parsing succeeds.
let mut background_${name} = background_${name}::SpecifiedValue(Vec::with_capacity(1)); let mut background_${name} = Vec::with_capacity(1);
% endfor % endfor
input.parse_comma_separated(|input| { input.parse_comma_separated(|input| {
// background-color can only be in the last element, so if it // background-color can only be in the last element, so if it
@ -99,17 +99,17 @@
any = any || background_color.is_some(); any = any || background_color.is_some();
if any { if any {
if let Some(position) = position { if let Some(position) = position {
background_position_x.0.push(position.horizontal); background_position_x.push(position.horizontal);
background_position_y.0.push(position.vertical); background_position_y.push(position.vertical);
} else { } else {
background_position_x.0.push(PositionComponent::zero()); background_position_x.push(PositionComponent::zero());
background_position_y.0.push(PositionComponent::zero()); background_position_y.push(PositionComponent::zero());
} }
% for name in "image repeat size attachment origin clip".split(): % for name in "image repeat size attachment origin clip".split():
if let Some(bg_${name}) = ${name} { if let Some(bg_${name}) = ${name} {
background_${name}.0.push(bg_${name}); background_${name}.push(bg_${name});
} else { } else {
background_${name}.0.push(background_${name}::single_value background_${name}.push(background_${name}::single_value
::get_initial_specified_value()); ::get_initial_specified_value());
} }
% endfor % endfor
@ -121,14 +121,9 @@
Ok(expanded! { Ok(expanded! {
background_color: background_color.unwrap_or(Color::transparent()), background_color: background_color.unwrap_or(Color::transparent()),
background_image: background_image, % for name in "image position_x position_y repeat size attachment origin clip".split():
background_position_x: background_position_x, background_${name}: background_${name}::SpecifiedValue(background_${name}.into()),
background_position_y: background_position_y, % endfor
background_repeat: background_repeat,
background_attachment: background_attachment,
background_size: background_size,
background_origin: background_origin,
background_clip: background_clip,
}) })
} }
@ -209,16 +204,16 @@
) -> Result<Longhands, ParseError<'i>> { ) -> Result<Longhands, ParseError<'i>> {
// Vec grows from 0 to 4 by default on first push(). So allocate with // Vec grows from 0 to 4 by default on first push(). So allocate with
// capacity 1, so in the common case of only one item we don't way // capacity 1, so in the common case of only one item we don't way
// overallocate. Note that we always push at least one item if parsing // overallocate, then shrink. Note that we always push at least one
// succeeds. // item if parsing succeeds.
let mut position_x = background_position_x::SpecifiedValue(Vec::with_capacity(1)); let mut position_x = Vec::with_capacity(1);
let mut position_y = background_position_y::SpecifiedValue(Vec::with_capacity(1)); let mut position_y = Vec::with_capacity(1);
let mut any = false; let mut any = false;
input.parse_comma_separated(|input| { input.parse_comma_separated(|input| {
let value = Position::parse_quirky(context, input, AllowQuirks::Yes)?; let value = Position::parse_quirky(context, input, AllowQuirks::Yes)?;
position_x.0.push(value.horizontal); position_x.push(value.horizontal);
position_y.0.push(value.vertical); position_y.push(value.vertical);
any = true; any = true;
Ok(()) Ok(())
})?; })?;
@ -227,8 +222,8 @@
} }
Ok(expanded! { Ok(expanded! {
background_position_x: position_x, background_position_x: background_position_x::SpecifiedValue(position_x.into()),
background_position_y: position_y, background_position_y: background_position_y::SpecifiedValue(position_y.into()),
}) })
} }

View file

@ -137,7 +137,7 @@ macro_rules! try_parse_one {
Ok(expanded! { Ok(expanded! {
% for prop in "property duration timing_function delay".split(): % for prop in "property duration timing_function delay".split():
transition_${prop}: transition_${prop}::SpecifiedValue(${prop}s), transition_${prop}: transition_${prop}::SpecifiedValue(${prop}s.into()),
% endfor % endfor
}) })
} }
@ -266,7 +266,7 @@ macro_rules! try_parse_one {
Ok(expanded! { Ok(expanded! {
% for prop in props: % for prop in props:
animation_${prop}: animation_${prop}::SpecifiedValue(${prop}s), animation_${prop}: animation_${prop}::SpecifiedValue(${prop}s.into()),
% endfor % endfor
}) })
} }

View file

@ -42,11 +42,11 @@
input: &mut Parser<'i, 't>, input: &mut Parser<'i, 't>,
) -> Result<Longhands, ParseError<'i>> { ) -> Result<Longhands, ParseError<'i>> {
% for name in "image mode position_x position_y size repeat origin clip composite".split(): % for name in "image mode position_x position_y size repeat origin clip composite".split():
// Vec grows from 0 to 4 by default on first push(). So allocate // Vec grows from 0 to 4 by default on first push(). So allocate with
// with capacity 1, so in the common case of only one item we don't // capacity 1, so in the common case of only one item we don't way
// way overallocate. Note that we always push at least one item if // overallocate, then shrink. Note that we always push at least one
// parsing succeeds. // item if parsing succeeds.
let mut mask_${name} = mask_${name}::SpecifiedValue(Vec::with_capacity(1)); let mut mask_${name} = Vec::with_capacity(1);
% endfor % endfor
input.parse_comma_separated(|input| { input.parse_comma_separated(|input| {
@ -96,17 +96,17 @@
% endfor % endfor
if any { if any {
if let Some(position) = position { if let Some(position) = position {
mask_position_x.0.push(position.horizontal); mask_position_x.push(position.horizontal);
mask_position_y.0.push(position.vertical); mask_position_y.push(position.vertical);
} else { } else {
mask_position_x.0.push(PositionComponent::zero()); mask_position_x.push(PositionComponent::zero());
mask_position_y.0.push(PositionComponent::zero()); mask_position_y.push(PositionComponent::zero());
} }
% for name in "image mode size repeat origin clip composite".split(): % for name in "image mode size repeat origin clip composite".split():
if let Some(m_${name}) = ${name} { if let Some(m_${name}) = ${name} {
mask_${name}.0.push(m_${name}); mask_${name}.push(m_${name});
} else { } else {
mask_${name}.0.push(mask_${name}::single_value mask_${name}.push(mask_${name}::single_value
::get_initial_specified_value()); ::get_initial_specified_value());
} }
% endfor % endfor
@ -118,7 +118,7 @@
Ok(expanded! { Ok(expanded! {
% for name in "image mode position_x position_y size repeat origin clip composite".split(): % for name in "image mode position_x position_y size repeat origin clip composite".split():
mask_${name}: mask_${name}, mask_${name}: mask_${name}::SpecifiedValue(mask_${name}.into()),
% endfor % endfor
}) })
} }
@ -209,16 +209,16 @@
) -> Result<Longhands, ParseError<'i>> { ) -> Result<Longhands, ParseError<'i>> {
// Vec grows from 0 to 4 by default on first push(). So allocate with // Vec grows from 0 to 4 by default on first push(). So allocate with
// capacity 1, so in the common case of only one item we don't way // capacity 1, so in the common case of only one item we don't way
// overallocate. Note that we always push at least one item if parsing // overallocate, then shrink. Note that we always push at least one
// succeeds. // item if parsing succeeds.
let mut position_x = mask_position_x::SpecifiedValue(Vec::with_capacity(1)); let mut position_x = Vec::with_capacity(1);
let mut position_y = mask_position_y::SpecifiedValue(Vec::with_capacity(1)); let mut position_y = Vec::with_capacity(1);
let mut any = false; let mut any = false;
input.parse_comma_separated(|input| { input.parse_comma_separated(|input| {
let value = Position::parse(context, input)?; let value = Position::parse(context, input)?;
position_x.0.push(value.horizontal); position_x.push(value.horizontal);
position_y.0.push(value.vertical); position_y.push(value.vertical);
any = true; any = true;
Ok(()) Ok(())
})?; })?;
@ -227,9 +227,10 @@
return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
} }
Ok(expanded! { Ok(expanded! {
mask_position_x: position_x, mask_position_x: mask_position_x::SpecifiedValue(position_x.into()),
mask_position_y: position_y, mask_position_y: mask_position_y::SpecifiedValue(position_y.into()),
}) })
} }

View file

@ -454,8 +454,7 @@ where
fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
self.iter() self.iter()
.map(|item| item.to_computed_value(context)) .map(|item| item.to_computed_value(context))
.collect::<Vec<_>>() .collect()
.into()
} }
#[inline] #[inline]
@ -463,8 +462,7 @@ where
computed computed
.iter() .iter()
.map(T::from_computed_value) .map(T::from_computed_value)
.collect::<Vec<_>>() .collect()
.into()
} }
} }

View file

@ -10,7 +10,7 @@ use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps};
use std::marker::PhantomData; use std::marker::PhantomData;
use std::ops::{Deref, DerefMut}; use std::ops::{Deref, DerefMut};
use std::ptr::NonNull; use std::ptr::NonNull;
use std::{fmt, mem, slice}; use std::{fmt, iter, mem, slice};
use to_shmem::{SharedMemoryBuilder, ToShmem}; use to_shmem::{SharedMemoryBuilder, ToShmem};
/// A struct that basically replaces a `Box<[T]>`, but which cbindgen can /// A struct that basically replaces a `Box<[T]>`, but which cbindgen can
@ -164,3 +164,10 @@ impl<T: ToShmem + Sized> ToShmem for OwnedSlice<T> {
} }
} }
} }
impl<T> iter::FromIterator<T> for OwnedSlice<T> {
#[inline]
fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
Vec::from_iter(iter).into()
}
}

View file

@ -5,6 +5,7 @@
//! Value information for devtools. //! Value information for devtools.
use crate::arc_slice::ArcSlice; use crate::arc_slice::ArcSlice;
use crate::owned_slice::OwnedSlice;
use servo_arc::Arc; use servo_arc::Arc;
use std::ops::Range; use std::ops::Range;
use std::sync::Arc as StdArc; use std::sync::Arc as StdArc;
@ -114,6 +115,7 @@ macro_rules! impl_generic_specified_value_info {
}; };
} }
impl_generic_specified_value_info!(Option<T>); impl_generic_specified_value_info!(Option<T>);
impl_generic_specified_value_info!(OwnedSlice<T>);
impl_generic_specified_value_info!(Vec<T>); impl_generic_specified_value_info!(Vec<T>);
impl_generic_specified_value_info!(Arc<T>); impl_generic_specified_value_info!(Arc<T>);
impl_generic_specified_value_info!(StdArc<T>); impl_generic_specified_value_info!(StdArc<T>);