Menu
▾
▴
glsdk-devel
|
From: H. H. <hen...@gm...> - 2008-08-26 19:28:32
|
The current implementation of glmMatInverse2f is defined thus:
GLMmat2f *
glmMatInverse2f (GLMmat2f *out, const GLMmat2f *m)
{
GLfloat m00, m01, m10, m11;
GLfloat inv_det;
assert (out);
assert (m);
inv_det = 1.0f / glmMatDeterm2f (m);
m00 = m->m00;
m01 = m->m01;
m10 = m->m10;
m11 = m->m11;
out->m00 = m11 * inv_det;
out->m01 = - m01 * inv_det;
out->m10 = - m10 * inv_det;
out->m11 = m00 * inv_det;
return out;
}
(with similar implementations for GLMmat3f and GLMmat4f)
The problem here is that the implementation is incomplete and contains a
bug. Consider the case when determinant is 0. For matrices that have det ==
0 inverses are non-existing. Therefore we will need to include a test to
check whether that is the case:
det = glmMatDeterm2f (m);
if (fabs (det) < error_tolerance)
...
Now two issues arise:
1) What should be the value of error_tolerance? Should we set it up as a
constant or something that the caller provides?
My thoughts: we should set it up as a constant as we want to keep thing
simple. I propose the value of 0.0005
2) What should the routine do if det == 0? Return NULL or do we just do
assertion failure?
My thoughts: we should return NULL and the out matrix is unaffected
--
Henri 'henux' Häkkinen |
|
From: Orhun B. <orh...@gm...> - 2008-08-26 20:35:56
|
I'll ask a question from a completely different view. Note that I am not excatly proposing this, I am just mentioning it so that we can discuss... Do we need to check for a tolerance? In debug, we can do whatever we want (i.e. assert etc) but for release I don't believe we have to check anything. Just continue the calculations as if nothing happened. What happens when you load a singular matrix to OpenGL with glLoadMatrix(or glMultMatrix)? Spec mentions singular matrices several times and it basically says the results are undefined. >From OpenGL Spec 3.0 page 58 If the GL implementation determines that the modelview matrix is uninvertible, then the entries in the inverted matrix are arbitrary. In any case, neither normal transformation nor use of the transformed normal may lead to GL interruption or termination. Why should glmMatInverse be any different? Such low level functions should not have such checks (or such behaviours defined). Higher level code should check whether the matrices they generate are well conditioned. <semiserious> We can always provide "safe" versions, which will have more defined behaviours. glmMatInverse4f_s anyone? </semiserious> -- Orhun Birsoy |
|
From: H. H. <hen...@gm...> - 2008-08-27 15:14:14
|
Yes. I think this is a good point. We do an assert check in debug version, but otherwise continue. I vote for this. On Tue, Aug 26, 2008 at 11:36 PM, Orhun Birsoy <orh...@gm...>wrote: > I'll ask a question from a completely different view. Note that I am not > excatly proposing this, I am just mentioning it so that we can discuss... > > Do we need to check for a tolerance? In debug, we can do whatever we want > (i.e. assert etc) but for release I don't believe we have to check anything. > Just continue the calculations as if nothing happened. > > What happens when you load a singular matrix to OpenGL with glLoadMatrix(or > glMultMatrix)? Spec mentions singular matrices several times and it > basically says the results are undefined. > > From OpenGL Spec 3.0 page 58 > If the GL implementation determines that the modelview > matrix is uninvertible, then the entries in the inverted matrix are > arbitrary. In > any case, neither normal transformation nor use of the transformed normal > may > lead to GL interruption or termination. > > Why should glmMatInverse be any different? Such low level functions should > not have such checks (or such behaviours defined). Higher level code should > check whether the matrices they generate are well conditioned. > > <semiserious> > We can always provide "safe" versions, which will have more defined > behaviours. glmMatInverse4f_s anyone? > </semiserious> > > -- > Orhun Birsoy > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's > challenge > Build the coolest Linux based applications with Moblin SDK & win great > prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > Glsdk-devel mailing list > Gls...@li... > https://lists.sourceforge.net/lists/listinfo/glsdk-devel > > -- Henri 'henux' Häkkinen |
|
From: Andrew G. <and...@gm...> - 2008-08-27 15:18:35
|
A note about unit testing: If you are going to provide error behavior (in
this case 'assert()') you should provide test cases that verify correct
error behavior ("test all code paths").
pudman
On Wed, Aug 27, 2008 at 11:14 AM, Henri Häkkinen <hen...@gm...> wrote:
> Yes. I think this is a good point. We do an assert check in debug version,
> but otherwise continue. I vote for this.
>
> On Tue, Aug 26, 2008 at 11:36 PM, Orhun Birsoy <orh...@gm...>wrote:
>
>> I'll ask a question from a completely different view. Note that I am not
>> excatly proposing this, I am just mentioning it so that we can discuss...
>>
>> Do we need to check for a tolerance? In debug, we can do whatever we want
>> (i.e. assert etc) but for release I don't believe we have to check anything.
>> Just continue the calculations as if nothing happened.
>>
>> What happens when you load a singular matrix to OpenGL with
>> glLoadMatrix(or glMultMatrix)? Spec mentions singular matrices several times
>> and it basically says the results are undefined.
>>
>> From OpenGL Spec 3.0 page 58
>> If the GL implementation determines that the modelview
>> matrix is uninvertible, then the entries in the inverted matrix are
>> arbitrary. In
>> any case, neither normal transformation nor use of the transformed normal
>> may
>> lead to GL interruption or termination.
>>
>> Why should glmMatInverse be any different? Such low level functions should
>> not have such checks (or such behaviours defined). Higher level code should
>> check whether the matrices they generate are well conditioned.
>>
>> <semiserious>
>> We can always provide "safe" versions, which will have more defined
>> behaviours. glmMatInverse4f_s anyone?
>> </semiserious>
>>
>> --
>> Orhun Birsoy
>>
>> -------------------------------------------------------------------------
>> This SF.Net email is sponsored by the Moblin Your Move Developer's
>> challenge
>> Build the coolest Linux based applications with Moblin SDK & win great
>> prizes
>> Grand prize is a trip for two to an Open Source event anywhere in the
>> world
>> http://moblin-contest.org/redirect.php?banner_id=100&url=/
>> _______________________________________________
>> Glsdk-devel mailing list
>> Gls...@li...
>> https://lists.sourceforge.net/lists/listinfo/glsdk-devel
>>
>>
>
>
> --
> Henri 'henux' Häkkinen
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Glsdk-devel mailing list
> Gls...@li...
> https://lists.sourceforge.net/lists/listinfo/glsdk-devel
>
>
|
|
From: Orhun B. <orh...@gm...> - 2008-08-27 16:00:23
|
Is assert a code path? I think it is not. I mean that`s the whole point, the code should have a single path. I always thought unit tests are for release (i.e production), so there will be no error behavior (i.e. assert) in release builds. If this is a problem then I would suggest to remove the assert all together. More on removing the assert: Inverse operation is not something you use when you are a newbie (drawing a cube does not require one or at least not in CPU). When you start using matrix inverses, a well placed, briefly commented assert(glmMatDeterm4f (m) != 0) [or something similar] in a glsdk tutorial will not confuse anybody. Or will it? -- Orhun Birsoy |
|
From: H. H. <hen...@gm...> - 2008-08-27 16:36:51
|
Well no it does not but is that assert even necessary in tutorials? On Wed, Aug 27, 2008 at 7:00 PM, Orhun Birsoy <orh...@gm...> wrote: > Is assert a code path? I think it is not. I mean that`s the whole point, > the code should have a single path. I always thought unit tests are for > release (i.e production), so there will be no error behavior (i.e. assert) > in release builds. If this is a problem then I would suggest to remove the > assert all together. > > More on removing the assert: > Inverse operation is not something you use when you are a newbie (drawing a > cube does not require one or at least not in CPU). When you start using > matrix inverses, a well placed, briefly commented assert(glmMatDeterm4f (m) > != 0) [or something similar] in a glsdk tutorial will not confuse anybody. > Or will it? > > -- > Orhun Birsoy > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's > challenge > Build the coolest Linux based applications with Moblin SDK & win great > prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > Glsdk-devel mailing list > Gls...@li... > https://lists.sourceforge.net/lists/listinfo/glsdk-devel > > -- Henri 'henux' Häkkinen |
|
From: H. H. <hen...@gm...> - 2008-08-27 16:42:22
|
More on the invert: A generic 4x4 invert matrix routine is not actually even very usable or needed routine. You get better performance by noting on special matrix types. For example, inverse of rotation is negative rotation or transpose of the 3x3 matrix and it is faster to calculate inverse of a perspective projection than inverse of generic 4x4 matrix. The reason I included the MatInverse in the API is that people would always ask us "where is the inverse?". Generic inverse is nice to have and "necessary" but usually it is better to do a specialized inverse. On Wed, Aug 27, 2008 at 7:37 PM, Henri Häkkinen <hen...@gm...> wrote: > Well no it does not but is that assert even necessary in tutorials? > > On Wed, Aug 27, 2008 at 7:00 PM, Orhun Birsoy <orh...@gm...>wrote: > >> Is assert a code path? I think it is not. I mean that`s the whole point, >> the code should have a single path. I always thought unit tests are for >> release (i.e production), so there will be no error behavior (i.e. assert) >> in release builds. If this is a problem then I would suggest to remove the >> assert all together. >> >> More on removing the assert: >> Inverse operation is not something you use when you are a newbie (drawing >> a cube does not require one or at least not in CPU). When you start using >> matrix inverses, a well placed, briefly commented assert(glmMatDeterm4f (m) >> != 0) [or something similar] in a glsdk tutorial will not confuse anybody. >> Or will it? >> >> -- >> Orhun Birsoy >> >> ------------------------------------------------------------------------- >> This SF.Net email is sponsored by the Moblin Your Move Developer's >> challenge >> Build the coolest Linux based applications with Moblin SDK & win great >> prizes >> Grand prize is a trip for two to an Open Source event anywhere in the >> world >> http://moblin-contest.org/redirect.php?banner_id=100&url=/ >> _______________________________________________ >> Glsdk-devel mailing list >> Gls...@li... >> https://lists.sourceforge.net/lists/listinfo/glsdk-devel >> >> > > > -- > Henri 'henux' Häkkinen > > -- Henri 'henux' Häkkinen |
|
From: Andrew G. <and...@gm...> - 2008-08-27 17:14:24
|
Re: Is assert a code path? In my opinion "error behavior" qualifies as a code path. If you submit a null to a function and "expect" it to fail assertion then you should be testing for that behavior. I bring it up because yesterday I found a typo in a GLM function which would have been caught if the unit tests tested for error behavior. That said, either test for it (assertions) or don't use them. I think it's perfectly acceptable to say functions will produce undefined results given null input. If that's not acceptable, create some reasonable error behavior *and test for that behavior*. This should be true whether the behavior is an assertion or a "return null and don't modify *out". pudman |
|
From: Orhun B. <orh...@gm...> - 2008-08-27 17:22:31
|
pudman OK, you convinced me for including such cases (asserts) in unit tests. Thanks... One question though, will CUnit be able to override the assert in that function so that the tests can continue? -- Orhun Birsoy |
|
From: H. H. <hen...@gm...> - 2008-08-27 19:59:54
|
ATM I am still in the process of writing test cases and correcting minor
bugs in the GLM. The current implementation holds that the behavior is
undefined if you pass NULL as function arguments:
glmMatInverse2f (&foo, NULL); // BOOM
I use assert's in the implementation to test this. However CUnit does not
have a way to override the assert inside the function glmMatInverse2f:
GLMmat2f *
glmMatInverse2f (GLMmat2f *out, const GLMmat2f *m)
{
...
assert (out);
assert (m);
...
}
And there cannot be because assert is usually defined as a macro.
On Wed, Aug 27, 2008 at 8:22 PM, Orhun Birsoy <orh...@gm...> wrote:
> pudman
> OK, you convinced me for including such cases (asserts) in unit tests.
> Thanks...
>
> One question though, will CUnit be able to override the assert in that
> function so that the tests can continue?
>
> --
> Orhun Birsoy
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Glsdk-devel mailing list
> Gls...@li...
> https://lists.sourceforge.net/lists/listinfo/glsdk-devel
>
>
--
Henri 'henux' Häkkinen |
|
From: H. H. <hen...@gm...> - 2008-08-27 20:01:10
|
... and GLM is compiled as a static lib and linked with glmtest.
On Wed, Aug 27, 2008 at 11:00 PM, Henri Häkkinen <hen...@gm...> wrote:
> ATM I am still in the process of writing test cases and correcting minor
> bugs in the GLM. The current implementation holds that the behavior is
> undefined if you pass NULL as function arguments:
>
> glmMatInverse2f (&foo, NULL); // BOOM
>
> I use assert's in the implementation to test this. However CUnit does not
> have a way to override the assert inside the function glmMatInverse2f:
>
> GLMmat2f *
> glmMatInverse2f (GLMmat2f *out, const GLMmat2f *m)
> {
> ...
> assert (out);
> assert (m);
> ...
> }
>
> And there cannot be because assert is usually defined as a macro.
>
> On Wed, Aug 27, 2008 at 8:22 PM, Orhun Birsoy <orh...@gm...>wrote:
>
>> pudman
>> OK, you convinced me for including such cases (asserts) in unit tests.
>> Thanks...
>>
>> One question though, will CUnit be able to override the assert in that
>> function so that the tests can continue?
>>
>> --
>> Orhun Birsoy
>>
>> -------------------------------------------------------------------------
>> This SF.Net email is sponsored by the Moblin Your Move Developer's
>> challenge
>> Build the coolest Linux based applications with Moblin SDK & win great
>> prizes
>> Grand prize is a trip for two to an Open Source event anywhere in the
>> world
>> http://moblin-contest.org/redirect.php?banner_id=100&url=/
>> _______________________________________________
>> Glsdk-devel mailing list
>> Gls...@li...
>> https://lists.sourceforge.net/lists/listinfo/glsdk-devel
>>
>>
>
>
> --
> Henri 'henux' Häkkinen
>
>
--
Henri 'henux' Häkkinen |
|
From: Andrew G. <and...@gm...> - 2008-08-27 21:18:33
|
> > The current implementation holds that the behavior is undefined if you pass > NULL as function arguments: > > glmMatInverse2f (&foo, NULL); // BOOM If the behavior (in DEBUG mode) is for this function to cause an assertion then that behavior IS in fact defined. If this function is called without assertions enabled then, while the behavior is definitely undefined, it will also likely crash the app (dereferencing null). Differences in behavior between debug and optimized mode can be toxic. I think a lot of the time when developers indicate behavior is "undefined" that the behavior is actually well defined for a particular build but should be relied on as it may change. When I say "build" here I do not mean the difference between a debug and release build. Which would be preferred? Invalid results (a call to MatInverse that seems to do nothing) or an app crash? I realize that I'm just babbling and not offering a solution. I think I would prefer a null or singular matrix to return NULL and not modify *out. That behavior is at least testable with unit testing. The disadvantage is that it may be difficult under normal circumstances to detect the root cause of a bug if a null or singular matrix doesn't cause an instant failure. However this function would not affect the stability of the app. Looking at the #ifndef's in MatInverse2f and 3f (missing in 4f) I see a reliance on assert() in release code. When compiled with NDEBUG the *assert( fabs( det ) < GLM_ERROR_TOLERANCE )* will be absent. Is this this (undefined) behavior what you expect? pudman |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X