Post Reply 
 
Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Is this a bug in code?
06-03-2011, 07:40 AM
Post: #1
Is this a bug in code?
File: src/drawable.cpp
Line: 105
Error: Uninitialized variable: srgb_alpha
Is this a bug?

Code:
// uniforms
    if (uniformsChanged)
    {
        renderModel.clearUniformCache();
        renderModel.uniforms.clear();
        if (transform != MATRIX4<float>()) // only add it if it's not the identity matrix
            renderModel.uniforms.push_back(RenderUniformEntry(transformId, transform.GetArray(), 16));
        if (r != 1 || g != 1 || b != 1 || a != 1) // only add it if it's not the default
        {
            float srgb_alpha[4];
            for (int i = 0; i < 3; i++)
            {
                srgb_alpha[i] = srgb_alpha[i] < 1 ? pow((&r)[i],2.2) : (&r)[i];   // <------This is the line
            }
            srgb_alpha[3] = a;
            renderModel.uniforms.push_back(RenderUniformEntry(colorId, srgb_alpha, 4));
        }
        /*if (decal) // only add it if it's not the default of zero
        {
            float depthOffset = -0.005;
            renderModel.uniforms.push_back(RenderUniformEntry(depthOffsetId, &depthOffset, 1));
        }*/
        
        uniformsChanged = false;
    }



File: src/graphics_gl3v.cpp
Lines: 290, 298 and 306
Error: index 4 out of bounds

Code:
// transform to eyespace (view space)
    MATHVECTOR <float, 4> lightDirection4;
    for (int i = 0; i < 3; i++)
        lightDirection4[i] = lightDirection[i];
    lightDirection4[3] = 0;
    defaultCamera.viewMatrix.MultiplyVector4(&lightDirection4[0]);
    
    // upload to the shaders
    RenderUniformEntry lightDirectionUniform(stringMap.addStringId("eyespaceLightDirection"), &lightDirection4[0], 3);
    renderer.setGlobalUniform(lightDirectionUniform);
    
    // set the reflection strength
    // TODO: read this from the track definition
    float reflectedLightColor[4];
    for (int i = 0; i < 3; i++)
        reflectedLightColor[i] = 1.0;
    reflectedLightColor[4] = 1.;  //<------- HERE!
    renderer.setGlobalUniform(RenderUniformEntry(stringMap.addStringId("reflectedLightColor"), reflectedLightColor, 4));
    
    // set the ambient strength
    // TODO: read this from the track definition
    float ambientLightColor[4];
    for (int i = 0; i < 3; i++)
        ambientLightColor[i] = 1.5;
    ambientLightColor[4] = 1.;  //<------- HERE!
    renderer.setGlobalUniform(RenderUniformEntry(stringMap.addStringId("ambientLightColor"), ambientLightColor, 4));
    
    // set the sun strength
    // TODO: read this from the track definition
    float directionalLightColor[4];
    for (int i = 0; i < 3; i++)
        directionalLightColor[i] = 1.4;
    directionalLightColor[4] = 1.;  //<------- HERE!
    renderer.setGlobalUniform(RenderUniformEntry(stringMap.addStringId("directionalLightColor"), directionalLightColor, 4));

File: src/pathmanager.cpp
Lines: around 80
Error: Conditionals aren't ok...

Code:
//find settings file
    settings_path = home_directory;
    #ifndef _WIN32 //NO WIN32
    settings_path += "/";
    settings_path += SETTINGS_DIR;
    MakeDir(settings_path);
    #else //NO NO WIN32=WIN32
    #ifdef _WIN32 //REDUNDANT
    MakeDir(settings_path+"\\My Documents");
    MakeDir(settings_path+"\\My Documents\\My Games");
    settings_path += "\\My Documents\\My Games\\VDrift";
    MakeDir(settings_path);
    #else //EVEN MORE REDUNDANT
    {
        settings_path += "/";
        settings_path += SETTINGS_DIR;
    }
    #endif
    #endif

I think it should be:
Code:
//find settings file
    settings_path = home_directory;
    #ifdef _WIN32
    MakeDir(settings_path+"\\My Documents");
    MakeDir(settings_path+"\\My Documents\\My Games");
    settings_path += "\\My Documents\\My Games\\VDrift";
    MakeDir(settings_path);
    #else
    settings_path += "/";
    settings_path += SETTINGS_DIR;
    MakeDir(settings_path);
    #endif


File: src/soundbuffer.cpp
Error: Resource leak (unclosed fp var)
Line: around 162?

Code:
bool SOUNDBUFFER::LoadWAV(const std::string & filename, const SOUNDINFO & sound_device_info, std::ostream & error_output)
{
    if (loaded)
        Unload();

    name = filename;

    FILE *fp;

    unsigned int size;

    fp = fopen(filename.c_str(), "rb");
    if (fp)
    {
        char id[5]; //four bytes to hold 'RIFF'

        if (fread(id,sizeof(char),4,fp) != 4) return false; //read in first four bytes
        id[4] = '\0';
        if (!strcmp(id,"RIFF"))
        { //we had 'RIFF' let's continue
            if (fread(&size,sizeof(unsigned int),1,fp) != 1) return false; //read in 32bit size value
            size = ENDIAN_SWAP_32(size);
            if (fread(id,sizeof(char),4,fp)!= 4) return false; //read in 4 byte string now
            if (!strcmp(id,"WAVE"))
            { //this is probably a wave file since it contained "WAVE"
                if (fread(id,sizeof(char),4,fp)!= 4) return false; //read in 4 bytes "fmt ";
                if (!strcmp(id,"fmt "))
                {
                    unsigned int format_length, sample_rate, avg_bytes_sec;
                    short format_tag, channels, block_align, bits_per_sample;

                    if (fread(&format_length, sizeof(unsigned int),1,fp) != 1) return false;
                    format_length = ENDIAN_SWAP_32(format_length);
                    if (fread(&format_tag, sizeof(short), 1, fp) != 1) return false;
                    format_tag = ENDIAN_SWAP_16(format_tag);
                    if (fread(&channels, sizeof(short),1,fp) != 1) return false;
                    channels = ENDIAN_SWAP_16(channels);
                    if (fread(&sample_rate, sizeof(unsigned int), 1, fp) != 1) return false;
                    sample_rate = ENDIAN_SWAP_32(sample_rate);
                    if (fread(&avg_bytes_sec, sizeof(unsigned int), 1, fp) != 1) return false;
                    avg_bytes_sec = ENDIAN_SWAP_32(avg_bytes_sec);
                    if (fread(&block_align, sizeof(short), 1, fp) != 1) return false;
                    block_align = ENDIAN_SWAP_16(block_align);
                    if (fread(&bits_per_sample, sizeof(short), 1, fp) != 1) return false;
                    bits_per_sample = ENDIAN_SWAP_16(bits_per_sample);


                    //new wave seeking code
                    //find data chunk
                    bool found_data_chunk = false;
                    long filepos = format_length + 4 + 4 + 4 + 4 + 4;
                    int chunknum = 0;
                    while (!found_data_chunk && chunknum < 10)
                    {
                        fseek(fp, filepos, SEEK_SET); //seek to the next chunk
                        if (fread(id, sizeof(char), 4, fp) != 4) return false; //read in 'data'
                        if (fread(&size, sizeof(unsigned int), 1, fp) != 1) return false; //how many bytes of sound data we have
                        size = ENDIAN_SWAP_32(size);
                        if (!strcmp(id,"data"))
                        {
                            found_data_chunk = true;
                        }
                        else
                        {
                            filepos += size + 4 + 4;
                        }

                        chunknum++;
                    }

                    if (chunknum >= 10)
                    {
                        //cerr << __FILE__ << "," << __LINE__ << ": Sound file contains more than 10 chunks before the data chunk: " + filename << std::endl;
                        error_output << "Couldn't find wave data in first 10 chunks of " << filename << std::endl;
                        return false;
                    }

                    sound_buffer = new char[size];

                    if (fread(sound_buffer, sizeof(char), size, fp) != size) return false; //read in our whole sound data chunk

#if SDL_BYTEORDER == SDL_BIG_ENDIAN
                    if (bits_per_sample == 16)
                    {
                        for (unsigned int i = 0; i < size/2; i++)
                        {
                            //cout << "preswap i: " << sound_buffer[i] << "preswap i+1: " << sound_buffer[i+1] << std::endl;
                            //short preswap = ((short *)sound_buffer)[i];
                            ((short *)sound_buffer)[i] = ENDIAN_SWAP_16(((short *)sound_buffer)[i]);
                            //cout << "postswap i: " << sound_buffer[i] << "postswap i+1: " << sound_buffer[i+1] << std::endl;
                            //cout << (int) i << "/" << (int) size << std::endl;
                            //short postswap = ((short *)sound_buffer)[i];
                            //cout << preswap << "/" << postswap << std::endl;

                        }
                    }
                    //else if (bits_per_sample != 8)
                    else
                    {
                        error_output << "Sound file with " << bits_per_sample << " bits per sample not supported" << std::endl;
                        return false;
                    }
#endif

                    info = SOUNDINFO(size/(bits_per_sample/8), sample_rate, channels, bits_per_sample/8);
                    SOUNDINFO original_info(size/(bits_per_sample/8), sample_rate, channels, bits_per_sample/8);

                    loaded = true;
                    SOUNDINFO desired_info(original_info.samples, sound_device_info.frequency, original_info.channels, sound_device_info.bytespersample);
                    //ConvertTo(desired_info);
                    if (desired_info == original_info)
                    {

                    }
                    else
                    {
                        error_output << "SOUND FORMAT:" << std::endl;
                        original_info.DebugPrint(error_output);
                        error_output << "DESIRED FORMAT:" << std::endl;
                        desired_info.DebugPrint(error_output);
                        //throw EXCEPTION(__FILE__, __LINE__, "Sound file isn't in desired format: " + filename);
                        //cerr << __FILE__ << "," << __LINE__ << ": Sound file isn't in desired format: " + filename << std::endl;
                        error_output << "Sound file isn't in desired format: "+filename << std::endl;
                        return false;
                    }
                }
                else
                {
                    //throw EXCEPTION(__FILE__, __LINE__, "Sound file doesn't have \"fmt \" header: " + filename);
                    //cerr << __FILE__ << "," << __LINE__ << ": Sound file doesn't have \"fmt \" header: " + filename << std::endl;
                    error_output << "Sound file doesn't have \"fmt\" header: "+filename << std::endl;
                    return false;
                }
            }
            else
            {
                //throw EXCEPTION(__FILE__, __LINE__, "Sound file doesn't have WAVE header: " + filename);
                //cerr << __FILE__ << "," << __LINE__ << ": Sound file doesn't have WAVE header: " + filename << std::endl;
                error_output << "Sound file doesn't have WAVE header: "+filename << std::endl;
                return false;
            }
        }
        else
        {
            //throw EXCEPTION(__FILE__, __LINE__, "Sound file doesn't have RIFF header: " + filename);
            //cerr << __FILE__ << "," << __LINE__ << ": Sound file doesn't have WAVE header: " + filename << std::endl;
            error_output << "Sound file doesn't have RIFF header: "+filename << std::endl;
            return false;
        }
    }
    else
    {
        //throw EXCEPTION(__FILE__, __LINE__, "Can't open sound file: " + filename);
        //cerr << __FILE__ << "," << __LINE__ << ": Can't open sound file: " + filename << std::endl;
        error_output << "Can't open sound file: "+filename << std::endl;
        return false;
    }

    //cout << size << std::endl;
    return true;
}


And a security one: Null pointer deference...
File: src/soundsource.cpp
Line: 34
Code:
SOUNDFILTER & SOUNDSOURCE::GetFilter(int num)
{
    int curnum = 0;
    for (std::list <SOUNDFILTER>::iterator i = filters.begin(); i != filters.end(); ++i)
    {
        if (num == curnum)
            return *i;
        curnum++;
    }
    assert(0);
    SOUNDFILTER * nullfilt = NULL; //Theese 2 last lines!
    return *nullfilt;
}
Find all posts by this user
Quote this message in a reply
06-03-2011, 05:08 PM
Post: #2
 
You've got a patch? Smile
Find all posts by this user
Quote this message in a reply
06-03-2011, 05:18 PM
Post: #3
 
NaN Wrote:You've got a patch? Smile

Give me five minutes! Big Grin
Find all posts by this user
Quote this message in a reply
06-03-2011, 05:41 PM
Post: #4
 
Here is the patch:
In order to view links, you must have to reply to this thread.

Here unsolved code as I'm not shure what to do and don't wan't to screw it... Tongue

Quote:File: src/drawable.cpp
Line: 105
Error: Uninitialized variable: srgb_alpha
Is this a bug?

Code:
// uniforms
   if (uniformsChanged)
   {
      renderModel.clearUniformCache();
      renderModel.uniforms.clear();
      if (transform != MATRIX4<float>()) // only add it if it's not the identity matrix
         renderModel.uniforms.push_back(RenderUniformEntry(transformId, transform.GetArray(), 16));
      if (r != 1 || g != 1 || b != 1 || a != 1) // only add it if it's not the default
      {
         float srgb_alpha[4];
         for (int i = 0; i < 3; i++)
         {
            srgb_alpha[i] = srgb_alpha[i] < 1 ? pow((&r)[i],2.2) : (&r)[i];   // <------This is the line
         }
         srgb_alpha[3] = a;
         renderModel.uniforms.push_back(RenderUniformEntry(colorId, srgb_alpha, 4));
      }
      /*if (decal) // only add it if it's not the default of zero
      {
         float depthOffset = -0.005;
         renderModel.uniforms.push_back(RenderUniformEntry(depthOffsetId, &depthOffset, 1));
      }*/
      
      uniformsChanged = false;
   }

And a security one: Null pointer deference...
File: src/soundsource.cpp
Line: 34

Code:
SOUNDFILTER & SOUNDSOURCE::GetFilter(int num)
{
   int curnum = 0;
   for (std::list <SOUNDFILTER>::iterator i = filters.begin(); i != filters.end(); ++i)
   {
      if (num == curnum)
         return *i;
      curnum++;
   }
   assert(0);
   SOUNDFILTER * nullfilt = NULL; //Theese 2 last lines!
   return *nullfilt;
}
Find all posts by this user
Quote this message in a reply
06-03-2011, 05:47 PM
Post: #5
 
I'll take care of it, thanks.
Find all posts by this user
Quote this message in a reply
06-03-2011, 05:51 PM
Post: #6
 
I've been searching and found this interesting but don't know if it is useful In order to view links, you must have to reply to this thread.

I've never received C or C++ classes. Well I've never been tought programming so I may find interesting things you see normal Tongue
Find all posts by this user
Quote this message in a reply
06-03-2011, 05:59 PM
Post: #7
 
Code:
assert(0);
   SOUNDFILTER * nullfilt = NULL; //Theese 2 last lines!
   return *nullfilt;
The failing assert says that we should never get there, makes the following code unreachable(in the debug build). It is still a hack.
Find all posts by this user
Quote this message in a reply
06-03-2011, 06:04 PM
Post: #8
 
NaN Wrote:
Code:
assert(0);
   SOUNDFILTER * nullfilt = NULL; //Theese 2 last lines!
   return *nullfilt;
The failing assert says that we should never get there, makes the following code unreachable(in the debug build). It is still a hack.

Why not using pointers? Thanks.
Find all posts by this user
Quote this message in a reply
06-03-2011, 06:15 PM
Post: #9
 
A reference guaranties an object instance. A pointer would have to be checked before dereferencing. I guess the original idea has been to have the check(assert) in one place.
Find all posts by this user
Quote this message in a reply
06-03-2011, 06:27 PM
Post: #10
 
Ok. Thanks!
Find all posts by this user
Quote this message in a reply
06-05-2011, 06:37 AM
Post: #11
Re: Is this a bug in code?
Everything fixed except the following bug:

antoniovazquez Wrote:File: src/drawable.cpp
Line: 105
Error: Uninitialized variable: srgb_alpha
Is this a bug?

Code:
// uniforms
    if (uniformsChanged)
    {
        renderModel.clearUniformCache();
        renderModel.uniforms.clear();
        if (transform != MATRIX4<float>()) // only add it if it's not the identity matrix
            renderModel.uniforms.push_back(RenderUniformEntry(transformId, transform.GetArray(), 16));
        if (r != 1 || g != 1 || b != 1 || a != 1) // only add it if it's not the default
        {
            float srgb_alpha[4];
            for (int i = 0; i < 3; i++)
            {
                srgb_alpha[i] = srgb_alpha[i] < 1 ? pow((&r)[i],2.2) : (&r)[i];   // <------This is the line
            }
            srgb_alpha[3] = a;
            renderModel.uniforms.push_back(RenderUniformEntry(colorId, srgb_alpha, 4));
        }
        /*if (decal) // only add it if it's not the default of zero
        {
            float depthOffset = -0.005;
            renderModel.uniforms.push_back(RenderUniformEntry(depthOffsetId, &depthOffset, 1));
        }*/
        
        uniformsChanged = false;
    }

I can't understand what are you trying to do with theese lines:
Code:
float srgb_alpha[4];
            for (int i = 0; i < 3; i++)
            {
                srgb_alpha[i] = srgb_alpha[i] < 1 ? pow((&r)[i],2.2) : (&r)[i];   // <------This is the line
            }

Can anybody explain them?
Find all posts by this user
Quote this message in a reply
06-05-2011, 07:22 AM
Post: #12
 
Looks like gamma correction: In order to view links, you must have to reply to this thread.

I guess the code is meant to be:
Code:
            srgb_alpha[0] = r < 1 ? pow(r, 2.2f) : r;
            srgb_alpha[1] = g < 1 ? pow(g, 2.2f) : g;
            srgb_alpha[2] = b < 1 ? pow(b, 2.2f) : b;
            srgb_alpha[3] = a;

But Joe can tell more I think.
Find all posts by this user
Quote this message in a reply
06-05-2011, 07:48 AM
Post: #13
 
NaN Wrote:Looks like gamma correction: In order to view links, you must have to reply to this thread.

I guess the code is meant to be:
Code:
            srgb_alpha[0] = r < 1 ? pow(r, 2.2f) : r;
            srgb_alpha[1] = g < 1 ? pow(g, 2.2f) : g;
            srgb_alpha[2] = b < 1 ? pow(b, 2.2f) : b;
            srgb_alpha[3] = a;

But Joe can tell more I think.

OK. I could understand ganmma correction but not the code. What you've written is understandable but not the other. Thanks Smile
Find all posts by this user
Quote this message in a reply
Post Reply 


Forum Jump:


User(s) browsing this thread: 1 Guest(s)