From 634c3dc2a7a49c5b3b0cdb222d3f4e0ba08e5d62 Mon Sep 17 00:00:00 2001 From: Jonathan De Wachter Date: Sun, 17 Nov 2013 05:31:40 +0100 Subject: [PATCH] [Android] Fixed memory leak when loading asset files --- include/SFML/Graphics/Font.hpp | 9 +- include/SFML/Graphics/Image.hpp | 9 ++ src/SFML/Audio/SoundFile.cpp | 50 +++------ src/SFML/Audio/SoundFile.hpp | 3 + src/SFML/Graphics/Font.cpp | 50 +++------ src/SFML/Graphics/Image.cpp | 61 +++++------ src/SFML/System/Android/ResourceStream.cpp | 85 ++++++++++++++++ src/SFML/System/Android/ResourceStream.hpp | 113 +++++++++++++++++++++ src/SFML/System/CMakeLists.txt | 4 +- 9 files changed, 275 insertions(+), 109 deletions(-) create mode 100644 src/SFML/System/Android/ResourceStream.cpp create mode 100644 src/SFML/System/Android/ResourceStream.hpp diff --git a/include/SFML/Graphics/Font.hpp b/include/SFML/Graphics/Font.hpp index cbbc4a121..245f24a25 100644 --- a/include/SFML/Graphics/Font.hpp +++ b/include/SFML/Graphics/Font.hpp @@ -305,6 +305,9 @@ private : Info m_info; ///< Information about the font mutable PageTable m_pages; ///< Table containing the glyphs pages by character size mutable std::vector m_pixelBuffer; ///< Pixel buffer holding a glyph's pixels before being written to the texture + #ifdef SFML_SYSTEM_ANDROID + void* m_stream; ///< Asset file streamer (if loaded from file) + #endif }; } // namespace sf @@ -351,19 +354,19 @@ private : /// \code /// // Declare a new font /// sf::Font font; -/// +/// /// // Load it from a file /// if (!font.loadFromFile("arial.ttf")) /// { /// // error... /// } -/// +/// /// // Create a text which uses our font /// sf::Text text1; /// text1.setFont(font); /// text1.setCharacterSize(30); /// text1.setStyle(sf::Text::Regular); -/// +/// /// // Create another text using the same font, but with different parameters /// sf::Text text2; /// text2.setFont(font); diff --git a/include/SFML/Graphics/Image.hpp b/include/SFML/Graphics/Image.hpp index dea9675d6..a201c775f 100644 --- a/include/SFML/Graphics/Image.hpp +++ b/include/SFML/Graphics/Image.hpp @@ -55,6 +55,12 @@ public : //////////////////////////////////////////////////////////// Image(); + //////////////////////////////////////////////////////////// + /// \brief Destructor + /// + //////////////////////////////////////////////////////////// + ~Image(); + //////////////////////////////////////////////////////////// /// \brief Create the image and fill it with a unique color /// @@ -259,6 +265,9 @@ private : //////////////////////////////////////////////////////////// Vector2u m_size; ///< Image size std::vector m_pixels; ///< Pixels of the image + #ifdef SFML_SYSTEM_ANDROID + void* m_stream; ///< Asset file streamer (if loaded from file) + #endif }; } // namespace sf diff --git a/src/SFML/Audio/SoundFile.cpp b/src/SFML/Audio/SoundFile.cpp index a16b24206..f49515e80 100644 --- a/src/SFML/Audio/SoundFile.cpp +++ b/src/SFML/Audio/SoundFile.cpp @@ -27,8 +27,7 @@ //////////////////////////////////////////////////////////// #include #ifdef SFML_SYSTEM_ANDROID - #include - #include + #include #endif #include #include @@ -59,7 +58,11 @@ m_sampleCount (0), m_channelCount(0), m_sampleRate (0) { + #ifdef SFML_SYSTEM_ANDROID + m_resourceStream = NULL; + + #endif } @@ -68,6 +71,13 @@ SoundFile::~SoundFile() { if (m_file) sf_close(m_file); + + #ifdef SFML_SYSTEM_ANDROID + + if (m_resourceStream) + delete (priv::ResourceStream*)m_resourceStream; + + #endif } @@ -118,39 +128,11 @@ bool SoundFile::openRead(const std::string& filename) #else - priv::ActivityStates* states = priv::getActivity(NULL); - Lock lock(states->mutex); + if (m_resourceStream) + delete (priv::ResourceStream*)m_resourceStream; - // Open the file - AAsset* file = NULL; - file = AAssetManager_open(states->activity->assetManager, filename.c_str(), AASSET_MODE_UNKNOWN); - - if (!file) - { - // File not found, abording... - err() << "Failed to load sound \"" << filename << "\" (couldn't find it)" << std::endl; - return false; - } - - // Copy into memory - off_t size = AAsset_getLength(file); - void* data = malloc(size); - int status = AAsset_read(file, data, size); - - if (status <= 0) - { - // Something went wrong while we were copying, reading error... - err() << "Failed to load sound \"" << filename << "\" (couldn't read it)" << std::endl; - return false; - } - - // Load from our fresh memory - bool ret = openRead(data, size); - - // Close the file - AAsset_close(file); - - return ret; + m_resourceStream = new priv::ResourceStream(filename); + return openRead(*(priv::ResourceStream*)m_resourceStream); #endif } diff --git a/src/SFML/Audio/SoundFile.hpp b/src/SFML/Audio/SoundFile.hpp index 8bf043bde..16bfe6f08 100644 --- a/src/SFML/Audio/SoundFile.hpp +++ b/src/SFML/Audio/SoundFile.hpp @@ -218,6 +218,9 @@ private : std::size_t m_sampleCount; ///< Total number of samples in the file unsigned int m_channelCount; ///< Number of channels used by the sound unsigned int m_sampleRate; ///< Number of samples per second + #ifdef SFML_SYSTEM_ANDROID + void* m_resourceStream; ///< Asset file streamer (if loaded from file) + #endif }; } // namespace priv diff --git a/src/SFML/Graphics/Font.cpp b/src/SFML/Graphics/Font.cpp index d4655953d..db7148f9c 100644 --- a/src/SFML/Graphics/Font.cpp +++ b/src/SFML/Graphics/Font.cpp @@ -28,8 +28,7 @@ #include #include #ifdef SFML_SYSTEM_ANDROID - #include - #include + #include #endif #include #include @@ -88,6 +87,10 @@ m_info (copy.m_info), m_pages (copy.m_pages), m_pixelBuffer(copy.m_pixelBuffer) { + #ifdef SFML_SYSTEM_ANDROID + m_stream = NULL; + #endif + // Note: as FreeType doesn't provide functions for copying/cloning, // we must share all the FreeType pointers @@ -100,6 +103,13 @@ m_pixelBuffer(copy.m_pixelBuffer) Font::~Font() { cleanup(); + + #ifdef SFML_SYSTEM_ANDROID + + if (m_stream) + delete (priv::ResourceStream*)m_stream; + + #endif } @@ -149,39 +159,11 @@ bool Font::loadFromFile(const std::string& filename) #else - priv::ActivityStates* states = priv::getActivity(NULL); - Lock lock(states->mutex); + if (m_stream) + delete (priv::ResourceStream*)m_stream; - // Open the file - AAsset* file = NULL; - file = AAssetManager_open(states->activity->assetManager, filename.c_str(), AASSET_MODE_UNKNOWN); - - if (!file) - { - // File not found, abording... - err() << "Failed to load font \"" << filename << "\" (couldn't find it)" << std::endl; - return false; - } - - // Copy into memory - off_t size = AAsset_getLength(file); - void* data = malloc(size); - int status = AAsset_read(file, data, size); - - if (status <= 0) - { - // Something went wrong while we were copying, reading error... - err() << "Failed to load font \"" << filename << "\" (couldn't read it)" << std::endl; - return false; - } - - // Load from our fresh memory - bool ret = loadFromMemory(data, size); - - // Close the file - AAsset_close(file); - - return ret; + m_stream = new priv::ResourceStream(filename); + return loadFromStream(*(priv::ResourceStream*)m_stream); #endif } diff --git a/src/SFML/Graphics/Image.cpp b/src/SFML/Graphics/Image.cpp index 215378a0d..a27ab92d5 100644 --- a/src/SFML/Graphics/Image.cpp +++ b/src/SFML/Graphics/Image.cpp @@ -27,11 +27,10 @@ //////////////////////////////////////////////////////////// #include #include -#ifdef SFML_SYSTEM_ANDROID - #include - #include -#endif #include +#ifdef SFML_SYSTEM_ANDROID + #include +#endif #include #include @@ -42,7 +41,23 @@ namespace sf Image::Image() : m_size(0, 0) { + #ifdef SFML_SYSTEM_ANDROID + m_stream = NULL; + + #endif +} + + +//////////////////////////////////////////////////////////// +Image::~Image() +{ + #ifdef SFML_SYSTEM_ANDROID + + if (m_stream) + delete (priv::ResourceStream*)m_stream; + + #endif } @@ -108,43 +123,15 @@ bool Image::loadFromFile(const std::string& filename) { #ifndef SFML_SYSTEM_ANDROID - return priv::ImageLoader::getInstance().loadImageFromFile(filename, m_pixels, m_size); + return priv::ImageLoader::getInstance().loadImageFromFile(filename, m_pixels, m_size); #else - priv::ActivityStates* states = priv::getActivity(NULL); - Lock lock(states->mutex); + if (m_stream) + delete (priv::ResourceStream*)m_stream; - // Open the file - AAsset* file = NULL; - file = AAssetManager_open(states->activity->assetManager, filename.c_str(), AASSET_MODE_UNKNOWN); - - if (!file) - { - // File not found, abording... - err() << "Failed to load image \"" << filename << "\" (couldn't find it)" << std::endl; - return false; - } - - // Copy into memory - off_t size = AAsset_getLength(file); - void* data = malloc(size); - int status = AAsset_read(file, data, size); - - if (status <= 0) - { - // Something went wrong while we were copying, reading error... - err() << "Failed to load image \"" << filename << "\" (couldn't read it)" << std::endl; - return false; - } - - // Load from our fresh memory - bool ret = loadFromMemory(data, size); - - // Close the file - AAsset_close(file); - - return ret; + m_stream = new priv::ResourceStream(filename); + return loadFromStream(*(priv::ResourceStream*)m_stream); #endif } diff --git a/src/SFML/System/Android/ResourceStream.cpp b/src/SFML/System/Android/ResourceStream.cpp new file mode 100644 index 000000000..abfa29e6e --- /dev/null +++ b/src/SFML/System/Android/ResourceStream.cpp @@ -0,0 +1,85 @@ +//////////////////////////////////////////////////////////// +// +// SFML - Simple and Fast Multimedia Library +// Copyright (C) 2013 Jonathan De Wachter (dewachter.jonathan@gmail.com) +// +// This software is provided 'as-is', without any express or implied warranty. +// In no event will the authors be held liable for any damages arising from the use of this software. +// +// Permission is granted to anyone to use this software for any purpose, +// including commercial applications, and to alter it and redistribute it freely, +// subject to the following restrictions: +// +// 1. The origin of this software must not be misrepresented; +// you must not claim that you wrote the original software. +// If you use this software in a product, an acknowledgment +// in the product documentation would be appreciated but is not required. +// +// 2. Altered source versions must be plainly marked as such, +// and must not be misrepresented as being the original software. +// +// 3. This notice may not be removed or altered from any source distribution. +// +//////////////////////////////////////////////////////////// + + +//////////////////////////////////////////////////////////// +// Headers +//////////////////////////////////////////////////////////// +#include +#include +#include + + +namespace sf +{ +namespace priv +{ + +//////////////////////////////////////////////////////////// +ResourceStream::ResourceStream(const std::string& filename) : +m_file (NULL) +{ + ActivityStates* states = getActivity(NULL); + Lock(states->mutex); + m_file = AAssetManager_open(states->activity->assetManager, filename.c_str(), AASSET_MODE_UNKNOWN); +} + + +//////////////////////////////////////////////////////////// +ResourceStream::~ResourceStream() +{ + AAsset_close(m_file); +} + + +//////////////////////////////////////////////////////////// +Int64 ResourceStream::read(void *data, Int64 size) +{ + return AAsset_read(m_file, data, size); +} + + +//////////////////////////////////////////////////////////// +Int64 ResourceStream::seek(Int64 position) +{ + AAsset_seek(m_file, position, SEEK_SET); +} + + +//////////////////////////////////////////////////////////// +Int64 ResourceStream::tell() +{ + return getSize() - AAsset_getRemainingLength(m_file); +} + + +//////////////////////////////////////////////////////////// +Int64 ResourceStream::getSize() +{ + return AAsset_getLength(m_file); +} + + +} // namespace priv +} // namespace sf diff --git a/src/SFML/System/Android/ResourceStream.hpp b/src/SFML/System/Android/ResourceStream.hpp new file mode 100644 index 000000000..183f55fe8 --- /dev/null +++ b/src/SFML/System/Android/ResourceStream.hpp @@ -0,0 +1,113 @@ +//////////////////////////////////////////////////////////// +// +// SFML - Simple and Fast Multimedia Library +// Copyright (C) 2013 Jonathan De Wachter (dewachter.jonathan@gmail.com) +// +// This software is provided 'as-is', without any express or implied warranty. +// In no event will the authors be held liable for any damages arising from the use of this software. +// +// Permission is granted to anyone to use this software for any purpose, +// including commercial applications, and to alter it and redistribute it freely, +// subject to the following restrictions: +// +// 1. The origin of this software must not be misrepresented; +// you must not claim that you wrote the original software. +// If you use this software in a product, an acknowledgment +// in the product documentation would be appreciated but is not required. +// +// 2. Altered source versions must be plainly marked as such, +// and must not be misrepresented as being the original software. +// +// 3. This notice may not be removed or altered from any source distribution. +// +//////////////////////////////////////////////////////////// + +#ifndef SFML_RESOURCESTREAM_HPP +#define SFML_RESOURCESTREAM_HPP + +//////////////////////////////////////////////////////////// +// Headers +//////////////////////////////////////////////////////////// +#include +#include +#include +#include + + +namespace sf +{ +namespace priv +{ +//////////////////////////////////////////////////////////// +/// \brief Read from Android asset files +/// +//////////////////////////////////////////////////////////// +class SFML_SYSTEM_API ResourceStream : public InputStream +{ +public : + + //////////////////////////////////////////////////////////// + /// \brief Default constructor + /// + /// \param filename Filename of the asset + /// + //////////////////////////////////////////////////////////// + ResourceStream(const std::string& filename); + + //////////////////////////////////////////////////////////// + /// \brief Destructor + /// + //////////////////////////////////////////////////////////// + ~ResourceStream(); + + //////////////////////////////////////////////////////////// + /// \brief Read data from the asset + /// + /// \param data Buffer where the asset data is copied + /// \param size Number of bytes read + /// + /// \return The number of bytes actually read, or -1 on error + /// + //////////////////////////////////////////////////////////// + Int64 read(void *data, Int64 size); + + //////////////////////////////////////////////////////////// + /// \brief Change the current reading position in the asset file + /// + /// \param position The position to seek to, from the beginning + /// + /// \return The position actually sought to, or -1 on error + /// + //////////////////////////////////////////////////////////// + Int64 seek(Int64 position); + + //////////////////////////////////////////////////////////// + /// \brief Get the current reading position in the asset file + /// + /// \return The current position, or -1 on error. + /// + //////////////////////////////////////////////////////////// + Int64 tell(); + + //////////////////////////////////////////////////////////// + /// \brief Return the size of the asset file + /// + /// \return The total number of bytes available in the asset, or -1 on error + /// + //////////////////////////////////////////////////////////// + Int64 getSize(); + +private : + + //////////////////////////////////////////////////////////// + // Member data + //////////////////////////////////////////////////////////// + AAsset* m_file; ///< The asset file to read +}; + +} // namespace priv + +} // namespace sf + + +#endif // SFML_RESOURCESTREAM_HPP diff --git a/src/SFML/System/CMakeLists.txt b/src/SFML/System/CMakeLists.txt index a874293dd..2eacc0dce 100644 --- a/src/SFML/System/CMakeLists.txt +++ b/src/SFML/System/CMakeLists.txt @@ -71,6 +71,8 @@ else() set(PLATFORM_SRC ${PLATFORM_SRC} ${SRCROOT}/Android/Activity.hpp ${SRCROOT}/Android/Activity.cpp + ${SRCROOT}/Android/ResourceStream.cpp + ${SRCROOT}/Android/ResourceStream.cpp ) endif() @@ -91,7 +93,7 @@ endif() # pthread is implicitly linked by the NDK if(SFML_OS_ANDROID) - set(SYSTEM_EXT_LIBS "") + set(SYSTEM_EXT_LIBS android) endif() # define the sfml-system target