would you consider changing the API of segseg to reduce memory garbage?

[mreinstein]

Proposals:

Have an out parameter somewhere in the argument list that gets the intersection point copied into it. e.g.,

function segseg (out, pos1, pos2, pos3, pos4) { ... } // returns boolean

I'd also propose using gl-matrix's vec2 convention (an array of 2 numbers): [ x, y ]

benefits:

• not generating a new array object every time this function is invoked will result in less garbage collection. (not a big deal for a few calls to this function, but for a game/simulation which can call this thousands of times per second this can make a big difference
• making the return object of segseg always a boolean rather than a mixed type makes it easier for the vm to optimize
• this format of having an out field first matches very closely what gl-matrix does, moving us torwards slightly better interoperability
• easier to read

cons:

• forms an opinion on vec2/point representation on behalf of the user (the [ x, y ] vs { x, y } data representation debate)
• breaking API change requiring major version bump

thoughts?

[mreinstein]

to give some context on why this came up, I've collected a number of 2d collision calls into one repository, with the goal of standardizing some kind of interface for doing these checks. Your segseg routine is what is powering the segment-segment and segments-segment functions that I've setup (modified with the above changes.)

https://github.com/mreinstein/collision-2d

Ideally I could just import your segseg module directly rather than duplicating and modifying it within my repo.

[tmpvar]

I love this! I wrote vec2 while trying to rewrite a physics engine that made copies of vectors on every op (sigh), but this was before gl-matrix, which I'd much rather use than my own vec2 library in most cases!

How do we handle the collinear case?

Perhaps adding some constants that we can return that signal "yes we intersected, but the intersection is between two collinear lines". Something like segseg.NONE / segseg.COLLINEAR / segseg.INTERSECT hung off of the exported function which aid in testing the result. I'm definitely not wed to this idea, but handing the return code has always been a wart of this library IMO. Open to ideas here, but I don't think a boolean can give enough information... unless I'm wrong about the utility of distinguishing collinear from normal intersections.

forms an opinion on vec2/point representation on behalf of the user (the [ x, y ] vs { x, y } data representation debate)

I'd argue that this is the "standard" way of doing things if you want to go fast 😊

breaking API change requiring major version bump

Again, a non-issue - this is what semver is for! I see your point though, and this can probably be addressed via a few lines in the readme.

[mreinstein]

How do we handle the collinear case?

good question. For the use cases I've been focused on (collision handling) NONE and COLLINEAR are not required because we're just trying to determine if a collision happened or not. But I don't want to assume collinear is completely useless, it's probably useful for some use cases of this function.

a few possible solutions:

• continue to ignore COLLINEAR as a unique return value and just alias the return to a boolean intersection result
• use either string or int enums ('COLLINEAR' | 'NONE' | 'INTERSECT' or 0 | 1 | 2). These both are a little ugly in that string enums are a tiny bit inefficient for comparison and int enums are a tiny bit unintuitive to humans.
• expose some kind of export constants in the library like you mentioned. Something about attaching constants to a function feels icky, I love the idea of this module just exposing a pure, simple function, but that's just my own personal bias probably. :)

[tmpvar]

continue to ignore COLLINEAR as a unique return value and just alias the return to a boolean intersection result

I'm coming around to this approach - in the collinear case, do we care what point is returned as the intersection point? I believe your code doesn't populate intersection if the segments are collinear (here).

If Infinity / NaN weren't so damn insidious in JS I'd maybe suggest packing [Infinity, Infinity] into the out array, but this is likely a terrible, terrible idea. Perhaps copying the first provided point (pos1 in your OP example) to out could act as a signal to a caller that cares about collinear intersections.

[mreinstein]

in the collinear case, do we care what point is returned as the intersection point?

assuming we decide to alias NONE and COLLINEAR to false, we should make it clear in the API doc that intersection point only gets a value copied into it if segseg(...) returns true. A simple usage example could probably convey this clearly, something like:

const intersection = [ 0, 0 ]

if (segseg(seg1Point1, seg1Point2, seg2Point1, seg2Point2, intersection)) {
    // if we get here, intersection is filled in with where the 2 segments overlap
}

[mreinstein]

by the way, I'm happy to submit a PR for this and es modules issue if either of these are something you'd like.

[tmpvar]

sorry for the delay, I'm a big 👍 for this.

[mreinstein]

nice. I'll jam together a PR in the next few hours!

[mreinstein]

closed via 565f1da