bug #2 - Mogre.Math.Sin() has problems

19 monts ago, user manski wrote about a bug (hidden in a topic as off-topic).
Later I added it to the bugtracker #2 Mogre.Math.Sin() has problems

Here the quote:

Specifically in my case (using Mogre 1.7.1 beta): Mogre.Math.Sin(0.0f, true) throws a DivisionByZero exception but Mogre.Math.Sin(0.0f, false) doesn't. The same is true for "Cos(0.0f, true)".

Update: Neither of both functions seem to work at all when using lookup tables - no matter what angle is specified.

Update2: Found out that one needs to create a new instance of "Mogre.Math" to initialize its static trig table lookup functions to work. (WTF?) It's not done automatically for whatever reason.


Now I checked the source code.
I don't see what's the problem. (On the other hand, I'm very very tired and unconcentrated now.)
The only crash possibility I see, if the Math constructor has trigTableSize = 0 as parameter.
On the other hand manski wrote about a DivisionByZeroException.

Maybe others have an idea how to fix the bug.

Here is the related code:


The related code in in file MogreMath.h

static inline Real Sin (Real fValue, bool useTables)
{
return (!useTables) ? Real(System::Math::Sin(fValue)) : SinTable(fValue);
}

The method SinTable is in file MogreMath.cpp

Real Math::SinTable (Real fValue)
{
// Convert range to index values, wrap if required
int idx;
if (fValue >= 0)
{
idx = int(fValue * mTrigTableFactor) % mTrigTableSize;
}
else
{
idx = mTrigTableSize - (int(-fValue * mTrigTableFactor) % mTrigTableSize) - 1;
}

return mSinTable[idx];
}


Related code in the same file:

Math::Math() // constructor
{
msAngleUnit = AngleUnit::AU_DEGREE;

mTrigTableSize = 4096;
mTrigTableFactor = mTrigTableSize / Math::TWO_PI;

mSinTable = gcnew array<Real>(mTrigTableSize);
mTanTable = gcnew array<Real>(mTrigTableSize);

buildTrigTables();
}


Math::Math( unsigned int trigTableSize ) // constructor 2
{
msAngleUnit = AngleUnit::AU_DEGREE;

mTrigTableSize = trigTableSize;
mTrigTableFactor = mTrigTableSize / Math::TWO_PI;

mSinTable = gcnew array<Real>(mTrigTableSize);
mTanTable = gcnew array<Real>(mTrigTableSize);

buildTrigTables();
}

//---------------------------------------


void Math::buildTrigTables(void)
{
// Build trig lookup tables
// Could get away with building only PI sized Sin table but simpler this
// way. Who cares, it'll ony use an extra 8k of memory anyway and I like
// simplicity.
Real angle;
for (int i = 0; i < mTrigTableSize; ++i)
{
angle = Math::TWO_PI * i / mTrigTableSize;
mSinTable = sin(angle);
mTanTable = tan(angle);
}
}



Oh, now I get an idea.
A DivisionByZero crash can come from here, when mTrigTableSize is 0.
angle = Math::TWO_PI * i / mTrigTableSize;

This only can happen when constructor Math::Math( unsigned int trigTableSize ) was called with trigTableSize = 0.
Then there will be a crash at building the table.
But as Manski wrote, the crash only happens when calling the Sin() method, not when calling the Math() constructor.

I don't know how to check which Math() constuctor will be called by default.
And in the case with param, how to check the value size.

Perhaps calling the code from a Mogre.dll binary and attach Visual Studio to the process?

A DivisionByZero crash can come from here, when mTrigTableSize is 0.
No, it's caused by:
idx = int(fValue * mTrigTableFactor) % mTrigTableSize;
or
idx = mTrigTableSize - (int(-fValue * mTrigTableFactor) % mTrigTableSize) - 1
in 'Math::SinTable' since 'mTrigTableSize' is zero.

In order to use table lookup you have to call the constructor first:
new Math();
Math.Sin(0, true);

Oh, yes. Modulo can cause a crash, too.
Now my brain state is freshed up (in oposite to last night).

I thought, a math class instance will be created automatically.
Is it wrong? Or just with start value 1?

Perhaps the Ogre code changed to use a different way. We should compare it.
(Our MogreMath class is not wrapped. It's ported for performance reason.)

If it's still needed to create an instance on your own, it's bad for programming, because people need to know this fact.
Perhaps there is a better solution:

1)
If a user calls Math.Sin(x, true) and no lookup-table exists,
build the table and store it in a singleton.
(I'm not shure if this is good/possible. I'm not shure about Singleton usage.)

2)
If a user calls Math.Sin(x, true) and no lookup-table exists,
throw an exception, which tells the programmer, that he needs to create a math instance before.

3)
Better/Other solutions?

new Math();
Math.Sin(0, true);
I don't understand, how it works.
There is no variable which stores a reference to the math class instance.

Or do you mean like this:
Math myMath = new Math();
myMath.Sin(0, true);

On the other hand Math.Sin(,) is static. So how the method will get access to the non-static class members?

I thought, a math class instance will be created automatically.
Is it wrong? Or just with start value 1?

Perhaps the Ogre code changed to use a different way. We should compare it.
(Our MogreMath class is not wrapped. It's ported for performance reason.)
Ogre doesn't create it either (I just put a breakpoint in the constructor), and would probably divide by zero if anyone ever tried to use the table version of Sin and Cos.

Thanks for testing and your feedback.
Good point for noticing Math.Cos(). So we can try to improve both at once.

Interesting to see that you are present in the Mogre forum now, although you use Mogre only one time per year.

At least I added a notice to the XML comments to the source code of both methods.
IMPORTANT: Create an instance of the Math class before you use tables.