From c87a35e4d5f3db206a512f953cc8fb4627b24c72 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Thu, 12 Jul 2018 15:59:21 -0400 Subject: [PATCH 1/6] Ensure canvas path stroking and filling use the same paths. --- components/canvas/canvas_data.rs | 42 +++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index 04540c8a495..2b3e7e1e091 100644 --- a/components/canvas/canvas_data.rs +++ b/components/canvas/canvas_data.rs @@ -8,7 +8,7 @@ use azure::azure_hl::{AntialiasMode, CapStyle, CompositionOp, JoinStyle}; use azure::azure_hl::{ BackendType, DrawOptions, DrawTarget, Pattern, StrokeOptions, SurfaceFormat, }; -use azure::azure_hl::{Color, ColorPattern, DrawSurfaceOptions, Filter, PathBuilder}; +use azure::azure_hl::{Color, ColorPattern, DrawSurfaceOptions, Filter, PathBuilder, Path}; use azure::azure_hl::{ExtendMode, GradientStop, LinearGradientPattern, RadialGradientPattern}; use canvas_traits::canvas::*; use cssparser::RGBA; @@ -23,6 +23,7 @@ pub struct CanvasData<'a> { drawtarget: DrawTarget, /// TODO(pcwalton): Support multiple paths. path_builder: PathBuilder, + path: Option, state: CanvasPaintState<'a>, saved_states: Vec>, webrender_api: webrender_api::RenderApi, @@ -47,6 +48,7 @@ impl<'a> CanvasData<'a> { CanvasData { drawtarget: draw_target, path_builder: path_builder, + path: None, state: CanvasPaintState::new(antialias), saved_states: vec![], webrender_api: webrender_api, @@ -206,40 +208,54 @@ impl<'a> CanvasData<'a> { } pub fn begin_path(&mut self) { - self.path_builder = self.drawtarget.create_path_builder() + self.path_builder = self.drawtarget.create_path_builder(); + self.path = None; } pub fn close_path(&self) { self.path_builder.close() } - pub fn fill(&self) { + fn ensure_path(&mut self) { + if self.path.is_none() { + self.path = Some(self.path_builder.finish()); + } + } + + fn path(&self) -> &Path { + self.path.as_ref().expect("Should have called ensure_path()") + } + + pub fn fill(&mut self) { if is_zero_size_gradient(&self.state.fill_style) { return; // Paint nothing if gradient size is zero. } + self.ensure_path(); self.drawtarget.fill( - &self.path_builder.finish(), + &self.path(), self.state.fill_style.to_pattern_ref(), &self.state.draw_options, ); } - pub fn stroke(&self) { + pub fn stroke(&mut self) { if is_zero_size_gradient(&self.state.stroke_style) { return; // Paint nothing if gradient size is zero. } + self.ensure_path(); self.drawtarget.stroke( - &self.path_builder.finish(), + &self.path(), self.state.stroke_style.to_pattern_ref(), &self.state.stroke_opts, &self.state.draw_options, ); } - pub fn clip(&self) { - self.drawtarget.push_clip(&self.path_builder.finish()); + pub fn clip(&mut self) { + self.ensure_path(); + self.drawtarget.push_clip(&self.path()); } pub fn is_point_in_path( @@ -249,9 +265,13 @@ impl<'a> CanvasData<'a> { _fill_rule: FillRule, chan: IpcSender, ) { - let path = self.path_builder.finish(); - let result = path.contains_point(x, y, &self.state.transform); - self.path_builder = path.copy_to_builder(); + self.ensure_path(); + let (path_builder, result) = { + let path = self.path(); + let result = path.contains_point(x, y, &self.state.transform); + (path.copy_to_builder(), result) + }; + self.path_builder = path_builder; chan.send(result).unwrap(); } From e7edc367f9dc8fc311c4f79cb97f3fd9ee22b5a5 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 1 Aug 2018 12:19:29 -0400 Subject: [PATCH 2/6] Transform paths between user and device space when there is a 2D canvas transformation present. --- components/canvas/canvas_data.rs | 268 ++++++++++++++---- .../path-objects/2d.path.arcTo.scale.html.ini | 5 - .../2d.path.arcTo.transformation.html.ini | 5 - .../2d.path.clip.unaffected.html.ini | 5 - .../2d.path.fill.closed.unaffected.html.ini | 5 - ...2d.path.isPointInPath.transform.2.html.ini | 5 - ...2d.path.isPointInPath.transform.4.html.ini | 5 - .../2d.path.stroke.scale1.html.ini | 5 - .../2d.path.stroke.unaffected.html.ini | 5 - .../2d.path.transformation.basic.html.ini | 5 - .../2d.path.transformation.changing.html.ini | 5 - 11 files changed, 220 insertions(+), 98 deletions(-) delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.arcTo.scale.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.arcTo.transformation.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.clip.unaffected.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.fill.closed.unaffected.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.isPointInPath.transform.2.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.isPointInPath.transform.4.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.stroke.scale1.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.stroke.unaffected.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.transformation.basic.html.ini delete mode 100644 tests/wpt/metadata/2dcontext/path-objects/2d.path.transformation.changing.html.ini diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index 2b3e7e1e091..e05a3c2e3c4 100644 --- a/components/canvas/canvas_data.rs +++ b/components/canvas/canvas_data.rs @@ -4,7 +4,7 @@ use azure::azure::AzFloat; use azure::azure_hl::SurfacePattern; -use azure::azure_hl::{AntialiasMode, CapStyle, CompositionOp, JoinStyle}; +use azure::azure_hl::{AntialiasMode, AsAzurePoint, CapStyle, CompositionOp, JoinStyle}; use azure::azure_hl::{ BackendType, DrawOptions, DrawTarget, Pattern, StrokeOptions, SurfaceFormat, }; @@ -21,8 +21,13 @@ use webrender::api::DirtyRect; pub struct CanvasData<'a> { drawtarget: DrawTarget, - /// TODO(pcwalton): Support multiple paths. - path_builder: PathBuilder, + /// User-space path builder. + path_builder: Option, + /// Device-space path builder, if transforms are added during path building. + device_space_path_builder: Option, + /// Transformation required to move between user-space and device-space. + path_to_device_space: Option>, + /// The user-space path. path: Option, state: CanvasPaintState<'a>, saved_states: Vec>, @@ -43,11 +48,12 @@ impl<'a> CanvasData<'a> { canvas_id: CanvasId, ) -> CanvasData<'a> { let draw_target = CanvasData::create(size); - let path_builder = draw_target.create_path_builder(); let webrender_api = webrender_api_sender.create_api(); CanvasData { drawtarget: draw_target, - path_builder: path_builder, + path_builder: None, + device_space_path_builder: None, + path_to_device_space: None, path: None, state: CanvasPaintState::new(antialias), saved_states: vec![], @@ -208,18 +214,58 @@ impl<'a> CanvasData<'a> { } pub fn begin_path(&mut self) { - self.path_builder = self.drawtarget.create_path_builder(); + // Erase any traces of previous paths that existed before this. + self.path_builder = None; + self.device_space_path_builder = None; self.path = None; + self.path_to_device_space = None; } - pub fn close_path(&self) { - self.path_builder.close() + pub fn close_path(&mut self) { + self.ensure_path_builder(); + match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(builder), None) | + (None, Some(builder)) => builder.close(), + _ => unreachable!(), + } } fn ensure_path(&mut self) { - if self.path.is_none() { - self.path = Some(self.path_builder.finish()); + // If there's no record of any path yet, create a new builder in user-space. + if self.path.is_none() && self.path_builder.is_none() && self.device_space_path_builder.is_none() { + self.path_builder = Some(self.drawtarget.create_path_builder()); } + + // If a user-space builder exists, create a finished path from it. + if let Some(path_builder) = self.path_builder.take() { + self.path = Some(path_builder.finish()); + } + + // If a user-space path exists, create a device-space builder based on it if + // any transform is present. + if self.path.is_some() { + if let Some(transform) = self.path_to_device_space.take() { + let path = self.path.take().unwrap(); + self.device_space_path_builder = Some(path.transformed_copy_to_builder(&transform)); + } + } + + // If a device-space builder is present, create a user-space path from its + // finished path by inverting the initial transformation. + if let Some(path_builder) = self.device_space_path_builder.take() { + let path = path_builder.finish(); + let inverse = match self.drawtarget.get_transform().inverse() { + Some(m) => m, + None => { + warn!("Couldn't invert canvas transformation."); + return; + } + }; + let builder = path.transformed_copy_to_builder(&inverse); + self.path = Some(builder.finish()); + } + + assert!(self.path.is_some()); } fn path(&self) -> &Path { @@ -268,64 +314,183 @@ impl<'a> CanvasData<'a> { self.ensure_path(); let (path_builder, result) = { let path = self.path(); - let result = path.contains_point(x, y, &self.state.transform); + let transform = match self.path_to_device_space { + Some(ref transform) => transform, + None => &self.state.transform, + }; + let result = path.contains_point(x, y, transform); (path.copy_to_builder(), result) }; - self.path_builder = path_builder; + self.path_builder = Some(path_builder); chan.send(result).unwrap(); } - pub fn move_to(&self, point: &Point2D) { - self.path_builder.move_to(*point) + pub fn move_to(&mut self, point: &Point2D) { + self.ensure_path_builder(); + match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(builder), None) => + builder.move_to(*point), + (None, Some(builder)) => { + let xform = self.drawtarget.get_transform(); + builder.move_to(xform.transform_point(point)); + } + _ => unreachable!(), + } } - pub fn line_to(&self, point: &Point2D) { - self.path_builder.line_to(*point) + pub fn line_to(&mut self, point: &Point2D) { + self.ensure_path_builder(); + match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(builder), None) => + builder.line_to(*point), + (None, Some(builder)) => { + let xform = self.drawtarget.get_transform(); + builder.line_to(xform.transform_point(point)); + } + _ => unreachable!(), + } } - pub fn rect(&self, rect: &Rect) { - self.path_builder - .move_to(Point2D::new(rect.origin.x, rect.origin.y)); - self.path_builder - .line_to(Point2D::new(rect.origin.x + rect.size.width, rect.origin.y)); - self.path_builder.line_to(Point2D::new( - rect.origin.x + rect.size.width, - rect.origin.y + rect.size.height, - )); - self.path_builder.line_to(Point2D::new( - rect.origin.x, - rect.origin.y + rect.size.height, - )); - self.path_builder.close(); + fn ensure_path_builder(&mut self) { + // If a device-space builder is present, we're done. + if self.device_space_path_builder.is_some() { + return; + } + + // If a user-space builder is present, convert it to a device-space builder if + // any transform is present. + if self.path_builder.is_some() { + if let Some(transform) = self.path_to_device_space.take() { + let path = self.path_builder.take().unwrap().finish(); + self.device_space_path_builder = Some(path.transformed_copy_to_builder(&transform)); + } + return; + } + + // If there is a path, create a new builder, transforming the path if there is + // a transform present. Otherwise, create a new builder from scratch. + match (self.path.take(), self.path_to_device_space.take()) { + (Some(path), None) => { + self.path_builder = Some(path.copy_to_builder()); + self.path = Some(path); + } + (Some(path), Some(transform)) => { + self.device_space_path_builder = Some(path.transformed_copy_to_builder(&transform)); + } + (None, transform) => { + assert!(transform.is_none()); + self.path_builder = Some(self.drawtarget.create_path_builder()); + } + } } - pub fn quadratic_curve_to(&self, cp: &Point2D, endpoint: &Point2D) { - self.path_builder.quadratic_curve_to(cp, endpoint) + pub fn rect(&mut self, rect: &Rect) { + self.ensure_path_builder(); + match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(path_builder), None) => { + path_builder.move_to(Point2D::new(rect.origin.x, rect.origin.y)); + path_builder.line_to(Point2D::new(rect.origin.x + rect.size.width, rect.origin.y)); + path_builder.line_to(Point2D::new(rect.origin.x + rect.size.width, + rect.origin.y + rect.size.height)); + path_builder.line_to(Point2D::new(rect.origin.x, rect.origin.y + rect.size.height)); + path_builder.close(); + } + (None, Some(path_builder)) => { + let xform = self.drawtarget.get_transform(); + path_builder.move_to(xform.transform_point( + &Point2D::new(rect.origin.x, rect.origin.y))); + path_builder.line_to(xform.transform_point( + &Point2D::new(rect.origin.x + rect.size.width, rect.origin.y))); + path_builder.line_to(xform.transform_point( + &Point2D::new(rect.origin.x + rect.size.width, + rect.origin.y + rect.size.height))); + path_builder.line_to(xform.transform_point( + &Point2D::new(rect.origin.x, rect.origin.y + rect.size.height))); + path_builder.close(); + } + _ => unreachable!(), + } + } + + pub fn quadratic_curve_to( + &mut self, + cp: &Point2D, + endpoint: &Point2D + ) { + self.ensure_path_builder(); + match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(builder), None) => + builder.quadratic_curve_to(cp, endpoint), + (None, Some(builder)) => { + let xform = self.drawtarget.get_transform(); + builder.quadratic_curve_to(&xform.transform_point(cp), + &xform.transform_point(endpoint)); + } + _ => unreachable!(), + } } pub fn bezier_curve_to( - &self, + &mut self, cp1: &Point2D, cp2: &Point2D, endpoint: &Point2D, ) { - self.path_builder.bezier_curve_to(cp1, cp2, endpoint) + self.ensure_path_builder(); + match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(builder), None) => + builder.bezier_curve_to(cp1, cp2, endpoint), + (None, Some(builder)) => { + let xform = self.drawtarget.get_transform(); + builder.bezier_curve_to(&xform.transform_point(cp1), + &xform.transform_point(cp2), + &xform.transform_point(endpoint)); + } + _ => unreachable!(), + } } pub fn arc( - &self, + &mut self, center: &Point2D, radius: AzFloat, start_angle: AzFloat, end_angle: AzFloat, ccw: bool, ) { - self.path_builder - .arc(*center, radius, start_angle, end_angle, ccw) + self.ensure_path_builder(); + match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(builder), None) => + builder.arc(*center, radius, start_angle, end_angle, ccw), + (None, Some(builder)) => { + let xform = self.drawtarget.get_transform(); + builder.arc(xform.transform_point(center), radius, start_angle, end_angle, ccw) + } + _ => unreachable!(), + } } - pub fn arc_to(&self, cp1: &Point2D, cp2: &Point2D, radius: AzFloat) { - let cp0 = self.path_builder.get_current_point(); + pub fn arc_to( + &mut self, + cp1: &Point2D, + cp2: &Point2D, + radius: AzFloat + ) { + self.ensure_path_builder(); + let cp0 = match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(builder), None) => + builder.get_current_point(), + (None, Some(builder)) => { + let inverse = match self.drawtarget.get_transform().inverse() { + Some(m) => m, + None => return, + }; + let current_point = builder.get_current_point(); + let transformed = inverse.transform_point(&Point2D::new(current_point.x, current_point.y)); + transformed.as_azure_point() + } + _ => unreachable!(), + }; let cp1 = *cp1; let cp2 = *cp2; @@ -394,15 +559,17 @@ impl<'a> CanvasData<'a> { end_angle: AzFloat, ccw: bool, ) { - self.path_builder.ellipse( - *center, - radius_x, - radius_y, - rotation_angle, - start_angle, - end_angle, - ccw, - ); + self.ensure_path_builder(); + match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { + (Some(builder), None) => + builder.ellipse(*center, radius_x, radius_y, rotation_angle, start_angle, end_angle, ccw), + (None, Some(builder)) => { + let xform = self.drawtarget.get_transform(); + builder.ellipse(xform.transform_point(center), radius_x, radius_y, rotation_angle, + start_angle, end_angle, ccw) + } + _ => unreachable!(), + } } pub fn set_fill_style(&mut self, style: FillOrStrokeStyle) { @@ -434,6 +601,11 @@ impl<'a> CanvasData<'a> { } pub fn set_transform(&mut self, transform: &Transform2D) { + // If there is an in-progress path, store the existing transformation required + // to move between device and user space. + if (self.path.is_some() || self.path_builder.is_some()) && self.path_to_device_space.is_none() { + self.path_to_device_space = Some(self.drawtarget.get_transform()); + } self.state.transform = transform.clone(); self.drawtarget.set_transform(transform) } diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.arcTo.scale.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.arcTo.scale.html.ini deleted file mode 100644 index 9176d25b683..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.arcTo.scale.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.arcTo.scale.html] - type: testharness - [arcTo scales the curve, not just the control points] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.arcTo.transformation.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.arcTo.transformation.html.ini deleted file mode 100644 index 3f170878e71..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.arcTo.transformation.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.arcTo.transformation.html] - type: testharness - [arcTo joins up to the last subpath point correctly] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.clip.unaffected.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.clip.unaffected.html.ini deleted file mode 100644 index 5cc73cbf53c..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.clip.unaffected.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.clip.unaffected.html] - type: testharness - [Canvas test: 2d.path.clip.unaffected] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.fill.closed.unaffected.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.fill.closed.unaffected.html.ini deleted file mode 100644 index 68acf76f5c9..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.fill.closed.unaffected.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.fill.closed.unaffected.html] - type: testharness - [Canvas test: 2d.path.fill.closed.unaffected] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.isPointInPath.transform.2.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.isPointInPath.transform.2.html.ini deleted file mode 100644 index 52e5f19ca7d..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.isPointInPath.transform.2.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.isPointInPath.transform.2.html] - type: testharness - [isPointInPath() handles transformations correctly] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.isPointInPath.transform.4.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.isPointInPath.transform.4.html.ini deleted file mode 100644 index 9e3698f4221..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.isPointInPath.transform.4.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.isPointInPath.transform.4.html] - type: testharness - [isPointInPath() handles transformations correctly] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.stroke.scale1.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.stroke.scale1.html.ini deleted file mode 100644 index 50af8df36ee..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.stroke.scale1.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.stroke.scale1.html] - type: testharness - [Stroke line widths are scaled by the current transformation matrix] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.stroke.unaffected.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.stroke.unaffected.html.ini deleted file mode 100644 index b79fc7eb008..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.stroke.unaffected.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.stroke.unaffected.html] - type: testharness - [Stroking does not start a new path or subpath] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.transformation.basic.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.transformation.basic.html.ini deleted file mode 100644 index e6cec3c806d..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.transformation.basic.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.transformation.basic.html] - type: testharness - [Canvas test: 2d.path.transformation.basic] - expected: FAIL - diff --git a/tests/wpt/metadata/2dcontext/path-objects/2d.path.transformation.changing.html.ini b/tests/wpt/metadata/2dcontext/path-objects/2d.path.transformation.changing.html.ini deleted file mode 100644 index 13e13c05992..00000000000 --- a/tests/wpt/metadata/2dcontext/path-objects/2d.path.transformation.changing.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[2d.path.transformation.changing.html] - type: testharness - [Transformations are applied while building paths, not when drawing] - expected: FAIL - From 53ae73bdec8c5808db54da0aaeb1a65045869735 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 3 Aug 2018 16:25:01 -0400 Subject: [PATCH 3/6] canvas: Refactor implicit path/path builder state machine into a single enum. --- components/canvas/canvas_data.rs | 477 +++++++++++++++++++------------ 1 file changed, 294 insertions(+), 183 deletions(-) diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index e05a3c2e3c4..dc192309516 100644 --- a/components/canvas/canvas_data.rs +++ b/components/canvas/canvas_data.rs @@ -19,16 +19,175 @@ use std::mem; use std::sync::Arc; use webrender::api::DirtyRect; +/// The canvas data stores a state machine for the current status of +/// the path data and any relevant transformations that are +/// applied to it. The Azure drawing API expects the path to be in +/// userspace. However, when a path is being built but the canvas' +/// transform changes, we choose to transform the path and perform +/// further operations to it in device space. When it's time to +/// draw the path, we convert it back to userspace and draw it +/// with the correct transform applied. +enum PathState { + /// Path builder in user-space. If a transform has been applied + /// but no further path operations have occurred, it is stored + /// in the optional field. + UserSpacePathBuilder(PathBuilder, Option>), + /// Path builder in device-space. + DeviceSpacePathBuilder(PathBuilder), + /// Path in user-space. If a transform has been applied but + /// but no further path operations have occurred, it is stored + /// in the optional field. + UserSpacePath(Path, Option>), +} + +impl PathState { + fn is_path(&self) -> bool { + match *self { + PathState::UserSpacePath(..) => true, + PathState::UserSpacePathBuilder(..) | + PathState::DeviceSpacePathBuilder(..) => false, + } + } + + fn path(&self) -> &Path { + match *self { + PathState::UserSpacePath(ref p, _) => p, + PathState::UserSpacePathBuilder(..) | + PathState::DeviceSpacePathBuilder(..) => panic!("should have called ensure_path"), + } + } +} + +/// A wrapper around a stored PathBuilder and an optional transformation that should be +/// applied to any points to ensure they are in the matching device space. +struct PathBuilderRef<'a> { + builder: &'a PathBuilder, + transform: Option>, +} + +impl<'a> PathBuilderRef<'a> { + fn line_to(&self, pt: &Point2D) { + let pt = match self.transform { + Some(ref t) => t.transform_point(pt), + None => *pt, + }; + self.builder.line_to(pt); + } + + fn move_to(&self, pt: &Point2D) { + let pt = match self.transform { + Some(ref t) => t.transform_point(pt), + None => *pt, + }; + self.builder.move_to(pt); + } + + fn rect(&self, rect: &Rect) { + let (first, second, third, fourth) = + (Point2D::new(rect.origin.x, rect.origin.y), + Point2D::new(rect.origin.x + rect.size.width, rect.origin.y), + Point2D::new(rect.origin.x + rect.size.width, rect.origin.y + rect.size.height), + Point2D::new(rect.origin.x, rect.origin.y + rect.size.height)); + let (first, second, third, fourth) = match self.transform { + Some(ref t) => + (t.transform_point(&first), + t.transform_point(&second), + t.transform_point(&third), + t.transform_point(&fourth)), + None => (first, second, third, fourth), + }; + self.builder.move_to(first); + self.builder.line_to(second); + self.builder.line_to(third); + self.builder.line_to(fourth); + self.builder.close(); + } + + fn quadratic_curve_to(&self, cp: &Point2D, endpoint: &Point2D) { + let (cp2, endpoint2); + let (cp, endpoint) = match self.transform { + Some(ref t) => { + cp2 = t.transform_point(cp); + endpoint2 = t.transform_point(endpoint); + (&cp2, &endpoint2) + } + None => (cp, endpoint), + }; + self.builder.quadratic_curve_to(cp, endpoint) + } + + fn bezier_curve_to( + &self, + cp1: &Point2D, + cp2: &Point2D, + endpoint: &Point2D + ) { + let (cp1_t, cp2_t, endpoint_t); + let (cp1, cp2, endpoint) = match self.transform { + Some(ref t) => { + cp1_t = t.transform_point(cp1); + cp2_t = t.transform_point(cp2); + endpoint_t = t.transform_point(endpoint); + (&cp1_t, &cp2_t, &endpoint_t) + } + None => (cp1, cp2, endpoint), + }; + self.builder.bezier_curve_to(cp1, cp2, endpoint) + } + + fn arc( + &self, + center: &Point2D, + radius: AzFloat, + start_angle: AzFloat, + end_angle: AzFloat, + ccw: bool + ) { + let center = match self.transform { + Some(ref t) => t.transform_point(center), + None => *center, + }; + self.builder.arc(center, radius, start_angle, end_angle, ccw); + } + + pub fn ellipse( + &self, + center: &Point2D, + radius_x: AzFloat, + radius_y: AzFloat, + rotation_angle: AzFloat, + start_angle: AzFloat, + end_angle: AzFloat, + ccw: bool + ) { + let center = match self.transform { + Some(ref t) => t.transform_point(center), + None => *center, + }; + self.builder.ellipse(center, radius_x, radius_y, rotation_angle, start_angle, end_angle, ccw); + } + + fn current_point(&self) -> Option> { + match self.transform { + Some(ref t) => { + let inverse = match t.inverse() { + Some(i) => i, + None => return None, + }; + let current_point = self.builder.get_current_point(); + Some(inverse.transform_point(&Point2D::new(current_point.x, current_point.y))) + } + None => { + let current = self.builder.get_current_point(); + Some(Point2D::new(current.x, current.y)) + } + } + } +} + pub struct CanvasData<'a> { drawtarget: DrawTarget, - /// User-space path builder. - path_builder: Option, - /// Device-space path builder, if transforms are added during path building. - device_space_path_builder: Option, - /// Transformation required to move between user-space and device-space. - path_to_device_space: Option>, - /// The user-space path. - path: Option, + path_state: Option, state: CanvasPaintState<'a>, saved_states: Vec>, webrender_api: webrender_api::RenderApi, @@ -51,10 +210,7 @@ impl<'a> CanvasData<'a> { let webrender_api = webrender_api_sender.create_api(); CanvasData { drawtarget: draw_target, - path_builder: None, - device_space_path_builder: None, - path_to_device_space: None, - path: None, + path_state: None, state: CanvasPaintState::new(antialias), saved_states: vec![], webrender_api: webrender_api, @@ -215,61 +371,72 @@ impl<'a> CanvasData<'a> { pub fn begin_path(&mut self) { // Erase any traces of previous paths that existed before this. - self.path_builder = None; - self.device_space_path_builder = None; - self.path = None; - self.path_to_device_space = None; + self.path_state = None; } pub fn close_path(&mut self) { - self.ensure_path_builder(); - match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(builder), None) | - (None, Some(builder)) => builder.close(), - _ => unreachable!(), - } + self.path_builder().builder.close(); } fn ensure_path(&mut self) { // If there's no record of any path yet, create a new builder in user-space. - if self.path.is_none() && self.path_builder.is_none() && self.device_space_path_builder.is_none() { - self.path_builder = Some(self.drawtarget.create_path_builder()); + if self.path_state.is_none() { + self.path_state = Some(PathState::UserSpacePathBuilder( + self.drawtarget.create_path_builder(), None)); } // If a user-space builder exists, create a finished path from it. - if let Some(path_builder) = self.path_builder.take() { - self.path = Some(path_builder.finish()); + let new_state = match *self.path_state.as_mut().unwrap() { + PathState::UserSpacePathBuilder(ref builder, ref mut transform) => + Some((builder.finish(), transform.take())), + PathState::DeviceSpacePathBuilder(..) | PathState::UserSpacePath(..) => + None, + }; + if let Some((path, transform)) = new_state { + self.path_state = Some(PathState::UserSpacePath(path, transform)); } // If a user-space path exists, create a device-space builder based on it if // any transform is present. - if self.path.is_some() { - if let Some(transform) = self.path_to_device_space.take() { - let path = self.path.take().unwrap(); - self.device_space_path_builder = Some(path.transformed_copy_to_builder(&transform)); - } + let new_state = match *self.path_state.as_ref().unwrap() { + PathState::UserSpacePath(ref path, Some(ref transform)) => + Some(path.transformed_copy_to_builder(transform)), + PathState::UserSpacePath(..) | + PathState::UserSpacePathBuilder(..) | + PathState::DeviceSpacePathBuilder(..) => + None, + }; + if let Some(builder) = new_state { + self.path_state = Some(PathState::DeviceSpacePathBuilder(builder)); } // If a device-space builder is present, create a user-space path from its // finished path by inverting the initial transformation. - if let Some(path_builder) = self.device_space_path_builder.take() { - let path = path_builder.finish(); - let inverse = match self.drawtarget.get_transform().inverse() { - Some(m) => m, - None => { - warn!("Couldn't invert canvas transformation."); - return; - } - }; - let builder = path.transformed_copy_to_builder(&inverse); - self.path = Some(builder.finish()); + let new_state = match self.path_state.as_ref().unwrap() { + PathState::DeviceSpacePathBuilder(ref builder) => { + let path = builder.finish(); + let inverse = match self.drawtarget.get_transform().inverse() { + Some(m) => m, + None => { + warn!("Couldn't invert canvas transformation."); + return; + } + }; + let builder = path.transformed_copy_to_builder(&inverse); + Some(builder.finish()) + } + PathState::UserSpacePathBuilder(..) | PathState::UserSpacePath(..) => + None, + }; + if let Some(path) = new_state { + self.path_state = Some(PathState::UserSpacePath(path, None)); } - assert!(self.path.is_some()); + assert!(self.path_state.as_ref().unwrap().is_path()) } fn path(&self) -> &Path { - self.path.as_ref().expect("Should have called ensure_path()") + self.path_state.as_ref().expect("Should have called ensure_path()").path() } pub fn fill(&mut self) { @@ -312,104 +479,92 @@ impl<'a> CanvasData<'a> { chan: IpcSender, ) { self.ensure_path(); - let (path_builder, result) = { - let path = self.path(); - let transform = match self.path_to_device_space { - Some(ref transform) => transform, - None => &self.state.transform, - }; - let result = path.contains_point(x, y, transform); - (path.copy_to_builder(), result) + let (result, new_state) = match *self.path_state.as_mut().unwrap() { + PathState::UserSpacePath(ref path, ref mut transform) => { + let result = { + let path_transform = transform.as_ref().unwrap_or(&self.state.transform); + path.contains_point(x, y, path_transform) + }; + let state = PathState::UserSpacePathBuilder(path.copy_to_builder(), transform.take()); + (result, state) + } + PathState::UserSpacePathBuilder(..) | + PathState::DeviceSpacePathBuilder(..) => unreachable!(), }; - self.path_builder = Some(path_builder); + self.path_state = Some(new_state); chan.send(result).unwrap(); } pub fn move_to(&mut self, point: &Point2D) { - self.ensure_path_builder(); - match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(builder), None) => - builder.move_to(*point), - (None, Some(builder)) => { - let xform = self.drawtarget.get_transform(); - builder.move_to(xform.transform_point(point)); - } - _ => unreachable!(), - } + self.path_builder().move_to(point); } pub fn line_to(&mut self, point: &Point2D) { - self.ensure_path_builder(); - match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(builder), None) => - builder.line_to(*point), - (None, Some(builder)) => { - let xform = self.drawtarget.get_transform(); - builder.line_to(xform.transform_point(point)); - } - _ => unreachable!(), - } + self.path_builder().line_to(point); } - fn ensure_path_builder(&mut self) { - // If a device-space builder is present, we're done. - if self.device_space_path_builder.is_some() { - return; + fn path_builder(&mut self) -> PathBuilderRef { + if self.path_state.is_none() { + self.path_state = Some(PathState::UserSpacePathBuilder( + self.drawtarget.create_path_builder(), None)); } - // If a user-space builder is present, convert it to a device-space builder if - // any transform is present. - if self.path_builder.is_some() { - if let Some(transform) = self.path_to_device_space.take() { - let path = self.path_builder.take().unwrap().finish(); - self.device_space_path_builder = Some(path.transformed_copy_to_builder(&transform)); + // Rust is not pleased by returning a reference to a builder in some branches + // and overwriting path_state in other ones. The following awkward use of duplicate + // matches works around the resulting borrow errors. + let new_state = { + match self.path_state.as_ref().unwrap() { + &PathState::UserSpacePathBuilder(_, None) | + &PathState::DeviceSpacePathBuilder(_) => + None, + &PathState::UserSpacePathBuilder(ref builder, Some(ref transform)) => { + let path = builder.finish(); + Some(PathState::DeviceSpacePathBuilder(path.transformed_copy_to_builder(transform))) + } + &PathState::UserSpacePath(ref path, Some(ref transform)) => + Some(PathState::DeviceSpacePathBuilder(path.transformed_copy_to_builder(transform))), + &PathState::UserSpacePath(ref path, None) => + Some(PathState::UserSpacePathBuilder(path.copy_to_builder(), None)), + } + }; + match new_state { + // There's a new builder value that needs to be stored. + Some(state) => self.path_state = Some(state), + // There's an existing builder value that can be returned immediately. + None => match self.path_state.as_ref().unwrap() { + &PathState::UserSpacePathBuilder(ref builder, None) => + return PathBuilderRef { + builder, + transform: None, + }, + &PathState::DeviceSpacePathBuilder(ref builder) => + return PathBuilderRef { + builder, + transform: Some(self.drawtarget.get_transform()), + }, + _ => unreachable!(), } - return; } - // If there is a path, create a new builder, transforming the path if there is - // a transform present. Otherwise, create a new builder from scratch. - match (self.path.take(), self.path_to_device_space.take()) { - (Some(path), None) => { - self.path_builder = Some(path.copy_to_builder()); - self.path = Some(path); - } - (Some(path), Some(transform)) => { - self.device_space_path_builder = Some(path.transformed_copy_to_builder(&transform)); - } - (None, transform) => { - assert!(transform.is_none()); - self.path_builder = Some(self.drawtarget.create_path_builder()); - } + match self.path_state.as_ref().unwrap() { + &PathState::UserSpacePathBuilder(ref builder, None) => + PathBuilderRef { + builder, + transform: None, + }, + &PathState::DeviceSpacePathBuilder(ref builder) => + PathBuilderRef { + builder, + transform: Some(self.drawtarget.get_transform()), + }, + &PathState::UserSpacePathBuilder(..) | + &PathState::UserSpacePath(..) => + unreachable!(), } } pub fn rect(&mut self, rect: &Rect) { - self.ensure_path_builder(); - match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(path_builder), None) => { - path_builder.move_to(Point2D::new(rect.origin.x, rect.origin.y)); - path_builder.line_to(Point2D::new(rect.origin.x + rect.size.width, rect.origin.y)); - path_builder.line_to(Point2D::new(rect.origin.x + rect.size.width, - rect.origin.y + rect.size.height)); - path_builder.line_to(Point2D::new(rect.origin.x, rect.origin.y + rect.size.height)); - path_builder.close(); - } - (None, Some(path_builder)) => { - let xform = self.drawtarget.get_transform(); - path_builder.move_to(xform.transform_point( - &Point2D::new(rect.origin.x, rect.origin.y))); - path_builder.line_to(xform.transform_point( - &Point2D::new(rect.origin.x + rect.size.width, rect.origin.y))); - path_builder.line_to(xform.transform_point( - &Point2D::new(rect.origin.x + rect.size.width, - rect.origin.y + rect.size.height))); - path_builder.line_to(xform.transform_point( - &Point2D::new(rect.origin.x, rect.origin.y + rect.size.height))); - path_builder.close(); - } - _ => unreachable!(), - } + self.path_builder().rect(rect); } pub fn quadratic_curve_to( @@ -417,17 +572,7 @@ impl<'a> CanvasData<'a> { cp: &Point2D, endpoint: &Point2D ) { - self.ensure_path_builder(); - match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(builder), None) => - builder.quadratic_curve_to(cp, endpoint), - (None, Some(builder)) => { - let xform = self.drawtarget.get_transform(); - builder.quadratic_curve_to(&xform.transform_point(cp), - &xform.transform_point(endpoint)); - } - _ => unreachable!(), - } + self.path_builder().quadratic_curve_to(cp, endpoint); } pub fn bezier_curve_to( @@ -436,18 +581,7 @@ impl<'a> CanvasData<'a> { cp2: &Point2D, endpoint: &Point2D, ) { - self.ensure_path_builder(); - match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(builder), None) => - builder.bezier_curve_to(cp1, cp2, endpoint), - (None, Some(builder)) => { - let xform = self.drawtarget.get_transform(); - builder.bezier_curve_to(&xform.transform_point(cp1), - &xform.transform_point(cp2), - &xform.transform_point(endpoint)); - } - _ => unreachable!(), - } + self.path_builder().bezier_curve_to(cp1, cp2, endpoint); } pub fn arc( @@ -458,16 +592,7 @@ impl<'a> CanvasData<'a> { end_angle: AzFloat, ccw: bool, ) { - self.ensure_path_builder(); - match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(builder), None) => - builder.arc(*center, radius, start_angle, end_angle, ccw), - (None, Some(builder)) => { - let xform = self.drawtarget.get_transform(); - builder.arc(xform.transform_point(center), radius, start_angle, end_angle, ccw) - } - _ => unreachable!(), - } + self.path_builder().arc(center, radius, start_angle, end_angle, ccw); } pub fn arc_to( @@ -476,20 +601,9 @@ impl<'a> CanvasData<'a> { cp2: &Point2D, radius: AzFloat ) { - self.ensure_path_builder(); - let cp0 = match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(builder), None) => - builder.get_current_point(), - (None, Some(builder)) => { - let inverse = match self.drawtarget.get_transform().inverse() { - Some(m) => m, - None => return, - }; - let current_point = builder.get_current_point(); - let transformed = inverse.transform_point(&Point2D::new(current_point.x, current_point.y)); - transformed.as_azure_point() - } - _ => unreachable!(), + let cp0 = match self.path_builder().current_point() { + Some(p) => p.as_azure_point(), + None => return, }; let cp1 = *cp1; let cp2 = *cp2; @@ -559,17 +673,7 @@ impl<'a> CanvasData<'a> { end_angle: AzFloat, ccw: bool, ) { - self.ensure_path_builder(); - match (self.path_builder.as_ref(), self.device_space_path_builder.as_ref()) { - (Some(builder), None) => - builder.ellipse(*center, radius_x, radius_y, rotation_angle, start_angle, end_angle, ccw), - (None, Some(builder)) => { - let xform = self.drawtarget.get_transform(); - builder.ellipse(xform.transform_point(center), radius_x, radius_y, rotation_angle, - start_angle, end_angle, ccw) - } - _ => unreachable!(), - } + self.path_builder().ellipse(center, radius_x, radius_y, rotation_angle, start_angle, end_angle, ccw); } pub fn set_fill_style(&mut self, style: FillOrStrokeStyle) { @@ -603,8 +707,15 @@ impl<'a> CanvasData<'a> { pub fn set_transform(&mut self, transform: &Transform2D) { // If there is an in-progress path, store the existing transformation required // to move between device and user space. - if (self.path.is_some() || self.path_builder.is_some()) && self.path_to_device_space.is_none() { - self.path_to_device_space = Some(self.drawtarget.get_transform()); + match self.path_state.as_mut() { + None | + Some(PathState::DeviceSpacePathBuilder(..)) => (), + Some(PathState::UserSpacePathBuilder(_, ref mut transform)) | + Some(PathState::UserSpacePath(_, ref mut transform)) => { + if transform.is_none() { + *transform = Some(self.drawtarget.get_transform()); + } + } } self.state.transform = transform.clone(); self.drawtarget.set_transform(transform) From 2c5a6ebc53fe5aaebb0d9482a7e6b26121bef180 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 6 Feb 2019 19:17:35 -0500 Subject: [PATCH 4/6] Always store a transform in PathBuilderRef. --- components/canvas/canvas_data.rs | 96 ++++++++++---------------------- 1 file changed, 28 insertions(+), 68 deletions(-) diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index dc192309516..6563debc0d7 100644 --- a/components/canvas/canvas_data.rs +++ b/components/canvas/canvas_data.rs @@ -62,23 +62,17 @@ impl PathState { /// applied to any points to ensure they are in the matching device space. struct PathBuilderRef<'a> { builder: &'a PathBuilder, - transform: Option>, + transform: Transform2D, } impl<'a> PathBuilderRef<'a> { fn line_to(&self, pt: &Point2D) { - let pt = match self.transform { - Some(ref t) => t.transform_point(pt), - None => *pt, - }; + let pt = self.transform.transform_point(pt); self.builder.line_to(pt); } fn move_to(&self, pt: &Point2D) { - let pt = match self.transform { - Some(ref t) => t.transform_point(pt), - None => *pt, - }; + let pt = self.transform.transform_point(pt); self.builder.move_to(pt); } @@ -88,32 +82,18 @@ impl<'a> PathBuilderRef<'a> { Point2D::new(rect.origin.x + rect.size.width, rect.origin.y), Point2D::new(rect.origin.x + rect.size.width, rect.origin.y + rect.size.height), Point2D::new(rect.origin.x, rect.origin.y + rect.size.height)); - let (first, second, third, fourth) = match self.transform { - Some(ref t) => - (t.transform_point(&first), - t.transform_point(&second), - t.transform_point(&third), - t.transform_point(&fourth)), - None => (first, second, third, fourth), - }; - self.builder.move_to(first); - self.builder.line_to(second); - self.builder.line_to(third); - self.builder.line_to(fourth); + self.builder.move_to(self.transform.transform_point(&first)); + self.builder.line_to(self.transform.transform_point(&second)); + self.builder.line_to(self.transform.transform_point(&third)); + self.builder.line_to(self.transform.transform_point(&fourth)); self.builder.close(); } fn quadratic_curve_to(&self, cp: &Point2D, endpoint: &Point2D) { - let (cp2, endpoint2); - let (cp, endpoint) = match self.transform { - Some(ref t) => { - cp2 = t.transform_point(cp); - endpoint2 = t.transform_point(endpoint); - (&cp2, &endpoint2) - } - None => (cp, endpoint), - }; - self.builder.quadratic_curve_to(cp, endpoint) + self.builder.quadratic_curve_to( + &self.transform.transform_point(cp), + &self.transform.transform_point(endpoint), + ) } fn bezier_curve_to( @@ -122,17 +102,11 @@ impl<'a> PathBuilderRef<'a> { cp2: &Point2D, endpoint: &Point2D ) { - let (cp1_t, cp2_t, endpoint_t); - let (cp1, cp2, endpoint) = match self.transform { - Some(ref t) => { - cp1_t = t.transform_point(cp1); - cp2_t = t.transform_point(cp2); - endpoint_t = t.transform_point(endpoint); - (&cp1_t, &cp2_t, &endpoint_t) - } - None => (cp1, cp2, endpoint), - }; - self.builder.bezier_curve_to(cp1, cp2, endpoint) + self.builder.bezier_curve_to( + &self.transform.transform_point(cp1), + &self.transform.transform_point(cp2), + &self.transform.transform_point(endpoint), + ) } fn arc( @@ -143,10 +117,7 @@ impl<'a> PathBuilderRef<'a> { end_angle: AzFloat, ccw: bool ) { - let center = match self.transform { - Some(ref t) => t.transform_point(center), - None => *center, - }; + let center = self.transform.transform_point(center); self.builder.arc(center, radius, start_angle, end_angle, ccw); } @@ -160,28 +131,17 @@ impl<'a> PathBuilderRef<'a> { end_angle: AzFloat, ccw: bool ) { - let center = match self.transform { - Some(ref t) => t.transform_point(center), - None => *center, - }; + let center = self.transform.transform_point(center); self.builder.ellipse(center, radius_x, radius_y, rotation_angle, start_angle, end_angle, ccw); } fn current_point(&self) -> Option> { - match self.transform { - Some(ref t) => { - let inverse = match t.inverse() { - Some(i) => i, - None => return None, - }; - let current_point = self.builder.get_current_point(); - Some(inverse.transform_point(&Point2D::new(current_point.x, current_point.y))) - } - None => { - let current = self.builder.get_current_point(); - Some(Point2D::new(current.x, current.y)) - } - } + let inverse = match self.transform.inverse() { + Some(i) => i, + None => return None, + }; + let current_point = self.builder.get_current_point(); + Some(inverse.transform_point(&Point2D::new(current_point.x, current_point.y))) } } @@ -535,12 +495,12 @@ impl<'a> CanvasData<'a> { &PathState::UserSpacePathBuilder(ref builder, None) => return PathBuilderRef { builder, - transform: None, + transform: Transform2D::identity(), }, &PathState::DeviceSpacePathBuilder(ref builder) => return PathBuilderRef { builder, - transform: Some(self.drawtarget.get_transform()), + transform: self.drawtarget.get_transform(), }, _ => unreachable!(), } @@ -550,12 +510,12 @@ impl<'a> CanvasData<'a> { &PathState::UserSpacePathBuilder(ref builder, None) => PathBuilderRef { builder, - transform: None, + transform: Transform2D::identity(), }, &PathState::DeviceSpacePathBuilder(ref builder) => PathBuilderRef { builder, - transform: Some(self.drawtarget.get_transform()), + transform: self.drawtarget.get_transform(), }, &PathState::UserSpacePathBuilder(..) | &PathState::UserSpacePath(..) => From decf88331b4c5b5cb055b25ca2b5c4214c41b363 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Thu, 7 Feb 2019 15:09:59 -0500 Subject: [PATCH 5/6] Keep is_point_in_path using a path, rather than a path builder. --- components/canvas/canvas_data.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index 6563debc0d7..084e9019f21 100644 --- a/components/canvas/canvas_data.rs +++ b/components/canvas/canvas_data.rs @@ -439,19 +439,14 @@ impl<'a> CanvasData<'a> { chan: IpcSender, ) { self.ensure_path(); - let (result, new_state) = match *self.path_state.as_mut().unwrap() { - PathState::UserSpacePath(ref path, ref mut transform) => { - let result = { - let path_transform = transform.as_ref().unwrap_or(&self.state.transform); - path.contains_point(x, y, path_transform) - }; - let state = PathState::UserSpacePathBuilder(path.copy_to_builder(), transform.take()); - (result, state) + let result = match self.path_state.as_ref() { + Some(PathState::UserSpacePath(ref path, ref transform)) => { + let target_transform = self.drawtarget.get_transform(); + let path_transform = transform.as_ref().unwrap_or(&target_transform); + path.contains_point(x, y, path_transform) } - PathState::UserSpacePathBuilder(..) | - PathState::DeviceSpacePathBuilder(..) => unreachable!(), + Some(_) | None => false, }; - self.path_state = Some(new_state); chan.send(result).unwrap(); } From 18282b8071a3a2759d69a0f6641fc6a653144a93 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Thu, 7 Feb 2019 15:13:13 -0500 Subject: [PATCH 6/6] Format canvas code. --- components/canvas/canvas_data.rs | 172 +++++++++++++++++-------------- 1 file changed, 97 insertions(+), 75 deletions(-) diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index 084e9019f21..81e4bbe4bba 100644 --- a/components/canvas/canvas_data.rs +++ b/components/canvas/canvas_data.rs @@ -8,7 +8,7 @@ use azure::azure_hl::{AntialiasMode, AsAzurePoint, CapStyle, CompositionOp, Join use azure::azure_hl::{ BackendType, DrawOptions, DrawTarget, Pattern, StrokeOptions, SurfaceFormat, }; -use azure::azure_hl::{Color, ColorPattern, DrawSurfaceOptions, Filter, PathBuilder, Path}; +use azure::azure_hl::{Color, ColorPattern, DrawSurfaceOptions, Filter, Path, PathBuilder}; use azure::azure_hl::{ExtendMode, GradientStop, LinearGradientPattern, RadialGradientPattern}; use canvas_traits::canvas::*; use cssparser::RGBA; @@ -44,16 +44,16 @@ impl PathState { fn is_path(&self) -> bool { match *self { PathState::UserSpacePath(..) => true, - PathState::UserSpacePathBuilder(..) | - PathState::DeviceSpacePathBuilder(..) => false, + PathState::UserSpacePathBuilder(..) | PathState::DeviceSpacePathBuilder(..) => false, } } fn path(&self) -> &Path { match *self { PathState::UserSpacePath(ref p, _) => p, - PathState::UserSpacePathBuilder(..) | - PathState::DeviceSpacePathBuilder(..) => panic!("should have called ensure_path"), + PathState::UserSpacePathBuilder(..) | PathState::DeviceSpacePathBuilder(..) => { + panic!("should have called ensure_path") + }, } } } @@ -77,15 +77,21 @@ impl<'a> PathBuilderRef<'a> { } fn rect(&self, rect: &Rect) { - let (first, second, third, fourth) = - (Point2D::new(rect.origin.x, rect.origin.y), - Point2D::new(rect.origin.x + rect.size.width, rect.origin.y), - Point2D::new(rect.origin.x + rect.size.width, rect.origin.y + rect.size.height), - Point2D::new(rect.origin.x, rect.origin.y + rect.size.height)); + let (first, second, third, fourth) = ( + Point2D::new(rect.origin.x, rect.origin.y), + Point2D::new(rect.origin.x + rect.size.width, rect.origin.y), + Point2D::new( + rect.origin.x + rect.size.width, + rect.origin.y + rect.size.height, + ), + Point2D::new(rect.origin.x, rect.origin.y + rect.size.height), + ); self.builder.move_to(self.transform.transform_point(&first)); - self.builder.line_to(self.transform.transform_point(&second)); + self.builder + .line_to(self.transform.transform_point(&second)); self.builder.line_to(self.transform.transform_point(&third)); - self.builder.line_to(self.transform.transform_point(&fourth)); + self.builder + .line_to(self.transform.transform_point(&fourth)); self.builder.close(); } @@ -100,7 +106,7 @@ impl<'a> PathBuilderRef<'a> { &self, cp1: &Point2D, cp2: &Point2D, - endpoint: &Point2D + endpoint: &Point2D, ) { self.builder.bezier_curve_to( &self.transform.transform_point(cp1), @@ -115,10 +121,11 @@ impl<'a> PathBuilderRef<'a> { radius: AzFloat, start_angle: AzFloat, end_angle: AzFloat, - ccw: bool + ccw: bool, ) { let center = self.transform.transform_point(center); - self.builder.arc(center, radius, start_angle, end_angle, ccw); + self.builder + .arc(center, radius, start_angle, end_angle, ccw); } pub fn ellipse( @@ -129,10 +136,18 @@ impl<'a> PathBuilderRef<'a> { rotation_angle: AzFloat, start_angle: AzFloat, end_angle: AzFloat, - ccw: bool + ccw: bool, ) { let center = self.transform.transform_point(center); - self.builder.ellipse(center, radius_x, radius_y, rotation_angle, start_angle, end_angle, ccw); + self.builder.ellipse( + center, + radius_x, + radius_y, + rotation_angle, + start_angle, + end_angle, + ccw, + ); } fn current_point(&self) -> Option> { @@ -342,15 +357,17 @@ impl<'a> CanvasData<'a> { // If there's no record of any path yet, create a new builder in user-space. if self.path_state.is_none() { self.path_state = Some(PathState::UserSpacePathBuilder( - self.drawtarget.create_path_builder(), None)); + self.drawtarget.create_path_builder(), + None, + )); } // If a user-space builder exists, create a finished path from it. let new_state = match *self.path_state.as_mut().unwrap() { - PathState::UserSpacePathBuilder(ref builder, ref mut transform) => - Some((builder.finish(), transform.take())), - PathState::DeviceSpacePathBuilder(..) | PathState::UserSpacePath(..) => - None, + PathState::UserSpacePathBuilder(ref builder, ref mut transform) => { + Some((builder.finish(), transform.take())) + }, + PathState::DeviceSpacePathBuilder(..) | PathState::UserSpacePath(..) => None, }; if let Some((path, transform)) = new_state { self.path_state = Some(PathState::UserSpacePath(path, transform)); @@ -359,12 +376,12 @@ impl<'a> CanvasData<'a> { // If a user-space path exists, create a device-space builder based on it if // any transform is present. let new_state = match *self.path_state.as_ref().unwrap() { - PathState::UserSpacePath(ref path, Some(ref transform)) => - Some(path.transformed_copy_to_builder(transform)), + PathState::UserSpacePath(ref path, Some(ref transform)) => { + Some(path.transformed_copy_to_builder(transform)) + }, PathState::UserSpacePath(..) | PathState::UserSpacePathBuilder(..) | - PathState::DeviceSpacePathBuilder(..) => - None, + PathState::DeviceSpacePathBuilder(..) => None, }; if let Some(builder) = new_state { self.path_state = Some(PathState::DeviceSpacePathBuilder(builder)); @@ -380,13 +397,12 @@ impl<'a> CanvasData<'a> { None => { warn!("Couldn't invert canvas transformation."); return; - } + }, }; let builder = path.transformed_copy_to_builder(&inverse); Some(builder.finish()) - } - PathState::UserSpacePathBuilder(..) | PathState::UserSpacePath(..) => - None, + }, + PathState::UserSpacePathBuilder(..) | PathState::UserSpacePath(..) => None, }; if let Some(path) = new_state { self.path_state = Some(PathState::UserSpacePath(path, None)); @@ -396,7 +412,10 @@ impl<'a> CanvasData<'a> { } fn path(&self) -> &Path { - self.path_state.as_ref().expect("Should have called ensure_path()").path() + self.path_state + .as_ref() + .expect("Should have called ensure_path()") + .path() } pub fn fill(&mut self) { @@ -444,7 +463,7 @@ impl<'a> CanvasData<'a> { let target_transform = self.drawtarget.get_transform(); let path_transform = transform.as_ref().unwrap_or(&target_transform); path.contains_point(x, y, path_transform) - } + }, Some(_) | None => false, }; chan.send(result).unwrap(); @@ -461,7 +480,9 @@ impl<'a> CanvasData<'a> { fn path_builder(&mut self) -> PathBuilderRef { if self.path_state.is_none() { self.path_state = Some(PathState::UserSpacePathBuilder( - self.drawtarget.create_path_builder(), None)); + self.drawtarget.create_path_builder(), + None, + )); } // Rust is not pleased by returning a reference to a builder in some branches @@ -470,16 +491,20 @@ impl<'a> CanvasData<'a> { let new_state = { match self.path_state.as_ref().unwrap() { &PathState::UserSpacePathBuilder(_, None) | - &PathState::DeviceSpacePathBuilder(_) => - None, + &PathState::DeviceSpacePathBuilder(_) => None, &PathState::UserSpacePathBuilder(ref builder, Some(ref transform)) => { let path = builder.finish(); - Some(PathState::DeviceSpacePathBuilder(path.transformed_copy_to_builder(transform))) - } - &PathState::UserSpacePath(ref path, Some(ref transform)) => - Some(PathState::DeviceSpacePathBuilder(path.transformed_copy_to_builder(transform))), - &PathState::UserSpacePath(ref path, None) => - Some(PathState::UserSpacePathBuilder(path.copy_to_builder(), None)), + Some(PathState::DeviceSpacePathBuilder( + path.transformed_copy_to_builder(transform), + )) + }, + &PathState::UserSpacePath(ref path, Some(ref transform)) => Some( + PathState::DeviceSpacePathBuilder(path.transformed_copy_to_builder(transform)), + ), + &PathState::UserSpacePath(ref path, None) => Some(PathState::UserSpacePathBuilder( + path.copy_to_builder(), + None, + )), } }; match new_state { @@ -487,34 +512,32 @@ impl<'a> CanvasData<'a> { Some(state) => self.path_state = Some(state), // There's an existing builder value that can be returned immediately. None => match self.path_state.as_ref().unwrap() { - &PathState::UserSpacePathBuilder(ref builder, None) => + &PathState::UserSpacePathBuilder(ref builder, None) => { return PathBuilderRef { builder, transform: Transform2D::identity(), - }, - &PathState::DeviceSpacePathBuilder(ref builder) => + }; + }, + &PathState::DeviceSpacePathBuilder(ref builder) => { return PathBuilderRef { builder, transform: self.drawtarget.get_transform(), - }, + }; + }, _ => unreachable!(), - } + }, } match self.path_state.as_ref().unwrap() { - &PathState::UserSpacePathBuilder(ref builder, None) => - PathBuilderRef { - builder, - transform: Transform2D::identity(), - }, - &PathState::DeviceSpacePathBuilder(ref builder) => - PathBuilderRef { - builder, - transform: self.drawtarget.get_transform(), - }, - &PathState::UserSpacePathBuilder(..) | - &PathState::UserSpacePath(..) => - unreachable!(), + &PathState::UserSpacePathBuilder(ref builder, None) => PathBuilderRef { + builder, + transform: Transform2D::identity(), + }, + &PathState::DeviceSpacePathBuilder(ref builder) => PathBuilderRef { + builder, + transform: self.drawtarget.get_transform(), + }, + &PathState::UserSpacePathBuilder(..) | &PathState::UserSpacePath(..) => unreachable!(), } } @@ -522,11 +545,7 @@ impl<'a> CanvasData<'a> { self.path_builder().rect(rect); } - pub fn quadratic_curve_to( - &mut self, - cp: &Point2D, - endpoint: &Point2D - ) { + pub fn quadratic_curve_to(&mut self, cp: &Point2D, endpoint: &Point2D) { self.path_builder().quadratic_curve_to(cp, endpoint); } @@ -547,15 +566,11 @@ impl<'a> CanvasData<'a> { end_angle: AzFloat, ccw: bool, ) { - self.path_builder().arc(center, radius, start_angle, end_angle, ccw); + self.path_builder() + .arc(center, radius, start_angle, end_angle, ccw); } - pub fn arc_to( - &mut self, - cp1: &Point2D, - cp2: &Point2D, - radius: AzFloat - ) { + pub fn arc_to(&mut self, cp1: &Point2D, cp2: &Point2D, radius: AzFloat) { let cp0 = match self.path_builder().current_point() { Some(p) => p.as_azure_point(), None => return, @@ -628,7 +643,15 @@ impl<'a> CanvasData<'a> { end_angle: AzFloat, ccw: bool, ) { - self.path_builder().ellipse(center, radius_x, radius_y, rotation_angle, start_angle, end_angle, ccw); + self.path_builder().ellipse( + center, + radius_x, + radius_y, + rotation_angle, + start_angle, + end_angle, + ccw, + ); } pub fn set_fill_style(&mut self, style: FillOrStrokeStyle) { @@ -663,14 +686,13 @@ impl<'a> CanvasData<'a> { // If there is an in-progress path, store the existing transformation required // to move between device and user space. match self.path_state.as_mut() { - None | - Some(PathState::DeviceSpacePathBuilder(..)) => (), + None | Some(PathState::DeviceSpacePathBuilder(..)) => (), Some(PathState::UserSpacePathBuilder(_, ref mut transform)) | Some(PathState::UserSpacePath(_, ref mut transform)) => { if transform.is_none() { *transform = Some(self.drawtarget.get_transform()); } - } + }, } self.state.transform = transform.clone(); self.drawtarget.set_transform(transform)