Bug 1549593 - Use OwnedSlice in the specified and computed values of most vector properties. r=heycam

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-06 15:56:53 +02:00
parent 78d9c69080
commit 6d9623f982
10 changed files with 101 additions and 108 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_URL;
let mut filters = Vec::new();
for filter in self.gecko.mFilters.iter(){
longhands::filter::computed_value::List(self.gecko.mFilters.iter().map(|filter| {
match filter.mType {
% for func in FILTER_FUNCTIONS:
NS_STYLE_FILTER_${func.upper()} => {
filters.push(Filter::${func}(
Filter::${func}(
GeckoStyleCoordConvertible::from_gecko_style_coord(
&filter.mFilterParameter).unwrap()));
&filter.mFilterParameter
).unwrap()
)
},
% endfor
NS_STYLE_FILTER_BLUR => {
filters.push(Filter::Blur(NonNegativeLength::from_gecko_style_coord(
&filter.mFilterParameter).unwrap()));
Filter::Blur(NonNegativeLength::from_gecko_style_coord(
&filter.mFilterParameter
).unwrap())
},
NS_STYLE_FILTER_HUE_ROTATE => {
filters.push(Filter::HueRotate(
GeckoStyleCoordConvertible::from_gecko_style_coord(
&filter.mFilterParameter).unwrap()));
Filter::HueRotate(GeckoStyleCoordConvertible::from_gecko_style_coord(
&filter.mFilterParameter,
).unwrap())
},
NS_STYLE_FILTER_DROP_SHADOW => {
filters.push(unsafe {
Filter::DropShadow(
(**filter.__bindgen_anon_1.mDropShadow.as_ref()).mArray[0].to_simple_shadow(),
)
});
Filter::DropShadow(unsafe {
(**filter.__bindgen_anon_1.mDropShadow.as_ref()).mArray[0].to_simple_shadow()
})
},
NS_STYLE_FILTER_URL => {
filters.push(Filter::Url(unsafe {
Filter::Url(unsafe {
let url = RefPtr::new(*filter.__bindgen_anon_1.mURL.as_ref());
ComputedUrl::from_url_value(url)
}));
})
}
_ => {},
_ => unreachable!("Unknown filter function?"),
}
}
longhands::filter::computed_value::List(filters)
}).collect())
}
</%self:impl_trait>
@ -3793,7 +3792,8 @@ fn static_assert() {
${impl_keyword('text_align', 'mTextAlign', text_align_keyword)}
pub fn set_text_shadow<I>(&mut self, v: I)
where I: IntoIterator<Item = SimpleShadow>,
where
I: IntoIterator<Item = SimpleShadow>,
I::IntoIter: ExactSizeIterator
{
let v = v.into_iter();
@ -3812,8 +3812,8 @@ fn static_assert() {
}
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();
longhands::text_shadow::computed_value::List(buf)
let buf = self.gecko.mTextShadow.iter().map(|v| v.to_simple_shadow()).collect::<Vec<_>>();
longhands::text_shadow::computed_value::List(buf.into())
}
fn clear_text_emphasis_style_if_string(&mut self) {

View File

@ -113,10 +113,8 @@
pub mod computed_value {
pub use super::single_value::computed_value as single_value;
pub use self::single_value::T as SingleComputedValue;
% if allow_empty and allow_empty != "NotInitial":
use std::vec::IntoIter;
% else:
use smallvec::{IntoIter, SmallVec};
% if not allow_empty or allow_empty == "NotInitial":
use smallvec::SmallVec;
% endif
use crate::values::computed::ComputedVecIter;
@ -144,14 +142,16 @@
#[css(if_empty = "none", iterable)]
% endif
% if allow_empty and allow_empty != "NotInitial":
pub Vec<T>,
pub crate::OwnedSlice<T>,
% 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]>,
% endif
);
/// The computed value, effectively a list of single values.
% if vector_animation_type:
% if not animation_value_type:
Sorry, this is stupid but needed for now.
@ -191,21 +191,10 @@
}
% endif
/// The computed value, effectively a list of single values.
pub type T = List<single_value::T>;
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}.
@ -219,12 +208,12 @@
% else:
#[css(if_empty = "none", iterable)]
% endif
pub Vec<single_value::SpecifiedValue>,
pub crate::OwnedSlice<single_value::SpecifiedValue>,
);
pub fn get_initial_value() -> computed_value::T {
% if allow_empty and allow_empty != "NotInitial":
computed_value::List(vec![])
computed_value::List(Default::default())
% else:
let mut v = SmallVec::new();
v.push(single_value::get_initial_value());
@ -240,19 +229,20 @@
% if allow_empty:
if input.try(|input| input.expect_ident_matching("none")).is_ok() {
return Ok(SpecifiedValue(Vec::new()))
return Ok(SpecifiedValue(Default::default()))
}
% endif
style_traits::${separator}::parse(input, |parser| {
let v = style_traits::${separator}::parse(input, |parser| {
single_value::parse(context, parser)
}).map(SpecifiedValue)
})?;
Ok(SpecifiedValue(v.into()))
}
pub use self::single_value::SpecifiedValue as SingleSpecifiedValue;
impl SpecifiedValue {
pub fn compute_iter<'a, 'cx, 'cx_a>(
fn compute_iter<'a, 'cx, 'cx_a>(
&'a self,
context: &'cx Context<'cx_a>,
) -> computed_value::Iter<'a, 'cx, 'cx_a> {
@ -265,14 +255,13 @@
#[inline]
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]
fn from_computed_value(computed: &computed_value::T) -> Self {
SpecifiedValue(computed.0.iter()
.map(ToComputedValue::from_computed_value)
.collect())
let iter = computed.0.iter().map(ToComputedValue::from_computed_value);
SpecifiedValue(iter.collect())
}
}
</%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 Vec<T>);

View File

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

View File

@ -137,7 +137,7 @@ macro_rules! try_parse_one {
Ok(expanded! {
% 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
})
}
@ -266,7 +266,7 @@ macro_rules! try_parse_one {
Ok(expanded! {
% for prop in props:
animation_${prop}: animation_${prop}::SpecifiedValue(${prop}s),
animation_${prop}: animation_${prop}::SpecifiedValue(${prop}s.into()),
% endfor
})
}

View File

@ -42,11 +42,11 @@
input: &mut Parser<'i, 't>,
) -> Result<Longhands, ParseError<'i>> {
% 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
// with 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 succeeds.
let mut mask_${name} = mask_${name}::SpecifiedValue(Vec::with_capacity(1));
// 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
// overallocate, then shrink. Note that we always push at least one
// item if parsing succeeds.
let mut mask_${name} = Vec::with_capacity(1);
% endfor
input.parse_comma_separated(|input| {
@ -96,17 +96,17 @@
% endfor
if any {
if let Some(position) = position {
mask_position_x.0.push(position.horizontal);
mask_position_y.0.push(position.vertical);
mask_position_x.push(position.horizontal);
mask_position_y.push(position.vertical);
} else {
mask_position_x.0.push(PositionComponent::zero());
mask_position_y.0.push(PositionComponent::zero());
mask_position_x.push(PositionComponent::zero());
mask_position_y.push(PositionComponent::zero());
}
% for name in "image mode size repeat origin clip composite".split():
if let Some(m_${name}) = ${name} {
mask_${name}.0.push(m_${name});
mask_${name}.push(m_${name});
} else {
mask_${name}.0.push(mask_${name}::single_value
mask_${name}.push(mask_${name}::single_value
::get_initial_specified_value());
}
% endfor
@ -118,7 +118,7 @@
Ok(expanded! {
% 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
})
}
@ -209,16 +209,16 @@
) -> Result<Longhands, ParseError<'i>> {
// 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
// overallocate. Note that we always push at least one item if parsing
// succeeds.
let mut position_x = mask_position_x::SpecifiedValue(Vec::with_capacity(1));
let mut position_y = mask_position_y::SpecifiedValue(Vec::with_capacity(1));
// overallocate, then shrink. Note that we always push at least one
// item if parsing succeeds.
let mut position_x = Vec::with_capacity(1);
let mut position_y = Vec::with_capacity(1);
let mut any = false;
input.parse_comma_separated(|input| {
let value = Position::parse(context, input)?;
position_x.0.push(value.horizontal);
position_y.0.push(value.vertical);
position_x.push(value.horizontal);
position_y.push(value.vertical);
any = true;
Ok(())
})?;
@ -227,9 +227,10 @@
return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
}
Ok(expanded! {
mask_position_x: position_x,
mask_position_y: position_y,
mask_position_x: mask_position_x::SpecifiedValue(position_x.into()),
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 {
self.iter()
.map(|item| item.to_computed_value(context))
.collect::<Vec<_>>()
.into()
.collect()
}
#[inline]
@ -463,8 +462,7 @@ where
computed
.iter()
.map(T::from_computed_value)
.collect::<Vec<_>>()
.into()
.collect()
}
}

View File

@ -10,7 +10,7 @@ use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps};
use std::marker::PhantomData;
use std::ops::{Deref, DerefMut};
use std::ptr::NonNull;
use std::{fmt, mem, slice};
use std::{fmt, iter, mem, slice};
use to_shmem::{SharedMemoryBuilder, ToShmem};
/// 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.
use crate::arc_slice::ArcSlice;
use crate::owned_slice::OwnedSlice;
use servo_arc::Arc;
use std::ops::Range;
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!(OwnedSlice<T>);
impl_generic_specified_value_info!(Vec<T>);
impl_generic_specified_value_info!(Arc<T>);
impl_generic_specified_value_info!(StdArc<T>);

View File

@ -4725,7 +4725,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetBackgroundImage(
let url = SpecifiedImageUrl::parse_from_string(string.into(), &context);
let decl = PropertyDeclaration::BackgroundImage(BackgroundImage(vec![Either::Second(
Image::Url(url),
)]));
)].into()));
write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
decls.push(decl, Importance::Normal);
});