Skip to content

Commit

Permalink
style: Use the owned slice type for basic shape polygon coordinates.
Browse files Browse the repository at this point in the history 
This enables destructors for tagged unions in cbindgen, implemented in:

 * mozilla/cbindgen#333

Which allow us to properly generate a destructor for the cbindgen-generated
StyleBasicShape (which now contains an OwnedSlice).

For now, we still use the glue code to go from Box<BasicShape> to
UniquePtr<BasicShape>. But that will change in the future when we generate even
more stuff and remove all the glue.

I could add support for copy-constructor generation to cbindgen for tagged
enums, but I'm not sure if it'll end up being needed, and copy-constructing
unions in C++ is always very tricky.

Differential Revision: https://phabricator.services.mozilla.com/D29769
  • Loading branch information
@emilio
emilio committed May 10, 2019
1 parent 330bccd commit 559235e
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 218 deletions.
78 changes: 3 additions & 75 deletions components/style/gecko/conversions.rs
View file
Expand Up @@ -532,28 +532,17 @@ impl nsStyleImage {

pub mod basic_shape {
//! Conversions from and to CSS shape representations.

use crate::gecko::values::GeckoStyleCoordConvertible;
use crate::gecko_bindings::structs::nsStyleCoord;
use crate::gecko_bindings::structs::{StyleBasicShape, StyleBasicShapeType};
use crate::gecko_bindings::structs::{
StyleGeometryBox, StyleShapeSource, StyleShapeSourceType,
};
use crate::gecko_bindings::sugar::refptr::RefPtr;
use crate::values::computed::basic_shape::{
BasicShape, ClippingShape, FloatAreaShape, ShapeRadius,
BasicShape, ClippingShape, FloatAreaShape,
};
use crate::values::computed::length::LengthPercentage;
use crate::values::computed::motion::OffsetPath;
use crate::values::computed::url::ComputedUrl;
use crate::values::generics::basic_shape::{
BasicShape as GenericBasicShape, InsetRect, Polygon,
};
use crate::values::generics::basic_shape::{Circle, Ellipse, Path, PolygonCoord};
use crate::values::generics::basic_shape::{GeometryBox, ShapeBox, ShapeSource};
use crate::values::generics::rect::Rect;
use crate::values::generics::basic_shape::{Path, GeometryBox, ShapeBox, ShapeSource};
use crate::values::specified::SVGPathData;
use std::borrow::Borrow;

impl StyleShapeSource {
/// Convert StyleShapeSource to ShapeSource except URL and Image
Expand All @@ -569,7 +558,7 @@ pub mod basic_shape {
StyleShapeSourceType::Box => Some(ShapeSource::Box(self.mReferenceBox.into())),
StyleShapeSourceType::Shape => {
let other_shape = unsafe { &*self.__bindgen_anon_1.mBasicShape.as_ref().mPtr };
let shape = other_shape.into();
let shape = Box::new(other_shape.clone());
let reference_box = if self.mReferenceBox == StyleGeometryBox::NoBox {
None
} else {
Expand Down Expand Up @@ -653,67 +642,6 @@ pub mod basic_shape {
}
}

impl<'a> From<&'a StyleBasicShape> for BasicShape {
fn from(other: &'a StyleBasicShape) -> Self {
match other.mType {
StyleBasicShapeType::Inset => {
let t = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[0]);
let r = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[1]);
let b = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[2]);
let l = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[3]);
let round = other.mRadius;
let rect = Rect::new(
t.expect("inset() offset should be a length, percentage, or calc value"),
r.expect("inset() offset should be a length, percentage, or calc value"),
b.expect("inset() offset should be a length, percentage, or calc value"),
l.expect("inset() offset should be a length, percentage, or calc value"),
);
GenericBasicShape::Inset(InsetRect { rect, round })
},
StyleBasicShapeType::Circle => GenericBasicShape::Circle(Circle {
radius: (&other.mCoordinates[0]).into(),
position: other.mPosition,
}),
StyleBasicShapeType::Ellipse => GenericBasicShape::Ellipse(Ellipse {
semiaxis_x: (&other.mCoordinates[0]).into(),
semiaxis_y: (&other.mCoordinates[1]).into(),
position: other.mPosition,
}),
StyleBasicShapeType::Polygon => {
let mut coords = Vec::with_capacity(other.mCoordinates.len() / 2);
for i in 0..(other.mCoordinates.len() / 2) {
let x = 2 * i;
let y = x + 1;
coords.push(PolygonCoord(
LengthPercentage::from_gecko_style_coord(&other.mCoordinates[x])
.expect(
"polygon() coordinate should be a length, percentage, \
or calc value",
),
LengthPercentage::from_gecko_style_coord(&other.mCoordinates[y])
.expect(
"polygon() coordinate should be a length, percentage, \
or calc value",
),
))
}
GenericBasicShape::Polygon(Polygon {
fill: other.mFillRule,
coordinates: coords,
})
},
}
}
}

impl<'a> From<&'a nsStyleCoord> for ShapeRadius {
fn from(other: &'a nsStyleCoord) -> Self {
let other = other.borrow();
ShapeRadius::from_gecko_style_coord(other)
.expect("<shape-radius> should be a length, percentage, calc, or keyword value")
}
}

impl From<ShapeBox> for StyleGeometryBox {
fn from(reference: ShapeBox) -> Self {
use crate::gecko_bindings::structs::StyleGeometryBox::*;
Expand Down
33 changes: 1 addition & 32 deletions components/style/gecko/values.rs
View file
Expand Up @@ -8,12 +8,10 @@

use crate::counter_style::{Symbol, Symbols};
use crate::gecko_bindings::structs::{nsStyleCoord, CounterStylePtr};
use crate::gecko_bindings::structs::{StyleGridTrackBreadth, StyleShapeRadius};
use crate::gecko_bindings::structs::StyleGridTrackBreadth;
use crate::gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue};
use crate::values::computed::basic_shape::ShapeRadius as ComputedShapeRadius;
use crate::values::computed::{Angle, Length, LengthPercentage};
use crate::values::computed::{Number, NumberOrPercentage, Percentage};
use crate::values::generics::basic_shape::ShapeRadius;
use crate::values::generics::gecko::ScrollSnapPoint;
use crate::values::generics::grid::{TrackBreadth, TrackKeyword};
use crate::values::generics::length::LengthPercentageOrAuto;
Expand Down Expand Up @@ -192,35 +190,6 @@ impl<L: GeckoStyleCoordConvertible> GeckoStyleCoordConvertible for TrackBreadth<
}
}

impl GeckoStyleCoordConvertible for ComputedShapeRadius {
fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
match *self {
ShapeRadius::ClosestSide => coord.set_value(CoordDataValue::Enumerated(
StyleShapeRadius::ClosestSide as u32,
)),
ShapeRadius::FarthestSide => coord.set_value(CoordDataValue::Enumerated(
StyleShapeRadius::FarthestSide as u32,
)),
ShapeRadius::Length(lp) => lp.to_gecko_style_coord(coord),
}
}

fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self> {
match coord.as_value() {
CoordDataValue::Enumerated(v) => {
if v == StyleShapeRadius::ClosestSide as u32 {
Some(ShapeRadius::ClosestSide)
} else if v == StyleShapeRadius::FarthestSide as u32 {
Some(ShapeRadius::FarthestSide)
} else {
None
}
},
_ => GeckoStyleCoordConvertible::from_gecko_style_coord(coord).map(ShapeRadius::Length),
}
}
}

impl<T: GeckoStyleCoordConvertible> GeckoStyleCoordConvertible for Option<T> {
fn to_gecko_style_coord<U: CoordDataMut>(&self, coord: &mut U) {
if let Some(ref me) = *self {
Expand Down
3 changes: 3 additions & 0 deletions components/style/owned_slice.rs
View file
Expand Up @@ -20,6 +20,9 @@ use to_shmem::{SharedMemoryBuilder, ToShmem};
///
/// But handling fat pointers with cbindgen both in structs and argument
/// positions more generally is a bit tricky.
///
/// cbindgen:derive-eq=false
/// cbindgen:derive-neq=false
#[repr(C)]
pub struct OwnedSlice<T: Sized> {
ptr: NonNull<T>,
Expand Down
83 changes: 11 additions & 72 deletions components/style/properties/gecko.mako.rs
View file
Expand Up @@ -4046,16 +4046,15 @@ fn set_style_svg_path(

<%def name="impl_shape_source(ident, gecko_ffi_name)">
pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
use crate::gecko_bindings::bindings::{Gecko_NewBasicShape, Gecko_DestroyShapeSource};
use crate::gecko_bindings::structs::{StyleBasicShape, StyleBasicShapeType, StyleShapeSourceType};
use crate::gecko_bindings::structs::{StyleGeometryBox, StyleShapeSource};
use crate::gecko::values::GeckoStyleCoordConvertible;
use crate::values::generics::basic_shape::{BasicShape, ShapeSource};
use crate::values::generics::basic_shape::ShapeSource;
use crate::gecko_bindings::structs::StyleShapeSourceType;
use crate::gecko_bindings::structs::StyleGeometryBox;

let ref mut ${ident} = self.gecko.${gecko_ffi_name};

// clean up existing struct
unsafe { Gecko_DestroyShapeSource(${ident}) };
// clean up existing struct.
unsafe { bindings::Gecko_DestroyShapeSource(${ident}) };

${ident}.mType = StyleShapeSourceType::None;

match v {
Expand Down Expand Up @@ -4084,72 +4083,12 @@ fn set_style_svg_path(
}
ShapeSource::Path(p) => set_style_svg_path(${ident}, &p.path, p.fill),
ShapeSource::Shape(servo_shape, maybe_box) => {
fn init_shape(${ident}: &mut StyleShapeSource, basic_shape_type: StyleBasicShapeType)
-> &mut StyleBasicShape {
unsafe {
// Create StyleBasicShape in StyleShapeSource. mReferenceBox and mType
// will be set manually later.
Gecko_NewBasicShape(${ident}, basic_shape_type);
&mut *${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr
}
}
match servo_shape {
BasicShape::Inset(inset) => {
let shape = init_shape(${ident}, StyleBasicShapeType::Inset);
unsafe { shape.mCoordinates.set_len(4) };

// set_len() can't call constructors, so the coordinates
// can contain any value. set_value() attempts to free
// allocated coordinates, so we don't want to feed it
// garbage values which it may misinterpret.
// Instead, we use leaky_set_value to blindly overwrite
// the garbage data without
// attempting to clean up.
shape.mCoordinates[0].leaky_set_null();
inset.rect.0.to_gecko_style_coord(&mut shape.mCoordinates[0]);
shape.mCoordinates[1].leaky_set_null();
inset.rect.1.to_gecko_style_coord(&mut shape.mCoordinates[1]);
shape.mCoordinates[2].leaky_set_null();
inset.rect.2.to_gecko_style_coord(&mut shape.mCoordinates[2]);
shape.mCoordinates[3].leaky_set_null();
inset.rect.3.to_gecko_style_coord(&mut shape.mCoordinates[3]);
shape.mRadius = inset.round;
}
BasicShape::Circle(circ) => {
let shape = init_shape(${ident}, StyleBasicShapeType::Circle);
unsafe { shape.mCoordinates.set_len(1) };
shape.mCoordinates[0].leaky_set_null();
circ.radius.to_gecko_style_coord(&mut shape.mCoordinates[0]);

shape.mPosition = circ.position.into();
}
BasicShape::Ellipse(el) => {
let shape = init_shape(${ident}, StyleBasicShapeType::Ellipse);
unsafe { shape.mCoordinates.set_len(2) };
shape.mCoordinates[0].leaky_set_null();
el.semiaxis_x.to_gecko_style_coord(&mut shape.mCoordinates[0]);
shape.mCoordinates[1].leaky_set_null();
el.semiaxis_y.to_gecko_style_coord(&mut shape.mCoordinates[1]);

shape.mPosition = el.position.into();
}
BasicShape::Polygon(poly) => {
let shape = init_shape(${ident}, StyleBasicShapeType::Polygon);
unsafe {
shape.mCoordinates.set_len(poly.coordinates.len() as u32 * 2);
}
for (i, coord) in poly.coordinates.iter().enumerate() {
shape.mCoordinates[2 * i].leaky_set_null();
shape.mCoordinates[2 * i + 1].leaky_set_null();
coord.0.to_gecko_style_coord(&mut shape.mCoordinates[2 * i]);
coord.1.to_gecko_style_coord(&mut shape.mCoordinates[2 * i + 1]);
}
shape.mFillRule = poly.fill;
}
unsafe {
${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr =
Box::into_raw(servo_shape);
}

${ident}.mReferenceBox = maybe_box.map(Into::into)
.unwrap_or(StyleGeometryBox::NoBox);
${ident}.mReferenceBox =
maybe_box.map(Into::into).unwrap_or(StyleGeometryBox::NoBox);
${ident}.mType = StyleShapeSourceType::Shape;
}
}
Expand Down
2 changes: 1 addition & 1 deletion components/style/properties/longhands/svg.mako.rs
View file
Expand Up @@ -87,8 +87,8 @@ ${helpers.predefined_type(
"basic_shape::ClippingShape",
"generics::basic_shape::ShapeSource::None",
products="gecko",
boxed=True,
animation_value_type="basic_shape::ClippingShape",
boxed=True,
flags="CREATES_STACKING_CONTEXT",
spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path",
)}
Expand Down
31 changes: 21 additions & 10 deletions components/style/values/animated/mod.rs
View file
Expand Up @@ -16,7 +16,6 @@ use crate::values::computed::Image;
use crate::values::specified::SVGPathData;
use crate::values::CSSFloat;
use app_units::Au;
use euclid::Point2D;
use smallvec::SmallVec;
use std::cmp;

Expand Down Expand Up @@ -241,16 +240,10 @@ impl Animate for Au {
}
}

impl<T> Animate for Point2D<T>
where
T: Animate,
{
impl<T: Animate> Animate for Box<T> {
#[inline]
fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
Ok(Point2D::new(
self.x.animate(&other.x, procedure)?,
self.y.animate(&other.y, procedure)?,
))
Ok(Box::new((**self).animate(&other, procedure)?))
}
}

Expand Down Expand Up @@ -288,6 +281,24 @@ where
}
}

impl<T> ToAnimatedValue for Box<T>
where
T: ToAnimatedValue,
{
type AnimatedValue = Box<<T as ToAnimatedValue>::AnimatedValue>;

#[inline]
fn to_animated_value(self) -> Self::AnimatedValue {
Box::new((*self).to_animated_value())
}

#[inline]
fn from_animated_value(animated: Self::AnimatedValue) -> Self {
Box::new(T::from_animated_value(*animated))
}
}


impl<T> ToAnimatedValue for Box<[T]>
where
T: ToAnimatedValue,
Expand Down Expand Up @@ -328,7 +339,7 @@ where

#[inline]
fn from_animated_value(animated: Self::AnimatedValue) -> Self {
Box::from_animated_value(animated.into_box()).into()
Self::from(Box::from_animated_value(animated.into_box()))
}
}

Expand Down
4 changes: 2 additions & 2 deletions components/style/values/computed/basic_shape.rs
View file
Expand Up @@ -23,7 +23,7 @@ pub type ClippingShape = generic::ClippingShape<BasicShape, ComputedUrl>;
pub type FloatAreaShape = generic::FloatAreaShape<BasicShape, Image>;

/// A computed basic shape.
pub type BasicShape = generic::BasicShape<
pub type BasicShape = generic::GenericBasicShape<
LengthPercentage,
LengthPercentage,
LengthPercentage,
Expand All @@ -41,7 +41,7 @@ pub type Ellipse =
generic::Ellipse<LengthPercentage, LengthPercentage, NonNegativeLengthPercentage>;

/// The computed value of `ShapeRadius`
pub type ShapeRadius = generic::ShapeRadius<NonNegativeLengthPercentage>;
pub type ShapeRadius = generic::GenericShapeRadius<NonNegativeLengthPercentage>;

impl ToCss for Circle {
fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
Expand Down

0 comments on commit 559235e

Please sign in to comment.