Skip to main content
gamedev.net
🔒 Locked

mymath library

Started by Yours3!f 23 replies 3.6k views
KulSeran
KulSeran
First off, nice job. I hope the constructive criticism doesn't come off too harsh. But:
1) You're still missing some const correctness, for instance your operator[] on the vector class.
2) You put all the function definitions in cpp files! For such simple functions it is usually better to keep them in the header as inline so that the compiler can, if it chooses to (probably will), inline the function for better speed.
3) You prepended everything with "my". It would probably be better to have put everything in a namespace like "math".
4) <rant> why did you shorten float/bool/int to single letters??????????????????????????? </rant>
5) Why did you sperate all the normal vector / matrix functions off into a single utility header? It makes a lot more sense for stuff like dotproduct to either be in a "vector_functions.h" or in the vector headers themselves.
Zakwayda
Zakwayda
(Some of this will probably duplicate what KulSeran said...)

- Be sure to include constant versions of your operator[]() functions in addition to non-constant versions
- Make sure scalar * vector is supported as well as vector * scalar
- Binary operators should generally be made non-member functions (search the forum archives for 'vector class' for more on this topic)
- Don't implement empty non-virtual destructors (the compiler-generated destructor will suffice in this case)
- operator<<() should take a constant myvec4f&
- Use a namespace rather than the 'my' prefix (as previously mentioned)
- You might consider adding an assertion in your [] operator to catch invalid indices
- Definitely don't typedef built-in types like that (only typedef types that you want to abstract away, such as a real-number type)
- I'd suggest using unsigned integer types wherever negative values wouldn't be sensible (e.g. for the argument to the [] operator)
- Some might suggest inverting and multiplying rather than dividing repeatedly (I don't know how much that matters these days though)
- I think this was just an oversight, but you should be streaming to 'output' in your << operator rather than 'cout'
- Rather than a separate 'return output' statement, you can just write 'return output << "( "...'
- Be sure not to put 'using' statements in header files at file scope (since they will then 'pollute' any file that includes that header)
- Using templates could eliminate a lot of extraneous code that's currently duplicated between your different classes
- The #defines in my_math should be constants (as it is now, those symbols won't observe normal scoping rules)
- Constants should be expressed in terms of other constants where possible (e.g. my_pi_div_180 should be expressed as my_pi / 180.f)
- I doubt 'mymath' should really be derived from all those classes
- The input to acos() should be clamped to the range [-1, 1] to correct for any numerical error
- You might want to note in the documentation that my_get_angle_bw_vec() requires the input vectors to be unit length
- To reduce code duplication, implement your length function in terms of your squared length function
- For the same reason, implement the squared length function in terms of the dot product function
- For reasons of performance and clarity, write x * x rather than pow(x, 2)
- In my_find_normal_vec(), it looks like you have some stuff written out longhand that should be written in terms of operators
- my_rotate_vec3() should probably be written as a matrix-vector multiplication to eliminate code duplication
- Ditto any other transform functions that do the same thing as a matrix-vector mult function
- IMO, matrix storage order should be abstracted away so that you can use 2-d indexing in your code
- There are a few other places where you're doing stuff manually that should be written in terms of operators
- Similarly, you're computing the vector length manually in at least one place rather than using a 'length' function

I didn't proof all the higher-level stuff, or look at all the files, but I think everything else I'd mention is probably along the same lines as the above.

The code is very clean, easy-to-read, and well organized, and the feature set looks good.

Although I'm sure I (or anyone) could figure it out by examining the code, perhaps you could clarify what the conventions used by the library are. In particular, what matrix storage order is used, whether vectors are treated as row vectors or column vectors, and what handedness and z clipping range are assumed by the transform functions for which those conventions matter.

Very nice job overall :)
Yours3&#33;f
Yours3&#33;f
Thank you for the constructive criticism KulSeran, it didnt come off harsh, it's rather the opposite: since it is my first library I created, and I suppose lot of stuff still needs to be updated your comment is really helpful.

1) you mean
instead of this: f &operator[](i num);
I should use this: f &operator[](const i num)?

2)yes I did... It is just how I do things, I like it this way, being organized in a clear way separating the two thing into separate files.

3)yes it was intentional, I wanted to create some kind of identity for the functions, because in IDEs you just type "my_" and you can access all the functions and objects.

4)ooops I left it in... in the game engine I'm working on I use this shortening... I don't know why, that's probably a bad habit (I will correct it.)

5)my concept was to separate operators, functions which can be accessed like this "a.a_function()" --> those belonging to the matrix and vector headers
and functions which may require other type of objects like matrix and vector types, the reason why I put the normalizing function to the mymath.h file is that it isn't really an operator
however if you'd like to i may put it in the myvec3f.h file, so that it can be accessed like this a.my_normalize_vec().

all in all this library helped a LOT this far and I hope everyone can benefit from it.

best regards,
Yours3lf
Yours3&#33;f
Yours3&#33;f
OMG, jyk, lot of useful stuff you wrote... I'm impressed

I will probably correct all the things you wrote (see my reply to KulSeran).

I didn't neither, but I've been using it a while, and I haven't encountered any errors.

Thanks I intend to keep it up.

Yes, it's been on my mind to put these thing in a readme, and I will definitely.

Probably this weekend or next week I'll have some time to correct things so stay tuned

Best regards,
Yours3!f
kdmiller3
kdmiller3
You need two different array index operators:

float &operator[](size_t const index);
float const &operator[](size_t const index) const;


The first is a non-const method returning a non-const reference, allowing assignment to a non-const vector (e.g. "normal[2] = 1.0f").
The second is a const method returning returns a const reference, allowing read-only access to a const vector (e.g. "float z = normal[2]").
Neither alone is sufficient.
Yours3&#33;f
Yours3&#33;f
Hi,

in connection with namespace usage:

is it good if I use it this way?

//vec2 header file
namespace mymath
{
class myvec2f
{
.....
}
}

//vec2 cpp file
mymath::myvec2f::myvec2f(float x, float y)
{
...
}


also then I guess I have to put the mymath class into the same namespace (like above), right?

I haven't decided what namespace to use still thinking (It cannot be mymath because I've got a class named like that right? should I rename the class and use mymath as the namespace name?)

Best regards,
Yours3!f
Yours3&#33;f
Yours3&#33;f
and about templates:

I think they shouldn't be used. I read an article on cplusplus.com, which convinced me of not using them, at least in our case.
I mean do they have any advantage besides letting the programmer write less code? they don't allow separation of interface and implementation, and they make the whole code less understandable. I mean beginner programmers don't even have an idea what templates are... To add because the classes' functions' implementations are very different we wouldn't benefit from using templates.

Best regards,
Yours3!f
Zakwayda
Zakwayda
Regarding namespaces, the idea (or an aspect of it at least) is that once you have a namespace, you no longer need to name your vector class 'myvec2f'; you can just name it 'vec2f' instead. Then, people can reference the class in whatever way they prefer (e.g. scope it explicitly, use a typedef, use a 'using' statement, etc.).

Also, you can avoid having to write mymath::myvec2f::myvec2f(...) by placing all of the associated code in a namespace block, e.g.:

namespace mymath {

myvec2f::myvec2f()
{
}

} // namespace mymath


and about templates:

I think they shouldn't be used. I read an article on cplusplus.com, which convinced me of not using them, at least in our case.
I mean do they have any advantage besides letting the programmer write less code? they don't allow separation of interface and implementation, and they make the whole code less understandable. I mean beginner programmers don't even have an idea what templates are... To add because the classes' functions' implementations are very different we wouldn't benefit from using templates.

Whether to use templates is entirely up to you. However, there's no general "should" or "shouldn't" about it; it really just depends on the circumstances and what your requirements are.

If you need evidence that using templates for a math library is a valid choice, just look at some of the math libraries that do use them (Eigen, GLM, CML, etc). People do use these libraries, which is good evidence, I think, that use of templates is valid in this context. (In fact, it seems to me that more publicly available math libraries use templates than not, although I could be wrong about that.)

Like any other software methodology, use of templates has advantages and disadvantages, and you just have to weigh it all out and decide what's best for you based on your preferences and requirements.

As for how they can be useful, I think that like many tools, the usefulness of templates often first becomes clear when you encounter a problem for which they'd be a good solution. Say you'd never seen a nail before and someone handed you a hammer; you might think that such a tool doesn't have much use. But, the moment you have a nail that needs to be pounded in, the usefulness of said tool immediately becomes completely clear. And so it often is (IMO) with programming tools such as templates as well.

To address some of your specific points:

I mean do they have any advantage besides letting the programmer write less code?[/quote]
Writing less code is good enough reason by itself. Code duplication makes code harder to write, debug, test, and maintain, and should be avoided wherever possible and practical. A math library that offers, say 2-d, 3-d, and 4-d vector classes and 2x2, 3x3, and 4x4 matrix classes will have reams of duplicate or similar code, which is undesirable by any reasonable standard. Use of templates can fix that problem.

To be fair, some might argue that having separate 2-d, 3-d, and 4-d vector classes is preferable, as the required functionality differs somewhat based on dimension. Just remember that there's no 'one size fits all' solution; what works for some may not work for others.

they don't allow separation of interface and implementation[/quote]
I think that ship has sailed, more or less :) Many if not most of the libraries commonly used with C++ (the standard library, Boost, etc.) are heavily template-based. The fact is, templates are not only part of the C++ language, they're arguably integral to it. You can certainly write C++ code without touching templates, but use of them (and the potential exposure of implementation that goes along with them) is entirely idiomatic.

and they make the whole code less understandable[/quote]
Arguably true :) Complex template code can verge on the impenetrable. But, that doesn't mean you shouldn't use templates. Plus, you can write clear code even when using templates, and how extensively you want to use them is entirely up to you. (For example, simply parameterizing the real-number type of a vector class will add very little in terms of code complexity, and will affect readability hardly at all.)

To add because the classes' functions' implementations are very different we wouldn't benefit from using templates.[/quote]
As I mentioned before, differences in functionality between related classes is certainly a consideration. But, the fact that there are libraries that do abstract away these differences shows that it can be done. Granted though, it's not necessarily trivial to do so.

To sum up, use of templates is just an option to consider; no one's saying you have to do it that way. If it's not immediately obvious to you how it would be advantageous, then I'd just stick with what you have.
Yours3&#33;f
Yours3&#33;f
Hi,

thank you jyk it cleared a lot of stuff in my mind about templates...

about binary operators:

I found this in C++ Standard, "Binary Operators": "A binary operator shall be implemented either by a non-static member function with one parameter or by a non-member function with two parameters"

so I guess a binary operator should be non-member only if lval's type is different than rval's, right?

Best regards,
Yours3!f
Zakwayda
Zakwayda

so I guess a binary operator should be non-member only if lval's type is different than rval's, right?

If by 'lval' and 'rval' you mean the types of the left and right arguments to the operator, then no, you can and should implement binary operators as non-member functions even when the arguments are of the same type. For example, a '+' operator that adds two vectors will generally be implemented as a non-member function.
Yours3&#33;f
Yours3&#33;f
Hi jyk,

I've got one more thing to correct, which is:

- Some might suggest inverting and multiplying rather than dividing repeatedly (I don't know how much that matters these days though) [/quote]

where did you find this?

after correcting it I write some examples, and hopefully on Thursday I'll release libmymath v1.1.0

Best regards,
Yours3!f
Zakwayda
Zakwayda

where did you find this?

I can't remember, but it was probably in a vector/scalar division function (i.e. operator/() for a vector and a scalar).
Yours3&#33;f
Yours3&#33;f
Hi everyone,

recently I tried to validate every function libmymath has, and I ran in this issue.
When comparing m3d to mymath, I found that when you call
m3dMatrixMultiply44(result, a, b);
what it does is it multiplies b with a. Now, as far as I know the order of multiplying matrices is important, thus a * b is not equal to b * a.
so when in m3d I write modelview_matrix * projection matrix it does the opposite.
I checked it with some online calculators whether I'm doing something wrong or m3d is just a little bit messed up, and mymath does it right, it really multiplies a with b. So should I just pay attention to whenever I see in the higher level m3d stuff matrix multiplication (for ex. at matrix stacks) I should multiply the matrices in the opposite order?

for ex.:
instead of writing:

void matrix_stack::mult_matrix(m4x4 m)
{
stack[stack_pointer] *= m;
}


should I write the following?

void matrix_stack::mult_matrix(m4x4 m)
{
stack[stack_pointer] = m * stack[stack_pointer] ;
}



Best regards,
Yours3!f
Zakwayda
Zakwayda
I'm not familiar with m3d, but whether you want A*B or B*A generally depends on whether your matrices are set up for use with row vectors or column vectors. With row vectors, A*B applies the respective transforms in the order A->B, while with column vectors it applies the transforms in the order B->A.

If the order seems to be reversed between your math library and another math library, it may be because opposite vector notation conventions are being used. (Note however that from a purely mathematical standpoint, matrix multiplication is always defined in the same way; the differences in order just have to do with how the matrices are being used.)
Yours3&#33;f
Yours3&#33;f

I'm not familiar with m3d, but whether you want A*B or B*A generally depends on whether your matrices are set up for use with row vectors or column vectors. With row vectors, A*B applies the respective transforms in the order A->B, while with column vectors it applies the transforms in the order B->A.

If the order seems to be reversed between your math library and another math library, it may be because opposite vector notation conventions are being used. (Note however that from a purely mathematical standpoint, matrix multiplication is always defined in the same way; the differences in order just have to do with how the matrices are being used.)


umm, ok so that must be because mymath uses row major order, and m3d uses column major order. nice:)
then I'll just do b * a than it'll work nice:)
Zakwayda
Zakwayda

umm, ok so that must be because mymath uses row major order, and m3d uses column major order.

No, row/column-major has to do with the underlying layout in memory; it doesn't affect matrix multiplication order. How matrix multiplication order relates to the order in which transforms are applied is a product of notational convention (i.e. row vs. column vectors), not storage order.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.