From 87b207edc7dcdf1d296e9b6d38623b69d629db19 Mon Sep 17 00:00:00 2001 From: LaurentGom Date: Sun, 26 Dec 2010 15:05:07 +0000 Subject: [PATCH] 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 | 38 ++--- src/SFML/Graphics/Image.cpp | 233 +++++++++++++----------------- src/SFML/Graphics/ImageLoader.cpp | 111 +++++++------- 3 files changed, 179 insertions(+), 203 deletions(-) diff --git a/include/SFML/Graphics/Image.hpp b/include/SFML/Graphics/Image.hpp index 55bb70bc..a447038d 100644 --- a/include/SFML/Graphics/Image.hpp +++ b/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: - /// - /// [r0 g0 b0 a0 r1 g1 b1 a1 r2...] + /// this memory layout: [r0 g0 b0 a0 r1 g1 b1 a1 r2...] + /// If this function fails, the image is left unchanged. /// /// \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 //////////////////////////////////////////////////////////// diff --git a/src/SFML/Graphics/Image.cpp b/src/SFML/Graphics/Image.cpp index 364a49ce..baf3d750 100644 --- a/src/SFML/Graphics/Image.cpp +++ b/src/SFML/Graphics/Image.cpp @@ -74,7 +74,7 @@ Resource() myPixelsFlipped = copy.myPixelsFlipped; // Create the texture - CreateTexture(); + CreateTexture(myWidth, myHeight); } @@ -82,26 +82,32 @@ Resource() Image::~Image() { // Destroy the OpenGL texture - DestroyTexture(); + if (myTexture) + { + GLuint Texture = static_cast(myTexture); + GLCheck(glDeleteTextures(1, &Texture)); + } } //////////////////////////////////////////////////////////// bool Image::LoadFromFile(const std::string& filename) { - // Let the image loader load the image into our pixel array - bool success = priv::ImageLoader::GetInstance().LoadImageFromFile(filename, myPixels, myWidth, myHeight); - - if (success) + // Forward the job to the image loader + std::vector pixels; + unsigned int width; + 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,26 +115,21 @@ bool Image::LoadFromFile(const std::string& filename) //////////////////////////////////////////////////////////// bool Image::LoadFromMemory(const void* data, std::size_t size) { - // Check parameters - if (!data || (size == 0)) - { - Err() << "Failed to image font from memory, no data provided" << std::endl; - return false; - } - - // Let the image loader load the image into our pixel array - bool success = priv::ImageLoader::GetInstance().LoadImageFromMemory(data, size, myPixels, myWidth, myHeight); - - if (success) + // Forward the job to the image loader + std::vector pixels; + unsigned int width; + unsigned int height; + if (priv::ImageLoader::GetInstance().LoadImageFromMemory(data, size, 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; } @@ -138,23 +139,17 @@ bool Image::LoadFromPixels(unsigned int width, unsigned int height, const Uint8* { if (data) { - // Store the texture dimensions - myWidth = width; - 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()) + // Create the internal texture + if (CreateTexture(width, height)) { + // 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 - myWidth = width; - myHeight = height; - - // Resize the pixel buffer - myPixels.resize(myWidth * myHeight * 4); - - // Fill it with the specified color - Uint8* p = &myPixels[0]; - Uint8* end = p + myWidth * myHeight * 4; - while (p < end) + // First create the texture + if (CreateTexture(width, height)) { - *p++ = color.r; - *p++ = color.g; - *p++ = color.b; - *p++ = color.a; - } + // Resize the pixel buffer + myPixels.resize(width * height * 4); + + // Fill it with the specified color + Uint8* ptr = &*myPixels.begin(); + Uint8* end = &*myPixels.end(); + while (ptr < end) + { + *ptr++ = color.r; + *ptr++ = color.g; + *ptr++ = color.b; + *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 - Uint8* p = &myPixels[0]; - Uint8* end = p + myWidth * myHeight * 4; - while (p < end) + // Make sure that the image is not empty + if (!myPixels.empty()) { - if ((p[0] == color.r) && (p[1] == color.g) && (p[2] == color.b) && (p[3] == color.a)) - p[3] = alpha; - p += 4; - } + // Replace the alpha of the pixels that match the transparent color + Uint8* ptr = &*myPixels.begin(); + 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; + // 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(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; } } @@ -424,17 +408,20 @@ const Uint8* Image::GetPixelsPtr() const //////////////////////////////////////////////////////////// void Image::UpdatePixels(const Uint8* pixels) { - GLint previous; - GLCheck(glGetIntegerv(GL_TEXTURE_BINDING_2D, &previous)); + if (pixels && myTexture) + { + GLint previous; + GLCheck(glGetIntegerv(GL_TEXTURE_BINDING_2D, &previous)); - // Update the texture from the array of pixels - GLCheck(glBindTexture(GL_TEXTURE_2D, myTexture)); - GLCheck(glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, myWidth, myHeight, GL_RGBA, GL_UNSIGNED_BYTE, pixels)); + // Update the texture from the array of pixels + GLCheck(glBindTexture(GL_TEXTURE_2D, myTexture)); + GLCheck(glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, myWidth, myHeight, GL_RGBA, GL_UNSIGNED_BYTE, pixels)); - GLCheck(glBindTexture(GL_TEXTURE_2D, previous)); + GLCheck(glBindTexture(GL_TEXTURE_2D, previous)); - myArrayUpdated = false; - myTextureUpdated = true; + myArrayUpdated = false; + myTextureUpdated = true; + } } @@ -444,17 +431,20 @@ void Image::UpdatePixels(const Uint8* pixels, const IntRect& rectangle) // Make sure that the texture is up-to-date EnsureTextureUpdate(); - GLint previous; - GLCheck(glGetIntegerv(GL_TEXTURE_BINDING_2D, &previous)); + if (pixels && myTexture) + { + GLint previous; + GLCheck(glGetIntegerv(GL_TEXTURE_BINDING_2D, &previous)); - // Update the texture from the array of pixels - GLCheck(glBindTexture(GL_TEXTURE_2D, myTexture)); - GLCheck(glTexSubImage2D(GL_TEXTURE_2D, 0, rectangle.Left, rectangle.Top, rectangle.Width, rectangle.Height, GL_RGBA, GL_UNSIGNED_BYTE, pixels)); + // Update the texture from the array of pixels + GLCheck(glBindTexture(GL_TEXTURE_2D, myTexture)); + GLCheck(glTexSubImage2D(GL_TEXTURE_2D, 0, rectangle.Left, rectangle.Top, rectangle.Width, rectangle.Height, GL_RGBA, GL_UNSIGNED_BYTE, pixels)); - GLCheck(glBindTexture(GL_TEXTURE_2D, previous)); + GLCheck(glBindTexture(GL_TEXTURE_2D, previous)); - // The pixel cache is no longer up-to-date - myArrayUpdated = false; + // 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 - myTextureWidth = GetValidSize(myWidth); - myTextureHeight = GetValidSize(myHeight); + // Compute the internal texture dimensions depending on NPOT textures support + unsigned int textureWidth = GetValidSize(width); + 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(myTexture); - GLCheck(glDeleteTextures(1, &Texture)); - myTexture = 0; - myTextureUpdated = true; - myArrayUpdated = true; - } -} - } // namespace sf diff --git a/src/SFML/Graphics/ImageLoader.cpp b/src/SFML/Graphics/ImageLoader.cpp index 122a5851..a154b2df 100644 --- a/src/SFML/Graphics/ImageLoader.cpp +++ b/src/SFML/Graphics/ImageLoader.cpp @@ -87,7 +87,7 @@ bool ImageLoader::LoadImageFromFile(const std::string& filename, std::vector& pixels, unsigned int& width, unsigned int& height) { - // Clear the array (just in case) - pixels.clear(); - - // Load the image and get a pointer to the pixels in memory - int imgWidth, imgHeight, imgChannels; - const unsigned char* buffer = static_cast(data); - unsigned char* ptr = stbi_load_from_memory(buffer, static_cast(size), &imgWidth, &imgHeight, &imgChannels, STBI_rgb_alpha); - - if (ptr) + // Check input parameters + if (data && size) { - // Assign the image properties - width = imgWidth; - height = imgHeight; + // Clear the array (just in case) + pixels.clear(); - // Copy the loaded pixels to the pixel buffer - pixels.resize(width * height * 4); - memcpy(&pixels[0], ptr, pixels.size()); + // Load the image and get a pointer to the pixels in memory + int imgWidth, imgHeight, imgChannels; + const unsigned char* buffer = static_cast(data); + unsigned char* ptr = stbi_load_from_memory(buffer, static_cast(size), &imgWidth, &imgHeight, &imgChannels, STBI_rgb_alpha); - // Free the loaded pixels (they are now in our own pixel buffer) - stbi_image_free(ptr); + if (ptr && imgWidth && imgHeight) + { + // Assign the image properties + width = imgWidth; + height = imgHeight; - return true; + // Copy the loaded pixels to the pixel buffer + pixels.resize(width * height * 4); + memcpy(&pixels[0], ptr, pixels.size()); + + // Free the loaded pixels (they are now in our own pixel buffer) + stbi_image_free(ptr); + + return true; + } + else + { + // Error, failed to load the image + Err() << "Failed to load image from memory. Reason : " << stbi_failure_reason() << std::endl; + + return false; + } } else { - // Error, failed to load the image - Err() << "Failed to load image from memory. Reason : " << stbi_failure_reason() << std::endl; - + Err() << "Failed to load image from memory, no data provided" << std::endl; return false; } } @@ -151,35 +160,39 @@ bool ImageLoader::LoadImageFromMemory(const void* data, std::size_t size, std::v //////////////////////////////////////////////////////////// bool ImageLoader::SaveImageToFile(const std::string& filename, const std::vector& pixels, unsigned int width, unsigned int height) { - // Deduce the image type from its extension - if (filename.size() > 3) + // Make sure the image is not empty + if (!pixels.empty() && width && height) { - // Extract the extension - std::string extension = filename.substr(filename.size() - 3); + // Deduce the image type from its extension + if (filename.size() > 3) + { + // Extract the extension + std::string extension = filename.substr(filename.size() - 3); - if (ToLower(extension) == "bmp") - { - // BMP format - if (stbi_write_bmp(filename.c_str(), width, height, 4, &pixels[0])) - return true; - } - else if (ToLower(extension) == "tga") - { - // TGA format - if (stbi_write_tga(filename.c_str(), width, height, 4, &pixels[0])) - return true; - } - else if(ToLower(extension) == "png") - { - // PNG format - if (stbi_write_png(filename.c_str(), width, height, 4, &pixels[0], 0)) - return true; - } - else if (ToLower(extension) == "jpg") - { - // JPG format - if (WriteJpg(filename, pixels, width, height)) - return true; + if (ToLower(extension) == "bmp") + { + // BMP format + if (stbi_write_bmp(filename.c_str(), width, height, 4, &pixels[0])) + return true; + } + else if (ToLower(extension) == "tga") + { + // TGA format + if (stbi_write_tga(filename.c_str(), width, height, 4, &pixels[0])) + return true; + } + else if(ToLower(extension) == "png") + { + // PNG format + if (stbi_write_png(filename.c_str(), width, height, 4, &pixels[0], 0)) + return true; + } + else if (ToLower(extension) == "jpg") + { + // JPG format + if (WriteJpg(filename, pixels, width, height)) + return true; + } } }