Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiline.split and Ray.split are inconsistent with other shapes #196

Open
stanch opened this issue Oct 1, 2024 · 1 comment
Open

Multiline.split and Ray.split are inconsistent with other shapes #196

stanch opened this issue Oct 1, 2024 · 1 comment

Comments

@stanch
Copy link
Contributor

stanch commented Oct 1, 2024

When Segment, Arc or Line are split by a point, they return an array with the two parts (or null and the entire shape).

Multiline

The split method of Multiline is very different. To begin with, it accepts an array of points, which is ok, but not consistent. The major inconsistency though is that it just adds the split point(s) to the multiline and returns the mutated multiline. This is very inconvenient as it does not allow to treat Multiline generically as any other shape. The workaround is to iterate the edges, find the one(s) that end in the splitting point(s), etc, which is quite cumbersome. (Or just ignore this method, iterate the underlying shapes, find one that contains the point and split there.)

It would be much easier if split accepted a single point and returned two multilines¹, like with other shapes. It’s very easy to go back to a single multiline with the added split points — just do new Multiline([...multiline1.toShapes(), ...multiline2.toShapes()]).

Note

¹ Edit: better yet, if one of the split sections is a single shape, it should not be wrapped into a multiline.

I suppose the same could be done for an array of points, but I don’t feel strongly about it as my use case only involves one point 🙃 So the existing split method could be renamed to something like splitEdges and maintain its existing implementation.

Ray

The behavior of Ray is also a bit inconsistent with the rest. When the point coincides with the start of the ray shape, the return value is [shape] whereas for a Segment or Arc, the return value would be [null, shape]. This is, however, much easier to work around.

Note

Edit: This is probably not a sensible change, because both [null, shape] and [shape, null] are equally valid.

Edit: Rays within multilines

What should happen when multilines containing rays are split? This is a bit tricky. I think if a ray is the only thing inside the multiline, it should behave exactly the same as a standalone ray. Otherwise, two parts are always returned.

To illustrate, let’s say we are splitting the following multilines with point(0, 0):

  • multiline([ray(point(0, 0), ...)])[ray(point(0, 0), ...)]
  • multiline([ray(point(0, 0), ...), segment(point(0, 0), ...)])[ray(point(0, 0), ...), segment(point(0, 0), ...)]
  • multiline([segment(..., point(0, 0)), ray(point(0, 0), ...)])[segment(..., point(0, 0)), ray(point(0, 0), ...)]
@stanch
Copy link
Contributor Author

stanch commented Oct 12, 2024

A few more notes:

  • Multilines can cross themselves, so a point can lie on two or more non-adjacent edges. In my use case this does not happen, so it’s sufficient to split at the first edge that contains the point. For a generic method, however, it would be more correct to return more than two multilines when this happens.
  • When the point is on a ray that is at the start of the multiline (and it has more edges), the result of splitting the ray needs to be reversed.

Here’s my current implementation (using lodash):

const splitMultiline = (multiline, point) => {
  const shapes = multiline.toShapes()
  const before = _.takeWhile(shapes, s => !s.contains(point))
  const split = shapes[before.length].split(point)
  // when the point coincides with the origin of the ray, only one shape is returned
  // make it consistent with segments and arcs by adding a second null shape
  const rayFix1 = split.length == 1 ? [null, split[0]] : split
  // when the ray is the first edge, the shapes are in the wrong direction
  // swap them to go from infinity to origin and reverse the segment, if any
  const rayFix2 = before.length == 0 && shapes[0] instanceof Ray ?
    _.reverse(rayFix1.map(s => s instanceof Segment ? s.reverse() : s)) :
    rayFix1
  const [b, a] = rayFix2
  const after = _.drop(shapes, before.length+1)
  const wrap = shapes => {
    const compacted = _.compact(shapes)
    return compacted.length > 1 ? new Multiline(compacted) : compacted[0]
  }
  return [
    wrap([...before, b]),
    wrap([a, ...after])
  ]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant