Turn a PathBuilder into an immutable Path with into_boxed_slice()

A Path contains a boxed slice of PathCommand, which is the
shrunk-to-fit Vec<PathCommand> from the PathBuilder.  The boxed slice
doesn't have the overhead of the capacity value.

For the huge map file in issue #574, this reduces path overhead by
a nice bit:

Before:
--------------------------------------------------------------------------------
  n        time(i)         total(B)   useful-heap(B) extra-heap(B)    stacks(B)
--------------------------------------------------------------------------------
 23 22,751,613,855    1,560,916,408    1,493,746,540    67,169,868            0
95.70% (1,493,746,540B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc.
->43.94% (685,929,024B) 0x49FF0BB: alloc (alloc.rs:84)
| ->43.94% (685,929,024B) 0x49FF0BB: exchange_malloc (alloc.rs:206)
|   ->43.94% (685,929,024B) 0x49FF0BB: new<rsvg_internals::element::Element> (boxed.rs:121)
|   ...
|
->36.05% (562,764,384B) 0x49CC457: realloc (alloc.rs:128)
| ->36.05% (562,764,384B) 0x49CC457: realloc (alloc.rs:187)
|   ->36.05% (562,764,384B) 0x49CC457: reserve_internal<rsvg_internals::path_builder::PathCommand,alloc::alloc::Global> (raw_vec.rs:693)
|     ->36.05% (562,764,384B) 0x49CC457: alloc::raw_vec::RawVec<T,A>::reserve (raw_vec.rs:520)
|       ->22.36% (349,090,560B) 0x49927B0: reserve<rsvg_internals::path_builder::PathCommand> (vec.rs:501)
|       | ->22.36% (349,090,560B) 0x49927B0: push<rsvg_internals::path_builder::PathCommand> (vec.rs:1150)

After:
--------------------------------------------------------------------------------
  n        time(i)         total(B)   useful-heap(B) extra-heap(B)    stacks(B)
--------------------------------------------------------------------------------
 30 22,796,106,012    1,553,581,072    1,329,943,324   223,637,748            0
85.61% (1,329,943,324B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc.
->44.15% (685,929,024B) 0x49FF25B: alloc (alloc.rs:84)
| ->44.15% (685,929,024B) 0x49FF25B: exchange_malloc (alloc.rs:206)
|   ->44.15% (685,929,024B) 0x49FF25B: new<rsvg_internals::element::Element> (boxed.rs:121)
|
The ones with slack space:
->22.53% (350,014,176B) 0x4A23300: realloc (alloc.rs:128)
| ->22.53% (350,014,176B) 0x4A23300: realloc (alloc.rs:187)
|   ->22.53% (350,014,176B) 0x4A23300: shrink_to_fit<rsvg_internals::path_builder::PathCommand,alloc::alloc::Global> (raw_vec.rs:633)
|     ->22.53% (350,014,176B) 0x4A23300: shrink_to_fit<rsvg_internals::path_builder::PathCommand> (vec.rs:623)
|       ->22.53% (350,014,176B) 0x4A23300: alloc::vec::Vec<T>::into_boxed_slice (vec.rs:679)
|         ->22.53% (350,014,176B) 0x4A03410: into_path (path_builder.rs:316)
|
| ...
|
The ones without slack space:
->03.48% (54,029,952B) 0x49CC577: realloc (alloc.rs:128)
| ->03.48% (54,029,952B) 0x49CC577: realloc (alloc.rs:187)
|   ->03.48% (54,029,952B) 0x49CC577: reserve_internal<rsvg_internals::path_builder::PathCommand,alloc::alloc::Global> (raw_vec.rs:693)
|     ->03.48% (54,029,952B) 0x49CC577: alloc::raw_vec::RawVec<T,A>::reserve (raw_vec.rs:520)
|       ->02.98% (46,292,544B) 0x49927E0: reserve<rsvg_internals::path_builder::PathCommand> (vec.rs:501)
|       | ->02.98% (46,292,544B) 0x49927E0: push<rsvg_internals::path_builder::PathCommand> (vec.rs:1150)
|       |   ->02.98% (46,292,544B) 0x49927E0: line_to (path_builder.rs:325)
parent 12904969
......@@ -970,19 +970,19 @@ impl DrawingCtx {
pub fn draw_path(
&mut self,
builder: &PathBuilder,
path: &Path,
node: &Node,
acquired_nodes: &mut AcquiredNodes,
values: &ComputedValues,
markers: Markers,
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
if !builder.is_empty() {
if !path.is_empty() {
let bbox =
self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
let cr = dc.get_cairo_context();
builder.to_cairo(&cr)?;
path.to_cairo(&cr)?;
if clipping {
cr.set_fill_rule(cairo::FillRule::from(values.clip_rule));
......@@ -994,8 +994,8 @@ impl DrawingCtx {
})?;
if markers == Markers::Yes {
marker::render_markers_for_path_builder(
builder,
marker::render_markers_for_path(
path.get_path_commands(),
self,
acquired_nodes,
values,
......
......@@ -19,7 +19,7 @@ use crate::iri::IRI;
use crate::length::*;
use crate::node::{CascadedValues, Node, NodeBorrow, NodeDraw};
use crate::parsers::{Parse, ParseValue};
use crate::path_builder::*;
use crate::path_builder::{arc_segment, ArcParameterization, CubicBezierCurve, PathCommand};
use crate::properties::{ComputedValues, ParsedProperty, SpecifiedValue, SpecifiedValues};
use crate::property_bag::PropertyBag;
use crate::rect::Rect;
......@@ -616,8 +616,8 @@ where
emit_fn(marker_type, x, y, orient)
}
pub fn render_markers_for_path_builder(
builder: &PathBuilder,
pub fn render_markers_for_path(
path_commands: &[PathCommand],
draw_ctx: &mut DrawingCtx,
acquired_nodes: &mut AcquiredNodes,
values: &ComputedValues,
......@@ -640,8 +640,8 @@ pub fn render_markers_for_path_builder(
return Ok(draw_ctx.empty_bbox());
}
emit_markers_for_path_builder(
builder,
emit_markers_for_path(
path_commands,
draw_ctx.empty_bbox(),
&mut |marker_type: MarkerType, x: f64, y: f64, computed_angle: Angle| {
if let &IRI::Resource(ref marker) = match marker_type {
......@@ -666,8 +666,8 @@ pub fn render_markers_for_path_builder(
)
}
fn emit_markers_for_path_builder<E>(
builder: &PathBuilder,
fn emit_markers_for_path<E>(
path_commands: &[PathCommand],
empty_bbox: BoundingBox,
emit_fn: &mut E,
) -> Result<BoundingBox, RenderingError>
......@@ -682,7 +682,7 @@ where
let mut bbox = empty_bbox;
// Convert the path to a list of segments and bare points
let segments = Segments::from(builder.get_path_commands());
let segments = Segments::from(path_commands);
let mut subpath_state = SubpathState::NoSubpath;
......@@ -774,7 +774,7 @@ where
.unwrap_or_else(|| Angle::new(0.0));
let angle = {
if let PathCommand::ClosePath = builder.get_path_commands()[segments.len()] {
if let PathCommand::ClosePath = path_commands[segments.len()] {
let outgoing = segments
.find_outgoing_angle_forwards(0)
.unwrap_or_else(|| Angle::new(0.0));
......@@ -857,6 +857,7 @@ mod parser_tests {
#[cfg(test)]
mod directionality_tests {
use super::*;
use crate::path_builder::PathBuilder;
// Single open path; the easy case
fn setup_open_path() -> Segments {
......@@ -866,7 +867,7 @@ mod directionality_tests {
builder.line_to(20.0, 10.0);
builder.line_to(20.0, 20.0);
Segments::from(builder.get_path_commands())
Segments::from(builder.into_path().get_path_commands())
}
#[test]
......@@ -891,7 +892,7 @@ mod directionality_tests {
builder.curve_to(50.0, 35.0, 60.0, 60.0, 70.0, 70.0);
builder.line_to(80.0, 90.0);
Segments::from(builder.get_path_commands())
Segments::from(builder.into_path().get_path_commands())
}
#[test]
......@@ -916,7 +917,7 @@ mod directionality_tests {
builder.line_to(20.0, 20.0);
builder.close_path();
Segments::from(builder.get_path_commands())
Segments::from(builder.into_path().get_path_commands())
}
#[test]
......@@ -946,7 +947,7 @@ mod directionality_tests {
builder.line_to(80.0, 90.0);
builder.close_path();
Segments::from(builder.get_path_commands())
Segments::from(builder.into_path().get_path_commands())
}
#[test]
......@@ -976,7 +977,7 @@ mod directionality_tests {
builder.line_to(40.0, 30.0);
Segments::from(builder.get_path_commands())
Segments::from(builder.into_path().get_path_commands())
}
#[test]
......@@ -1010,7 +1011,7 @@ mod directionality_tests {
// builder.move_to (30.0, 30.0);
// builder.move_to (40.0, 40.0);
//
// Segments::from(&builder)
// Segments::from(builder.into_path().get_path_commands())
// }
//
// #[test]
......@@ -1105,6 +1106,7 @@ mod directionality_tests {
#[cfg(test)]
mod marker_tests {
use super::*;
use crate::path_builder::PathBuilder;
#[test]
fn emits_for_open_subpath() {
......@@ -1116,8 +1118,8 @@ mod marker_tests {
let mut v = Vec::new();
assert!(emit_markers_for_path_builder(
&builder,
assert!(emit_markers_for_path(
builder.into_path().get_path_commands(),
BoundingBox::new(),
&mut |marker_type: MarkerType,
x: f64,
......@@ -1152,8 +1154,8 @@ mod marker_tests {
let mut v = Vec::new();
assert!(emit_markers_for_path_builder(
&builder,
assert!(emit_markers_for_path(
builder.into_path().get_path_commands(),
BoundingBox::new(),
&mut |marker_type: MarkerType,
x: f64,
......
......@@ -290,16 +290,33 @@ impl PathCommand {
}
}
/// Constructs a path out of commands
///
/// When you are finished constructing a path builder, turn it into
/// a `Path` with `into_path`.
#[derive(Clone, Default)]
pub struct PathBuilder {
path_commands: Vec<PathCommand>,
}
/// An immutable path
///
/// This is constructed from a `PathBuilder` once it is finished.
pub struct Path {
path_commands: Box<[PathCommand]>,
}
impl PathBuilder {
pub fn new() -> PathBuilder {
PathBuilder::default()
}
pub fn into_path(self) -> Path {
Path {
path_commands: self.path_commands.into_boxed_slice(),
}
}
pub fn move_to(&mut self, x: f64, y: f64) {
self.path_commands.push(PathCommand::MoveTo(x, y));
}
......@@ -343,7 +360,9 @@ impl PathBuilder {
pub fn close_path(&mut self) {
self.path_commands.push(PathCommand::ClosePath);
}
}
impl Path {
pub fn get_path_commands(&self) -> &[PathCommand] {
&self.path_commands
}
......@@ -355,7 +374,7 @@ impl PathBuilder {
pub fn to_cairo(&self, cr: &cairo::Context) -> Result<(), cairo::Status> {
assert!(!self.is_empty());
for s in &self.path_commands {
for s in self.path_commands.iter() {
s.to_cairo(cr);
}
......
......@@ -976,7 +976,8 @@ mod tests {
let mut builder = PathBuilder::new();
let result = parse_path_into_builder(path_str, &mut builder);
let commands = builder.get_path_commands();
let path = builder.into_path();
let commands = path.get_path_commands();
assert_eq!(expected_commands, commands);
assert_eq!(expected_result, result);
......
......@@ -13,7 +13,7 @@ use crate::error::*;
use crate::length::*;
use crate::node::{CascadedValues, Node};
use crate::parsers::{optional_comma, Parse, ParseValue};
use crate::path_builder::*;
use crate::path_builder::{LargeArc, Path as SvgPath, PathBuilder, Sweep};
use crate::path_parser;
use crate::properties::ComputedValues;
use crate::property_bag::PropertyBag;
......@@ -26,13 +26,13 @@ pub enum Markers {
}
pub struct Shape {
builder: Rc<PathBuilder>,
path: Rc<SvgPath>,
markers: Markers,
}
impl Shape {
fn new(builder: Rc<PathBuilder>, markers: Markers) -> Shape {
Shape { builder, markers }
fn new(path: Rc<SvgPath>, markers: Markers) -> Shape {
Shape { path, markers }
}
fn draw(
......@@ -44,7 +44,7 @@ impl Shape {
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
draw_ctx.draw_path(
&self.builder,
&self.path,
node,
acquired_nodes,
values,
......@@ -54,12 +54,12 @@ impl Shape {
}
}
fn make_ellipse(cx: f64, cy: f64, rx: f64, ry: f64) -> PathBuilder {
fn make_ellipse(cx: f64, cy: f64, rx: f64, ry: f64) -> SvgPath {
let mut builder = PathBuilder::new();
// Per the spec, rx and ry must be nonnegative
if rx <= 0.0 || ry <= 0.0 {
return builder;
return builder.into_path();
}
// 4/3 * (1-cos 45°)/sin 45° = 4/3 * sqrt(2) - 1
......@@ -107,12 +107,12 @@ fn make_ellipse(cx: f64, cy: f64, rx: f64, ry: f64) -> PathBuilder {
builder.close_path();
builder
builder.into_path()
}
#[derive(Default)]
pub struct Path {
builder: Option<Rc<PathBuilder>>,
path: Option<Rc<SvgPath>>,
}
impl ElementTrait for Path {
......@@ -126,7 +126,7 @@ impl ElementTrait for Path {
rsvg_log!("could not parse path: {}", e);
}
self.builder = Some(Rc::new(builder));
self.path = Some(Rc::new(builder.into_path()));
}
}
......@@ -141,9 +141,9 @@ impl ElementTrait for Path {
draw_ctx: &mut DrawingCtx,
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
if let Some(builder) = self.builder.as_ref() {
if let Some(path) = self.path.as_ref() {
let values = cascaded.get();
Shape::new(builder.clone(), Markers::Yes).draw(
Shape::new(path.clone(), Markers::Yes).draw(
node,
acquired_nodes,
values,
......@@ -194,7 +194,7 @@ impl Parse for Points {
}
}
fn make_poly(points: Option<&Points>, closed: bool) -> PathBuilder {
fn make_poly(points: Option<&Points>, closed: bool) -> SvgPath {
let mut builder = PathBuilder::new();
if let Some(points) = points {
......@@ -211,7 +211,7 @@ fn make_poly(points: Option<&Points>, closed: bool) -> PathBuilder {
}
}
builder
builder.into_path()
}
#[derive(Default)]
......@@ -314,16 +314,18 @@ impl ElementTrait for Line {
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
let values = cascaded.get();
Shape::new(
Rc::new(self.make_path_builder(values, draw_ctx)),
Markers::Yes,
Shape::new(Rc::new(self.make_path(values, draw_ctx)), Markers::Yes).draw(
node,
acquired_nodes,
values,
draw_ctx,
clipping,
)
.draw(node, acquired_nodes, values, draw_ctx, clipping)
}
}
impl Line {
fn make_path_builder(&self, values: &ComputedValues, draw_ctx: &mut DrawingCtx) -> PathBuilder {
fn make_path(&self, values: &ComputedValues, draw_ctx: &mut DrawingCtx) -> SvgPath {
let mut builder = PathBuilder::new();
let params = draw_ctx.get_view_params();
......@@ -336,7 +338,7 @@ impl Line {
builder.move_to(x1, y1);
builder.line_to(x2, y2);
builder
builder.into_path()
}
}
......@@ -392,16 +394,18 @@ impl ElementTrait for Rect {
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
let values = cascaded.get();
Shape::new(
Rc::new(self.make_path_builder(values, draw_ctx)),
Markers::No,
Shape::new(Rc::new(self.make_path(values, draw_ctx)), Markers::No).draw(
node,
acquired_nodes,
values,
draw_ctx,
clipping,
)
.draw(node, acquired_nodes, values, draw_ctx, clipping)
}
}
impl Rect {
fn make_path_builder(&self, values: &ComputedValues, draw_ctx: &mut DrawingCtx) -> PathBuilder {
fn make_path(&self, values: &ComputedValues, draw_ctx: &mut DrawingCtx) -> SvgPath {
let params = draw_ctx.get_view_params();
let x = self.x.normalize(values, &params);
......@@ -438,12 +442,12 @@ impl Rect {
// Per the spec, w,h must be >= 0
if w <= 0.0 || h <= 0.0 {
return builder;
return builder.into_path();
}
// ... and rx,ry must be nonnegative
if rx < 0.0 || ry < 0.0 {
return builder;
return builder.into_path();
}
let half_w = w / 2.0;
......@@ -569,7 +573,7 @@ impl Rect {
builder.close_path();
}
builder
builder.into_path()
}
}
......@@ -605,16 +609,18 @@ impl ElementTrait for Circle {
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
let values = cascaded.get();
Shape::new(
Rc::new(self.make_path_builder(values, draw_ctx)),
Markers::No,
Shape::new(Rc::new(self.make_path(values, draw_ctx)), Markers::No).draw(
node,
acquired_nodes,
values,
draw_ctx,
clipping,
)
.draw(node, acquired_nodes, values, draw_ctx, clipping)
}
}
impl Circle {
fn make_path_builder(&self, values: &ComputedValues, draw_ctx: &mut DrawingCtx) -> PathBuilder {
fn make_path(&self, values: &ComputedValues, draw_ctx: &mut DrawingCtx) -> SvgPath {
let params = draw_ctx.get_view_params();
let cx = self.cx.normalize(values, &params);
......@@ -663,16 +669,18 @@ impl ElementTrait for Ellipse {
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
let values = cascaded.get();
Shape::new(
Rc::new(self.make_path_builder(values, draw_ctx)),
Markers::No,
Shape::new(Rc::new(self.make_path(values, draw_ctx)), Markers::No).draw(
node,
acquired_nodes,
values,
draw_ctx,
clipping,
)
.draw(node, acquired_nodes, values, draw_ctx, clipping)
}
}
impl Ellipse {
fn make_path_builder(&self, values: &ComputedValues, draw_ctx: &mut DrawingCtx) -> PathBuilder {
fn make_path(&self, values: &ComputedValues, draw_ctx: &mut DrawingCtx) -> SvgPath {
let params = draw_ctx.get_view_params();
let cx = self.cx.normalize(values, &params);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment