Fix deleting wrong data when releasing font faces for some font types…

…, see #217.
mikke89 · Aug 5, 2021 · 95f7709 · 95f7709
1 parent 36f76c2
commit 95f7709
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 28 deletions.
diff --git a/Source/Core/FontEngineDefault/FontFace.cpp b/Source/Core/FontEngineDefault/FontFace.cpp
@@ -33,20 +33,21 @@
 
 namespace Rml {
 
-FontFace::FontFace(FontFaceHandleFreetype _face, Style::FontStyle _style, Style::FontWeight _weight, bool _release_stream)
+FontFace::FontFace(FontFaceHandleFreetype _face, Style::FontStyle _style, Style::FontWeight _weight, UniquePtr<byte[]> _face_memory)
 {
 	style = _style;
 	weight = _weight;
 	face = _face;
 
-	release_stream = _release_stream;
+	face_memory = std::move(_face_memory);
 }
 
 FontFace::~FontFace()
 {
 	if (face) 
 	{
-		FreeType::ReleaseFace(face, release_stream);
+		FreeType::ReleaseFace(face);
+		face_memory.reset();
 		face = 0;
 	}
 	handles.clear();

diff --git a/Source/Core/FontEngineDefault/FontFace.h b/Source/Core/FontEngineDefault/FontFace.h
@@ -43,7 +43,7 @@ class FontFaceHandleDefault;
 class FontFace
 {
 public:
-	FontFace(FontFaceHandleFreetype face, Style::FontStyle style, Style::FontWeight weight, bool release_stream);
+	FontFace(FontFaceHandleFreetype face, Style::FontStyle style, Style::FontWeight weight, UniquePtr<byte[]> face_memory);
 	~FontFace();
 
 	Style::FontStyle GetStyle() const;
@@ -58,7 +58,8 @@ class FontFace
 	Style::FontStyle style;
 	Style::FontWeight weight;
 
-	bool release_stream;
+	// Only filled if we own the memory used by the FreeType face handle.
+	UniquePtr<byte[]> face_memory;
 
 	// Key is font size
 	using HandleMap = UnorderedMap< int, UniquePtr<FontFaceHandleDefault> >;

diff --git a/Source/Core/FontEngineDefault/FontFamily.cpp b/Source/Core/FontEngineDefault/FontFamily.cpp
@@ -61,9 +61,9 @@ FontFaceHandleDefault* FontFamily::GetFaceHandle(Style::FontStyle style, Style::
 
 
 // Adds a new face to the family.
-FontFace* FontFamily::AddFace(FontFaceHandleFreetype ft_face, Style::FontStyle style, Style::FontWeight weight, bool release_stream)
+FontFace* FontFamily::AddFace(FontFaceHandleFreetype ft_face, Style::FontStyle style, Style::FontWeight weight, UniquePtr<byte[]> face_memory)
 {
-	auto face = MakeUnique<FontFace>(ft_face, style, weight, release_stream);
+	auto face = MakeUnique<FontFace>(ft_face, style, weight, std::move(face_memory));
 	FontFace* result = face.get();
 
 	font_faces.push_back(std::move(face));

diff --git a/Source/Core/FontEngineDefault/FontFamily.h b/Source/Core/FontEngineDefault/FontFamily.h
@@ -57,9 +57,9 @@ class FontFamily
 	/// @param[in] ft_face The previously loaded FreeType face.
 	/// @param[in] style The style of the new face.
 	/// @param[in] weight The weight of the new face.
-	/// @param[in] release_stream True if the application must free the face's memory stream.
+	/// @param[in] face_memory Optionally pass ownership of the face's memory to the face itself, automatically releasing it on destruction.
 	/// @return True if the face was loaded successfully, false otherwise.
-	FontFace* AddFace(FontFaceHandleFreetype ft_face, Style::FontStyle style, Style::FontWeight weight, bool release_stream);
+	FontFace* AddFace(FontFaceHandleFreetype ft_face, Style::FontStyle style, Style::FontWeight weight, UniquePtr<byte[]> face_memory);
 
 protected:
 	String name;

diff --git a/Source/Core/FontEngineDefault/FontProvider.cpp b/Source/Core/FontEngineDefault/FontProvider.cpp
@@ -115,11 +115,12 @@ bool FontProvider::LoadFontFace(const String& file_name, bool fallback_face)
 
 	size_t length = file_interface->Length(handle);
 
-	byte* buffer = new byte[length];
+	auto buffer_ptr = UniquePtr<byte[]>(new byte[length]);
+	byte* buffer = buffer_ptr.get();
 	file_interface->Read(buffer, length, handle);
 	file_interface->Close(handle);
 
-	bool result = Get().LoadFontFace(buffer, (int)length, fallback_face, true, file_name);
+	bool result = Get().LoadFontFace(buffer, (int)length, fallback_face, std::move(buffer_ptr), file_name);
 
 	return result;
 }
@@ -129,21 +130,18 @@ bool FontProvider::LoadFontFace(const byte* data, int data_size, const String& f
 {
 	const String source = "memory";
 
-	bool result = Get().LoadFontFace(data, data_size, fallback_face, false, source, font_family, style, weight);
+	bool result = Get().LoadFontFace(data, data_size, fallback_face, nullptr, source, font_family, style, weight);
 
 	return result;
 }
 
-bool FontProvider::LoadFontFace(const byte* data, int data_size, bool fallback_face, bool local_data, const String& source,
+bool FontProvider::LoadFontFace(const byte* data, int data_size, bool fallback_face, UniquePtr<byte[]> face_memory, const String& source,
 	String font_family, Style::FontStyle style, Style::FontWeight weight)
 {
 	FontFaceHandleFreetype ft_face = FreeType::LoadFace(data, data_size, source);
 
 	if (!ft_face)
 	{
-		if (local_data)
-			delete[] data;
-
 		Log::Message(Log::LT_ERROR, "Failed to load font face %s (from %s).", font_family.c_str(), source.c_str());
 		return false;
 	}
@@ -153,7 +151,7 @@ bool FontProvider::LoadFontFace(const byte* data, int data_size, bool fallback_f
 		FreeType::GetFaceStyle(ft_face, font_family, style, weight);
 	}
 
-	if (!AddFace(ft_face, font_family, style, weight, fallback_face, local_data))
+	if (!AddFace(ft_face, font_family, style, weight, fallback_face, std::move(face_memory)))
 	{
 		Log::Message(Log::LT_ERROR, "Failed to load font face %s (from %s).", font_family.c_str(), source.c_str());
 		return false;
@@ -163,7 +161,7 @@ bool FontProvider::LoadFontFace(const byte* data, int data_size, bool fallback_f
 	return true;
 }
 
-bool FontProvider::AddFace(FontFaceHandleFreetype face, const String& family, Style::FontStyle style, Style::FontWeight weight, bool fallback_face, bool release_stream)
+bool FontProvider::AddFace(FontFaceHandleFreetype face, const String& family, Style::FontStyle style, Style::FontWeight weight, bool fallback_face, UniquePtr<byte[]> face_memory)
 {
 	String family_lower = StringUtilities::ToLower(family);
 	FontFamily* font_family = nullptr;
@@ -179,7 +177,7 @@ bool FontProvider::AddFace(FontFaceHandleFreetype face, const String& family, St
 		font_families[family_lower] = std::move(font_family_ptr);
 	}
 
-	FontFace* font_face_result = font_family->AddFace(face, style, weight, release_stream);
+	FontFace* font_face_result = font_family->AddFace(face, style, weight, std::move(face_memory));
 
 	if (font_face_result && fallback_face)
 	{

diff --git a/Source/Core/FontEngineDefault/FontProvider.h b/Source/Core/FontEngineDefault/FontProvider.h
@@ -78,10 +78,10 @@ class FontProvider
 
 	static FontProvider& Get();
 
-	bool LoadFontFace(const byte* data, int data_size, bool fallback_face, bool local_data, const String& source,
+	bool LoadFontFace(const byte* data, int data_size, bool fallback_face, UniquePtr<byte[]> face_memory, const String& source,
 		String font_family = {}, Style::FontStyle style = Style::FontStyle::Normal, Style::FontWeight weight = Style::FontWeight::Normal);
 
-	bool AddFace(FontFaceHandleFreetype face, const String& family, Style::FontStyle style, Style::FontWeight weight, bool fallback_face, bool release_stream);
+	bool AddFace(FontFaceHandleFreetype face, const String& family, Style::FontStyle style, Style::FontWeight weight, bool fallback_face, UniquePtr<byte[]> face_memory);
 
 	using FontFaceList = Vector<FontFace*>;
 	using FontFamilyMap = UnorderedMap< String, UniquePtr<FontFamily>>;

diff --git a/Source/Core/FontEngineDefault/FreeTypeInterface.cpp b/Source/Core/FontEngineDefault/FreeTypeInterface.cpp
@@ -95,16 +95,11 @@ FontFaceHandleFreetype FreeType::LoadFace(const byte* data, int data_length, con
 	return (FontFaceHandleFreetype)face;
 }
 
-bool FreeType::ReleaseFace(FontFaceHandleFreetype in_face, bool release_stream)
+bool FreeType::ReleaseFace(FontFaceHandleFreetype in_face)
 {
 	FT_Face face = (FT_Face)in_face;
-
-	FT_Byte* face_memory = face->stream->base;
 	FT_Error error = FT_Done_Face(face);
 
-	if (release_stream)
-		delete[] face_memory;
-
 	return (error == 0);
 }
 

diff --git a/Source/Core/FontEngineDefault/FreeTypeInterface.h b/Source/Core/FontEngineDefault/FreeTypeInterface.h
@@ -44,7 +44,7 @@ void Shutdown();
 FontFaceHandleFreetype LoadFace(const byte* data, int data_length, const String& source);
 
 // Releases the FreeType face.
-bool ReleaseFace(FontFaceHandleFreetype face, bool release_stream);
+bool ReleaseFace(FontFaceHandleFreetype face);
 
 // Retrieves the font family, style and weight of the given font face.
 void GetFaceStyle(FontFaceHandleFreetype face, String& font_family, Style::FontStyle& style, Style::FontWeight& weight);