Repository navigation
[p5.js 2.0+ Bug Report]: Vector.cross() assumes the vectors have three dimensions #8925
Description
Most appropriate sub-area of p5.js?
- Accessibility
- Color
- Core/Environment/Rendering
- Data
- DOM
- Events
- Image
- IO
- Math
- Typography
- Utilities
- WebGL
- WebGPU
- p5.strands
- Build process
- Unit testing
- Internationalization
- Friendly errors
- Other (specify if possible)
p5.js version
2.3.0
Web browser and version
Brave 1.90.122
Operating system
Linux Mint 22.1
Steps to reproduce this
Steps:
- Create two vectors with more than three dimensions
- Use Vector.cross() to compute their cross product
- Observe no errors and a (meaningless) 3D result
Snippet:
function setup() {
let v1 = createVector(1, 0, 0, 2);
let v2 = createVector(0, 0, 1, -1);
let cp = v1.cross(v2);
print(cp.toString());
}Discussion
The vector cross product is only defined for 3D vectors. I think Vector.cross() should give an error if either vector is not 3D.
Perhaps an special case should be made for 2D vectors, setting their "z" value to zero for the cross product computation. This is in fact what is currently done since the code uses Vector.z, which returns 0 for 2D vectors. Also, the cross() documentation, after stating "This method should only be used with 3D vectors", uses 2D vectors in the examples! Those examples would break if a strict check for three dimensions is implemented. So would the current implementations of angleBetween() and slerp().
This issue applies to both the instance and static methods.
Activity
harshiltewari2004 commentedon Jun 16, 2026
Happy to take this up @ksen0 @davepagurek !
ksen0 commentedon Jun 17, 2026
Sure @harshiltewari2004 ! this one should:
Thanks all!
sidwellr commentedon Jun 17, 2026
Be careful with the timing on this one. Ideally, wait until
angleBetween()andslerp()are fixed (issues #8926 and #8928), which currently work fine for 2D vectors even though they usecross(). But ifcross()prints a warning for 2D vectors, so willangleBetween()andslerp(), which will be confusing to users.ksen0 commentedon Jun 17, 2026
@sidwellr thanks for flagging! Maybe @harshiltewari2004 in your PR you can add a unit test or two to make sure there are no excessive FES calls in anglebetween and slerp? That would be helpful to make sure this doesn't get lost
harshiltewari2004 commentedon Jun 18, 2026
Thanks @ksen0! Confirming my understanding before I start so I build the right thing.
Plan:
Add a non-3D check at the top of the instance cross() (the static p5.Vector.cross() just delegates to it, so this covers both). When either vector's dimensions !== 3, emit a friendly warning via _friendlyError — the FES path that respects disableFriendlyErrors and doesn't throw — then continue and return the result as it does today. So it's purely additive: existing behavior is unchanged except for the heads-up.
Fix the docstring @example blocks: they currently cross two 2D vectors (createVector(1, 0) and createVector(3, 4)), which would trip the new warning. I'll switch them to 3D vectors and update the expected printed output.
One thing to confirm: "anything not 3D" includes the 2D case, which is currently shown in the docs as a supported convenience (z defaults to 0). So 2D cross() calls will start emitting the warning too — that's the reason the docs example needs fixing. Just want to make sure that's the intended scope and not only the >3D case.
I'll open the PR once my current one (#8823) merges, keeping to one PR at a time. Sound good?
harshiltewari2004 commentedon Jun 18, 2026
Good catch @sidwellr — angleBetween() and slerp() both call cross() internally and are meant to work with 2D vectors, so a naive warning would leak through them. I'll scope the warning to direct cross() calls so those two stay quiet on 2D input, and @ksen0 yes — I'll add regression tests covering exactly that: no excessive FES calls from angleBetween()/slerp(), plus a check that a direct non-3D cross() still warns. Thanks both!
ksen0 commentedon Jun 18, 2026
Hi @harshiltewari2004 overall sounds good, some notes:
Yes, 2d (or 1d) should also emit warning
I think the fix should not introduce any exceptional behavior (the general intent with all the Vector updates is to have as little exceptional behavior as possible, but balancing that with no/minimal breaking changes). As sidwellr mentioned, until those other 2 fixes (and make slerp and angleBetween no longer use cross) are in, this fix will be failing the tests. Once the other fixes are in, the tests will pass. The purpose of the tests is to make it very clear when this PR is ready to merge. Does that make sense?
Taking a look at your open PR!
harshiltewari2004 commentedon Jun 18, 2026
Makes sense, thanks @ksen0 — I'll drop the internal bypass and have cross() emit the warning uniformly whenever either vector isn't 3D, no special path. I'll keep the no-excessive-FES tests for angleBetween()/slerp() in this PR; understood they'll fail until #8926/#8928 land and those two stop calling cross(), and that the red→green flip is the signal it's ready to merge. I'll open it as a draft until then so the intent's clear. Thanks for looking at #8823!
ksen0 commentedon Jun 19, 2026
Hi @harshiltewari2004 I've just merged your open FES one, please feel free to file a PR for this one. Thanks!
harshiltewari2004 commentedon Jun 19, 2026
Thanks @ksen0 — glad #8823 is in! I'll start on this now and open it as the draft we discussed: the cross() warning + docs fix, with the angleBetween()/slerp() no-leak tests staying red until #8926/#8928 land and decouple them. Thanks!
harshiltewari2004 commentedon Jun 19, 2026
Hi @ksen0 ,I have opened #8944 (draft) with the warning + docs fix we discussed. It's a draft on purpose — the angleBetween/slerp no-leak tests fail for now since both call cross() internally, and they'll go green once #8926 and #8928 decouple those methods. I'll mark it ready for review once those land. Happy to tweak the warning wording or tests in the meantime.