tidying / performance updates to vec module #92
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -17,7 +17,7 @@ export default function MovableEllipse() { | |||
| color: Theme.blue, | |||
| // Constrain this point to only move in a circle | |||
| constrain: (position) => | |||
| vec.scale(vec.normalize(position), hintRadius), | |||
| vec.withMag(position, hintRadius), | |||
There was a problem hiding this comment.
might as well use it since we have it!
sritchie
changed the title
| */ | ||
| export type Matrix = [number, number, number, number, number, number, number, number, number] | ||
| export type Matrix = [number, number, number, number, number, number] |
There was a problem hiding this comment.
As discussed in Discord, this PR removes the 0 0 1 redundant row from the matrix representation; this change brings the types along for the ride.
| @@ -53,19 +54,18 @@ export namespace vec { | |||
| export function lerp(v1: Vector2, v2: Vector2, t: number): Vector2 { | |||
| const d = sub(v2, v1) | |||
| const m = mag(d) | |||
| return add(v1, scale(normalize(d), t * m)) | |||
| return add(v1, withMag(d, t * m)) | |||
There was a problem hiding this comment.
this should be slightly more efficient by skipping the double scaling.
| m[6] * m2[1] + m[7] * m2[4] + m[8] * m2[7], | ||
| m[6] * m2[2] + m[7] * m2[5] + m[8] * m2[8], | ||
| ] | ||
| return matrixCreate( |
There was a problem hiding this comment.
I made this change by find-replacing each of the [6] and [7] entries with 0 and the [8] entry with 1, then manually simplifying :)
| } | ||
|
|
||
| /** | ||
| * Rotates a vector around the origin. Shorthand for a rotation matrix | ||
| */ | ||
| export function rotate(v: Vector2, a: number): Vector2 { | ||
| return [v[0] * Math.cos(a) - v[1] * Math.sin(a), v[0] * Math.sin(a) + v[1] * Math.cos(a)] | ||
| const c = Math.cos(a) |
There was a problem hiding this comment.
calculate the sin and cos just once.
| v2[0] * Math.cos(a) - v2[1] * Math.sin(a), | ||
| v2[0] * Math.sin(a) + v2[1] * Math.cos(a), | ||
| ]) | ||
| return add(cp, rotate(v2, a)) |
There was a problem hiding this comment.
slightly tidier, yeah?
| export function matrixInvert(matrix: Matrix): Matrix | null { | ||
| const a = matrix | ||
| const out: Matrix = new Array(9) as Matrix | ||
| export function matrixInvert(a: Matrix): Matrix | null { |
There was a problem hiding this comment.
same as above, I did this by find-replacing the 0 and 1 entries and simplifying.
| invDet * -a10, | ||
| invDet * -a01, | ||
| invDet * a00, | ||
| invDet * (a12 * a01 - a02 * a11), |
There was a problem hiding this comment.
the translation back is more "complicated" because the translation happens along the NEW basis vectors, while the translation to get there happened along the old basis vectors.
| const s = Math.sin(a) | ||
| return matrixBuilder(matrixMult(matrixCreate(c, s, -s, c), _m)) | ||
| }, | ||
| scale: (x: number, y: number) => matrixBuilder(matrixMult(matrixCreate(x, 0, 0, y), _m)), |
There was a problem hiding this comment.
it might be nice to pull these out as scaleMatrix, shearMatrix etc constructors.
There was a problem hiding this comment.
totally! I think a lot of things could be optimized/added. might be fun to add a benchmark suite to see what speed improvements are needed.
| @@ -235,5 +219,5 @@ export namespace vec { | |||
| * Create a matrix | |||
| */ | |||
| function matrixCreate(a = 1, b = 0, c = 0, d = 1, tx = 0, ty = 0): vec.Matrix { | |||
| return [a, c, tx, b, d, ty, 0, 0, 1] | |||
| return [a, c, tx, b, d, ty] | |||
|
this is a great contribution! I will release this as version v0.13.0. |
This PR modifies the internal 3x3 transform-and-translate matrix to a 2x3, removing the redundant final row. I then pushed these changes through the matrix inversion and multiplication code, for what should be a nice performance improvement.
I also made a few small changes to the vector code that should save some computation and, hopefully, make the code more understandable to new folks coming in.