From 95ef444c20ba48e4b05681f66d9614e2906d3078 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Sun, 27 Oct 2013 07:15:39 -0700 Subject: [PATCH] More unique_ptr --- aegisub/libaegisub/common/cajun/elements.cpp | 5 +- .../include/libaegisub/cajun/elements.h | 3 +- aegisub/src/ass_file.cpp | 3 +- aegisub/src/audio_renderer.cpp | 20 ++--- aegisub/src/audio_renderer.h | 11 +-- aegisub/src/audio_renderer_spectrum.cpp | 15 ++-- aegisub/src/auto4_lua.cpp | 16 +++- aegisub/src/auto4_lua.h | 4 +- aegisub/src/auto4_lua_assfile.cpp | 11 ++- aegisub/src/block_cache.h | 84 +++++++++---------- aegisub/src/utils.h | 16 ---- 11 files changed, 81 insertions(+), 107 deletions(-) diff --git a/aegisub/libaegisub/common/cajun/elements.cpp b/aegisub/libaegisub/common/cajun/elements.cpp index 7092f15af..a39341f43 100644 --- a/aegisub/libaegisub/common/cajun/elements.cpp +++ b/aegisub/libaegisub/common/cajun/elements.cpp @@ -88,7 +88,7 @@ UnknownElement::UnknownElement() : m_pImp(new Imp_T UnknownElement::UnknownElement(const UnknownElement& unknown) : m_pImp(unknown.m_pImp->Clone()) {} UnknownElement::UnknownElement(int number) : m_pImp(new Imp_T(number)) {} UnknownElement::UnknownElement(const char *string) : m_pImp(new Imp_T(string)) {} -UnknownElement::~UnknownElement() { delete m_pImp; } +UnknownElement::~UnknownElement() { } #define DEFINE_UE_TYPE(Type) \ UnknownElement::UnknownElement(Type const& val) : m_pImp(new Imp_T(val)) { } \ @@ -105,8 +105,7 @@ DEFINE_UE_TYPE(Null) UnknownElement& UnknownElement::operator =(const UnknownElement& unknown) { - delete m_pImp; - m_pImp = unknown.m_pImp->Clone(); + m_pImp.reset(unknown.m_pImp->Clone()); return *this; } diff --git a/aegisub/libaegisub/include/libaegisub/cajun/elements.h b/aegisub/libaegisub/include/libaegisub/cajun/elements.h index acf79f5c9..c3ffe7046 100644 --- a/aegisub/libaegisub/include/libaegisub/cajun/elements.h +++ b/aegisub/libaegisub/include/libaegisub/cajun/elements.h @@ -12,6 +12,7 @@ Author: Terry Caton #include #include #include +#include #include #include @@ -126,7 +127,7 @@ private: template ElementTypeT& CastTo(); - Imp* m_pImp; + std::unique_ptr m_pImp; }; ///////////////////////////////////////////////////////////////////////////////// diff --git a/aegisub/src/ass_file.cpp b/aegisub/src/ass_file.cpp index 5c3e723d1..b57326de2 100644 --- a/aegisub/src/ass_file.cpp +++ b/aegisub/src/ass_file.cpp @@ -87,8 +87,9 @@ void AssFile::swap(AssFile &that) throw() { } AssFile::AssFile(const AssFile &from) { - Line.clone_from(from.Line, std::mem_fun_ref(&AssEntry::Clone), delete_ptr()); + Line.clone_from(from.Line, std::mem_fun_ref(&AssEntry::Clone), [](AssEntry *e) { delete e; }); } + AssFile& AssFile::operator=(AssFile from) { std::swap(*this, from); return *this; diff --git a/aegisub/src/audio_renderer.cpp b/aegisub/src/audio_renderer.cpp index 65b579fe8..d245ee509 100644 --- a/aegisub/src/audio_renderer.cpp +++ b/aegisub/src/audio_renderer.cpp @@ -31,20 +31,20 @@ /// @brief Base classes for audio renderers (spectrum, waveform, ...) /// @ingroup audio_ui - -// Headers #include "config.h" #include "audio_renderer.h" +#include "include/aegisub/audio_provider.h" + +#include + #include #include #include #include -#include "include/aegisub/audio_provider.h" - using std::placeholders::_1; AudioRendererBitmapCacheBitmapFactory::AudioRendererBitmapCacheBitmapFactory(AudioRenderer *renderer) @@ -53,14 +53,9 @@ AudioRendererBitmapCacheBitmapFactory::AudioRendererBitmapCacheBitmapFactory(Aud assert(renderer); } -wxBitmap *AudioRendererBitmapCacheBitmapFactory::ProduceBlock(int /* i */) +std::unique_ptr AudioRendererBitmapCacheBitmapFactory::ProduceBlock(int /* i */) { - return new wxBitmap(renderer->cache_bitmap_width, renderer->pixel_height, 24); -} - -void AudioRendererBitmapCacheBitmapFactory::DisposeBlock(wxBitmap *bmp) -{ - delete bmp; + return agi::util::make_unique(renderer->cache_bitmap_width, renderer->pixel_height, 24); } size_t AudioRendererBitmapCacheBitmapFactory::GetBlockSize() const @@ -79,7 +74,8 @@ AudioRenderer::AudioRenderer() , renderer(0) , provider(0) { - bitmaps.resize(AudioStyle_MAX, AudioRendererBitmapCache(256, AudioRendererBitmapCacheBitmapFactory(this))); + for (int i = 0; i < AudioStyle_MAX; ++i) + bitmaps.emplace_back(256, AudioRendererBitmapCacheBitmapFactory(this)); // Make sure there's *some* values for those fields, and in the caches SetMillisecondsPerPixel(1); diff --git a/aegisub/src/audio_renderer.h b/aegisub/src/audio_renderer.h index 40493b6ea..ab6e87b7e 100644 --- a/aegisub/src/audio_renderer.h +++ b/aegisub/src/audio_renderer.h @@ -54,6 +54,8 @@ class AudioRenderer; /// @class AudioRendererBitmapCacheBitmapFactory /// @brief Produces wxBitmap objects for DataBlockCache storage for the audio renderer struct AudioRendererBitmapCacheBitmapFactory { + typedef std::unique_ptr BlockType; + /// The audio renderer we're producing bitmaps for AudioRenderer *renderer; @@ -66,13 +68,7 @@ struct AudioRendererBitmapCacheBitmapFactory { /// @return A fresh wxBitmap /// /// Produces a wxBitmap with dimensions pulled from our master AudioRenderer. - wxBitmap *ProduceBlock(int i); - - /// @brief Delete a bitmap - /// @param bmp The bitmap to delete - /// - /// Deletes said bitmap. - void DisposeBlock(wxBitmap *bmp); + std::unique_ptr ProduceBlock(int i); /// @brief Calculate the size of bitmaps /// @return The size of bitmaps created @@ -83,7 +79,6 @@ struct AudioRendererBitmapCacheBitmapFactory { typedef DataBlockCache AudioRendererBitmapCache; - /// @class AudioRenderer /// @brief Renders audio to bitmap images for display on screen /// diff --git a/aegisub/src/audio_renderer_spectrum.cpp b/aegisub/src/audio_renderer_spectrum.cpp index 7f8ba1d71..ad9e395f7 100644 --- a/aegisub/src/audio_renderer_spectrum.cpp +++ b/aegisub/src/audio_renderer_spectrum.cpp @@ -54,6 +54,8 @@ /// Allocates blocks of derived data for the audio spectrum struct AudioSpectrumCacheBlockFactory { + typedef std::unique_ptr> BlockType; + /// Pointer back to the owning spectrum renderer AudioSpectrumRenderer *spectrum; @@ -66,18 +68,11 @@ struct AudioSpectrumCacheBlockFactory { /// @return Newly allocated and filled block /// /// The filling is delegated to the spectrum renderer - float *ProduceBlock(size_t i) + BlockType ProduceBlock(size_t i) { float *res = new float[((size_t)1)<derivation_size]; spectrum->FillBlock(i, res); - return res; - } - - /// @brief De-allocate a cache block - /// @param block The block to dispose of - void DisposeBlock(float *block) - { - delete[] block; + return BlockType(res); } /// @brief Calculate the in-memory size of a spec @@ -118,7 +113,7 @@ AudioSpectrumRenderer::AudioSpectrumRenderer(std::string const& color_scheme_nam AudioSpectrumRenderer::~AudioSpectrumRenderer() { // This sequence will clean up - provider = 0; + provider = nullptr; RecreateCache(); } diff --git a/aegisub/src/auto4_lua.cpp b/aegisub/src/auto4_lua.cpp index 2d17060c2..8452d71c6 100644 --- a/aegisub/src/auto4_lua.cpp +++ b/aegisub/src/auto4_lua.cpp @@ -307,7 +307,7 @@ namespace Automation4 { std::string version; std::vector macros; - std::vector filters; + std::vector> filters; /// load script and create internal structures etc. void Create(); @@ -337,7 +337,7 @@ namespace Automation4 { bool GetLoadedState() const { return L != 0; } std::vector GetMacros() const { return macros; } - std::vector GetFilters() const { return filters; } + std::vector GetFilters() const; std::vector GetFormats() const { return std::vector(); } }; @@ -492,12 +492,20 @@ namespace Automation4 { for (int i = macros.size() - 1; i >= 0; --i) cmd::unreg(macros[i]->name()); - delete_clear(filters); + filters.clear(); lua_close(L); L = nullptr; } + std::vector LuaScript::GetFilters() const + { + std::vector ret; + ret.reserve(filters.size()); + for (auto& filter : filters) ret.push_back(filter.get()); + return ret; + } + void LuaScript::RegisterCommand(LuaCommand *command) { for (auto macro : macros) { @@ -517,7 +525,7 @@ namespace Automation4 { void LuaScript::RegisterFilter(LuaExportFilter *filter) { - filters.push_back(filter); + filters.emplace_back(filter); } LuaScript* LuaScript::GetScriptObject(lua_State *L) diff --git a/aegisub/src/auto4_lua.h b/aegisub/src/auto4_lua.h index ba1dfb35a..0102bc0f6 100644 --- a/aegisub/src/auto4_lua.h +++ b/aegisub/src/auto4_lua.h @@ -83,7 +83,7 @@ namespace Automation4 { /// Commits to apply once processing completes successfully std::deque pending_commits; /// Lines to delete once processing complete successfully - std::deque lines_to_delete; + std::deque> lines_to_delete; int ObjectIndexRead(lua_State *L); void ObjectIndexWrite(lua_State *L); @@ -101,7 +101,7 @@ namespace Automation4 { void LuaSetUndoPoint(lua_State *L); // LuaAssFile can only be deleted by the reference count hitting zero - ~LuaAssFile() { } + ~LuaAssFile(); public: static LuaAssFile *GetObjPointer(lua_State *L, int idx); diff --git a/aegisub/src/auto4_lua_assfile.cpp b/aegisub/src/auto4_lua_assfile.cpp index 112e29936..baaeb3d9e 100644 --- a/aegisub/src/auto4_lua_assfile.cpp +++ b/aegisub/src/auto4_lua_assfile.cpp @@ -129,6 +129,8 @@ namespace { } namespace Automation4 { + LuaAssFile::~LuaAssFile() { } + void LuaAssFile::CheckAllowModify() { if (!can_modify) @@ -376,7 +378,7 @@ namespace Automation4 { auto e = LuaToAssEntry(L); modification_type |= modification_mask(e.get()); CheckBounds(n); - lines_to_delete.push_back(lines[n - 1]); + lines_to_delete.emplace_back(lines[n - 1]); lines[n - 1] = e.release(); } else { @@ -416,7 +418,7 @@ namespace Automation4 { for (size_t i = 0; i < lines.size(); ++i) { if (id_idx < ids.size() && ids[id_idx] == i) { modification_type |= modification_mask(lines[i]); - lines_to_delete.push_back(lines[i]); + lines_to_delete.emplace_back(lines[i]); ++id_idx; } else { @@ -438,7 +440,7 @@ namespace Automation4 { for (; b < lines.size(); ++a, ++b) { modification_type |= modification_mask(lines[a]); - lines_to_delete.push_back(lines[a]); + lines_to_delete.emplace_back(lines[a]); lines[a] = lines[b]; } @@ -609,7 +611,7 @@ namespace Automation4 { if (modification_type && can_set_undo && !undo_description.empty()) ass->Commit(undo_description, modification_type); - delete_clear(lines_to_delete); + lines_to_delete.clear(); references--; if (!references) delete this; @@ -617,6 +619,7 @@ namespace Automation4 { void LuaAssFile::Cancel() { + for (auto& line : lines_to_delete) line.release(); references--; if (!references) delete this; } diff --git a/aegisub/src/block_cache.h b/aegisub/src/block_cache.h index dffcff6d2..69cf3e464 100644 --- a/aegisub/src/block_cache.h +++ b/aegisub/src/block_cache.h @@ -49,25 +49,18 @@ /// requested blocks in to avoid the default allocator. template struct BasicDataBlockFactory { + typedef std::unique_ptr BlockType; + /// @brief Allocates a block and returns it /// @param i Index of the block to allocate /// @return A pointer to the allocated block /// /// This default implementation does not use the i parameter. Custom implementations /// of block factories should use i to determine what data to fill into the block. - BlockT *ProduceBlock(size_t i) + std::unique_ptr ProduceBlock(size_t i) { (void)i; - return new BlockT; - } - - /// @brief De-allocate a block - /// @param block Pointer to the block to de-allocate - /// - /// It is guaranteed that block was returned by ProduceBlock. - void DisposeBlock(BlockT *block) - { - delete block; + return std::unique_ptr(new BlockT); } /// @brief Retrieve the amount of memory consumed by a single block @@ -94,9 +87,8 @@ template < typename BlockFactoryT = BasicDataBlockFactory > class DataBlockCache { - /// Type of an array of blocks - typedef std::vector BlockArray; + typedef std::vector BlockArray; struct MacroBlock { /// This macroblock's position in the age list @@ -105,6 +97,16 @@ class DataBlockCache { /// The blocks contained in the macroblock BlockArray blocks; + +#ifdef _MSC_VER + MacroBlock() { } + MacroBlock(MacroBlock&& rgt) : position(rgt.position), blocks(std::move(rgt.blocks)) { } + MacroBlock& operator=(MacroBlock&& rgt) { + position = rgt.position; + blocks = std::move(rgt.blocks); + return *this; + } +#endif }; /// Type of an array of macro blocks @@ -135,17 +137,21 @@ class DataBlockCache { if (mb.blocks.empty()) return; - for (auto block : mb.blocks) - { - if (block) - factory.DisposeBlock(block); - } - mb.blocks.clear(); age.erase(mb.position); } public: +#ifdef _MSC_VER + DataBlockCache(DataBlockCache&& rgt) + : data(std::move(rgt.data)) + , age(std::move(rgt.age)) + , macroblock_size(rgt.macroblock_size) + , macroblock_index_mask(rgt.macroblock_index_mask) + , factory(std::move(rgt.factory)) + { } +#endif + /// @brief Constructor /// @param block_count Total number of blocks the cache will manage /// @param factory Factory object to use for producing blocks @@ -155,22 +161,12 @@ public: /// /// The factory object passed must respond well to copying. DataBlockCache(size_t block_count, BlockFactoryT factory = BlockFactoryT()) - : factory(factory) + : factory(factory) { SetBlockCount(block_count); } - /// @brief Destructor - /// - /// Disposes of all cached blocks - ~DataBlockCache() - { - // Clear all blocks by aging to zero bytes - Age(0); - } - - /// @brief Change the number of blocks in cache /// @param block_count New number of blocks to hold /// @@ -184,7 +180,7 @@ public: macroblock_index_mask = ~(((~0) >> MacroblockExponent) << MacroblockExponent); - data.resize( (block_count + macroblock_size - 1) >> MacroblockExponent ); + data.resize((block_count + macroblock_size - 1) >> MacroblockExponent); } @@ -201,8 +197,10 @@ public: // Quick way out: get rid of everything if (max_size == 0) { - for (auto& mb : data) - KillMacroBlock(mb); + size_t block_count = data.size(); + data.clear(); + SetBlockCount(block_count); + age.clear(); return; } @@ -212,14 +210,12 @@ public: auto it = age.begin(); for (; it != age.end() && cur_size < max_size; ++it) { - BlockArray &ba = (*it)->blocks; + auto& ba = (*it)->blocks; cur_size += (ba.size() - std::count(ba.begin(), ba.end(), nullptr)) * block_size; } // Hit max, clear all remaining blocks - for (; it != age.end();) - { + for (; it != age.end(); ) KillMacroBlock(**it++); - } } @@ -229,21 +225,17 @@ public: /// @return A pointer to the block in cache /// /// It is legal to pass 0 (null) for created, in this case nothing is returned in it. - BlockT *Get(size_t i, bool *created = 0) + BlockT *Get(size_t i, bool *created = nullptr) { size_t mbi = i >> MacroblockExponent; assert(mbi < data.size()); MacroBlock &mb = data[mbi]; - if (mb.blocks.size() == 0) - { + if (mb.blocks.empty()) mb.blocks.resize(macroblock_size); - } else - { age.erase(mb.position); - } // Move this macroblock to the front of the age list age.push_front(&mb); @@ -252,13 +244,13 @@ public: size_t block_index = i & macroblock_index_mask; assert(block_index < mb.blocks.size()); - BlockT *b = mb.blocks[block_index]; + BlockT *b = mb.blocks[block_index].get(); if (!b) { - b = factory.ProduceBlock(i); + mb.blocks[block_index] = factory.ProduceBlock(i); + b = mb.blocks[block_index].get(); assert(b != 0); - mb.blocks[block_index] = b; if (created) *created = true; } diff --git a/aegisub/src/utils.h b/aegisub/src/utils.h index 0f19889d9..9ea8cac82 100644 --- a/aegisub/src/utils.h +++ b/aegisub/src/utils.h @@ -114,22 +114,6 @@ void SetClipboard(wxBitmap const& new_value); #define countof(array) (sizeof(array) / sizeof(array[0])) -/// Polymorphic delete functor -struct delete_ptr { - template void operator()(T* ptr) const { - delete ptr; - } -}; - -/// Delete all of the items in a container of pointers and clear the container -template -void delete_clear(T& container) { - if (!container.empty()) { - std::for_each(container.begin(), container.end(), delete_ptr()); - container.clear(); - } -} - template struct cast { template