Small fix for Image::setColourAt (added in 1.7.3)

[BigG]

Hey folks,
here's the recently added procedure Image::setColourAt

Code: Select all

    void Image::setColourAt(ColourValue const &cv, size_t x, size_t y, size_t z)
    {
        unsigned char pixelSize = PixelUtil::getNumElemBytes(getFormat());
        PixelUtil::packColour(cv, getFormat(), &((unsigned char *)getData())[pixelSize * (z * getWidth() * getHeight() + y * getWidth() + x)]);
    }

My compiler gave me a warning about conversion from size_t to unsigned char for PixelUtil::getNumElemBytes, so I investigated further and found out that Image already has a member variable m_ucPixelSize which holds exactly the same as pixelSize (this member variable is also used in getColourAt, duh!), so I'd propose simply copy&pasting getColourAt's code and basicly changing setColourAt to:

Code: Select all

    void Image::setColourAt(ColourValue const &cv, size_t x, size_t y, size_t z)
    {
        PixelUtil::packColour(cv, m_eFormat, &m_pBuffer[m_ucPixelSize * (z * m_uWidth * m_uHeight + y * m_uWidth + x)]);
    }

This will not only get rid of the warning, but also speed it up a bit by removing the unnecessary call to PixelUtil::getNumElemBytes!

[nmodprime]

My compiler gave me a warning about conversion from size_t to unsigned char for PixelUtil::getNumElemBytes,

What compiler are you using? And what warning level are you using? I'm using clang++ with default warning levels, and I don't get this warning. I'll redo with g++ and default warning levels to see what happens.

Anyway, I'd like to take care of this one. Unless there are objections to this. It seems reasonable to me.

[BigG]

I was using visual c++ 2010 with the default warning level set up by CMake - I guess it's W3. I don't know, however, whether I was compiling for 32 or 64 bit, I'll try to reproduce it if necessary However, as you said, it's not only the compiler warning that this small fix is about. Thanks in advance for taking initiative on it!

[nmodprime]

see the issue tracker for the patch.

Issue #0000443

[masterfalcon]

I will commit the fix very soon