Make SkFunctionWrapper a better wrapper.
SkFunctionWrapper was made to be a simple abstraction over existing
resource release functions which generally follow a specific pattern of
returning void and taking a pointer to the underlying type. However,
this has been observed to be an unnecessary limit. This makes it more
generic while also making the call sites a little less brittle.
This change also uncovered an issue in msvc v19.20 to v19.22, see
https://developercommunity.visualstudio.com/content/problem/698192/in-templateusing-decltype-is-void.html
To work around this, several otherwise redundant '&' are used.
There was an attempt to take references to functions instead of pointers
to functions which greatly simplifies the intermediate wrappers.
However, that uncovered another issue in msvc, see
https://developercommunity.visualstudio.com/content/problem/699878/function-to-pointer.html
Change-Id: I54ab945ed9d9cfd0204d4d6650c2fde47cc9e175
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/235105
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
diff --git a/include/private/SkTemplates.h b/include/private/SkTemplates.h
index 1697a29..0933c98 100644
--- a/include/private/SkTemplates.h
+++ b/include/private/SkTemplates.h
@@ -47,8 +47,12 @@
return reinterpret_cast<D*>(reinterpret_cast<sknonstd::same_cv_t<char, D>*>(ptr) + byteOffset);
}
-template <typename R, typename T, R (*P)(T*)> struct SkFunctionWrapper {
- R operator()(T* t) { return P(t); }
+// TODO: when C++17 the language is available, use template <auto P>
+template <typename T, T* P> struct SkFunctionWrapper {
+ template <typename... Args>
+ auto operator()(Args&&... args) const -> decltype(P(std::forward<Args>(args)...)) {
+ return P(std::forward<Args>(args)...);
+ }
};
/** \class SkAutoTCallVProc
@@ -60,9 +64,10 @@
function.
*/
template <typename T, void (*P)(T*)> class SkAutoTCallVProc
- : public std::unique_ptr<T, SkFunctionWrapper<void, T, P>> {
+ : public std::unique_ptr<T, SkFunctionWrapper<skstd::remove_pointer_t<decltype(P)>, P>> {
public:
- SkAutoTCallVProc(T* obj): std::unique_ptr<T, SkFunctionWrapper<void, T, P>>(obj) {}
+ SkAutoTCallVProc(T* obj)
+ : std::unique_ptr<T, SkFunctionWrapper<skstd::remove_pointer_t<decltype(P)>, P>>(obj) {}
operator T*() const { return this->get(); }
};
@@ -259,7 +264,7 @@
T* release() { return fPtr.release(); }
private:
- std::unique_ptr<T, SkFunctionWrapper<void, void, sk_free>> fPtr;
+ std::unique_ptr<T, SkFunctionWrapper<decltype(sk_free), sk_free>> fPtr;
};
template <size_t kCountRequested, typename T> class SkAutoSTMalloc {
@@ -434,7 +439,7 @@
SkAlignedSStorage<sizeof(T)*N> fStorage;
};
-using SkAutoFree = std::unique_ptr<void, SkFunctionWrapper<void, void, sk_free>>;
+using SkAutoFree = std::unique_ptr<void, SkFunctionWrapper<decltype(sk_free), sk_free>>;
template<typename C, std::size_t... Is>
constexpr auto SkMakeArrayFromIndexSequence(C c, skstd::index_sequence<Is...>)
diff --git a/modules/skparagraph/src/ParagraphImpl.cpp b/modules/skparagraph/src/ParagraphImpl.cpp
index b2eee68..39348bb 100644
--- a/modules/skparagraph/src/ParagraphImpl.cpp
+++ b/modules/skparagraph/src/ParagraphImpl.cpp
@@ -25,10 +25,9 @@
UErrorCode status = U_ZERO_ERROR;
fIterator = nullptr;
fSize = text.size();
- UText utf8UText = UTEXT_INITIALIZER;
- utext_openUTF8(&utf8UText, text.begin(), text.size(), &status);
- fAutoClose =
- std::unique_ptr<UText, SkFunctionWrapper<UText*, UText, utext_close>>(&utf8UText);
+ UText sUtf8UText = UTEXT_INITIALIZER;
+ std::unique_ptr<UText, SkFunctionWrapper<decltype(utext_close), utext_close>> utf8UText(
+ utext_openUTF8(&sUtf8UText, text.begin(), text.size(), &status));
if (U_FAILURE(status)) {
SkDebugf("Could not create utf8UText: %s", u_errorName(status));
return false;
@@ -39,7 +38,7 @@
SK_ABORT("");
}
- ubrk_setUText(fIterator.get(), &utf8UText, &status);
+ ubrk_setUText(fIterator.get(), utf8UText.get(), &status);
if (U_FAILURE(status)) {
SkDebugf("Could not setText on break iterator: %s", u_errorName(status));
return false;
diff --git a/modules/skparagraph/src/ParagraphImpl.h b/modules/skparagraph/src/ParagraphImpl.h
index 4c170d2..0922b91 100644
--- a/modules/skparagraph/src/ParagraphImpl.h
+++ b/modules/skparagraph/src/ParagraphImpl.h
@@ -75,8 +75,7 @@
bool eof() { return fPos == icu::BreakIterator::DONE; }
private:
- std::unique_ptr<UText, SkFunctionWrapper<UText*, UText, utext_close>> fAutoClose;
- std::unique_ptr<UBreakIterator, SkFunctionWrapper<void, UBreakIterator, ubrk_close>> fIterator;
+ std::unique_ptr<UBreakIterator, SkFunctionWrapper<decltype(ubrk_close), ubrk_close>> fIterator;
bool fInitialized;
int32_t fPos;
size_t fSize;
diff --git a/modules/skshaper/src/SkShaper_harfbuzz.cpp b/modules/skshaper/src/SkShaper_harfbuzz.cpp
index 49af86c7..34b80c5 100644
--- a/modules/skshaper/src/SkShaper_harfbuzz.cpp
+++ b/modules/skshaper/src/SkShaper_harfbuzz.cpp
@@ -55,13 +55,15 @@
}
namespace {
-template <class T, void(*P)(T*)> using resource = std::unique_ptr<T, SkFunctionWrapper<void, T, P>>;
-using HBBlob = resource<hb_blob_t , hb_blob_destroy >;
-using HBFace = resource<hb_face_t , hb_face_destroy >;
-using HBFont = resource<hb_font_t , hb_font_destroy >;
-using HBBuffer = resource<hb_buffer_t , hb_buffer_destroy>;
-using ICUBiDi = resource<UBiDi , ubidi_close >;
-using ICUBrk = resource<UBreakIterator, ubrk_close >;
+template <typename T, void(*P)(T*)> using resource =
+ std::unique_ptr<T, SkFunctionWrapper<skstd::remove_pointer_t<decltype(P)>, P>>;
+using HBBlob = resource<hb_blob_t , &hb_blob_destroy >;
+using HBFace = resource<hb_face_t , &hb_face_destroy >;
+using HBFont = resource<hb_font_t , &hb_font_destroy >;
+using HBBuffer = resource<hb_buffer_t , &hb_buffer_destroy>;
+using ICUBiDi = resource<UBiDi , &ubidi_close >;
+using ICUBrk = resource<UBreakIterator, &ubrk_close >;
+using ICUUText = std::unique_ptr<UText, SkFunctionWrapper<decltype(utext_close), utext_close>>;
HBBlob stream_to_blob(std::unique_ptr<SkStreamAsset> asset) {
size_t size = asset->getLength();
@@ -863,14 +865,13 @@
UBreakIterator& breakIterator = *fLineBreakIterator;
{
UErrorCode status = U_ZERO_ERROR;
- UText utf8UText = UTEXT_INITIALIZER;
- utext_openUTF8(&utf8UText, utf8Start, utf8runLength, &status);
- std::unique_ptr<UText, SkFunctionWrapper<UText*, UText, utext_close>> autoClose(&utf8UText);
+ UText sUtf8UText = UTEXT_INITIALIZER;
+ ICUUText utf8UText(utext_openUTF8(&sUtf8UText, utf8Start, utf8runLength, &status));
if (U_FAILURE(status)) {
SkDebugf("Could not create utf8UText: %s", u_errorName(status));
return;
}
- ubrk_setUText(&breakIterator, &utf8UText, &status);
+ ubrk_setUText(&breakIterator, utf8UText.get(), &status);
if (U_FAILURE(status)) {
SkDebugf("Could not setText on break iterator: %s", u_errorName(status));
return;
@@ -966,20 +967,19 @@
UBreakIterator& graphemeBreakIterator = *fGraphemeBreakIterator;
{
UErrorCode status = U_ZERO_ERROR;
- UText utf8UText = UTEXT_INITIALIZER;
- utext_openUTF8(&utf8UText, utf8, utf8Bytes, &status);
- std::unique_ptr<UText, SkFunctionWrapper<UText*, UText, utext_close>> autoClose(&utf8UText);
+ UText sUtf8UText = UTEXT_INITIALIZER;
+ ICUUText utf8UText(utext_openUTF8(&sUtf8UText, utf8, utf8Bytes, &status));
if (U_FAILURE(status)) {
SkDebugf("Could not create utf8UText: %s", u_errorName(status));
return;
}
- ubrk_setUText(&lineBreakIterator, &utf8UText, &status);
+ ubrk_setUText(&lineBreakIterator, utf8UText.get(), &status);
if (U_FAILURE(status)) {
SkDebugf("Could not setText on line break iterator: %s", u_errorName(status));
return;
}
- ubrk_setUText(&graphemeBreakIterator, &utf8UText, &status);
+ ubrk_setUText(&graphemeBreakIterator, utf8UText.get(), &status);
if (U_FAILURE(status)) {
SkDebugf("Could not setText on grapheme break iterator: %s", u_errorName(status));
return;
diff --git a/src/core/SkMask.h b/src/core/SkMask.h
index 61b6f71..29ae48e 100644
--- a/src/core/SkMask.h
+++ b/src/core/SkMask.h
@@ -237,7 +237,7 @@
* Stack class used to manage the fImage buffer in a SkMask.
* When this object loses scope, the buffer is freed with SkMask::FreeImage().
*/
-using SkAutoMaskFreeImage = std::unique_ptr<uint8_t,SkFunctionWrapper<void,void,SkMask::FreeImage>>;
+using SkAutoMaskFreeImage = std::unique_ptr<uint8_t, SkFunctionWrapper<decltype(SkMask::FreeImage), SkMask::FreeImage>>;
#define SkAutoMaskFreeImage(...) SK_REQUIRE_LOCAL_VAR(SkAutoMaskFreeImage)
#endif
diff --git a/src/pdf/SkPDFSubsetFont.cpp b/src/pdf/SkPDFSubsetFont.cpp
index 0bb92bb..7701d92 100644
--- a/src/pdf/SkPDFSubsetFont.cpp
+++ b/src/pdf/SkPDFSubsetFont.cpp
@@ -16,11 +16,12 @@
#include "hb.h"
#include "hb-subset.h"
-template <class T, void(*P)(T*)> using resource = std::unique_ptr<T, SkFunctionWrapper<void, T, P>>;
-using HBBlob = resource<hb_blob_t, hb_blob_destroy>;
-using HBFace = resource<hb_face_t, hb_face_destroy>;
-using HBSubsetInput = resource<hb_subset_input_t, hb_subset_input_destroy>;
-using HBSet = resource<hb_set_t, hb_set_destroy>;
+template <class T, void(*P)(T*)> using resource =
+ std::unique_ptr<T, SkFunctionWrapper<skstd::remove_pointer_t<decltype(P)>, P>>;
+using HBBlob = resource<hb_blob_t, &hb_blob_destroy>;
+using HBFace = resource<hb_face_t, &hb_face_destroy>;
+using HBSubsetInput = resource<hb_subset_input_t, &hb_subset_input_destroy>;
+using HBSet = resource<hb_set_t, &hb_set_destroy>;
static HBBlob to_blob(sk_sp<SkData> data) {
using blob_size_t = SkCallableTraits<decltype(hb_blob_create)>::argument<1>::type;
diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index 8c582a9..34f9f47 100644
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -297,7 +297,7 @@
struct SkFaceRec {
SkFaceRec* fNext;
- std::unique_ptr<FT_FaceRec, SkFunctionWrapper<FT_Error, FT_FaceRec, FT_Done_Face>> fFace;
+ std::unique_ptr<FT_FaceRec, SkFunctionWrapper<decltype(FT_Done_Face), FT_Done_Face>> fFace;
FT_StreamRec fFTStream;
std::unique_ptr<SkStreamAsset> fSkStream;
uint32_t fRefCnt;
@@ -529,7 +529,7 @@
void generateFontMetrics(SkFontMetrics*) override;
private:
- using UnrefFTFace = SkFunctionWrapper<void, SkFaceRec, unref_ft_face>;
+ using UnrefFTFace = SkFunctionWrapper<decltype(unref_ft_face), unref_ft_face>;
std::unique_ptr<SkFaceRec, UnrefFTFace> fFaceRec;
FT_Face fFace; // Borrowed face from gFaceRecHead.
@@ -932,7 +932,7 @@
fLoadGlyphFlags = loadFlags;
}
- using DoneFTSize = SkFunctionWrapper<FT_Error, skstd::remove_pointer_t<FT_Size>, FT_Done_Size>;
+ using DoneFTSize = SkFunctionWrapper<decltype(FT_Done_Size), FT_Done_Size>;
std::unique_ptr<skstd::remove_pointer_t<FT_Size>, DoneFTSize> ftSize([this]() -> FT_Size {
FT_Size size;
FT_Error err = FT_New_Size(fFaceRec->fFace.get(), &size);
diff --git a/src/utils/SkBitSet.h b/src/utils/SkBitSet.h
index 4d6b414..8ed0533 100644
--- a/src/utils/SkBitSet.h
+++ b/src/utils/SkBitSet.h
@@ -51,7 +51,7 @@
}
private:
- std::unique_ptr<uint32_t, SkFunctionWrapper<void, void, sk_free>> fBitData;
+ std::unique_ptr<uint32_t, SkFunctionWrapper<decltype(sk_free), sk_free>> fBitData;
size_t fDwordCount; // Dword (32-bit) count of the bitset.
uint32_t* internalGet(int index) const {
diff --git a/src/utils/mac/SkUniqueCFRef.h b/src/utils/mac/SkUniqueCFRef.h
index 13d452e..80640de 100644
--- a/src/utils/mac/SkUniqueCFRef.h
+++ b/src/utils/mac/SkUniqueCFRef.h
@@ -19,7 +19,7 @@
template <typename CFRef> using SkUniqueCFRef =
std::unique_ptr<skstd::remove_pointer_t<CFRef>,
- SkFunctionWrapper<void, skstd::remove_pointer_t<CFTypeRef>, CFRelease>>;
+ SkFunctionWrapper<decltype(CFRelease), CFRelease>>;
#endif
#endif