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

Minor issues with the Ogre API that can be trivial to fix
BigG
Gnoblar
Posts: 7
Joined: Sat May 29, 2010 11:05 am

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

Post by 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!
User avatar
nmodprime
Gnoblar
Posts: 11
Joined: Sat Apr 23, 2011 3:54 am

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

Post by nmodprime »

BigG wrote: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
Gnoblar
Posts: 7
Joined: Sat May 29, 2010 11:05 am

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

Post by 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!
User avatar
nmodprime
Gnoblar
Posts: 11
Joined: Sat Apr 23, 2011 3:54 am

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

Post by nmodprime »

see the issue tracker for the patch.

Issue #0000443
User avatar
masterfalcon
OGRE Team Member
OGRE Team Member
Posts: 4270
Joined: Sun Feb 25, 2007 4:56 am
Location: Bloomington, MN
x 126

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

Post by masterfalcon »

I will commit the fix very soon