diff --git a/src/iconvert/iconvert.cpp b/src/iconvert/iconvert.cpp index 66b513c381..239b74a11c 100644 --- a/src/iconvert/iconvert.cpp +++ b/src/iconvert/iconvert.cpp @@ -356,8 +356,8 @@ convert_file(const std::string& in_filename, const std::string& out_filename) // subimage appending, we gather them all first. std::vector subimagespecs; if (out->supports("multiimage") && !out->supports("appendsubimage")) { - ImageCache* imagecache = ImageCache::create(); - int nsubimages = 0; + auto imagecache = ImageCache::create(); + int nsubimages = 0; ustring ufilename(in_filename); imagecache->get_image_info(ufilename, 0, 0, ustring("subimages"), TypeInt, &nsubimages); @@ -370,7 +370,6 @@ convert_file(const std::string& in_filename, const std::string& out_filename) adjust_spec(in.get(), out.get(), inspec, subimagespecs[i]); } } - ImageCache::destroy(imagecache); } bool ok = true; diff --git a/src/idiff/idiff.cpp b/src/idiff/idiff.cpp index 814d854f12..b7aadef496 100644 --- a/src/idiff/idiff.cpp +++ b/src/idiff/idiff.cpp @@ -119,8 +119,9 @@ getargs(int argc, char* argv[]) static bool -read_input(const std::string& filename, ImageBuf& img, ImageCache* cache, - int subimage = 0, int miplevel = 0) +read_input(const std::string& filename, ImageBuf& img, + std::shared_ptr cache, int subimage = 0, + int miplevel = 0) { if (img.subimage() >= 0 && img.subimage() == subimage && img.miplevel() == miplevel) @@ -232,7 +233,7 @@ main(int argc, char* argv[]) // Create a private ImageCache so we can customize its cache size // and instruct it store everything internally as floats. - ImageCache* imagecache = ImageCache::create(true); + std::shared_ptr imagecache = ImageCache::create(true); imagecache->attribute("forcefloat", 1); if (sizeof(void*) == 4) // 32 bit or 64? imagecache->attribute("max_memory_MB", 512.0); @@ -418,7 +419,6 @@ main(int argc, char* argv[]) } imagecache->invalidate_all(true); - ImageCache::destroy(imagecache); shutdown(); return ret; } diff --git a/src/include/OpenImageIO/imagebuf.h b/src/include/OpenImageIO/imagebuf.h index de9b13a052..61946a17a4 100644 --- a/src/include/OpenImageIO/imagebuf.h +++ b/src/include/OpenImageIO/imagebuf.h @@ -77,7 +77,7 @@ enum class InitializePixels { No = 0, Yes = 1 }; /// translate into a different data format than appears in the file). /// /// ImageBuf data coming from disk files may optionally be backed by -/// ImageCache, by explicitly passing an `ImageCache*` to the ImageBuf +/// ImageCache, by explicitly passing an ImageCache to the ImageBuf /// constructor or `reset()` method (pass `ImageCache::create()` to get a /// pointer to the default global ImageCache), or by having previously set the /// global OIIO attribute `"imagebuf:use_imagecache"` to a nonzero value. When @@ -186,9 +186,9 @@ class OIIO_API ImageBuf { /// ensure that it remains valid for the lifetime of the ImageBuf. /// explicit ImageBuf(string_view name, int subimage = 0, int miplevel = 0, - ImageCache* imagecache = nullptr, - const ImageSpec* config = nullptr, - Filesystem::IOProxy* ioproxy = nullptr); + std::shared_ptr imagecache = {}, + const ImageSpec* config = nullptr, + Filesystem::IOProxy* ioproxy = nullptr); /// Construct a writable ImageBuf with the given specification /// (including resolution, data type, metadata, etc.). The ImageBuf will @@ -270,9 +270,9 @@ class OIIO_API ImageBuf { /// as if newly constructed with the same arguments, as a read-only /// representation of an existing image file. void reset(string_view name, int subimage = 0, int miplevel = 0, - ImageCache* imagecache = nullptr, - const ImageSpec* config = nullptr, - Filesystem::IOProxy* ioproxy = nullptr); + std::shared_ptr imagecache = {}, + const ImageSpec* config = nullptr, + Filesystem::IOProxy* ioproxy = nullptr); /// Destroy any previous contents of the ImageBuf and re-initialize it /// as if newly constructed with the same arguments, as a read/write @@ -1005,9 +1005,9 @@ class OIIO_API ImageBuf { /// image being in RAM somewhere? bool cachedpixels() const; - /// A pointer to the underlying ImageCache, or nullptr if this ImageBuf - /// is not backed by an ImageCache. - ImageCache* imagecache() const; + /// A shared pointer to the underlying ImageCache, or empty if this + /// ImageBuf is not backed by an ImageCache. + std::shared_ptr imagecache() const; /// Return the address where pixel `(x,y,z)`, channel `ch`, is stored in /// the image buffer. Use with extreme caution! Will return `nullptr` @@ -1151,7 +1151,7 @@ class OIIO_API ImageBuf { // Deprecated things -- might be removed at any time OIIO_DEPRECATED("Use `ImageBuf(name, 0, 0, imagecache, nullptr)` (2.2)") - ImageBuf(string_view name, ImageCache* imagecache) + ImageBuf(string_view name, std::shared_ptr imagecache) : ImageBuf(name, 0, 0, imagecache) { } @@ -1164,7 +1164,7 @@ class OIIO_API ImageBuf { } OIIO_DEPRECATED("Use `reset(name, 0, 0, imagecache)` (2.2)") - void reset(string_view name, ImageCache* imagecache) + void reset(string_view name, std::shared_ptr imagecache) { reset(name, 0, 0, imagecache); } diff --git a/src/include/OpenImageIO/imagecache.h b/src/include/OpenImageIO/imagecache.h index 2cb7bb89ad..63ff62c491 100644 --- a/src/include/OpenImageIO/imagecache.h +++ b/src/include/OpenImageIO/imagecache.h @@ -24,6 +24,9 @@ // Does invalidate() support the optional `force` flag? #define OIIO_IMAGECACHE_INVALIDATE_FORCE 1 +// Does ImageCache::create() return a shared pointer? +#define OIIO_IMAGECACHE_CREATE_SHARED 1 + OIIO_NAMESPACE_BEGIN @@ -56,8 +59,7 @@ class OIIO_API ImageCache { /// or destroy the concrete implementation, so two static methods of /// ImageCache are provided: - /// Create a ImageCache and return a raw pointer to it. This should - /// only be freed by passing it to `ImageCache::destroy()`! + /// Create a ImageCache and return a shared pointer to it. /// /// @param shared /// If `true`, the pointer returned will be a shared ImageCache (so @@ -66,22 +68,20 @@ class OIIO_API ImageCache { /// completely unique ImageCache will be created and returned. /// /// @returns - /// A raw pointer to an ImageCache, which can only be freed with + /// A shared pointer to an ImageCache, which can only be freed with /// `ImageCache::destroy()`. /// /// @see ImageCache::destroy - static ImageCache* create(bool shared = true); + static std::shared_ptr create(bool shared = true); - /// Destroy an allocated ImageCache, including freeing all system - /// resources that it holds. - /// - /// It is safe to destroy even a shared ImageCache, as the implementation - /// of `destroy()` will recognize a shared one and only truly release - /// its resources if it has been requested to be destroyed as many times as - /// shared ImageCache's were created. + /// Release the shared_ptr to an ImageCache, including freeing all + /// system resources that it holds if no one else is still using it. This + /// is not strictly necessary to call, simply destroying the shared_ptr + /// will do the same thing, but this call is for backward compatibility + /// and is helpful if you want to use the teardown option. /// /// @param cache - /// Raw pointer to the ImageCache to destroy. + /// Shared pointer to the ImageCache to destroy. /// /// @param teardown /// For a shared ImageCache, if the `teardown` parameter is @@ -89,7 +89,7 @@ class OIIO_API ImageCache { /// nobody else is still holding a reference (otherwise, it will /// leave it intact). This parameter has no effect if `cache` was /// not the single globally shared ImageCache. - static void destroy(ImageCache* cache, bool teardown = false); + static void destroy(std::shared_ptr& cache, bool teardown = false); /// @} diff --git a/src/include/OpenImageIO/texture.h b/src/include/OpenImageIO/texture.h index 3dea2d822f..43eba9a7f8 100644 --- a/src/include/OpenImageIO/texture.h +++ b/src/include/OpenImageIO/texture.h @@ -27,6 +27,9 @@ #define OIIO_TEXTURESYSTEM_SUPPORTS_STOCHASTIC 1 #define OIIO_TEXTURESYSTEM_SUPPORTS_DECODE_BY_USTRINGHASH 1 +// Does TextureSystem::create() return a shared pointer? +#define OIIO_TEXTURESYSTEM_CREATE_SHARED 1 + #ifndef INCLUDED_IMATHVEC_H // Placeholder declaration for Imath::V3f if no Imath headers have been // included. @@ -365,8 +368,7 @@ class OIIO_API TextureSystem { /// or destroy the concrete implementation, so two static methods of /// TextureSystem are provided: - /// Create a TextureSystem and return a pointer to it. This should only - /// be freed by passing it to TextureSystem::destroy()! + /// Create a TextureSystem and return a shared pointer to it. /// /// @param shared /// If `shared` is `true`, the pointer returned will be a shared @@ -376,32 +378,30 @@ class OIIO_API TextureSystem { /// completely unique TextureCache will be created and returned. /// /// @param imagecache - /// If `shared` is `false` and `imagecache` is not `nullptr`, the + /// If `shared` is `false` and `imagecache` is not empty, the /// TextureSystem will use this as its underlying ImageCache. In /// that case, it is the caller who is responsible for eventually /// freeing the ImageCache after the TextureSystem is destroyed. If - /// `shared` is `false` and `imagecache` is `nullptr`, then a custom + /// `shared` is `false` and `imagecache` is empty, then a custom /// ImageCache will be created, owned by the TextureSystem, and /// automatically freed when the TS destroys. /// /// @returns - /// A raw pointer to a TextureSystem, which can only be freed with - /// `TextureSystem::destroy()`. + /// A shared pointer to a TextureSystem which will be destroyed only + /// when the last shared_ptr to it is destroyed. /// /// @see TextureSystem::destroy - static TextureSystem *create (bool shared=true, - ImageCache *imagecache=nullptr); + static std::shared_ptr create(bool shared=true, + std::shared_ptr imagecache = {}); - /// Destroy an allocated TextureSystem, including freeing all system - /// resources that it holds. - /// - /// It is safe to destroy even a shared TextureSystem, as the - /// implementation of `destroy()` will recognize a shared one and only - /// truly release its resources if it has been requested to be destroyed - /// as many times as shared TextureSystem's were created. + /// Release the shared_ptr to a TextureSystem, including freeing all + /// system resources that it holds if no one else is still using it. This + /// is not strictly necessary to call, simply destroying the shared_ptr + /// will do the same thing, but this call is for backward compatibility + /// and is helpful if you want to use the teardown_imagecache option. /// /// @param ts - /// Raw pointer to the TextureSystem to destroy. + /// Shared pointer to the TextureSystem to destroy. /// /// @param teardown_imagecache /// For a shared TextureSystem, if the `teardown_imagecache` @@ -409,8 +409,8 @@ class OIIO_API TextureSystem { /// cache if nobody else is still holding a reference (otherwise, it /// will leave it intact). This parameter has no effect if `ts` was /// not the single globally shared TextureSystem. - static void destroy (TextureSystem *ts, - bool teardown_imagecache = false); + static void destroy(std::shared_ptr& ts, + bool teardown_imagecache = false); /// @} @@ -1652,7 +1652,7 @@ class OIIO_API TextureSystem { /// Return an opaque, non-owning pointer to the underlying ImageCache /// (if there is one). - virtual ImageCache *imagecache () const = 0; + virtual std::shared_ptr imagecache() const = 0; virtual ~TextureSystem () { } diff --git a/src/iv/imageviewer.cpp b/src/iv/imageviewer.cpp index a22c5eae81..e4e3822f37 100644 --- a/src/iv/imageviewer.cpp +++ b/src/iv/imageviewer.cpp @@ -791,7 +791,7 @@ ImageViewer::readSettings(bool ui_is_set_up) OIIO::attribute("imagebuf:use_imagecache", 1); - ImageCache* imagecache = ImageCache::create(true); + auto imagecache = ImageCache::create(true); imagecache->attribute("automip", autoMipmap->isChecked()); imagecache->attribute("max_memory_MB", (float)maxMemoryIC->value()); } diff --git a/src/iv/ivmain.cpp b/src/iv/ivmain.cpp index 1685a19ec2..f6850d4fba 100644 --- a/src/iv/ivmain.cpp +++ b/src/iv/ivmain.cpp @@ -129,7 +129,7 @@ main(int argc, char* argv[]) mainWin->show(); // Set up the imagecache with parameters that make sense for iv - ImageCache* imagecache = ImageCache::create(true); + auto imagecache = ImageCache::create(true); imagecache->attribute("autotile", 256); imagecache->attribute("deduplicate", (int)0); if (ap["no-autopremult"].get()) diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index 6781d7ef2a..25a4c5b94e 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -89,7 +89,7 @@ set_roi_full(ImageSpec& spec, const ROI& newroi) class ImageBufImpl { public: ImageBufImpl(string_view filename, int subimage, int miplevel, - ImageCache* imagecache = nullptr, + std::shared_ptr imagecache = {}, const ImageSpec* spec = nullptr, void* buffer = nullptr, const ImageSpec* config = nullptr, Filesystem::IOProxy* ioproxy = nullptr, @@ -100,7 +100,7 @@ class ImageBufImpl { void clear(); void reset(string_view name, int subimage, int miplevel, - ImageCache* imagecache, const ImageSpec* config, + std::shared_ptr imagecache, const ImageSpec* config, Filesystem::IOProxy* ioproxy); // Reset the buf to blank, given the spec. If nativespec is also // supplied, use it for the "native" spec, otherwise make the nativespec @@ -252,7 +252,7 @@ class ImageBufImpl { // Invalidate the file in our imagecache and the shared one void invalidate(ustring filename, bool force) { - ImageCache* shared_imagecache = ImageCache::create(true); + auto shared_imagecache = ImageCache::create(true); OIIO_DASSERT(shared_imagecache); if (m_imagecache) m_imagecache->invalidate(filename, force); // *our* IC @@ -298,11 +298,11 @@ class ImageBufImpl { stride_t m_zstride; stride_t m_channel_stride; bool m_contiguous; - ImageCache* m_imagecache = nullptr; ///< ImageCache to use - TypeDesc m_cachedpixeltype; ///< Data type stored in the cache - DeepData m_deepdata; ///< Deep data - size_t m_allocated_size; ///< How much memory we've allocated - std::vector m_blackpixel; ///< Pixel-sized zero bytes + std::shared_ptr m_imagecache; ///< ImageCache to use + TypeDesc m_cachedpixeltype; ///< Data type stored in the cache + DeepData m_deepdata; ///< Deep data + size_t m_allocated_size; ///< How much memory we've allocated + std::vector m_blackpixel; ///< Pixel-sized zero bytes std::vector m_write_format; /// Pixel data format to use for write() int m_write_tile_width; int m_write_tile_height; @@ -348,8 +348,9 @@ ImageBuf::impl_deleter(ImageBufImpl* todel) ImageBufImpl::ImageBufImpl(string_view filename, int subimage, int miplevel, - ImageCache* imagecache, const ImageSpec* spec, - void* buffer, const ImageSpec* config, + std::shared_ptr imagecache, + const ImageSpec* spec, void* buffer, + const ImageSpec* config, Filesystem::IOProxy* ioproxy, stride_t xstride, stride_t ystride, stride_t zstride) : m_storage(ImageBuf::UNINITIALIZED) @@ -401,7 +402,8 @@ ImageBufImpl::ImageBufImpl(string_view filename, int subimage, int miplevel, } else if (filename.length() > 0) { // filename being nonempty means this ImageBuf refers to a file. OIIO_DASSERT(buffer == nullptr); - reset(filename, subimage, miplevel, imagecache, config, ioproxy); + reset(filename, subimage, miplevel, std::move(imagecache), config, + ioproxy); } else { OIIO_DASSERT(buffer == nullptr); } @@ -503,11 +505,11 @@ ImageBuf::ImageBuf() ImageBuf::ImageBuf(string_view filename, int subimage, int miplevel, - ImageCache* imagecache, const ImageSpec* config, - Filesystem::IOProxy* ioproxy) - : m_impl(new ImageBufImpl(filename, subimage, miplevel, imagecache, - nullptr /*spec*/, nullptr /*buffer*/, config, - ioproxy), + std::shared_ptr imagecache, + const ImageSpec* config, Filesystem::IOProxy* ioproxy) + : m_impl(new ImageBufImpl(filename, subimage, miplevel, + std::move(imagecache), nullptr /*spec*/, + nullptr /*buffer*/, config, ioproxy), &impl_deleter) { } @@ -710,7 +712,7 @@ ImageBufImpl::clear() m_zstride = 0; m_channel_stride = 0; m_contiguous = false; - m_imagecache = nullptr; + m_imagecache.reset(); m_deepdata.free(); m_blackpixel.clear(); m_write_format.clear(); @@ -735,8 +737,8 @@ ImageBuf::clear() void ImageBufImpl::reset(string_view filename, int subimage, int miplevel, - ImageCache* imagecache, const ImageSpec* config, - Filesystem::IOProxy* ioproxy) + std::shared_ptr imagecache, + const ImageSpec* config, Filesystem::IOProxy* ioproxy) { clear(); m_name = ustring(filename); @@ -747,7 +749,7 @@ ImageBufImpl::reset(string_view filename, int subimage, int miplevel, } m_current_subimage = subimage; m_current_miplevel = miplevel; - m_imagecache = imagecache; + m_imagecache = std::move(imagecache); if (config) m_configspec.reset(new ImageSpec(*config)); m_rioproxy = ioproxy; @@ -768,10 +770,11 @@ ImageBufImpl::reset(string_view filename, int subimage, int miplevel, void ImageBuf::reset(string_view filename, int subimage, int miplevel, - ImageCache* imagecache, const ImageSpec* config, + std::shared_ptr imagecache, const ImageSpec* config, Filesystem::IOProxy* ioproxy) { - m_impl->reset(filename, subimage, miplevel, imagecache, config, ioproxy); + m_impl->reset(filename, subimage, miplevel, std::move(imagecache), config, + ioproxy); } @@ -899,7 +902,7 @@ ImageBufImpl::init_spec(string_view filename, int subimage, int miplevel, // If we weren't given an imagecache but "imagebuf:use_imagecache" // attribute was set, use a shared IC. - if (m_imagecache == nullptr && pvt::imagebuf_use_imagecache) + if (!m_imagecache && pvt::imagebuf_use_imagecache) m_imagecache = ImageCache::create(true); if (m_imagecache) { @@ -1849,7 +1852,7 @@ ImageBuf::cachedpixels() const -ImageCache* +std::shared_ptr ImageBuf::imagecache() const { return m_impl->m_imagecache; diff --git a/src/libOpenImageIO/imagebuf_test.cpp b/src/libOpenImageIO/imagebuf_test.cpp index c9d67a5524..912a7ef6d6 100644 --- a/src/libOpenImageIO/imagebuf_test.cpp +++ b/src/libOpenImageIO/imagebuf_test.cpp @@ -306,7 +306,7 @@ test_open_with_config() { // N.B. This function must run after ImageBuf_test_appbuffer, which // writes "A.tif". - ImageCache* ic = ImageCache::create(false); + auto ic = ImageCache::create(false); ImageSpec config; config.attribute("oiio:DebugOpenConfig!", 1); ImageBuf A("A_imagebuf_test.tif", 0, 0, ic, &config); @@ -315,7 +315,6 @@ test_open_with_config() // Clear A because it would be unwise to let the ImageBuf outlive the // custom ImageCache we passed it to use. A.clear(); - ic->destroy(ic); } diff --git a/src/libOpenImageIO/imagecache_test.cpp b/src/libOpenImageIO/imagecache_test.cpp index 54432be314..f766d88ca4 100644 --- a/src/libOpenImageIO/imagecache_test.cpp +++ b/src/libOpenImageIO/imagecache_test.cpp @@ -53,7 +53,7 @@ static void test_get_pixels_errors() { Strutil::print("\nTesting get_pixels error handling\n"); - ImageCache* ic = ImageCache::create(); + std::shared_ptr ic = ImageCache::create(); float fpixels[4 * 4 * 3]; const int fpixelsize = 3 * sizeof(float); @@ -121,7 +121,7 @@ test_get_pixels_cachechannels(int chbegin = 0, int chend = 4, std::cout << "\nTesting IC get_pixels of chans [" << chbegin << "," << chend << ") with cache range [" << cache_chbegin << "," << cache_chend << "):\n"; - ImageCache* imagecache = ImageCache::create(false /*not shared*/); + auto imagecache = ImageCache::create(false); // Create a 10 channel file ustring filename("tenchannels.tif"); @@ -151,8 +151,6 @@ test_get_pixels_cachechannels(int chbegin = 0, int chend = 4, } for (int c = 2 * nc; c < 2 * nchans; ++c) OIIO_CHECK_EQUAL(p[c], -1.0f); - - ImageCache::destroy(imagecache); } @@ -172,7 +170,7 @@ NullInputCreator() void test_app_buffer() { - ImageCache* imagecache = ImageCache::create(false /*not shared*/); + auto imagecache = ImageCache::create(false /*not shared*/); // Add a file entry with a "null" ImageInput proxy configured to look // like a 2x2 RGB float image. @@ -226,8 +224,6 @@ test_app_buffer() OIIO_CHECK_EQUAL(testpixel[0], pixels[1][1][0]); OIIO_CHECK_EQUAL(testpixel[1], pixels[1][1][1]); OIIO_CHECK_EQUAL(testpixel[2], pixels[1][1][2]); - - ImageCache::destroy(imagecache); } @@ -236,8 +232,8 @@ void test_custom_threadinfo() { Strutil::print("\nTesting creating/destroying custom IC and thread info\n"); - ImageCache* imagecache = ImageCache::create(true); - auto threadinfo = imagecache->create_thread_info(); + auto imagecache = ImageCache::create(true); + auto threadinfo = imagecache->create_thread_info(); OIIO_CHECK_ASSERT(threadinfo != nullptr); imagecache->destroy_thread_info(threadinfo); imagecache->close_all(); @@ -249,7 +245,7 @@ void test_tileptr() { Strutil::print("\nTesting tile ptr things\n"); - ImageCache* imagecache = ImageCache::create(); + auto imagecache = ImageCache::create(); auto hand = imagecache->get_image_handle(checkertex); ImageCache::Tile* tile = imagecache->get_tile(hand, nullptr, 0, 0, 4, 4, 0); OIIO_CHECK_ASSERT(tile != nullptr); @@ -277,7 +273,7 @@ static void test_imagespec() { Strutil::print("\nTesting imagespec retrieval\n"); - ImageCache* ic = ImageCache::create(); + auto ic = ImageCache::create(); { // basic get_imagespec() ImageSpec spec; @@ -350,7 +346,7 @@ main(int /*argc*/, char* /*argv*/[]) test_custom_threadinfo(); test_imagespec(); - ImageCache* ic = ImageCache::create(); + auto ic = ImageCache::create(); Strutil::print("\n\n{}\n", ic->getstats(5)); ic->reset_stats(); diff --git a/src/libOpenImageIO/imagespeed_test.cpp b/src/libOpenImageIO/imagespeed_test.cpp index 6c67991e04..d929023317 100644 --- a/src/libOpenImageIO/imagespeed_test.cpp +++ b/src/libOpenImageIO/imagespeed_test.cpp @@ -34,7 +34,7 @@ static std::string output_filename; static std::string output_format; static std::vector buffer; static ImageSpec bufspec, outspec; -static ImageCache* imagecache = NULL; +static std::shared_ptr imagecache; static imagesize_t total_image_pixels = 0; static float cache_size = 0; @@ -611,6 +611,5 @@ main(int argc, char** argv) if (verbose) std::cout << "\n" << imagecache->getstats(2) << "\n"; - ImageCache::destroy(imagecache); return unit_test_failures; } diff --git a/src/libtexture/imagecache.cpp b/src/libtexture/imagecache.cpp index c048b04fd1..9c7c480135 100644 --- a/src/libtexture/imagecache.cpp +++ b/src/libtexture/imagecache.cpp @@ -51,7 +51,7 @@ spin_mutex ImageCacheImpl::m_perthread_info_mutex; namespace { // anonymous static thread_local tsl::robin_map imcache_error_messages; -static std::shared_ptr shared_image_cache; +static std::shared_ptr shared_image_cache; static spin_mutex shared_image_cache_mutex; // Make some static ustring constants to avoid strcmp's @@ -3934,53 +3934,41 @@ ImageCacheImpl::append_error(string_view message) const -ImageCache* +std::shared_ptr ImageCache::create(bool shared) { if (shared) { // They requested a shared cache. If a shared cache already // exists, just return it, otherwise record the new cache. spin_lock guard(shared_image_cache_mutex); - if (!shared_image_cache.get()) - shared_image_cache.reset(aligned_new(), - aligned_delete); - -#if 0 - std::cerr << " shared ImageCache is " - << (void *)shared_image_cache.get() << "\n"; -#endif - return shared_image_cache.get(); + if (!shared_image_cache) + shared_image_cache = std::make_shared(); + return shared_image_cache; } // Doesn't need a shared cache - ImageCacheImpl* ic = aligned_new(); -#if 0 - std::cerr << "creating new ImageCache " << (void *)ic << "\n"; -#endif - return ic; + return std::make_shared(); } void -ImageCache::destroy(ImageCache* x, bool teardown) +ImageCache::destroy(std::shared_ptr& cache, bool teardown) { - if (!x) + if (!cache) return; spin_lock guard(shared_image_cache_mutex); - if (x == shared_image_cache.get()) { + if (cache.get() == shared_image_cache.get()) { // This is the shared cache, so don't really delete it. Invalidate // it fully, closing the files and throwing out any tiles that // nobody is currently holding references to. But only delete the // IC fully if 'teardown' is true, and even then, it won't destroy // until nobody else is still holding a shared_ptr to it. - ((ImageCacheImpl*)x)->invalidate_all(teardown); + cache->invalidate_all(teardown); if (teardown) shared_image_cache.reset(); - } else { - // Not a shared cache, we are the only owner, so truly destroy it. - aligned_delete(x); } + cache.reset(); } diff --git a/src/libtexture/texture_pvt.h b/src/libtexture/texture_pvt.h index a616c86623..b7d231c35c 100644 --- a/src/libtexture/texture_pvt.h +++ b/src/libtexture/texture_pvt.h @@ -37,7 +37,7 @@ class TextureSystemImpl final : public TextureSystem { public: typedef ImageCacheFile TextureFile; - TextureSystemImpl(ImageCache* imagecache); + TextureSystemImpl(std::shared_ptr imagecache); ~TextureSystemImpl() override; bool attribute(string_view name, TypeDesc type, const void* val) override; @@ -283,7 +283,10 @@ class TextureSystemImpl final : public TextureSystem { /// Return an opaque, non-owning pointer to the underlying ImageCache /// (if there is one). - ImageCache* imagecache() const override { return m_imagecache; } + std::shared_ptr imagecache() const override + { + return m_imagecache_sp; + } private: typedef ImageCacheTileRef TileRef; @@ -495,6 +498,7 @@ class TextureSystemImpl final : public TextureSystem { void visualize_ellipse(const std::string& name, float dsdx, float dtdx, float dsdy, float dtdy, float sblur, float tblur); + std::shared_ptr m_imagecache_sp; ImageCacheImpl* m_imagecache = nullptr; uint64_t m_id; // A unique ID for this TextureSystem Imath::M44f m_Mw2c; ///< world-to-"common" matrix diff --git a/src/libtexture/texturesys.cpp b/src/libtexture/texturesys.cpp index a29a43e2f3..8d5400ce6b 100644 --- a/src/libtexture/texturesys.cpp +++ b/src/libtexture/texturesys.cpp @@ -47,7 +47,7 @@ namespace { // anonymous // The only easy way to fix this is to make shared_texturesys be an ordinary // pointer and just let it leak (who cares? the app is done, and it only // contains a few hundred bytes). -static TextureSystemImpl* shared_texturesys = NULL; +static std::shared_ptr shared_texturesys; static spin_mutex shared_texturesys_mutex; static bool do_unit_test_texture = false; static float unit_test_texture_blur = 0.0f; @@ -81,14 +81,14 @@ static const OIIO_SIMD4_ALIGN vbool4 channel_masks[5] = { } // end anonymous namespace -TextureSystem* -TextureSystem::create(bool shared, ImageCache* imagecache) +std::shared_ptr +TextureSystem::create(bool shared, std::shared_ptr imagecache) { // Because the shared_texturesys is never deleted (by design) // we silence the otherwise useful compiler warning on newer GCC versions OIIO_PRAGMA_WARNING_PUSH #if OIIO_GNUC_VERSION > 100000 - OIIO_GCC_ONLY_PRAGMA(GCC diagnostic ignored "-Wmismatched-new-delete") + // OIIO_GCC_ONLY_PRAGMA(GCC diagnostic ignored "-Wmismatched-new-delete") #endif if (shared) { // They requested a shared texture system. If a shared one already @@ -96,11 +96,8 @@ TextureSystem::create(bool shared, ImageCache* imagecache) // as the shared one. spin_lock guard(shared_texturesys_mutex); if (!shared_texturesys) - shared_texturesys = new TextureSystemImpl(ImageCache::create(true)); -#if 0 - std::cerr << " shared TextureSystem is " - << (void *)shared_texturesys << "\n"; -#endif + shared_texturesys = std::make_shared( + ImageCache::create(true)); return shared_texturesys; } @@ -110,11 +107,8 @@ TextureSystem::create(bool shared, ImageCache* imagecache) imagecache = ImageCache::create(false); own_ic = true; } - TextureSystemImpl* ts = new TextureSystemImpl(imagecache); + auto ts = std::make_shared(imagecache); ts->m_imagecache_owner = own_ic; -#if 0 - std::cerr << "creating new ImageCache " << (void *)ts << "\n"; -#endif OIIO_PRAGMA_WARNING_POP return ts; } @@ -122,25 +116,20 @@ TextureSystem::create(bool shared, ImageCache* imagecache) void -TextureSystem::destroy(TextureSystem* x, bool teardown_imagecache) +TextureSystem::destroy(std::shared_ptr& ts, + bool teardown_imagecache) { - // std::cerr << "Destroying TS " << (void *)x << "\n"; - if (!x) + if (!ts) return; - TextureSystemImpl* impl = (TextureSystemImpl*)x; + TextureSystemImpl* impl = (TextureSystemImpl*)ts.get(); if (teardown_imagecache) { if (impl->m_imagecache_owner) - ImageCache::destroy(impl->m_imagecache, true); + ImageCache::destroy(impl->m_imagecache_sp, true); impl->m_imagecache = nullptr; + impl->m_imagecache_sp.reset(); } - spin_lock guard(shared_texturesys_mutex); - if (impl == shared_texturesys) { - // This is the shared TS, so don't really delete it. - } else { - // Not a shared cache, we are the only owner, so truly destroy it. - delete x; - } + ts.reset(); } @@ -324,10 +313,11 @@ texture_type_name(TexFormat f) -TextureSystemImpl::TextureSystemImpl(ImageCache* imagecache) +TextureSystemImpl::TextureSystemImpl(std::shared_ptr imagecache) : m_id(++txsys_next_id) { - m_imagecache = (ImageCacheImpl*)imagecache; + m_imagecache_sp = imagecache; + m_imagecache = (ImageCacheImpl*)m_imagecache_sp.get(); init(); } @@ -3120,7 +3110,7 @@ TextureSystem::unit_test_hash() Strutil::print("Testing hashing with {} files of {}x{} with {}x{} tiles:", nfiles, res, res, tilesize, tilesize); - ImageCache* imagecache = ImageCache::create(); + auto imagecache = ImageCache::create(); // Set up the ImageCacheFiles outside of the timing loop using OIIO::pvt::ImageCacheFile; @@ -3129,8 +3119,8 @@ TextureSystem::unit_test_hash() std::vector icf; for (int f = 0; f < nfiles; ++f) { ustring filename = ustring::fmtformat("{:06}.tif", f); - icf.push_back( - new ImageCacheFile(*(ImageCacheImpl*)imagecache, NULL, filename)); + icf.push_back(new ImageCacheFile(*(ImageCacheImpl*)imagecache.get(), + nullptr, filename)); } // First, just try to do raw timings of the hash @@ -3208,10 +3198,7 @@ TextureSystem::unit_test_hash() max = sixteenbits[i]; } Strutil::print("16-bit hash buckets range from {} to {}\n", min, max); - Strutil::print("\n"); - - ImageCache::destroy(imagecache); #endif } diff --git a/src/maketx/maketx.cpp b/src/maketx/maketx.cpp index 3feeaf4d64..4900f41eb1 100644 --- a/src/maketx/maketx.cpp +++ b/src/maketx/maketx.cpp @@ -502,7 +502,7 @@ getargs(int argc, char* argv[], ImageSpec& configspec) if (ignore_unassoc) { configspec.attribute("maketx:ignore_unassoc", (int)ignore_unassoc); - ImageCache* ic = ImageCache::create(); // get the shared one + auto ic = ImageCache::create(); // get the shared one ic->attribute("unassociatedalpha", (int)ignore_unassoc); } @@ -532,7 +532,7 @@ main(int argc, char* argv[]) OIIO::attribute("threads", nthreads); // N.B. This will apply to the default IC that any ImageBuf's get. - ImageCache* ic = ImageCache::create(); // get the shared one + auto ic = ImageCache::create(); // get the shared one ic->attribute("forcefloat", 1); // Force float upon read ic->attribute("max_memory_MB", 1024.0); // 1 GB cache diff --git a/src/oiiotool/imagerec.cpp b/src/oiiotool/imagerec.cpp index 4ea7fc2ac5..54703f8df8 100644 --- a/src/oiiotool/imagerec.cpp +++ b/src/oiiotool/imagerec.cpp @@ -182,7 +182,7 @@ ImageRec::ImageRec(ImageBufRef img, bool copy_pixels) ImageRec::ImageRec(const std::string& name, const ImageSpec& spec, - ImageCache* imagecache) + std::shared_ptr imagecache) : m_name(name) , m_elaborated(true) , m_pixels_modified(true) diff --git a/src/oiiotool/oiiotool.h b/src/oiiotool/oiiotool.h index 48706faedb..8f05bf58d5 100644 --- a/src/oiiotool/oiiotool.h +++ b/src/oiiotool/oiiotool.h @@ -140,7 +140,7 @@ class Oiiotool { ImageRecRef curimg; // current image std::vector image_stack; // stack of previous images std::map image_labels; // labeled images - ImageCache* imagecache = nullptr; // back ptr to ImageCache + std::shared_ptr imagecache; // back ptr to ImageCache ColorConfig colorconfig; // OCIO color config Timer total_runtime; // total_readtime is the amount of time for direct reads, and does not @@ -434,7 +434,7 @@ class SubimageRec { /// and potentially MIPmap levels for each subimage. class ImageRec { public: - ImageRec(const std::string& name, ImageCache* imagecache) + ImageRec(const std::string& name, std::shared_ptr imagecache) : m_name(name) , m_imagecache(imagecache) { @@ -471,7 +471,7 @@ class ImageRec { // Initialize an ImageRec with the given spec. ImageRec(const std::string& name, const ImageSpec& spec, - ImageCache* imagecache); + std::shared_ptr imagecache); ImageRec(const ImageRec& copy) = delete; // Disallow copy ctr @@ -618,7 +618,7 @@ class ImageRec { std::vector m_subimages; std::time_t m_time; //< Modification time of the input file TypeDesc m_input_dataformat; - ImageCache* m_imagecache = nullptr; + std::shared_ptr m_imagecache; mutable std::string m_err; std::unique_ptr m_configspec; diff --git a/src/python/py_imagecache.cpp b/src/python/py_imagecache.cpp index 5b2d69db09..6d88e48416 100644 --- a/src/python/py_imagecache.cpp +++ b/src/python/py_imagecache.cpp @@ -9,10 +9,7 @@ namespace PyOpenImageIO { // Make a special wrapper to help with the weirdo way we use create/destroy. class ImageCacheWrap { public: - struct ICDeleter { - void operator()(ImageCache* p) const { ImageCache::destroy(p); } - }; - std::unique_ptr m_cache; + std::shared_ptr m_cache; ImageCacheWrap(bool shared = true) : m_cache(ImageCache::create(shared)) @@ -23,7 +20,7 @@ class ImageCacheWrap { ~ImageCacheWrap() {} // will call the deleter on the IC static void destroy(ImageCacheWrap* x, bool teardown = false) { - ImageCache::destroy(x->m_cache.release(), teardown); + ImageCache::destroy(x->m_cache, teardown); } py::object get_pixels(const std::string& filename, int subimage, int miplevel, int xbegin, int xend, int ybegin, diff --git a/src/python/py_texturesys.cpp b/src/python/py_texturesys.cpp index 3699f8745b..b51e1bc10e 100644 --- a/src/python/py_texturesys.cpp +++ b/src/python/py_texturesys.cpp @@ -44,14 +44,11 @@ class TextureOptWrap : public TextureOpt { // Make a special wrapper to help with the weirdo way we use create/destroy. class TextureSystemWrap { public: - struct TSDeleter { - void operator()(TextureSystem* p) const { TextureSystem::destroy(p); } - }; - std::unique_ptr m_texsys; + std::shared_ptr m_texsys; TextureSystemWrap(bool shared = true) - : m_texsys(TextureSystem::create(shared, nullptr)) + : m_texsys(TextureSystem::create(shared)) { } TextureSystemWrap(const TextureSystemWrap&) = delete; @@ -59,7 +56,7 @@ class TextureSystemWrap { ~TextureSystemWrap() {} // will call the deleter on the m_texsys static void destroy(TextureSystemWrap* x) { - TextureSystem::destroy(x->m_texsys.release()); + TextureSystem::destroy(x->m_texsys); } }; diff --git a/src/testtex/testtex.cpp b/src/testtex/testtex.cpp index 95bce60338..95eebb43f7 100644 --- a/src/testtex/testtex.cpp +++ b/src/testtex/testtex.cpp @@ -56,7 +56,7 @@ static bool test_construction = false; static bool test_gettexels = false; static bool test_getimagespec = false; static bool filtertest = false; -static TextureSystem* texsys = NULL; +static std::shared_ptr texsys; static std::string searchpath; static bool batch = false; static bool nowarp = false; @@ -1649,7 +1649,7 @@ test_icwrite(int testicwrite) // The global "shared" ImageCache will be the same one the // TextureSystem uses. - ImageCache* ic = ImageCache::create(); + std::shared_ptr ic = ImageCache::create(); // Set up the fake file and add it int tw = 64, th = 64; // tile width and height