servo: Merge #17546 - Script paint worklets arguments (from asajeffrey:script-paint-worklets-arguments); r=emilio
authorAlan Jeffrey <ajeffrey@mozilla.com>
Sat, 29 Jul 2017 15:32:49 -0500
changeset 420544 bf1af6cd743df11f6e567c1f22f66e5d34ec82de
parent 420543 24dea787604ee66bc213977def7de4a62e4a602d
child 420545 33a4c8cd16c566c2b6347f293e5984954abfa007
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs17546, 17435
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #17546 - Script paint worklets arguments (from asajeffrey:script-paint-worklets-arguments); r=emilio <!-- Please describe your changes on the following line: --> Implement paint worklet arguments. This is a dependent PR, only the last commit is in this PR. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #17435 - [X] These changes do not require tests because there are tests in the most recent wpt. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 097e36beb1fc34f021044a19f029eed8780fa87f
servo/components/canvas/canvas_paint_thread.rs
servo/components/layout/display_list_builder.rs
servo/components/script/dom/paintworkletglobalscope.rs
servo/components/script_traits/lib.rs
servo/components/style/values/generics/image.rs
servo/components/style/values/specified/image.rs
--- a/servo/components/canvas/canvas_paint_thread.rs
+++ b/servo/components/canvas/canvas_paint_thread.rs
@@ -543,20 +543,25 @@ impl<'a> CanvasPaintThread<'a> {
         self.state.draw_options.set_composition_op(op.to_azure_style());
     }
 
     fn create(size: Size2D<i32>) -> DrawTarget {
         DrawTarget::new(BackendType::Skia, size, SurfaceFormat::B8G8R8A8)
     }
 
     fn recreate(&mut self, size: Size2D<i32>) {
+        // TODO: clear the thread state. https://github.com/servo/servo/issues/17533
         self.drawtarget = CanvasPaintThread::create(size);
         self.state = CanvasPaintState::new(self.state.draw_options.antialias);
         self.saved_states.clear();
         // Webrender doesn't let images change size, so we clear the webrender image key.
+        // TODO: there is an annying race condition here: the display list builder
+        // might still be using the old image key. Really, we should be scheduling the image
+        // for later deletion, not deleting it immediately.
+        // https://github.com/servo/servo/issues/17534
         if let Some(image_key) = self.image_key.take() {
             // If this executes, then we are in a new epoch since we last recreated the canvas,
             // so `old_image_key` must be `None`.
             debug_assert!(self.old_image_key.is_none());
             self.old_image_key = Some(image_key);
         }
     }
 
@@ -577,23 +582,25 @@ impl<'a> CanvasPaintThread<'a> {
                 format: webrender_api::ImageFormat::BGRA8,
                 offset: 0,
                 is_opaque: false,
             };
             let data = webrender_api::ImageData::Raw(Arc::new(element.into()));
 
             match self.image_key {
                 Some(image_key) => {
+                    debug!("Updating image {:?}.", image_key);
                     self.webrender_api.update_image(image_key,
                                                     descriptor,
                                                     data,
                                                     None);
                 }
                 None => {
                     self.image_key = Some(self.webrender_api.generate_image_key());
+                    debug!("New image {:?}.", self.image_key);
                     self.webrender_api.add_image(self.image_key.unwrap(),
                                                  descriptor,
                                                  data,
                                                  None);
                 }
             }
 
             if let Some(image_key) = mem::replace(&mut self.very_old_image_key, self.old_image_key.take()) {
--- a/servo/components/layout/display_list_builder.rs
+++ b/servo/components/layout/display_list_builder.rs
@@ -63,16 +63,17 @@ use style::values::computed::image::{End
 use style::values::generics::background::BackgroundSize;
 use style::values::generics::effects::Filter;
 use style::values::generics::image::{Circle, Ellipse, EndingShape as GenericEndingShape};
 use style::values::generics::image::{GradientItem as GenericGradientItem, GradientKind};
 use style::values::generics::image::{Image, ShapeExtent};
 use style::values::generics::image::PaintWorklet;
 use style::values::specified::position::{X, Y};
 use style_traits::CSSPixel;
+use style_traits::ToCss;
 use style_traits::cursor::Cursor;
 use table_cell::CollapsedBordersForCell;
 use webrender_api::{ClipId, ColorF, ComplexClipRegion, GradientStop, LocalClip, RepeatMode};
 use webrender_api::{LineStyle, ScrollPolicy, ScrollSensitivity, TransformStyle};
 use webrender_helpers::{ToBorderRadius, ToMixBlendMode, ToRectF, ToTransformStyle};
 
 trait ResolvePercentage {
     fn resolve(&self, length: u32) -> u32;
@@ -1168,35 +1169,41 @@ impl FragmentDisplayListBuilding for Fra
         // https://drafts.csswg.org/css-images-3/#concrete-object-size
         // Experimentally, chrome is using the size in px of the box,
         // including padding, but not border or margin, so we follow suit.
         // https://github.com/w3c/css-houdini-drafts/issues/417
         let unbordered_box = self.border_box - style.logical_border_width();
         let device_pixel_ratio = state.layout_context.style_context.device_pixel_ratio();
         let size_in_au = unbordered_box.size.to_physical(style.writing_mode);
         let size_in_px = TypedSize2D::new(size_in_au.width.to_f32_px(), size_in_au.height.to_f32_px());
+
+        // TODO: less copying.
         let name = paint_worklet.name.clone();
+        let arguments = paint_worklet.arguments.iter()
+            .map(|argument| argument.to_css_string())
+            .collect();
 
         // Get the painter, and the computed values for its properties.
+        // TODO: less copying.
         let (properties, painter) = match state.layout_context.registered_painters.read().get(&name) {
             Some(registered_painter) => (
                 registered_painter.properties
                     .iter()
                     .filter_map(|(name, id)| id.as_shorthand().err().map(|id| (name, id)))
                     .map(|(name, id)| (name.clone(), style.computed_value_to_string(id)))
                     .collect(),
                 registered_painter.painter.clone()
             ),
             None => return debug!("Worklet {} called before registration.", name),
         };
 
         // TODO: add a one-place cache to avoid drawing the paint image every time.
         // https://github.com/servo/servo/issues/17369
         debug!("Drawing a paint image {}({},{}).", name, size_in_px.width, size_in_px.height);
-        let mut draw_result = painter.draw_a_paint_image(size_in_px, device_pixel_ratio, properties);
+        let mut draw_result = painter.draw_a_paint_image(size_in_px, device_pixel_ratio, properties, arguments);
         let webrender_image = WebRenderImageInfo {
             width: draw_result.width,
             height: draw_result.height,
             format: draw_result.format,
             key: draw_result.image_key,
         };
 
         for url in draw_result.missing_image_urls.drain(..) {
--- a/servo/components/script/dom/paintworkletglobalscope.rs
+++ b/servo/components/script/dom/paintworkletglobalscope.rs
@@ -12,16 +12,17 @@ use dom::bindings::conversions::get_prop
 use dom::bindings::conversions::get_property_jsval;
 use dom::bindings::error::Error;
 use dom::bindings::error::Fallible;
 use dom::bindings::inheritance::Castable;
 use dom::bindings::js::JS;
 use dom::bindings::js::Root;
 use dom::bindings::reflector::DomObject;
 use dom::bindings::str::DOMString;
+use dom::cssstylevalue::CSSStyleValue;
 use dom::paintrenderingcontext2d::PaintRenderingContext2D;
 use dom::paintsize::PaintSize;
 use dom::stylepropertymapreadonly::StylePropertyMapReadOnly;
 use dom::worklet::WorkletExecutor;
 use dom::workletglobalscope::WorkletGlobalScope;
 use dom::workletglobalscope::WorkletGlobalScopeInit;
 use dom::workletglobalscope::WorkletTask;
 use dom_struct::dom_struct;
@@ -33,16 +34,17 @@ use js::jsapi::Construct1;
 use js::jsapi::HandleValue;
 use js::jsapi::HandleValueArray;
 use js::jsapi::Heap;
 use js::jsapi::IsCallable;
 use js::jsapi::IsConstructor;
 use js::jsapi::JSAutoCompartment;
 use js::jsapi::JS_ClearPendingException;
 use js::jsapi::JS_IsExceptionPending;
+use js::jsapi::JS_NewArrayObject;
 use js::jsval::JSVal;
 use js::jsval::ObjectValue;
 use js::jsval::UndefinedValue;
 use js::rust::Runtime;
 use msg::constellation_msg::PipelineId;
 use net_traits::image::base::PixelFormat;
 use net_traits::image_cache::ImageCache;
 use script_layout_interface::message::Msg;
@@ -95,47 +97,53 @@ impl PaintWorkletGlobalScope {
     }
 
     pub fn image_cache(&self) -> Arc<ImageCache> {
         self.image_cache.clone()
     }
 
     pub fn perform_a_worklet_task(&self, task: PaintWorkletTask) {
         match task {
-            PaintWorkletTask::DrawAPaintImage(name, size_in_px, device_pixel_ratio, properties, sender) => {
+            PaintWorkletTask::DrawAPaintImage(name, size_in_px, device_pixel_ratio, properties, arguments, sender) => {
                 let properties = StylePropertyMapReadOnly::from_iter(self.upcast(), properties);
-                let result = self.draw_a_paint_image(name, size_in_px, device_pixel_ratio, &*properties);
+                let result = self.draw_a_paint_image(name, size_in_px, device_pixel_ratio, &*properties, arguments);
                 let _ = sender.send(result);
             }
         }
     }
 
     /// https://drafts.css-houdini.org/css-paint-api/#draw-a-paint-image
     fn draw_a_paint_image(&self,
                           name: Atom,
                           size_in_px: TypedSize2D<f32, CSSPixel>,
                           device_pixel_ratio: ScaleFactor<f32, CSSPixel, DevicePixel>,
-                          properties: &StylePropertyMapReadOnly)
+                          properties: &StylePropertyMapReadOnly,
+                          arguments: Vec<String>)
                           -> DrawAPaintImageResult
     {
+        let size_in_dpx = size_in_px * device_pixel_ratio;
+        let size_in_dpx = TypedSize2D::new(size_in_dpx.width.abs() as u32, size_in_dpx.height.abs() as u32);
+
+        // TODO: Steps 1-5.
+
         // TODO: document paint definitions.
-        self.invoke_a_paint_callback(name, size_in_px, device_pixel_ratio, properties)
+        self.invoke_a_paint_callback(name, size_in_px, size_in_dpx, device_pixel_ratio, properties, arguments)
     }
 
     /// https://drafts.css-houdini.org/css-paint-api/#invoke-a-paint-callback
     #[allow(unsafe_code)]
     fn invoke_a_paint_callback(&self,
                                name: Atom,
                                size_in_px: TypedSize2D<f32, CSSPixel>,
+                               size_in_dpx: TypedSize2D<u32, DevicePixel>,
                                device_pixel_ratio: ScaleFactor<f32, CSSPixel, DevicePixel>,
-                               properties: &StylePropertyMapReadOnly)
+                               properties: &StylePropertyMapReadOnly,
+                               mut arguments: Vec<String>)
                                -> DrawAPaintImageResult
     {
-        let size_in_dpx = size_in_px * device_pixel_ratio;
-        let size_in_dpx = TypedSize2D::new(size_in_dpx.width.abs() as u32, size_in_dpx.height.abs() as u32);
         debug!("Invoking a paint callback {}({},{}) at {}.",
                name, size_in_px.width, size_in_px.height, device_pixel_ratio);
 
         let cx = self.worklet_global.get_cx();
         let _ac = JSAutoCompartment::new(cx, self.worklet_global.reflector().get_jsobject().get());
 
         // TODO: Steps 1-2.1.
         // Step 2.2-5.1.
@@ -193,20 +201,29 @@ impl PaintWorkletGlobalScope {
         rendering_context.set_bitmap_dimensions(size_in_px, device_pixel_ratio);
 
         // Step 9
         let paint_size = PaintSize::new(self, size_in_px);
 
         // TODO: Step 10
         // Steps 11-12
         debug!("Invoking paint function {}.", name);
+        rooted_vec!(let arguments_values <- arguments.drain(..)
+                    .map(|argument| CSSStyleValue::new(self.upcast(), argument)));
+        let arguments_value_vec: Vec<JSVal> = arguments_values.iter()
+            .map(|argument| ObjectValue(argument.reflector().get_jsobject().get()))
+            .collect();
+        let arguments_value_array = unsafe { HandleValueArray::from_rooted_slice(&*arguments_value_vec) };
+        rooted!(in(cx) let argument_object = unsafe { JS_NewArrayObject(cx, &arguments_value_array) });
+
         let args_slice = [
             ObjectValue(rendering_context.reflector().get_jsobject().get()),
             ObjectValue(paint_size.reflector().get_jsobject().get()),
             ObjectValue(properties.reflector().get_jsobject().get()),
+            ObjectValue(argument_object.get()),
         ];
         let args = unsafe { HandleValueArray::from_rooted_slice(&args_slice) };
 
         rooted!(in(cx) let mut result = UndefinedValue());
         unsafe { Call(cx, paint_instance.handle(), paint_function.handle(), &args, result.handle_mut()); }
         let missing_image_urls = rendering_context.take_missing_image_urls();
 
         // Step 13.
@@ -247,22 +264,27 @@ impl PaintWorkletGlobalScope {
 
     fn painter(&self, name: Atom) -> Arc<Painter> {
         // Rather annoyingly we have to use a mutex here to make the painter Sync.
         struct WorkletPainter(Atom, Mutex<WorkletExecutor>);
         impl Painter for WorkletPainter {
             fn draw_a_paint_image(&self,
                                   size: TypedSize2D<f32, CSSPixel>,
                                   device_pixel_ratio: ScaleFactor<f32, CSSPixel, DevicePixel>,
-                                  properties: Vec<(Atom, String)>)
-                                  -> DrawAPaintImageResult
-            {
+                                  properties: Vec<(Atom, String)>,
+                                  arguments: Vec<String>)
+                                  -> DrawAPaintImageResult {
                 let name = self.0.clone();
                 let (sender, receiver) = mpsc::channel();
-                let task = PaintWorkletTask::DrawAPaintImage(name, size, device_pixel_ratio, properties, sender);
+                let task = PaintWorkletTask::DrawAPaintImage(name,
+                                                             size,
+                                                             device_pixel_ratio,
+                                                             properties,
+                                                             arguments,
+                                                             sender);
                 self.1.lock().expect("Locking a painter.")
                     .schedule_a_worklet_task(WorkletTask::Paint(task));
                 receiver.recv().expect("Worklet thread died?")
             }
         }
         Arc::new(WorkletPainter(name, Mutex::new(self.worklet_global.executor())))
     }
 }
@@ -291,17 +313,17 @@ impl PaintWorkletGlobalScopeMethods for 
 
         // Step 4-6.
         let mut property_names: Vec<String> =
             unsafe { get_property(cx, paint_obj.handle(), "inputProperties", ()) }?
             .unwrap_or_default();
         let properties = property_names.drain(..).map(Atom::from).collect();
 
         // Step 7-9.
-        let _argument_names: Vec<String> =
+        let input_arguments: Vec<String> =
             unsafe { get_property(cx, paint_obj.handle(), "inputArguments", ()) }?
             .unwrap_or_default();
 
         // TODO: Steps 10-11.
 
         // Steps 12-13.
         let alpha: bool =
             unsafe { get_property(cx, paint_obj.handle(), "alpha", ()) }?
@@ -327,16 +349,17 @@ impl PaintWorkletGlobalScopeMethods for 
             return Err(Error::Type(String::from("Paint function is not callable.")));
         }
 
         // Step 19.
         let context = PaintRenderingContext2D::new(self);
         let definition = PaintDefinition::new(paint_val.handle(),
                                               paint_function.handle(),
                                               alpha,
+                                              input_arguments.len(),
                                               &*context);
 
         // Step 20.
         debug!("Registering definition {}.", name);
         self.paint_definitions.borrow_mut().insert(name.clone(), definition);
 
         // TODO: Step 21.
 
@@ -351,47 +374,52 @@ impl PaintWorkletGlobalScopeMethods for 
 }
 
 /// Tasks which can be peformed by a paint worklet
 pub enum PaintWorkletTask {
     DrawAPaintImage(Atom,
                     TypedSize2D<f32, CSSPixel>,
                     ScaleFactor<f32, CSSPixel, DevicePixel>,
                     Vec<(Atom, String)>,
+                    Vec<String>,
                     Sender<DrawAPaintImageResult>)
 }
 
 /// A paint definition
 /// https://drafts.css-houdini.org/css-paint-api/#paint-definition
 /// This type is dangerous, because it contains uboxed `Heap<JSVal>` values,
 /// which can't be moved.
 #[derive(JSTraceable, HeapSizeOf)]
 #[must_root]
 struct PaintDefinition {
     class_constructor: Heap<JSVal>,
     paint_function: Heap<JSVal>,
     constructor_valid_flag: Cell<bool>,
     context_alpha_flag: bool,
+    // TODO: this should be a list of CSS syntaxes.
+    input_arguments_len: usize,
     // TODO: the spec calls for fresh rendering contexts each time a paint image is drawn,
     // but to avoid having the primary worklet thread create a new renering context,
     // we recycle them.
     context: JS<PaintRenderingContext2D>,
 }
 
 impl PaintDefinition {
     fn new(class_constructor: HandleValue,
            paint_function: HandleValue,
            alpha: bool,
+           input_arguments_len: usize,
            context: &PaintRenderingContext2D)
            -> Box<PaintDefinition>
     {
         let result = Box::new(PaintDefinition {
             class_constructor: Heap::default(),
             paint_function: Heap::default(),
             constructor_valid_flag: Cell::new(true),
             context_alpha_flag: alpha,
+            input_arguments_len: input_arguments_len,
             context: JS::from_ref(context),
         });
         result.class_constructor.set(class_constructor.get());
         result.paint_function.set(paint_function.get());
         result
     }
 }
--- a/servo/components/script_traits/lib.rs
+++ b/servo/components/script_traits/lib.rs
@@ -817,17 +817,18 @@ impl From<RecvTimeoutError> for PaintWor
 }
 
 /// Execute paint code in the worklet thread pool.
 pub trait Painter: Sync + Send {
     /// https://drafts.css-houdini.org/css-paint-api/#draw-a-paint-image
     fn draw_a_paint_image(&self,
                           size: TypedSize2D<f32, CSSPixel>,
                           zoom: ScaleFactor<f32, CSSPixel, DevicePixel>,
-                          properties: Vec<(Atom, String)>)
+                          properties: Vec<(Atom, String)>,
+                          arguments: Vec<String>)
                           -> DrawAPaintImageResult;
 }
 
 /// The result of executing paint code: the image together with any image URLs that need to be loaded.
 /// TODO: this should return a WR display list. https://github.com/servo/servo/issues/17497
 #[derive(Debug, Deserialize, Serialize, Clone)]
 pub struct DrawAPaintImageResult {
     /// The image height
--- a/servo/components/style/values/generics/image.rs
+++ b/servo/components/style/values/generics/image.rs
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Generic types for the handling of [images].
 //!
 //! [images]: https://drafts.csswg.org/css-images/#image-values
 
 use Atom;
 use cssparser::serialize_identifier;
+use custom_properties::SpecifiedValue;
 use std::fmt;
 use style_traits::{HasViewportPercentage, ToCss};
 use values::computed::ComputedValueAsSpecified;
 use values::specified::url::SpecifiedUrl;
 
 /// An [image].
 ///
 /// [image]: https://drafts.csswg.org/css-images/#image-values
@@ -131,27 +132,36 @@ pub struct ColorStop<Color, LengthOrPerc
     /// The color of this stop.
     pub color: Color,
     /// The position of this stop.
     pub position: Option<LengthOrPercentage>,
 }
 
 /// Specified values for a paint worklet.
 /// https://drafts.css-houdini.org/css-paint-api/
-#[derive(Clone, Debug, PartialEq, ToComputedValue)]
+#[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct PaintWorklet {
     /// The name the worklet was registered with.
     pub name: Atom,
+    /// The arguments for the worklet.
+    /// TODO: store a parsed representation of the arguments.
+    pub arguments: Vec<SpecifiedValue>,
 }
 
+impl ComputedValueAsSpecified for PaintWorklet {}
+
 impl ToCss for PaintWorklet {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         dest.write_str("paint(")?;
         serialize_identifier(&*self.name.to_string(), dest)?;
+        for argument in &self.arguments {
+            dest.write_str(", ")?;
+            argument.to_css(dest)?;
+        }
         dest.write_str(")")
     }
 }
 
 /// Values for `moz-image-rect`.
 ///
 /// `-moz-image-rect(<uri>, top, right, bottom, left);`
 #[allow(missing_docs)]
--- a/servo/components/style/values/specified/image.rs
+++ b/servo/components/style/values/specified/image.rs
@@ -4,16 +4,17 @@
 
 //! CSS handling for the specified value of
 //! [`image`][image]s
 //!
 //! [image]: https://drafts.csswg.org/css-images/#image-values
 
 use Atom;
 use cssparser::{Parser, Token, BasicParseError};
+use custom_properties::SpecifiedValue;
 use parser::{Parse, ParserContext};
 use selectors::parser::SelectorParseError;
 #[cfg(feature = "servo")]
 use servo_url::ServoUrl;
 use std::cmp::Ordering;
 use std::f32::consts::PI;
 use std::fmt;
 use style_traits::{ToCss, ParseError, StyleParseError};
@@ -869,22 +870,27 @@ impl Parse for ColorStop {
         Ok(ColorStop {
             color: RGBAColor::parse(context, input)?,
             position: input.try(|i| LengthOrPercentage::parse(context, i)).ok(),
         })
     }
 }
 
 impl Parse for PaintWorklet {
-    fn parse<'i, 't>(_context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
+    fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         input.expect_function_matching("paint")?;
-        input.parse_nested_block(|i| {
-            let name = i.expect_ident()?;
+        input.parse_nested_block(|input| {
+            let name = Atom::from(&**input.expect_ident()?);
+            let arguments = input.try(|input| {
+                input.expect_comma()?;
+                input.parse_comma_separated(|input| Ok(*SpecifiedValue::parse(context, input)?))
+            }).unwrap_or(vec![]);
             Ok(PaintWorklet {
-                name: Atom::from(name.as_ref()),
+                name: name,
+                arguments: arguments,
             })
         })
     }
 }
 
 impl Parse for MozImageRect {
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         input.try(|i| i.expect_function_matching("-moz-image-rect"))?;