From fcfcca977c23e318769171f06fc8554869495870 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 26 Sep 2024 16:44:56 -0400 Subject: Ditch last instances of LL_LIBRARY_INCLUDE. --- indra/llcommon/llerror.h | 5 ----- indra/llcommon/llpointer.h | 5 ----- 2 files changed, 10 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index b17b9ff21e..b6d560a121 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -239,17 +239,12 @@ namespace LLError ~CallSite(); -#ifdef LL_LIBRARY_INCLUDE - bool shouldLog(); -#else // LL_LIBRARY_INCLUDE bool shouldLog() { return mCached ? mShouldLog : Log::shouldLog(*this); } - // this member function needs to be in-line for efficiency -#endif // LL_LIBRARY_INCLUDE void invalidate(); diff --git a/indra/llcommon/llpointer.h b/indra/llcommon/llpointer.h index 71c955c4c5..1b2a961c40 100644 --- a/indra/llcommon/llpointer.h +++ b/indra/llcommon/llpointer.h @@ -189,10 +189,6 @@ public: } protected: -#ifdef LL_LIBRARY_INCLUDE - void ref(); - void unref(); -#else void ref() { if (mPointer) @@ -215,7 +211,6 @@ protected: } } } -#endif // LL_LIBRARY_INCLUDE protected: Type* mPointer; -- cgit v1.2.3 From a2b76b69ca1a29e40616d9279a25f9dd22a81127 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 27 Sep 2024 05:38:39 -0400 Subject: Migrate ~LLPointer()'s peculiar warning case to llpointer.cpp. This allows removing #include "llerror.h" from llpointer.h. Also remove #include "llmutex.h" as a heavy way to get . That requires adding #include "llmutex.h" to llimage.h, llnotifications.h, llwatchdog.cpp and llvolumemgr.cpp, which were inheriting it from llpointer.h. --- indra/llcommon/CMakeLists.txt | 1 + indra/llcommon/llpointer.cpp | 26 ++++++++++++++++++++++++++ indra/llcommon/llpointer.h | 19 ++++++++++++++----- indra/llimage/llimage.h | 5 +++-- indra/llmath/llvolumemgr.cpp | 1 + indra/llui/llnotifications.h | 1 + indra/newview/llwatchdog.cpp | 1 + 7 files changed, 47 insertions(+), 7 deletions(-) create mode 100755 indra/llcommon/llpointer.cpp (limited to 'indra') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 78bfaade55..aa8810f00b 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -70,6 +70,7 @@ set(llcommon_SOURCE_FILES llmetrics.cpp llmortician.cpp llmutex.cpp + llpointer.cpp llpredicate.cpp llprocess.cpp llprocessor.cpp diff --git a/indra/llcommon/llpointer.cpp b/indra/llcommon/llpointer.cpp new file mode 100755 index 0000000000..adea447caa --- /dev/null +++ b/indra/llcommon/llpointer.cpp @@ -0,0 +1,26 @@ +/** + * @file llpointer.cpp + * @author Nat Goodspeed + * @date 2024-09-26 + * @brief Implementation for llpointer. + * + * $LicenseInfo:firstyear=2024&license=viewerlgpl$ + * Copyright (c) 2024, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llpointer.h" +// STL headers +// std headers +// external library headers +// other Linden headers +#include "llerror.h" + +void LLPointerBase::wild_dtor(std::string_view msg) +{ +// LL_WARNS() << msg << LL_ENDL; + llassert_msg(false, msg); +} diff --git a/indra/llcommon/llpointer.h b/indra/llcommon/llpointer.h index 1b2a961c40..b53cfcdd1a 100644 --- a/indra/llcommon/llpointer.h +++ b/indra/llcommon/llpointer.h @@ -26,8 +26,8 @@ #ifndef LLPOINTER_H #define LLPOINTER_H -#include "llerror.h" // *TODO: consider eliminating this -#include "llmutex.h" +#include +#include #include // std::swap() //---------------------------------------------------------------------------- @@ -43,8 +43,18 @@ //---------------------------------------------------------------------------- +class LLPointerBase +{ +protected: + // alert the coder that a referenced type's destructor did something very + // strange -- this is in a non-template base class so we can hide the + // implementation in llpointer.cpp + static void wild_dtor(std::string_view msg); +}; + // Note: relies on Type having ref() and unref() methods -template class LLPointer +template +class LLPointer: public LLPointerBase { public: template @@ -106,7 +116,6 @@ public: const Type& operator*() const { return *mPointer; } Type& operator*() { return *mPointer; } - operator BOOL() const { return (mPointer != nullptr); } operator bool() const { return (mPointer != nullptr); } bool operator!() const { return (mPointer == nullptr); } bool isNull() const { return (mPointer == nullptr); } @@ -206,7 +215,7 @@ protected: temp->unref(); if (mPointer != nullptr) { - LL_WARNS() << "Unreference did assignment to non-NULL because of destructor" << LL_ENDL; + wild_dtor("Unreference did assignment to non-NULL because of destructor"); unref(); } } diff --git a/indra/llimage/llimage.h b/indra/llimage/llimage.h index 6b14b68c78..1fb61673bd 100644 --- a/indra/llimage/llimage.h +++ b/indra/llimage/llimage.h @@ -27,10 +27,11 @@ #ifndef LL_LLIMAGE_H #define LL_LLIMAGE_H -#include "lluuid.h" -#include "llstring.h" +#include "llmutex.h" #include "llpointer.h" +#include "llstring.h" #include "lltrace.h" +#include "lluuid.h" constexpr S32 MIN_IMAGE_MIP = 2; // 4x4, only used for expand/contract power of 2 constexpr S32 MAX_IMAGE_MIP = 12; // 4096x4096 diff --git a/indra/llmath/llvolumemgr.cpp b/indra/llmath/llvolumemgr.cpp index bb0c94d513..d8f649140f 100644 --- a/indra/llmath/llvolumemgr.cpp +++ b/indra/llmath/llvolumemgr.cpp @@ -25,6 +25,7 @@ #include "linden_common.h" +#include "llmutex.h" #include "llvolumemgr.h" #include "llvolume.h" diff --git a/indra/llui/llnotifications.h b/indra/llui/llnotifications.h index 9b83da13ad..9d59b99349 100644 --- a/indra/llui/llnotifications.h +++ b/indra/llui/llnotifications.h @@ -94,6 +94,7 @@ #include "llinitparam.h" #include "llinstancetracker.h" #include "llmortician.h" +#include "llmutex.h" #include "llnotificationptr.h" #include "llpointer.h" #include "llrefcount.h" diff --git a/indra/newview/llwatchdog.cpp b/indra/newview/llwatchdog.cpp index bf171fe954..d9fcd5811d 100644 --- a/indra/newview/llwatchdog.cpp +++ b/indra/newview/llwatchdog.cpp @@ -27,6 +27,7 @@ #include "llviewerprecompiledheaders.h" #include "llwatchdog.h" +#include "llmutex.h" #include "llthread.h" constexpr U32 WATCHDOG_SLEEP_TIME_USEC = 1000000U; -- cgit v1.2.3 From d39c862f856f45f83cd11ac5eb127a0b826a5b3f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 27 Sep 2024 05:40:54 -0400 Subject: Automate memory management in JPEG2KEncode and JPEG2KDecode. Instead of using strdup() and free() to store comment_text in JPEG2KEncode, store a std::string. Similarly, instead of manually managing pointers to encoder, decoder, image, stream and codestream_info in JPEG2KDecode and JPEG2KEncode, use std::unique_ptr for each, specifying their respective deleter functions. --- indra/llimagej2coj/llimagej2coj.cpp | 232 ++++++++++++++---------------------- 1 file changed, 88 insertions(+), 144 deletions(-) (limited to 'indra') diff --git a/indra/llimagej2coj/llimagej2coj.cpp b/indra/llimagej2coj/llimagej2coj.cpp index 6037517103..a2a43e4baa 100644 --- a/indra/llimagej2coj/llimagej2coj.cpp +++ b/indra/llimagej2coj/llimagej2coj.cpp @@ -31,6 +31,8 @@ #include "openjpeg.h" #include "event.h" #include "cio.h" +#include // std::unique_ptr +#include #define MAX_ENCODED_DISCARD_LEVELS 5 @@ -231,33 +233,6 @@ public: parameters.cp_reduce = discardLevel; } - ~JPEG2KDecode() - { - if (decoder) - { - opj_destroy_codec(decoder); - } - decoder = nullptr; - - if (image) - { - opj_image_destroy(image); - } - image = nullptr; - - if (stream) - { - opj_stream_destroy(stream); - } - stream = nullptr; - - if (codestream_info) - { - opj_destroy_cstr_info(&codestream_info); - } - codestream_info = nullptr; - } - bool readHeader( U8* data, U32 dataSize, @@ -268,46 +243,42 @@ public: { parameters.flags |= OPJ_DPARAMETERS_DUMP_FLAG; - decoder = opj_create_decompress(OPJ_CODEC_J2K); + decoder.reset(opj_create_decompress(OPJ_CODEC_J2K)); - if (!opj_setup_decoder(decoder, ¶meters)) + if (!opj_setup_decoder(decoder.get(), ¶meters)) { return false; } - if (stream) - { - opj_stream_destroy(stream); - } - - stream = opj_stream_create(dataSize, true); + stream.reset(opj_stream_create(dataSize, true)); if (!stream) { return false; } - opj_stream_set_user_data(stream, this, opj_free_user_data); - opj_stream_set_user_data_length(stream, dataSize); - opj_stream_set_read_function(stream, opj_read); - opj_stream_set_write_function(stream, opj_write); - opj_stream_set_skip_function(stream, opj_skip); - opj_stream_set_seek_function(stream, opj_seek); + opj_stream_set_user_data(stream.get(), this, opj_free_user_data); + opj_stream_set_user_data_length(stream.get(), dataSize); + opj_stream_set_read_function(stream.get(), opj_read); + opj_stream_set_write_function(stream.get(), opj_write); + opj_stream_set_skip_function(stream.get(), opj_skip); + opj_stream_set_seek_function(stream.get(), opj_seek); buffer = data; size = dataSize; offset = 0; // enable decoding partially loaded images - opj_decoder_set_strict_mode(decoder, OPJ_FALSE); + opj_decoder_set_strict_mode(decoder.get(), OPJ_FALSE); /* Read the main header of the codestream and if necessary the JP2 boxes*/ - if (!opj_read_header((opj_stream_t*)stream, decoder, &image)) + opj_image_t* img; + if (!opj_read_header(stream.get(), decoder.get(), &img)) { return false; } + image.reset(img); - codestream_info = opj_get_cstr_info(decoder); - + codestream_info.reset(opj_get_cstr_info(decoder.get())); if (!codestream_info) { return false; @@ -337,51 +308,44 @@ public: { parameters.flags &= ~OPJ_DPARAMETERS_DUMP_FLAG; - decoder = opj_create_decompress(OPJ_CODEC_J2K); - opj_setup_decoder(decoder, ¶meters); + decoder.reset(opj_create_decompress(OPJ_CODEC_J2K)); + opj_setup_decoder(decoder.get(), ¶meters); - opj_set_info_handler(decoder, opj_info, this); - opj_set_warning_handler(decoder, opj_warn, this); - opj_set_error_handler(decoder, opj_error, this); + opj_set_info_handler(decoder.get(), opj_info, this); + opj_set_warning_handler(decoder.get(), opj_warn, this); + opj_set_error_handler(decoder.get(), opj_error, this); - if (stream) - { - opj_stream_destroy(stream); - } - - stream = opj_stream_create(dataSize, true); + stream.reset(opj_stream_create(dataSize, true)); if (!stream) { return false; } - opj_stream_set_user_data(stream, this, opj_free_user_data); - opj_stream_set_user_data_length(stream, dataSize); - opj_stream_set_read_function(stream, opj_read); - opj_stream_set_write_function(stream, opj_write); - opj_stream_set_skip_function(stream, opj_skip); - opj_stream_set_seek_function(stream, opj_seek); + opj_stream_set_user_data(stream.get(), this, opj_free_user_data); + opj_stream_set_user_data_length(stream.get(), dataSize); + opj_stream_set_read_function(stream.get(), opj_read); + opj_stream_set_write_function(stream.get(), opj_write); + opj_stream_set_skip_function(stream.get(), opj_skip); + opj_stream_set_seek_function(stream.get(), opj_seek); buffer = data; size = dataSize; offset = 0; - if (image) - { - opj_image_destroy(image); - image = nullptr; - } + image.reset(); // needs to happen before opj_read_header and opj_decode... - opj_set_decoded_resolution_factor(decoder, discard_level); + opj_set_decoded_resolution_factor(decoder.get(), discard_level); // enable decoding partially loaded images - opj_decoder_set_strict_mode(decoder, OPJ_FALSE); + opj_decoder_set_strict_mode(decoder.get(), OPJ_FALSE); - if (!opj_read_header(stream, decoder, &image)) + opj_image_t* img; + if (!opj_read_header(stream.get(), decoder.get(), &img)) { return false; } + image.reset(img); // needs to happen before decode which may fail if (channels) @@ -389,30 +353,36 @@ public: *channels = image->numcomps; } - OPJ_BOOL decoded = opj_decode(decoder, stream, image); + OPJ_BOOL decoded = opj_decode(decoder.get(), stream.get(), image.get()); // count was zero. The latter is just a sanity check before we // dereference the array. - if (!decoded || !image || !image->numcomps) - { - opj_end_decompress(decoder, stream); - return false; - } - - opj_end_decompress(decoder, stream); - - return true; + bool result = (decoded && image && image->numcomps); + opj_end_decompress(decoder.get(), stream.get()); + return result; } - opj_image_t* getImage() { return image; } + opj_image_t* getImage() { return image.get(); } private: + // opj_destroy_cstr_info(opj_codestream_info_v2_t**) requires a pointer to + // pointer, which is too bad because otherwise we could directly pass that + // function as the unique_ptr's deleter. + static void cstr_info_deleter(opj_codestream_info_v2_t* doomed) + { + opj_destroy_cstr_info(&doomed); + } + opj_dparameters_t parameters; opj_event_mgr_t event_mgr; - opj_image_t* image = nullptr; - opj_codec_t* decoder = nullptr; - opj_stream_t* stream = nullptr; - opj_codestream_info_v2_t* codestream_info = nullptr; + std::unique_ptr + codestream_info{ nullptr, cstr_info_deleter }; + std::unique_ptr + stream{ nullptr, opj_stream_destroy }; + std::unique_ptr + image{ nullptr, opj_image_destroy }; + std::unique_ptr + decoder{ nullptr, opj_destroy_codec }; }; class JPEG2KEncode : public JPEG2KBase @@ -447,43 +417,19 @@ public: parameters.irreversible = 1; } - if (comment_text) - { - free(comment_text); - } - comment_text = comment_text_in ? strdup(comment_text_in) : nullptr; + comment_text = { comment_text_in? comment_text_in : "no comment" }; - parameters.cp_comment = comment_text ? comment_text : (char*)"no comment"; + // Because comment_text is a member declared before parameters, + // it will outlive parameters, so we can safely store in parameters a + // pointer into comment_text's data. Unfortunately cp_comment is + // declared as (non-const) char*. We just have to trust that this is + // legacy C style coding, rather than any intention to modify the + // comment string. (If there was actual modification, we could use a + // std::vector instead, but let's only go there if we must.) + parameters.cp_comment = const_cast(comment_text.c_str()); llassert(parameters.cp_comment); } - ~JPEG2KEncode() - { - if (encoder) - { - opj_destroy_codec(encoder); - } - encoder = nullptr; - - if (image) - { - opj_image_destroy(image); - } - image = nullptr; - - if (stream) - { - opj_stream_destroy(stream); - } - stream = nullptr; - - if (comment_text) - { - free(comment_text); - } - comment_text = nullptr; - } - bool encode(const LLImageRaw& rawImageIn, LLImageJ2C &compressedImageOut) { LLImageDataSharedLock lockIn(&rawImageIn); @@ -491,7 +437,7 @@ public: setImage(rawImageIn); - encoder = opj_create_compress(OPJ_CODEC_J2K); + encoder.reset(opj_create_compress(OPJ_CODEC_J2K)); parameters.tcp_mct = (image->numcomps >= 3) ? 1 : 0; parameters.cod_format = OPJ_CODEC_J2K; @@ -542,14 +488,14 @@ public: parameters.max_cs_size = max_cs_size; } - if (!opj_setup_encoder(encoder, ¶meters, image)) + if (!opj_setup_encoder(encoder.get(), ¶meters, image.get())) { return false; } - opj_set_info_handler(encoder, opj_info, this); - opj_set_warning_handler(encoder, opj_warn, this); - opj_set_error_handler(encoder, opj_error, this); + opj_set_info_handler(encoder.get(), opj_info, this); + opj_set_warning_handler(encoder.get(), opj_warn, this); + opj_set_error_handler(encoder.get(), opj_error, this); U32 tile_count = (rawImageIn.getWidth() >> 6) * (rawImageIn.getHeight() >> 6); U32 data_size_guess = tile_count * TILE_SIZE; @@ -561,37 +507,32 @@ public: memset(buffer, 0, data_size_guess); - if (stream) - { - opj_stream_destroy(stream); - } - - stream = opj_stream_create(data_size_guess, false); + stream.reset(opj_stream_create(data_size_guess, false)); if (!stream) { return false; } - opj_stream_set_user_data(stream, this, opj_free_user_data_write); - opj_stream_set_user_data_length(stream, data_size_guess); - opj_stream_set_read_function(stream, opj_read); - opj_stream_set_write_function(stream, opj_write); - opj_stream_set_skip_function(stream, opj_skip); - opj_stream_set_seek_function(stream, opj_seek); + opj_stream_set_user_data(stream.get(), this, opj_free_user_data_write); + opj_stream_set_user_data_length(stream.get(), data_size_guess); + opj_stream_set_read_function(stream.get(), opj_read); + opj_stream_set_write_function(stream.get(), opj_write); + opj_stream_set_skip_function(stream.get(), opj_skip); + opj_stream_set_seek_function(stream.get(), opj_seek); - OPJ_BOOL started = opj_start_compress(encoder, image, stream); + OPJ_BOOL started = opj_start_compress(encoder.get(), image.get(), stream.get()); if (!started) { return false; } - if (!opj_encode(encoder, stream)) + if (!opj_encode(encoder.get(), stream.get())) { return false; } - OPJ_BOOL encoded = opj_end_compress(encoder, stream); + OPJ_BOOL encoded = opj_end_compress(encoder.get(), stream.get()); // if we successfully encoded, then stream out the compressed data... if (encoded) @@ -624,7 +565,7 @@ public: cmptparm[c].h = height; } - image = opj_image_create(numcomps, &cmptparm[0], OPJ_CLRSPC_SRGB); + image.reset(opj_image_create(numcomps, &cmptparm[0], OPJ_CLRSPC_SRGB)); image->x1 = width; image->y1 = height; @@ -739,15 +680,18 @@ public: }*/ } - opj_image_t* getImage() { return image; } + opj_image_t* getImage() { return image.get(); } private: + std::string comment_text; opj_cparameters_t parameters; opj_event_mgr_t event_mgr; - opj_image_t* image = nullptr; - opj_codec_t* encoder = nullptr; - opj_stream_t* stream = nullptr; - char* comment_text = nullptr; + std::unique_ptr + stream{ nullptr, opj_stream_destroy }; + std::unique_ptr + image{ nullptr, opj_image_destroy }; + std::unique_ptr + encoder{ nullptr, opj_destroy_codec }; }; -- cgit v1.2.3 From ecc2ada086b7cdc943abe11eb62b749c2b3d8798 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 27 Sep 2024 06:22:56 -0400 Subject: lltexlayerparams.h uses std::atomic, so #include --- indra/llappearance/lltexlayerparams.h | 1 + 1 file changed, 1 insertion(+) (limited to 'indra') diff --git a/indra/llappearance/lltexlayerparams.h b/indra/llappearance/lltexlayerparams.h index 5e785e4f3e..116eb3bee0 100644 --- a/indra/llappearance/lltexlayerparams.h +++ b/indra/llappearance/lltexlayerparams.h @@ -30,6 +30,7 @@ #include "llpointer.h" #include "v4color.h" #include "llviewervisualparam.h" +#include class LLAvatarAppearance; class LLImageRaw; -- cgit v1.2.3 From 5c9d60c0e58e2dc65e3e048bd42412997770c6d6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 27 Sep 2024 06:29:49 -0400 Subject: Linux GCC prefers the explicit std::string::assign() call. --- indra/llimagej2coj/llimagej2coj.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llimagej2coj/llimagej2coj.cpp b/indra/llimagej2coj/llimagej2coj.cpp index a2a43e4baa..6af321be3d 100644 --- a/indra/llimagej2coj/llimagej2coj.cpp +++ b/indra/llimagej2coj/llimagej2coj.cpp @@ -417,7 +417,7 @@ public: parameters.irreversible = 1; } - comment_text = { comment_text_in? comment_text_in : "no comment" }; + comment_text.assign(comment_text_in? comment_text_in : "no comment"); // Because comment_text is a member declared before parameters, // it will outlive parameters, so we can safely store in parameters a -- cgit v1.2.3 From 1c78829ab3177940adbdb7f0081b5f46c1e37763 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 27 Sep 2024 11:07:08 -0400 Subject: Introduce owning_ptr; use it for JPEG2KEncode and JPEG2KDecode. owning_ptr adapts std::unique_ptr to be a better drop-in replacement for a legacy class that formerly stored plain T* data members, and explicitly destroyed them using domain-specific destructor functions. Directly substituting std::unique_ptr into JPEG2KEncode and JPEG2KDecode was cumbersome because every such pointer declaration required a redundant template parameter describing the deleter function passed into its constructor. Moreover, it required lots of little syntax tweaks: changing every assignment to a reset() call, changing every reference to a get() call. Using owning_ptr allows us to leave the code more or less as it was before, save that assignment and destruction automatically handle the previous referenced T instance. --- indra/llcommon/owning_ptr.h | 71 ++++++++++++++++++ indra/llimagej2coj/llimagej2coj.cpp | 145 +++++++++++++++++------------------- 2 files changed, 140 insertions(+), 76 deletions(-) create mode 100755 indra/llcommon/owning_ptr.h (limited to 'indra') diff --git a/indra/llcommon/owning_ptr.h b/indra/llcommon/owning_ptr.h new file mode 100755 index 0000000000..c0da245265 --- /dev/null +++ b/indra/llcommon/owning_ptr.h @@ -0,0 +1,71 @@ +/** + * @file owning_ptr.h + * @author Nat Goodspeed + * @date 2024-09-27 + * @brief owning_ptr is like std::unique_ptr, but easier to integrate + * + * $LicenseInfo:firstyear=2024&license=viewerlgpl$ + * Copyright (c) 2024, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_OWNING_PTR_H) +#define LL_OWNING_PTR_H + +#include +#include + +/** + * owning_ptr adapts std::unique_ptr to make it easier to adopt into + * older code using dumb pointers. + * + * Consider a class Outer with a member Thing* mThing. After the constructor, + * each time a method wants to assign to mThing, it must test for nullptr and + * destroy the previous Thing instance. During Outer's lifetime, mThing is + * passed to legacy domain-specific functions accepting plain Thing*. Finally + * the destructor must again test for nullptr and destroy the remaining Thing + * instance. + * + * Multiply that by several different Outer members of different types, + * possibly with different domain-specific destructor functions. + * + * Dropping std::unique_ptr into Outer is cumbersome for a several + * reasons. First, if Thing requires a domain-specific destructor function, + * the unique_ptr declaration of mThing must explicitly state the type of that + * function (as a function pointer, for a typical legacy function). Second, + * every Thing* assignment to mThing must be changed to mThing.reset(). Third, + * every time we call a legacy domain-specific function, we must pass + * mThing.get(). + * + * owning_ptr is designed to drop into a situation like this. The domain- + * specific destructor function, if any, is passed to its constructor; it need + * not be encoded into the pointer type. owning_ptr supports plain pointer + * assignment, internally calling std::unique_ptr::reset(). It also + * supports implicit conversion to plain T*, to pass the owned pointer to + * legacy domain-specific functions. + * + * Obviously owning_ptr must not be used in situations where ownership of + * the referenced object is passed on to another pointer: use std::unique_ptr + * for that. Similarly, it is not for shared ownership. It simplifies lifetime + * management for classes that currently store (and explicitly destroy) plain + * T* pointers. + */ +template +class owning_ptr +{ + using deleter = std::function; +public: + owning_ptr(T* p=nullptr, const deleter& d=std::default_delete()): + mPtr(p, d) + {} + void reset(T* p=nullptr) { mPtr.reset(p); } + owning_ptr& operator=(T* p) { mPtr.reset(p); return *this; } + operator T*() const { return mPtr.get(); } + T& operator*() const { return *mPtr; } + T* operator->() const { return mPtr.operator->(); } + +private: + std::unique_ptr> mPtr; +}; + +#endif /* ! defined(LL_OWNING_PTR_H) */ diff --git a/indra/llimagej2coj/llimagej2coj.cpp b/indra/llimagej2coj/llimagej2coj.cpp index 6af321be3d..d8f17561b3 100644 --- a/indra/llimagej2coj/llimagej2coj.cpp +++ b/indra/llimagej2coj/llimagej2coj.cpp @@ -31,7 +31,7 @@ #include "openjpeg.h" #include "event.h" #include "cio.h" -#include // std::unique_ptr +#include "owning_ptr.h" #include #define MAX_ENCODED_DISCARD_LEVELS 5 @@ -243,42 +243,42 @@ public: { parameters.flags |= OPJ_DPARAMETERS_DUMP_FLAG; - decoder.reset(opj_create_decompress(OPJ_CODEC_J2K)); + decoder = opj_create_decompress(OPJ_CODEC_J2K); - if (!opj_setup_decoder(decoder.get(), ¶meters)) + if (!opj_setup_decoder(decoder, ¶meters)) { return false; } - stream.reset(opj_stream_create(dataSize, true)); + stream = opj_stream_create(dataSize, true); if (!stream) { return false; } - opj_stream_set_user_data(stream.get(), this, opj_free_user_data); - opj_stream_set_user_data_length(stream.get(), dataSize); - opj_stream_set_read_function(stream.get(), opj_read); - opj_stream_set_write_function(stream.get(), opj_write); - opj_stream_set_skip_function(stream.get(), opj_skip); - opj_stream_set_seek_function(stream.get(), opj_seek); + opj_stream_set_user_data(stream, this, opj_free_user_data); + opj_stream_set_user_data_length(stream, dataSize); + opj_stream_set_read_function(stream, opj_read); + opj_stream_set_write_function(stream, opj_write); + opj_stream_set_skip_function(stream, opj_skip); + opj_stream_set_seek_function(stream, opj_seek); buffer = data; size = dataSize; offset = 0; // enable decoding partially loaded images - opj_decoder_set_strict_mode(decoder.get(), OPJ_FALSE); + opj_decoder_set_strict_mode(decoder, OPJ_FALSE); /* Read the main header of the codestream and if necessary the JP2 boxes*/ opj_image_t* img; - if (!opj_read_header(stream.get(), decoder.get(), &img)) + if (!opj_read_header(stream, decoder, &img)) { return false; } - image.reset(img); + image = img; - codestream_info.reset(opj_get_cstr_info(decoder.get())); + codestream_info = opj_get_cstr_info(decoder); if (!codestream_info) { return false; @@ -308,44 +308,44 @@ public: { parameters.flags &= ~OPJ_DPARAMETERS_DUMP_FLAG; - decoder.reset(opj_create_decompress(OPJ_CODEC_J2K)); - opj_setup_decoder(decoder.get(), ¶meters); + decoder = opj_create_decompress(OPJ_CODEC_J2K); + opj_setup_decoder(decoder, ¶meters); - opj_set_info_handler(decoder.get(), opj_info, this); - opj_set_warning_handler(decoder.get(), opj_warn, this); - opj_set_error_handler(decoder.get(), opj_error, this); + opj_set_info_handler(decoder, opj_info, this); + opj_set_warning_handler(decoder, opj_warn, this); + opj_set_error_handler(decoder, opj_error, this); - stream.reset(opj_stream_create(dataSize, true)); + stream = opj_stream_create(dataSize, true); if (!stream) { return false; } - opj_stream_set_user_data(stream.get(), this, opj_free_user_data); - opj_stream_set_user_data_length(stream.get(), dataSize); - opj_stream_set_read_function(stream.get(), opj_read); - opj_stream_set_write_function(stream.get(), opj_write); - opj_stream_set_skip_function(stream.get(), opj_skip); - opj_stream_set_seek_function(stream.get(), opj_seek); + opj_stream_set_user_data(stream, this, opj_free_user_data); + opj_stream_set_user_data_length(stream, dataSize); + opj_stream_set_read_function(stream, opj_read); + opj_stream_set_write_function(stream, opj_write); + opj_stream_set_skip_function(stream, opj_skip); + opj_stream_set_seek_function(stream, opj_seek); buffer = data; size = dataSize; offset = 0; - image.reset(); + image = nullptr; // needs to happen before opj_read_header and opj_decode... - opj_set_decoded_resolution_factor(decoder.get(), discard_level); + opj_set_decoded_resolution_factor(decoder, discard_level); // enable decoding partially loaded images - opj_decoder_set_strict_mode(decoder.get(), OPJ_FALSE); + opj_decoder_set_strict_mode(decoder, OPJ_FALSE); opj_image_t* img; - if (!opj_read_header(stream.get(), decoder.get(), &img)) + if (!opj_read_header(stream, decoder, &img)) { return false; } - image.reset(img); + image = img; // needs to happen before decode which may fail if (channels) @@ -353,36 +353,32 @@ public: *channels = image->numcomps; } - OPJ_BOOL decoded = opj_decode(decoder.get(), stream.get(), image.get()); + OPJ_BOOL decoded = opj_decode(decoder, stream, image); // count was zero. The latter is just a sanity check before we // dereference the array. bool result = (decoded && image && image->numcomps); - opj_end_decompress(decoder.get(), stream.get()); + opj_end_decompress(decoder, stream); return result; } - opj_image_t* getImage() { return image.get(); } + opj_image_t* getImage() { return image; } private: - // opj_destroy_cstr_info(opj_codestream_info_v2_t**) requires a pointer to - // pointer, which is too bad because otherwise we could directly pass that - // function as the unique_ptr's deleter. - static void cstr_info_deleter(opj_codestream_info_v2_t* doomed) - { - opj_destroy_cstr_info(&doomed); - } - opj_dparameters_t parameters; opj_event_mgr_t event_mgr; - std::unique_ptr - codestream_info{ nullptr, cstr_info_deleter }; - std::unique_ptr - stream{ nullptr, opj_stream_destroy }; - std::unique_ptr - image{ nullptr, opj_image_destroy }; - std::unique_ptr - decoder{ nullptr, opj_destroy_codec }; + owning_ptr codestream_info{ + nullptr, + // opj_destroy_cstr_info(opj_codestream_info_v2_t**) requires a + // pointer to pointer, which is too bad because otherwise we could + // directly pass that function as the owning_ptr's deleter. + [](opj_codestream_info_v2_t* doomed) + { + opj_destroy_cstr_info(&doomed); + }}; + owning_ptr stream{ nullptr, opj_stream_destroy }; + owning_ptr image{ nullptr, opj_image_destroy }; + owning_ptr decoder{ nullptr, opj_destroy_codec }; }; class JPEG2KEncode : public JPEG2KBase @@ -437,7 +433,7 @@ public: setImage(rawImageIn); - encoder.reset(opj_create_compress(OPJ_CODEC_J2K)); + encoder = opj_create_compress(OPJ_CODEC_J2K); parameters.tcp_mct = (image->numcomps >= 3) ? 1 : 0; parameters.cod_format = OPJ_CODEC_J2K; @@ -488,14 +484,14 @@ public: parameters.max_cs_size = max_cs_size; } - if (!opj_setup_encoder(encoder.get(), ¶meters, image.get())) + if (!opj_setup_encoder(encoder, ¶meters, image)) { return false; } - opj_set_info_handler(encoder.get(), opj_info, this); - opj_set_warning_handler(encoder.get(), opj_warn, this); - opj_set_error_handler(encoder.get(), opj_error, this); + opj_set_info_handler(encoder, opj_info, this); + opj_set_warning_handler(encoder, opj_warn, this); + opj_set_error_handler(encoder, opj_error, this); U32 tile_count = (rawImageIn.getWidth() >> 6) * (rawImageIn.getHeight() >> 6); U32 data_size_guess = tile_count * TILE_SIZE; @@ -507,32 +503,32 @@ public: memset(buffer, 0, data_size_guess); - stream.reset(opj_stream_create(data_size_guess, false)); + stream = opj_stream_create(data_size_guess, false); if (!stream) { return false; } - opj_stream_set_user_data(stream.get(), this, opj_free_user_data_write); - opj_stream_set_user_data_length(stream.get(), data_size_guess); - opj_stream_set_read_function(stream.get(), opj_read); - opj_stream_set_write_function(stream.get(), opj_write); - opj_stream_set_skip_function(stream.get(), opj_skip); - opj_stream_set_seek_function(stream.get(), opj_seek); + opj_stream_set_user_data(stream, this, opj_free_user_data_write); + opj_stream_set_user_data_length(stream, data_size_guess); + opj_stream_set_read_function(stream, opj_read); + opj_stream_set_write_function(stream, opj_write); + opj_stream_set_skip_function(stream, opj_skip); + opj_stream_set_seek_function(stream, opj_seek); - OPJ_BOOL started = opj_start_compress(encoder.get(), image.get(), stream.get()); + OPJ_BOOL started = opj_start_compress(encoder, image, stream); if (!started) { return false; } - if (!opj_encode(encoder.get(), stream.get())) + if (!opj_encode(encoder, stream)) { return false; } - OPJ_BOOL encoded = opj_end_compress(encoder.get(), stream.get()); + OPJ_BOOL encoded = opj_end_compress(encoder, stream); // if we successfully encoded, then stream out the compressed data... if (encoded) @@ -565,7 +561,7 @@ public: cmptparm[c].h = height; } - image.reset(opj_image_create(numcomps, &cmptparm[0], OPJ_CLRSPC_SRGB)); + image = opj_image_create(numcomps, &cmptparm[0], OPJ_CLRSPC_SRGB); image->x1 = width; image->y1 = height; @@ -680,18 +676,15 @@ public: }*/ } - opj_image_t* getImage() { return image.get(); } + opj_image_t* getImage() { return image; } private: - std::string comment_text; - opj_cparameters_t parameters; - opj_event_mgr_t event_mgr; - std::unique_ptr - stream{ nullptr, opj_stream_destroy }; - std::unique_ptr - image{ nullptr, opj_image_destroy }; - std::unique_ptr - encoder{ nullptr, opj_destroy_codec }; + std::string comment_text; + opj_cparameters_t parameters; + opj_event_mgr_t event_mgr; + owning_ptr stream{ nullptr, opj_stream_destroy }; + owning_ptr image{ nullptr, opj_image_destroy }; + owning_ptr encoder{ nullptr, opj_destroy_codec }; }; -- cgit v1.2.3 From cf2f482ceca796e587dfe2fb8df552c09156fb50 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 27 Sep 2024 11:11:29 -0400 Subject: Slightly streamline owning_ptr class definition. --- indra/llcommon/owning_ptr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra') diff --git a/indra/llcommon/owning_ptr.h b/indra/llcommon/owning_ptr.h index c0da245265..7cf8d3f0ba 100755 --- a/indra/llcommon/owning_ptr.h +++ b/indra/llcommon/owning_ptr.h @@ -65,7 +65,7 @@ public: T* operator->() const { return mPtr.operator->(); } private: - std::unique_ptr> mPtr; + std::unique_ptr mPtr; }; #endif /* ! defined(LL_OWNING_PTR_H) */ -- cgit v1.2.3