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

Add Vector4.multiply() #21065

Merged
merged 2 commits into from
Jan 12, 2021
Merged

Add Vector4.multiply() #21065

merged 2 commits into from
Jan 12, 2021

Conversation

marcofugaro
Copy link
Contributor

Related issue: https://discord.com/channels/685241246557667386/685241247233081362/797517091993419796

Description

This PR adds the Vector4.multiply() method.

Any reason in particular this wasn't there?

@WestLangley
Copy link
Collaborator

Any reason in particular this wasn't there?

See #19495

src/math/Vector4.js Outdated Show resolved Hide resolved
@marcofugaro
Copy link
Contributor Author

See #19495

Wasn't aware of that discussion. It makes sense to me to keep it simple as proposed in #19495 (comment). The PR should be ok now.

@WestLangley
Copy link
Collaborator

@marcofugaro Can you please describe your use case? (Or modify your original post.)

@marcofugaro
Copy link
Contributor Author

@WestLangley there was a discussion about this on discord, where a user wondered why it's not there:

image

@gsimone
Copy link
Contributor

gsimone commented Jan 11, 2021

Chiming in, of course this can be solved in a number of way, but since the other ops are in, I don't see why multiply shouldn't, it's not much maintenance surface to add or documentation 🤷‍♂️

In my case I was trying to make something similar to shadertoy's mouse uniform, which is a v4 with x and y being used for initial POS and z w for final POS. Wanting to add axis invert, I tried multiplying by v4(invertX, invertY, invertX, invertY) and of course it doesn't work.

Ran around the issue by manually setting an m4, but that's a bit weird since I'm then applying the m4 to the v4 and yeah, it gets weird.

I want to keep the chaining style because it's way easier to read when reviewing, compact and maintainable than anything else

@mrdoob mrdoob added this to the r125 milestone Jan 12, 2021
@mrdoob mrdoob merged commit dbf57f8 into mrdoob:dev Jan 12, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jan 12, 2021

Thanks!

@marcofugaro marcofugaro deleted the vec4-multiply branch January 12, 2021 17:27
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

Successfully merging this pull request may close these issues.

None yet

5 participants