From 8ca77f27a16e8af123284a3a9d9f059336aaecbf Mon Sep 17 00:00:00 2001 From: Piotr Dziwinski Date: Thu, 16 Jul 2015 20:24:54 +0200 Subject: [PATCH] Extracted CSDLFileHandler from CResourceManager This also possibly fixes some rare error cases (e.g. #439) --- src/CMakeLists.txt | 1 + src/common/image.cpp | 24 +-- src/common/resources/resourcemanager.cpp | 113 +------------ src/common/resources/resourcemanager.h | 12 +- src/common/resources/sdl_file_wrapper.cpp | 184 ++++++++++++++++++++++ src/common/resources/sdl_file_wrapper.h | 53 +++++++ src/graphics/engine/text.cpp | 19 ++- 7 files changed, 268 insertions(+), 138 deletions(-) create mode 100644 src/common/resources/sdl_file_wrapper.cpp create mode 100644 src/common/resources/sdl_file_wrapper.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 717591b6..fa514232 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -93,6 +93,7 @@ set(BASE_SOURCES common/resources/outputstream.cpp common/resources/outputstreambuffer.cpp common/resources/resourcemanager.cpp + common/resources/sdl_file_wrapper.cpp common/resources/sndfile.cpp common/restext.cpp common/stringutils.cpp diff --git a/src/common/image.cpp b/src/common/image.cpp index b1ac57c7..882caf4a 100644 --- a/src/common/image.cpp +++ b/src/common/image.cpp @@ -389,16 +389,16 @@ bool CImage::Load(const std::string& fileName) m_error = ""; - SDL_RWops* pointer = CResourceManager::GetSDLFileHandler(fileName.c_str()); - if (pointer == nullptr) + auto file = CResourceManager::GetSDLFileHandler(fileName.c_str()); + if (!file->IsOpen()) { delete m_data; m_data = nullptr; - + m_error = "Unable to open file"; return false; } - m_data->surface = IMG_Load_RW(pointer, 1); + m_data->surface = IMG_Load_RW(file->GetHandler(), 1); if (m_data->surface == nullptr) { delete m_data; @@ -439,9 +439,9 @@ void CImage::SetDataPixels(void *pixels){ Uint8* srcPixels = static_cast (pixels); Uint8* resultPixels = static_cast (m_data->surface->pixels); - + Uint32 pitch = m_data->surface->pitch; - + for(int line = 0; line < m_data->surface->h; ++line) { Uint32 pos = line * pitch; memcpy(&resultPixels[pos], &srcPixels[pos], pitch); @@ -458,21 +458,21 @@ void CImage::flipVertically(){ m_data->surface->format->Gmask, m_data->surface->format->Bmask, m_data->surface->format->Amask); - + assert(result != nullptr); - + Uint8* srcPixels = static_cast (m_data->surface->pixels); Uint8* resultPixels = static_cast (result->pixels); - + Uint32 pitch = m_data->surface->pitch; Uint32 pxLength = pitch*m_data->surface->h; - + for(int line = 0; line < m_data->surface->h; ++line) { Uint32 pos = line * pitch; memcpy(&resultPixels[pos], &srcPixels[(pxLength-pos)-pitch], pitch); } - + SDL_FreeSurface(m_data->surface); - m_data->surface = result; + m_data->surface = result; } diff --git a/src/common/resources/resourcemanager.cpp b/src/common/resources/resourcemanager.cpp index b3688ab5..7143a2a4 100644 --- a/src/common/resources/resourcemanager.cpp +++ b/src/common/resources/resourcemanager.cpp @@ -34,11 +34,6 @@ namespace fs = boost::filesystem; -namespace -{ - const Uint32 PHYSFS_RWOPS_TYPE = 0xc010b04f; -} - CResourceManager::CResourceManager(const char *argv0) { @@ -112,40 +107,11 @@ std::string CResourceManager::GetSaveLocation() return ""; } - -SDL_RWops* CResourceManager::GetSDLFileHandler(const std::string &filename) +std::unique_ptr CResourceManager::GetSDLFileHandler(const std::string &filename) { - SDL_RWops *handler = SDL_AllocRW(); - if (!handler) - { - CLogger::GetInstancePointer()->Error("Unable to allocate SDL_RWops for \"%s\"\n", filename.c_str()); - return nullptr; - } - - if (!PHYSFS_isInit()) - { - SDL_FreeRW(handler); - return nullptr; - } - - PHYSFS_File *file = PHYSFS_openRead(CleanPath(filename).c_str()); - if (!file) - { - SDL_FreeRW(handler); - return nullptr; - } - - handler->seek = SDLSeek; - handler->read = SDLRead; - handler->write = SDLWrite; - handler->close = SDLClose; - handler->type = PHYSFS_RWOPS_TYPE; - handler->hidden.unknown.data1 = file; - - return handler; + return std::unique_ptr(new CSDLFileWrapper(CleanPath(filename))); } - std::unique_ptr CResourceManager::GetSNDFileHandler(const std::string &filename) { return std::unique_ptr(new CSNDFile(CleanPath(filename))); @@ -302,78 +268,3 @@ bool CResourceManager::Remove(const std::string& filename) } return false; } - -int CResourceManager::SDLClose(SDL_RWops *context) -{ - if (CheckSDLContext(context)) - { - PHYSFS_close(static_cast(context->hidden.unknown.data1)); - SDL_FreeRW(context); - - return 0; - } - - return 1; -} - - -int CResourceManager::SDLRead(SDL_RWops *context, void *ptr, int size, int maxnum) -{ - if (CheckSDLContext(context)) - { - PHYSFS_File *file = static_cast(context->hidden.unknown.data1); - SDL_memset(ptr, 0, size * maxnum); - - return PHYSFS_read(file, ptr, size, maxnum); - } - - return 0; -} - - -int CResourceManager::SDLWrite(SDL_RWops *context, const void *ptr, int size, int num) -{ - return 0; -} - - -int CResourceManager::SDLSeek(SDL_RWops *context, int offset, int whence) -{ - if (CheckSDLContext(context)) - { - PHYSFS_File *file = static_cast(context->hidden.unknown.data1); - int position, result; - - switch (whence) - { - default: - case RW_SEEK_SET: - result = PHYSFS_seek(file, offset); - return result > 0 ? offset : -1; - - case RW_SEEK_CUR: - position = offset + PHYSFS_tell(file); - result = PHYSFS_seek(file, position); - return result > 0 ? position : -1; - - case RW_SEEK_END: - position = PHYSFS_fileLength(file) - offset; - result = PHYSFS_seek(file, position); - return result > 0 ? position : -1; - } - } - - return -1; -} - - -bool CResourceManager::CheckSDLContext(SDL_RWops *context) -{ - if (context->type != PHYSFS_RWOPS_TYPE) - { - SDL_SetError("Wrong kind of RWops"); - return false; - } - - return true; -} diff --git a/src/common/resources/resourcemanager.h b/src/common/resources/resourcemanager.h index 86c7acb6..24a93758 100644 --- a/src/common/resources/resourcemanager.h +++ b/src/common/resources/resourcemanager.h @@ -20,13 +20,12 @@ #pragma once #include "common/resources/sndfile.h" +#include "common/resources/sdl_file_wrapper.h" #include #include #include -#include - class CResourceManager { public: @@ -41,7 +40,7 @@ public: static bool SetSaveLocation(const std::string &location); static std::string GetSaveLocation(); - static SDL_RWops* GetSDLFileHandler(const std::string &filename); + static std::unique_ptr GetSDLFileHandler(const std::string &filename); static std::unique_ptr GetSNDFileHandler(const std::string &filename); //! Check if file exists @@ -69,11 +68,4 @@ public: static bool Move(const std::string &from, const std::string &to); //! Remove file static bool Remove(const std::string& filename); - -private: - static int SDLSeek(SDL_RWops *context, int offset, int whence); - static int SDLRead(SDL_RWops *context, void *ptr, int size, int maxnum); - static int SDLWrite(SDL_RWops *context, const void *ptr, int size, int num); - static int SDLClose(SDL_RWops *context); - static bool CheckSDLContext(SDL_RWops *context); }; diff --git a/src/common/resources/sdl_file_wrapper.cpp b/src/common/resources/sdl_file_wrapper.cpp new file mode 100644 index 00000000..6422120d --- /dev/null +++ b/src/common/resources/sdl_file_wrapper.cpp @@ -0,0 +1,184 @@ +/* + * This file is part of the Colobot: Gold Edition source code + * Copyright (C) 2001-2014, Daniel Roux, EPSITEC SA & TerranovaTeam + * http://epsiteс.ch; http://colobot.info; http://github.com/colobot + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://gnu.org/licenses + */ + + +#include "common/resources/sdl_file_wrapper.h" + +#include "common/logger.h" + +#include + +namespace +{ + const Uint32 PHYSFS_RWOPS_TYPE = 0xc010b04f; +} + +CSDLFileWrapper::CSDLFileWrapper(const std::string& filename) + : m_rwops(nullptr) +{ + if (!PHYSFS_isInit()) + { + CLogger::GetInstancePointer()->Error("PHYSFS not initialized!\n"); + return; + } + + PHYSFS_File *file = PHYSFS_openRead(filename.c_str()); + if (file == nullptr) + { + CLogger::GetInstancePointer()->Error("Error opening file with PHYSFS: \"%s\"\n", filename.c_str()); + return; + } + + m_rwops = SDL_AllocRW(); + if (m_rwops == nullptr) + { + CLogger::GetInstancePointer()->Error("Unable to allocate SDL_RWops for \"%s\"\n", filename.c_str()); + return; + } + + m_rwops->type = PHYSFS_RWOPS_TYPE; + m_rwops->hidden.unknown.data1 = file; + m_rwops->seek = SDLSeek; + m_rwops->read = SDLRead; + m_rwops->write = SDLWrite; + // This is safe because SDL_FreeRW will be called in destructor + m_rwops->close = SDLCloseWithoutFreeRW; +} + +CSDLFileWrapper::~CSDLFileWrapper() +{ + SDLCloseWithFreeRW(m_rwops); +} + +SDL_RWops* CSDLFileWrapper::GetHandler() +{ + return m_rwops; +} + +SDL_RWops* CSDLFileWrapper::ReleaseHandler() +{ + SDL_RWops* rwops = m_rwops; + m_rwops = nullptr; + + // Destructor will not call SDL_FreeRW, so we must take care of it ourselves + rwops->close = SDLCloseWithFreeRW; + return rwops; +} + +bool CSDLFileWrapper::IsOpen() const +{ + return m_rwops != nullptr; +} + +int CSDLFileWrapper::SDLClose(SDL_RWops *context, bool freeRW) +{ + if (context == nullptr) + return 0; + + if (CheckSDLContext(context)) + { + if (context->hidden.unknown.data1 != nullptr) + { + PHYSFS_close(static_cast(context->hidden.unknown.data1)); + context->hidden.unknown.data1 = nullptr; + } + + if (freeRW) + { + SDL_FreeRW(context); + } + + return 0; + } + + return 1; +} + +int CSDLFileWrapper::SDLCloseWithoutFreeRW(SDL_RWops *context) +{ + return SDLClose(context, false); +} + +int CSDLFileWrapper::SDLCloseWithFreeRW(SDL_RWops *context) +{ + return SDLClose(context, true); +} + +bool CSDLFileWrapper::CheckSDLContext(SDL_RWops *context) +{ + if (context->type != PHYSFS_RWOPS_TYPE) + { + SDL_SetError("Wrong kind of RWops"); + return false; + } + + return true; +} + +int CSDLFileWrapper::SDLSeek(SDL_RWops *context, int offset, int whence) +{ + if (CheckSDLContext(context)) + { + PHYSFS_File *file = static_cast(context->hidden.unknown.data1); + + switch (whence) + { + default: + case RW_SEEK_SET: + { + int result = PHYSFS_seek(file, offset); + return result > 0 ? offset : -1; + } + + case RW_SEEK_CUR: + { + int position = offset + PHYSFS_tell(file); + int result = PHYSFS_seek(file, position); + return result > 0 ? position : -1; + } + + case RW_SEEK_END: + { + int position = PHYSFS_fileLength(file) - offset; + int result = PHYSFS_seek(file, position); + return result > 0 ? position : -1; + } + } + } + + return -1; +} + +int CSDLFileWrapper::SDLRead(SDL_RWops *context, void *ptr, int size, int maxnum) +{ + if (CheckSDLContext(context)) + { + PHYSFS_File *file = static_cast(context->hidden.unknown.data1); + SDL_memset(ptr, 0, size * maxnum); + + return PHYSFS_read(file, ptr, size, maxnum); + } + + return 0; +} + +int CSDLFileWrapper::SDLWrite(SDL_RWops *context, const void *ptr, int size, int num) +{ + return 0; +} diff --git a/src/common/resources/sdl_file_wrapper.h b/src/common/resources/sdl_file_wrapper.h new file mode 100644 index 00000000..dc943daa --- /dev/null +++ b/src/common/resources/sdl_file_wrapper.h @@ -0,0 +1,53 @@ +/* + * This file is part of the Colobot: Gold Edition source code + * Copyright (C) 2001-2014, Daniel Roux, EPSITEC SA & TerranovaTeam + * http://epsiteс.ch; http://colobot.info; http://github.com/colobot + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://gnu.org/licenses + */ + + +#pragma once + +#include + +#include + +class CSDLFileWrapper +{ +public: + CSDLFileWrapper(const std::string& filename); + ~CSDLFileWrapper(); + + CSDLFileWrapper(const CSDLFileWrapper&) = delete; + CSDLFileWrapper& operator=(const CSDLFileWrapper&) = delete; + + bool IsOpen() const; + SDL_RWops* GetHandler(); + + // TODO: this is kind of hacked for SDL_ttf, which keeps SDL_RWops open + SDL_RWops* ReleaseHandler(); + +private: + static int SDLSeek(SDL_RWops *context, int offset, int whence); + static int SDLRead(SDL_RWops *context, void *ptr, int size, int maxnum); + static int SDLWrite(SDL_RWops *context, const void *ptr, int size, int num); + static int SDLClose(SDL_RWops *context, bool freeRW); + static int SDLCloseWithoutFreeRW(SDL_RWops *context); + static int SDLCloseWithFreeRW(SDL_RWops *context); + static bool CheckSDLContext(SDL_RWops *context); + +private: + SDL_RWops* m_rwops; +}; diff --git a/src/graphics/engine/text.cpp b/src/graphics/engine/text.cpp index d8a3acc7..8e4f483d 100644 --- a/src/graphics/engine/text.cpp +++ b/src/graphics/engine/text.cpp @@ -350,7 +350,8 @@ float CText::GetStringWidth(std::string text, FontType font, float size) float CText::GetCharWidth(UTF8Char ch, FontType font, float size, float offset) { - if (font == FONT_BUTTON) { + if (font == FONT_BUTTON) + { Math::IntPoint windowSize = m_engine->GetWindowSize(); float height = GetHeight(FONT_COLOBOT, size); float width = height*(static_cast(windowSize.y)/windowSize.x); @@ -970,16 +971,24 @@ CachedFont* CText::GetOrOpenFont(FontType font, float size) return m_lastCachedFont; } - m_lastCachedFont = new CachedFont(); - SDL_RWops* file = CResourceManager::GetSDLFileHandler(mf->fileName); - if(file == nullptr) + auto file = CResourceManager::GetSDLFileHandler(mf->fileName); + if (!file->IsOpen()) { m_error = std::string("Unable to open file"); return nullptr; } - m_lastCachedFont->font = TTF_OpenFontRW(file, 1, pointSize); + + SDL_RWops* handler = file->ReleaseHandler(); + + m_lastCachedFont = new CachedFont(); + m_lastCachedFont->font = TTF_OpenFontRW(handler, 1, pointSize); if (m_lastCachedFont->font == nullptr) + { + SDL_RWclose(handler); m_error = std::string("TTF_OpenFont error ") + std::string(TTF_GetError()); + delete m_lastCachedFont; + return nullptr; + } mf->fonts[pointSize] = m_lastCachedFont;