PathSampler::split_range seems to disregard discontinuities in the original path

[arximboldi]

I am trying to use the PathSampler to created dashed patterns as suggested here #673

Here is what my code looks like more or less:

struct Dash {
    pub gap: Float,
    pub length: Float,
}

fn apply_dashes(
    path: lyon::path::Path,
    dashes: &Vec<Dash>,
    line_width: f32,
) -> lyon::path::Path {
    use lyon_algorithms::{
        measure::{PathMeasurements, SampleType},
        path::Path,
    };

    if dashes.len() == 0 || dashes.len() == 1 && dashes[0].gap == 0.0 {
        return path;
    }

    let measurements = PathMeasurements::from_path(&path, 0.001);
    let mut sampler = measurements.create_sampler(&path, SampleType::Distance);
    let length = sampler.length();
    let mut dashed = Path::builder();
    let mut pos = 0.0;
    while pos < length {
        for dash in dashes {
            sampler.split_range(pos..(pos + dash.length * line_width), &mut dashed);
            dashed.end(false);
            pos += dash.length * line_width + dash.gap * line_width;
        }
    }
    return dashed.build();
}

When the original path contains discontinuities (e.g. as produced when using Svg builder and doing move_to() to move to arbitrary points) the discontinuities do not seem to be respected by split_range(), and it create a line connecting the discontinuity.

I am gonna try as a workaround to produce the dashed path in multiple steps (producing one individual path per disjoint segment) but I am concerned with the performance implications.

Or is there perhaps a mistake in my code or something I am not considering?

[nical]

That sounds like a bug in the path sampler. I'll try to have a look soon-ish (no promises, I'm very short on spare time these days).

[arximboldi]

Just chiming back in to report that the workaround has worked: now I create a new path for every discontinuous segment and sample that (accumulating the results in a single builder) and this seems to be working. Something around these lines:

fn apply_dashes<B: lyon::path::builder::PathBuilder>(
    builder: &mut B,
    path: lyon::path::Path,
    dashes: &Vec<Dash>,
    line_width: f32,
) {
    use lyon_algorithms::measure::{PathMeasurements, SampleType};
    let measurements = PathMeasurements::from_path(&path, 0.001);
    let mut sampler = measurements.create_sampler(&path, SampleType::Distance);
    let length = sampler.length();
    let mut pos = 0.0;
    while pos < length {
        for dash in dashes {
            sampler.split_range(pos..(pos + dash.length * line_width), builder);
            pos += dash.length * line_width + dash.gap * line_width;
        }
    }
}

Which is used like this:

    let path = if has_dashes(&r.line_style.dashes) {
        let mut dashed = Path::builder();
        for shape in r.shapes {
            let mut builder = Path::svg_builder();
            draw_shape(&mut builder, shape);
            apply_dashes(
                &mut dashed,
                builder.build(),
                &r.line_style.dashes,
                line_width,
            );
        }
        dashed.build()
    } else {
        let mut builder = Path::svg_builder();
        for shape in r.shapes {
            draw_shape(&mut builder, shape);
        }
        builder.build()
    };

[nical]

A (currently failing) test case for this issue:

#[test]
fn multiple_sub_paths() {
    let mut path = Path::builder();

    path.begin(point(0.0, 0.0));
    path.line_to(point(10.0, 0.0));
    path.end(false);

    path.begin(point(10.0, 10.0));
    path.line_to(point(20.0, 10.0));
    path.end(false);

    let path = path.build();
    let measure = PathMeasurements::from_path(&path, 0.01);
    let mut sampler = measure.create_sampler(&path, SampleType::Normalized);

    let mut dashes = Path::builder();
    sampler.split_range(0.0 .. 0.25, &mut dashes);
    sampler.split_range(0.25 .. 0.5, &mut dashes);
    sampler.split_range(0.5 .. 0.75, &mut dashes);
    sampler.split_range(0.75 .. 1.0, &mut dashes);
    let dashes = dashes.build();

    let mut iter = dashes.iter();

    use crate::path::geom::euclid::approxeq::ApproxEq;

    fn expect_begin(event: Option<path::PathEvent>, pos: Point) {
        println!("- {:?}", event);
        if let Some(path::PathEvent::Begin { at }) = event {
            assert!(at.approx_eq(&pos), "Expected Begin {:?}, got {:?}", pos, at);
        } else {
            panic!("Expected begin, got {:?}", event);
        }    
    }

    fn expect_end(event: Option<path::PathEvent>, pos: Point) {
        println!("- {:?}", event);
        if let Some(path::PathEvent::End { last, .. }) = event {
            assert!(last.approx_eq(&pos), "Expected End {:?}, got {:?}", pos, last);
        } else {
            panic!("Expected end, got {:?}", event);
        }    
    }
    fn expect_line(event: Option<path::PathEvent>, expect_from: Point, expect_to: Point) {
        println!("- {:?}", event);
        if let Some(path::PathEvent::Line { from, to }) = event {
            assert!(from.approx_eq(&expect_from), "Expected line {:?} {:?}, got {:?} {:?}", expect_from, expect_to, from, to);
            assert!(to.approx_eq(&expect_to), "Expected line {:?} {:?}, got {:?} {:?}", expect_from, expect_to, from, to);
        } else {
            panic!("Expected a line {:?} {:?}, got {:?}", expect_from, expect_to, event);
        }    
    }

    expect_begin(iter.next(), point(0.0, 0.0));
    expect_line(iter.next(), point(0.0, 0.0), point(5.0, 0.0));
    expect_end(iter.next(), point(5.0, 0.0));

    expect_begin(iter.next(), point(5.0, 0.0));
    expect_line(iter.next(), point(5.0, 0.0), point(10.0, 0.0));
    expect_end(iter.next(), point(10.0, 0.0));

    expect_begin(iter.next(), point(10.0, 10.0));
    expect_line(iter.next(), point(10.0, 10.0), point(15.0, 10.0));
    expect_end(iter.next(), point(15.0, 10.0));

    expect_begin(iter.next(), point(15.0, 10.0));
    expect_line(iter.next(), point(15.0, 10.0), point(20.0, 10.0));
    expect_end(iter.next(), point(20.0, 10.0));
}

[timmb]

I have a fix for this. Will raise a PR

[timmb]

@nical are you happy for me to include your test in the PR?

[nical]

@timmb Great! Yes, please include the above test in the PR.

[timmb]

PR is up here: #924