Reviewed all sf::Image functions to make the behaviour always well-defined and robust

git-svn-id: https://sfml.svn.sourceforge.net/svnroot/sfml/branches/sfml2@1764 4e206d99-4929-0410-ac5d-dfc041789085

include/SFML/Graphics/Image.hpp

@@ -79,6 +79,7 @@ public :
     /// like progressive jpeg.
     /// The maximum size for an image depends on the graphics
     /// driver and can be retrieve with the GetMaximumSize function.
+    /// If this function fails, the image is left unchanged.
     ///
     /// \param filename Path of the image file to load
     ///
@@ -97,6 +98,7 @@ public :
     /// like progressive jpeg.
     /// The maximum size for an image depends on the graphics
     /// driver and can be retrieve with the GetMaximumSize function.
+    /// If this function fails, the image is left unchanged.
     ///
     /// \param data Pointer to the file data in memory
     /// \param size Size of the data to load, in bytes
@@ -113,9 +115,8 @@ public :
     ///
     /// The \a pixels argument must point to an array of 32 bits
     /// RGBA pixels. In other words, the pixel array must have
-    /// this memory layout:
+    /// this memory layout: [r0 g0 b0 a0 r1 g1 b1 a1 r2...]
-    ///
+    /// If this function fails, the image is left unchanged.
-    /// [r0 g0 b0 a0 r1 g1 b1 a1 r2...]
     ///
     /// \param width  Width of the image
     /// \param height Height of the image
@@ -134,7 +135,7 @@ public :
     /// The format of the image is automatically deduced from
     /// the extension. The supported image formats are bmp, png,
     /// tga and jpg. The destination file is overwritten
-    /// if it already exists.
+    /// if it already exists. This function fails if the image is empty.
     ///
     /// \param filename Path of the file to save
     ///
@@ -148,6 +149,8 @@ public :
     ////////////////////////////////////////////////////////////
     /// \brief Create the image and fill it with a unique color
     ///
+    /// If this function fails, the image is left unchanged.
+    ///
     /// \param width  Width of the image
     /// \param height Height of the image
     /// \param color  Fill color
@@ -198,6 +201,7 @@ public :
     /// If \a sourceRect is empty, the whole window is copied.
     /// Warning: this is a slow operation, if you need to draw
     /// dynamic contents to an image then use sf::RenderImage.
+    /// If this function fails, the image is left unchanged.
     ///
     /// \param window     Window to capture
     /// \param sourceRect Sub-rectangle of the screen to copy
@@ -243,9 +247,10 @@ public :
     ///
     /// The returned value points to an array of RGBA pixels made of
     /// 8 bits integers components. The size of the array is
-    /// width * height * 4.
+    /// GetWidth() * GetHeight() * 4.
     /// Warning: the returned pointer may become invalid if you
     /// modify the image, so you should never store it for too long.
+    /// If the image is empty, a null pointer is returned.
     ///
     /// \return Const pointer to the array of pixels
     ///
@@ -259,6 +264,7 @@ public :
     /// the image, and to store RGBA 32 bits pixels.
     /// See the other overload of this function to update only
     /// a sub-rectangle of the image.
+    /// This function does nothing if \a pixels is null.
     ///
     /// \param pixels Array of pixels to write to the image
     ///
@@ -273,6 +279,7 @@ public :
     /// perform any check; thus you're responsible of ensuring that
     /// \a rectangle does not exceed the image size, and that
     /// \a pixels contains enough elements.
+    /// This function does nothing if \a pixels is null.
     /// See the other overload of this function to update the
     /// whole image.
     ///
@@ -398,10 +405,15 @@ private :
     ////////////////////////////////////////////////////////////
     /// \brief Create the OpenGL texture
     ///
-    /// \return True if texture has been successfully created
+    /// If this function fails, the image's members are left unchanged.
+    ///
+    /// \param width Texture width
+    /// \param height Texture height
+    ///
+    /// \return True if texture was successfully created, false if it failed
     ///
     ////////////////////////////////////////////////////////////
-    bool CreateTexture();
+    bool CreateTexture(unsigned int width, unsigned int height);
 
     ////////////////////////////////////////////////////////////
     /// \brief Make sure that the texture in video memory is
@@ -423,18 +435,6 @@ private :
     ////////////////////////////////////////////////////////////
     void Use() const;
 
-    ////////////////////////////////////////////////////////////
-    /// \brief Reset the image attributes and leave a clean empty image
-    ///
-    ////////////////////////////////////////////////////////////
-    void Reset();
-
-    ////////////////////////////////////////////////////////////
-    /// \brief Destroy the OpenGL texture
-    ///
-    ////////////////////////////////////////////////////////////
-    void DestroyTexture();
-
     ////////////////////////////////////////////////////////////
     // Types
     ////////////////////////////////////////////////////////////

src/SFML/Graphics/Image.cpp

@@ -74,7 +74,7 @@ Resource<Image>()
     myPixelsFlipped  = copy.myPixelsFlipped;
 
     // Create the texture
-    CreateTexture();
+    CreateTexture(myWidth, myHeight);
 }
 
 
@@ -82,25 +82,31 @@ Resource<Image>()
 Image::~Image()
 {
     // Destroy the OpenGL texture
-    DestroyTexture();
+    if (myTexture)
+    {
+        GLuint Texture = static_cast<GLuint>(myTexture);
+        GLCheck(glDeleteTextures(1, &Texture));
+    }
 }
 
 
 ////////////////////////////////////////////////////////////
 bool Image::LoadFromFile(const std::string& filename)
 {
-    // Let the image loader load the image into our pixel array
+    // Forward the job to the image loader
-    bool success = priv::ImageLoader::GetInstance().LoadImageFromFile(filename, myPixels, myWidth, myHeight);
+    std::vector<Uint8> pixels;
-
+    unsigned int width;
-    if (success)
+    unsigned int height;
+    if (priv::ImageLoader::GetInstance().LoadImageFromFile(filename, pixels, width, height))
     {
         // Loading succeeded : we can create the texture
-        if (CreateTexture())
+        if (CreateTexture(width, height))
+        {
+            // Copy the pixels
+            myPixels.swap(pixels);
             return true;
         }
-
+    }
-    // Oops... something failed
-    Reset();
 
     return false;
 }
@@ -109,25 +115,20 @@ bool Image::LoadFromFile(const std::string& filename)
 ////////////////////////////////////////////////////////////
 bool Image::LoadFromMemory(const void* data, std::size_t size)
 {
-    // Check parameters
+    // Forward the job to the image loader
-    if (!data || (size == 0))
+    std::vector<Uint8> pixels;
-    {
+    unsigned int width;
-        Err() << "Failed to image font from memory, no data provided" << std::endl;
+    unsigned int height;
-        return false;
+    if (priv::ImageLoader::GetInstance().LoadImageFromMemory(data, size, pixels, width, height))
-    }
-
-    // Let the image loader load the image into our pixel array
-    bool success = priv::ImageLoader::GetInstance().LoadImageFromMemory(data, size, myPixels, myWidth, myHeight);
-
-    if (success)
     {
         // Loading succeeded : we can create the texture
-        if (CreateTexture())
+        if (CreateTexture(width, height))
+        {
+            // Copy the pixels
+            myPixels.swap(pixels);
             return true;
         }
-
+    }
-    // Oops... something failed
-    Reset();
 
     return false;
 }
@@ -138,23 +139,17 @@ bool Image::LoadFromPixels(unsigned int width, unsigned int height, const Uint8*
 {
     if (data)
     {
-        // Store the texture dimensions
+        // Create the internal texture
-        myWidth  = width;
+        if (CreateTexture(width, height))
-        myHeight = height;
-
-        // Fill the pixel buffer with the specified raw data
-        myPixels.resize(width * height * 4);
-        std::memcpy(&myPixels[0], data, myPixels.size());
-
-        // We can create the texture
-        if (CreateTexture())
         {
+            // Copy the pixels
+            std::size_t size = width * height * 4;
+            myPixels.resize(size);
+            std::memcpy(&myPixels[0], data, size); // faster than vector::assign
             return true;
         }
         else
         {
-            // Oops... something failed
-            Reset();
             return false;
         }
     }
@@ -180,33 +175,27 @@ bool Image::SaveToFile(const std::string& filename) const
 ////////////////////////////////////////////////////////////
 bool Image::Create(unsigned int width, unsigned int height, const Color& color)
 {
-    // Store the texture dimensions
+    // First create the texture
-    myWidth  = width;
+    if (CreateTexture(width, height))
-    myHeight = height;
+    {
-
         // Resize the pixel buffer
-    myPixels.resize(myWidth * myHeight * 4);
+        myPixels.resize(width * height * 4);
 
         // Fill it with the specified color
-    Uint8* p   = &myPixels[0];
+        Uint8* ptr = &*myPixels.begin();
-    Uint8* end = p + myWidth * myHeight * 4;
+        Uint8* end = &*myPixels.end();
-    while (p < end)
+        while (ptr < end)
         {
-        *p++ = color.r;
+            *ptr++ = color.r;
-        *p++ = color.g;
+            *ptr++ = color.g;
-        *p++ = color.b;
+            *ptr++ = color.b;
-        *p++ = color.a;
+            *ptr++ = color.a;
         }
 
-    // We can create the texture
-    if (CreateTexture())
-    {
         return true;
     }
     else
     {
-        // Oops... something failed
-        Reset();
         return false;
     }
 }
@@ -215,32 +204,32 @@ bool Image::Create(unsigned int width, unsigned int height, const Color& color)
 ////////////////////////////////////////////////////////////
 void Image::CreateMaskFromColor(const Color& color, Uint8 alpha)
 {
-    // No pixels to replace
-    if (myPixels.empty())
-        return;
-
     // Check if the array of pixels needs to be updated
     EnsureArrayUpdate();
 
-    // Replace the alpha of the pixels that match the transparent color
+    // Make sure that the image is not empty
-    Uint8* p   = &myPixels[0];
+    if (!myPixels.empty())
-    Uint8* end = p + myWidth * myHeight * 4;
-    while (p < end)
     {
-        if ((p[0] == color.r) && (p[1] == color.g) && (p[2] == color.b) && (p[3] == color.a))
+        // Replace the alpha of the pixels that match the transparent color
-            p[3] = alpha;
+        Uint8* ptr = &*myPixels.begin();
-        p += 4;
+        Uint8* end = &*myPixels.end();
+        while (ptr < end)
+        {
+            if ((ptr[0] == color.r) && (ptr[1] == color.g) && (ptr[2] == color.b) && (ptr[3] == color.a))
+                ptr[3] = alpha;
+            ptr += 4;
         }
 
         // The texture will need to be updated
         myTextureUpdated = false;
     }
+}
 
 
 ////////////////////////////////////////////////////////////
 void Image::Copy(const Image& source, unsigned int destX, unsigned int destY, const IntRect& sourceRect, bool applyAlpha)
 {
-    // Make sure both images are valid
+    // Make sure that both images are valid
     if ((source.myWidth == 0) || (source.myHeight == 0) || (myWidth == 0) || (myHeight == 0))
         return;
 
@@ -343,12 +332,8 @@ bool Image::CopyScreen(RenderWindow& window, const IntRect& sourceRect)
         if (srcRect.Height > static_cast<int>(window.GetHeight())) srcRect.Height = window.GetHeight();
     }
 
-    // Store the texture dimensions
-    myWidth  = srcRect.Width;
-    myHeight = srcRect.Height;
-
     // We can then create the texture
-    if (window.SetActive() && CreateTexture())
+    if (window.SetActive() && CreateTexture(srcRect.Width, srcRect.Height))
     {
         GLint previous;
         GLCheck(glGetIntegerv(GL_TEXTURE_BINDING_2D, &previous));
@@ -366,7 +351,6 @@ bool Image::CopyScreen(RenderWindow& window, const IntRect& sourceRect)
     }
     else
     {
-        Reset();
         return false;
     }
 }
@@ -423,6 +407,8 @@ const Uint8* Image::GetPixelsPtr() const
 
 ////////////////////////////////////////////////////////////
 void Image::UpdatePixels(const Uint8* pixels)
+{
+    if (pixels && myTexture)
     {
         GLint previous;
         GLCheck(glGetIntegerv(GL_TEXTURE_BINDING_2D, &previous));
@@ -436,6 +422,7 @@ void Image::UpdatePixels(const Uint8* pixels)
         myArrayUpdated   = false;
         myTextureUpdated = true;
     }
+}
 
 
 ////////////////////////////////////////////////////////////
@@ -444,6 +431,8 @@ void Image::UpdatePixels(const Uint8* pixels, const IntRect& rectangle)
     // Make sure that the texture is up-to-date
     EnsureTextureUpdate();
 
+    if (pixels && myTexture)
+    {
         GLint previous;
         GLCheck(glGetIntegerv(GL_TEXTURE_BINDING_2D, &previous));
 
@@ -456,6 +445,7 @@ void Image::UpdatePixels(const Uint8* pixels, const IntRect& rectangle)
         // The pixel cache is no longer up-to-date
         myArrayUpdated = false;
     }
+}
 
 
 ////////////////////////////////////////////////////////////
@@ -596,27 +586,33 @@ unsigned int Image::GetValidSize(unsigned int size)
 
 
 ////////////////////////////////////////////////////////////
-bool Image::CreateTexture()
+bool Image::CreateTexture(unsigned int width, unsigned int height)
 {
     // Check if texture parameters are valid before creating it
-    if (!myWidth || !myHeight)
+    if (!width || !height)
         return false;
 
-    // Adjust internal texture dimensions depending on NPOT textures support
+    // Compute the internal texture dimensions depending on NPOT textures support
-    myTextureWidth  = GetValidSize(myWidth);
+    unsigned int textureWidth  = GetValidSize(width);
-    myTextureHeight = GetValidSize(myHeight);
+    unsigned int textureHeight = GetValidSize(height);
 
     // Check the maximum texture size
     unsigned int maxSize = GetMaximumSize();
-    if ((myTextureWidth > maxSize) || (myTextureHeight > maxSize))
+    if ((textureWidth > maxSize) || (textureHeight > maxSize))
     {
         Err() << "Failed to create image, its internal size is too high "
-              << "(" << myTextureWidth << "x" << myTextureHeight << ", "
+              << "(" << textureWidth << "x" << textureHeight << ", "
               << "maximum is " << maxSize << "x" << maxSize << ")"
               << std::endl;
         return false;
     }
 
+    // All the validity checks passed, we can store the new texture settings
+    myWidth         = width;
+    myHeight        = height;
+    myTextureWidth  = textureWidth;
+    myTextureHeight = textureHeight;
+
     // Create the OpenGL texture if it doesn't exist yet
     if (!myTexture)
     {
@@ -729,37 +725,4 @@ void Image::Use() const
     EnsureTextureUpdate();
 }
 
-
-////////////////////////////////////////////////////////////
-void Image::Reset()
-{
-    DestroyTexture();
-
-    myWidth          = 0;
-    myHeight         = 0;
-    myTextureWidth   = 0;
-    myTextureHeight  = 0;
-    myTexture        = 0;
-    myIsSmooth       = true;
-    myTextureUpdated = true;
-    myArrayUpdated   = true;
-    myPixelsFlipped  = false;
-    myPixels.clear();
-}
-
-
-////////////////////////////////////////////////////////////
-void Image::DestroyTexture()
-{
-    // Destroy the internal texture
-    if (myTexture)
-    {
-        GLuint Texture = static_cast<GLuint>(myTexture);
-        GLCheck(glDeleteTextures(1, &Texture));
-        myTexture        = 0;
-        myTextureUpdated = true;
-        myArrayUpdated   = true;
-    }
-}
-
 } // namespace sf

src/SFML/Graphics/ImageLoader.cpp

@@ -87,7 +87,7 @@ bool ImageLoader::LoadImageFromFile(const std::string& filename, std::vector<Uin
     int imgWidth, imgHeight, imgChannels;
     unsigned char* ptr = stbi_load(filename.c_str(), &imgWidth, &imgHeight, &imgChannels, STBI_rgb_alpha);
 
-    if (ptr)
+    if (ptr && imgWidth && imgHeight)
     {
         // Assign the image properties
         width  = imgWidth;
@@ -114,6 +114,9 @@ bool ImageLoader::LoadImageFromFile(const std::string& filename, std::vector<Uin
 
 ////////////////////////////////////////////////////////////
 bool ImageLoader::LoadImageFromMemory(const void* data, std::size_t size, std::vector<Uint8>& pixels, unsigned int& width, unsigned int& height)
+{
+    // Check input parameters
+    if (data && size)
     {
         // Clear the array (just in case)
         pixels.clear();
@@ -123,7 +126,7 @@ bool ImageLoader::LoadImageFromMemory(const void* data, std::size_t size, std::v
         const unsigned char* buffer = static_cast<const unsigned char*>(data);
         unsigned char* ptr = stbi_load_from_memory(buffer, static_cast<int>(size), &imgWidth, &imgHeight, &imgChannels, STBI_rgb_alpha);
 
-    if (ptr)
+        if (ptr && imgWidth && imgHeight)
         {
             // Assign the image properties
             width  = imgWidth;
@@ -146,10 +149,19 @@ bool ImageLoader::LoadImageFromMemory(const void* data, std::size_t size, std::v
             return false;
         }
     }
+    else
+    {
+        Err() << "Failed to load image from memory, no data provided" << std::endl;
+        return false;
+    }
+}
 
 
 ////////////////////////////////////////////////////////////
 bool ImageLoader::SaveImageToFile(const std::string& filename, const std::vector<Uint8>& pixels, unsigned int width, unsigned int height)
+{
+    // Make sure the image is not empty
+    if (!pixels.empty() && width && height)
     {
         // Deduce the image type from its extension
         if (filename.size() > 3)
@@ -182,6 +194,7 @@ bool ImageLoader::SaveImageToFile(const std::string& filename, const std::vector
                     return true;
             }
         }
+    }
 
     Err() << "Failed to save image \"" << filename << "\"" << std::endl;
     return false;