toji / gl-matrix

Javascript Matrix and Vector library for High Performance WebGL apps
glmatrix.net
MIT License
5.4k stars 725 forks source link

Add mat4 set scaling #435

Closed signorpipo closed 3 years ago

signorpipo commented 3 years ago

Mat4 has getScaling and scale, but no set, and u need to set it by do some "inverse" math. Nothin complex but just add all the methods, not half of them

stefnotch commented 3 years ago

Due to how matrices work, setting the scale isn't quite as easy as setting a few components. One first has to calculate the scaling vector and then multiply it accordingly to get the correct result. https://github.com/toji/gl-matrix/blob/d30bf3ba16449c2228a56754822059dd79d196a4/src/mat4.js#L1152-L1168

I suppose the API, while somewhat inconvenient, reflects on that. My personal suggestion is to

  1. store translation, rotation and scaling as individual parts
  2. operate on those
  3. when it's time to send them to the GPU or something, convert them to matrices
signorpipo commented 3 years ago

I get that it requires some work to create the fuction, but why not adding it anyway to the api, to make them more complete and coherent

stefnotch commented 3 years ago

I fully agree that setter methods like mat4.setScale would be convenient. The downside is that I'd have to add all setter methods (consistency) which in turn would increase the file size. And then unit tests for everything would have to be written.

I barely have the time to answer to issues as is.

signorpipo commented 3 years ago

Of course, don't worry it's still a great work