Skip to content

Commit d0d9c8e

Browse files
committed
Fix major bug when computing the measure of a point after unnamed anchor
This could result in a big drift when both part didn’t progress at the same pace Signed-off-by: Tristram Gräbener <tristram+git@tristramg.eu>
1 parent 5f8b952 commit d0d9c8e

1 file changed

Lines changed: 28 additions & 39 deletions

File tree

src/lrm_scale.rs

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -216,35 +216,38 @@ impl LrmScale {
216216
+ curve_interval * (scale_position - anchors[0].scale_position()) / scale_interval)
217217
}
218218

219-
/// Returns a measure given a distance along the `Curve`.
219+
/// Returns a [LrmScaleMeasure] given a distance along the `Curve`.
220+
///
220221
/// The corresponding [Anchor] is the named `Anchor` that gives the smallest positive `offset`.
221-
/// If such an `Anchor` does not exists, the first named `Anchor` is used.
222+
/// If such an `Anchor` does not exists, the first named `Anchor` is used and the offset can be negative.
222223
pub fn locate_anchor(
223224
&self,
224225
curve_position: CurvePosition,
225226
) -> Result<LrmScaleMeasure, LrmScaleError> {
226227
// First, we find the nearest named Anchor to the Curve.
228+
// It will be the reference point from which we will compute the offset to the point on the curve
227229
let named_anchor = self
228230
.nearest_named(curve_position)
229231
.ok_or(LrmScaleError::NoAnchorFound)?;
230232

231-
// Then we search the nearest Anchor that will be the reference
232-
// to convert from Curve units to scale units.
233-
let nearest_anchor = if named_anchor.curve_position < curve_position {
234-
self.next_anchor(&named_anchor.name)
235-
.or(self.previous_anchor(&named_anchor.name))
236-
} else {
237-
self.previous_anchor(&named_anchor.name)
238-
.or(self.next_anchor(&named_anchor.name))
239-
}
240-
.ok_or(LrmScaleError::NoAnchorFound)?;
233+
// We need the anchor just before and just after the position to interpolate the scale position
234+
// If we are looking for a curve position that is after the last anchor, we extrapolate from the last two
235+
let anchors = self
236+
.anchors
237+
.windows(2)
238+
.find(|window| window[1].curve_position() >= curve_position)
239+
.or_else(|| self.anchors.windows(2).last())
240+
.ok_or(LrmScaleError::NoAnchorFound)?;
241241

242-
let ratio = (nearest_anchor.scale_position() - named_anchor.scale_position)
243-
/ (nearest_anchor.curve_position() - named_anchor.curve_position);
242+
// We compute a ratio to know how much the scale increases per unit of curve.
243+
// This ratio isn’t always constant due to irregularities in anchor measurements
244+
let ratio = (anchors[0].scale_position() - anchors[1].scale_position())
245+
/ (anchors[0].curve_position() - anchors[1].curve_position());
244246

245247
Ok(LrmScaleMeasure {
246248
anchor_name: named_anchor.name.clone(),
247-
scale_offset: (curve_position - named_anchor.curve_position) * ratio,
249+
scale_offset: (anchors[0].scale_position() - named_anchor.scale_position)
250+
+ (curve_position - anchors[0].curve_position()) * ratio,
248251
})
249252
}
250253

@@ -299,29 +302,6 @@ impl LrmScale {
299302
.or_else(|| self.iter_named().next())
300303
}
301304

302-
// Finds the closest Anchor before the Anchor having the name `name`
303-
fn previous_anchor(&self, name: &str) -> Option<&Anchor> {
304-
self.anchors
305-
.iter()
306-
.rev()
307-
.skip_while(|anchor| match anchor {
308-
Anchor::Named(anchor) => anchor.name != name,
309-
Anchor::Unnamed(_) => true,
310-
})
311-
.nth(1)
312-
}
313-
314-
// Finds the closest Anchor after the Anchor having the name `name`
315-
fn next_anchor(&self, name: &str) -> Option<&Anchor> {
316-
self.anchors
317-
.iter()
318-
.skip_while(|anchor| match anchor {
319-
Anchor::Named(anchor) => anchor.name != name,
320-
Anchor::Unnamed(_) => true,
321-
})
322-
.nth(1)
323-
}
324-
325305
// Iterates only on named Anchor objects
326306
fn iter_named(&self) -> impl DoubleEndedIterator<Item = &NamedAnchor> + '_ {
327307
self.anchors.iter().filter_map(|anchor| match anchor {
@@ -405,12 +385,13 @@ pub(crate) mod tests {
405385

406386
#[test]
407387
fn locate_anchor_with_unnamed() {
408-
// ----Unnamed(100)----A(200)----B(300)----Unnamed(400)---
388+
// ----Unnamed(100)----A(200)----Unnamed(250)----B(300)----Unnamed(400)---
409389
let scale = LrmScale {
410390
id: "id".to_owned(),
411391
anchors: vec![
412392
Anchor::new_unnamed(0., 100., None, properties!()),
413393
Anchor::new_named("a", 1., 200., None, properties!()),
394+
Anchor::new_unnamed(1.2, 250., None, properties!()), // 250 is between A and B, but the scale is a voluntarily off
414395
Anchor::new_named("b", 3., 300., None, properties!()),
415396
Anchor::new_unnamed(4., 400., None, properties!()),
416397
],
@@ -426,12 +407,20 @@ pub(crate) mod tests {
426407
assert_eq!(measure.anchor_name, "a");
427408
assert_eq!(measure.scale_offset, -1.5);
428409

410+
// Named----Unnamed----position----Named
411+
let measure = scale.locate_anchor(275.).unwrap();
412+
assert_eq!(measure.anchor_name, "a");
413+
// The first unnamed is at 0.2 from a.
414+
// The position to find is right in the middle of 250 and 300, so 0.9 from the first unnamed
415+
assert_eq!(measure.scale_offset, 0.2 + 0.9);
416+
429417
// Unnamed----Named----position----Unnamed
430418
let measure = scale.locate_anchor(350.).unwrap();
431419
assert_eq!(measure.anchor_name, "b");
432420
assert_eq!(measure.scale_offset, 0.5);
433421

434422
// Unnamed----Named----Unnamed----position
423+
// This tests we are also able to extrapolate when the position is not between two anchors
435424
let measure = scale.locate_anchor(500.).unwrap();
436425
assert_eq!(measure.anchor_name, "b");
437426
assert_eq!(measure.scale_offset, 2.);

0 commit comments

Comments
 (0)