From c678cae4985352ac7273717769b46da09ce06ad3 Mon Sep 17 00:00:00 2001 From: Fytch Date: Fri, 11 Nov 2016 11:52:20 +0100 Subject: [PATCH] fixed sf::Image::create Discussion thread: http://en.sfml-dev.org/forums/index.php?topic=20875.0 Basically, the issue with sf::Image::create was, that it would either occupy space, it doesn't need, because std::vector::resize doesn't actually shrink the vector, or reallocate in an inefficient way by needlessly copying the whole old image over. Neither did it grant strong exception safety guarantee because it changed the non-throwing members (m_size) prior to doing the critical stuff (reallocating) which may throw. Changing the order and using a temporary (create-temporary-and-swap idiom; see http://www.gotw.ca/gotw/059.htm) fixes both of these problems. --- src/SFML/Graphics/Image.cpp | 44 ++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/src/SFML/Graphics/Image.cpp b/src/SFML/Graphics/Image.cpp index be3f33fd3..8e5e67eab 100644 --- a/src/SFML/Graphics/Image.cpp +++ b/src/SFML/Graphics/Image.cpp @@ -66,16 +66,12 @@ void Image::create(unsigned int width, unsigned int height, const Color& color) { if (width && height) { - // Assign the new size - m_size.x = width; - m_size.y = height; - - // Resize the pixel buffer - m_pixels.resize(width * height * 4); - + // Create a new pixel buffer first for exception safety's sake + std::vector newPixels(width * height * 4); + // Fill it with the specified color - Uint8* ptr = &m_pixels[0]; - Uint8* end = ptr + m_pixels.size(); + Uint8* ptr = &newPixels[0]; + Uint8* end = ptr + newPixels.size(); while (ptr < end) { *ptr++ = color.r; @@ -83,13 +79,22 @@ void Image::create(unsigned int width, unsigned int height, const Color& color) *ptr++ = color.b; *ptr++ = color.a; } + + // Commit the new pixel buffer + m_pixels.swap(newPixels); + + // Assign the new size + m_size.x = width; + m_size.y = height; } else { - // Create an empty image + // Dump the pixel buffer + std::vector().swap(m_pixels); + + // Assign the new size m_size.x = 0; m_size.y = 0; - m_pixels.clear(); } } @@ -99,21 +104,24 @@ void Image::create(unsigned int width, unsigned int height, const Uint8* pixels) { if (pixels && width && height) { + // Create a new pixel buffer first for exception safety's sake + std::vector newPixels(pixels, pixels + width * height * 4); + + // Commit the new pixel buffer + m_pixels.swap(newPixels); + // Assign the new size m_size.x = width; m_size.y = height; - - // Copy the pixels - std::size_t size = width * height * 4; - m_pixels.resize(size); - std::memcpy(&m_pixels[0], pixels, size); // faster than vector::assign } else { - // Create an empty image + // Dump the pixel buffer + std::vector().swap(m_pixels); + + // Assign the new size m_size.x = 0; m_size.y = 0; - m_pixels.clear(); } }