Fix issues with harfbuzz strings (#1034)

* Add unit tests to detect #1031
* Correctly marshal C `char*` as C# `byte*`

binding/HarfBuzzSharp.Shared/Font.cs

@@ -8,7 +8,7 @@ namespace HarfBuzzSharp
 {
 	public class Font : NativeObject
 	{
-		private const int NameBufferLength = 128;
+		internal const int NameBufferLength = 128;
 
 		public Font (Face face)
 			: base (IntPtr.Zero)
@@ -144,18 +144,18 @@ namespace HarfBuzzSharp
 
 		public unsafe bool TryGetGlyphName (uint glyph, out string name)
 		{
-			var buffer = ArrayPool<char>.Shared.Rent (NameBufferLength);
+			var buffer = ArrayPool<byte>.Shared.Rent (NameBufferLength);
 			try {
-				fixed (char* first = buffer) {
+				fixed (byte* first = buffer) {
 					if (!HarfBuzzApi.hb_font_get_glyph_name (Handle, glyph, first, buffer.Length)) {
 						name = string.Empty;
 						return false;
 					}
-					name = new string (first);
+					name = Marshal.PtrToStringAnsi ((IntPtr)first);
 					return true;
 				}
 			} finally {
-				ArrayPool<char>.Shared.Return (buffer);
+				ArrayPool<byte>.Shared.Return (buffer);
 			}
 		}
 
@@ -206,14 +206,14 @@ namespace HarfBuzzSharp
 
 		public unsafe string GlyphToString (uint glyph)
 		{
-			var buffer = ArrayPool<char>.Shared.Rent (NameBufferLength);
+			var buffer = ArrayPool<byte>.Shared.Rent (NameBufferLength);
 			try {
-				fixed (char* first = buffer) {
+				fixed (byte* first = buffer) {
 					HarfBuzzApi.hb_font_glyph_to_string (Handle, glyph, first, buffer.Length);
-					return new string (first);
+					return Marshal.PtrToStringAnsi ((IntPtr)first);
 				}
 			} finally {
-				ArrayPool<char>.Shared.Return (buffer);
+				ArrayPool<byte>.Shared.Return (buffer);
 			}
 		}
 

binding/HarfBuzzSharp.Shared/HarfBuzzApi.cs

@@ -190,7 +190,7 @@ namespace HarfBuzzSharp
 		public extern static hb_bool_t hb_font_get_glyph_contour_point (hb_font_t font, hb_codepoint_t glyph, uint point_index, out hb_position_t x, out hb_position_t y);
 		[DllImport (HARFBUZZ, CallingConvention = CallingConvention.Cdecl)]
 		[return: MarshalAs (UnmanagedType.I1)]
-		public extern static hb_bool_t hb_font_get_glyph_name (hb_font_t font, hb_codepoint_t glyph, char* nameBuffer, int size);
+		public extern static hb_bool_t hb_font_get_glyph_name (hb_font_t font, hb_codepoint_t glyph, byte* nameBuffer, int size);
 		[DllImport (HARFBUZZ, CallingConvention = CallingConvention.Cdecl)]
 		[return: MarshalAs (UnmanagedType.I1)]
 		public extern static hb_bool_t hb_font_get_glyph_from_name (hb_font_t font, [MarshalAs (UnmanagedType.LPStr)]string name, int len, out hb_codepoint_t glyph);
@@ -208,7 +208,7 @@ namespace HarfBuzzSharp
 		public extern static hb_bool_t hb_font_get_glyph_contour_point_for_origin (hb_font_t font, hb_codepoint_t glyph, uint point_index, hb_direction_t direction, out hb_position_t x, out hb_position_t y);
 		/* Generates gidDDD if glyph has no name. */
 		[DllImport (HARFBUZZ, CallingConvention = CallingConvention.Cdecl)]
-		public extern static void hb_font_glyph_to_string (hb_font_t font, hb_codepoint_t glyph, char* s, int size);
+		public extern static void hb_font_glyph_to_string (hb_font_t font, hb_codepoint_t glyph, byte* s, int size);
 		/* Parses gidDDD and uniUUUU strings automatically. */
 		[DllImport (HARFBUZZ, CallingConvention = CallingConvention.Cdecl)]
 		[return: MarshalAs (UnmanagedType.I1)]

binding/HarfBuzzSharp.Shared/Properties/HarfBuzzSharpAssemblyInfo.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Reflection;
 using System.Resources;
+using System.Runtime.CompilerServices;
 
 [assembly: AssemblyTitle("HarfBuzzSharp")]
 [assembly: AssemblyDescription("HarfBuzzSharp is a cross-platform OpenType text shaping engine for .NET platforms.")]
@@ -14,3 +15,10 @@ using System.Resources;
 #else
 [assembly: AssemblyConfiguration("Release")]
 #endif
+
+[assembly: InternalsVisibleTo ("SkiaSharp.Tests, PublicKey=" +
+	"002400000480000094000000060200000024000052534131000400000100010079159977d2d03a" +
+	"8e6bea7a2e74e8d1afcc93e8851974952bb480a12c9134474d04062447c37e0e68c080536fcf3c" +
+	"3fbe2ff9c979ce998475e506e8ce82dd5b0f350dc10e93bf2eeecf874b24770c5081dbea7447fd" +
+	"dafa277b22de47d6ffea449674a4f9fccf84d15069089380284dbdd35f46cdff12a1bd78e4ef00" +
+	"65d016df")]

tests/Tests/HBFontFuncsTest.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Buffers;
 using Xunit;
 
 namespace HarfBuzzSharp.Tests
@@ -137,6 +138,35 @@ namespace HarfBuzzSharp.Tests
 			}
 		}
 
+		[SkippableFact]
+		public void TryGetGlyphNameIsCorrectWithDelegate()
+		{
+			// get an array and fill it with things
+			var buffer = ArrayPool<byte>.Shared.Rent(Font.NameBufferLength);
+			for (int i = 0; i < buffer.Length; i++)
+				buffer[i] = (byte)i;
+			ArrayPool<byte>.Shared.Return(buffer);
+
+			using (var font = new Font(Font))
+			using (var fontFuncs = new FontFunctions())
+			{
+				fontFuncs.SetGlyphNameDelegate((Font f, object fd, uint g, out string n) =>
+				{
+					n = ((char)g).ToString();
+					return true;
+				});
+
+				fontFuncs.MakeImmutable();
+
+				font.SetFontFunctions(fontFuncs, "FontData");
+
+				var result = font.TryGetGlyphName('H', out var name);
+
+				Assert.True(result);
+				Assert.Equal("H", name);
+			}
+		}
+
 		[SkippableFact]
 		public void ShouldSetHorizontalGlyphAdvanceDelegate()
 		{

tests/Tests/HBFontTest.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Buffers;
 using System.Text;
 using Xunit;
 
@@ -158,6 +159,24 @@ namespace HarfBuzzSharp.Tests
 			}
 		}
 
+		[SkippableFact]
+		public void GlyphToStringIsCorrectWithDelegate()
+		{
+			// get an array and fill it with things
+			var buffer = ArrayPool<byte>.Shared.Rent(Font.NameBufferLength);
+			for (int i = 0; i < buffer.Length; i++)
+				buffer[i] = (byte)i;
+			ArrayPool<byte>.Shared.Return(buffer);
+
+			using (var face = new Face(Blob, 0))
+			using (var font = new Font(face))
+			{
+				var s = font.GlyphToString(49);
+
+				Assert.Equal("H", s);
+			}
+		}
+
 		[SkippableFact]
 		public void ShouldGetGlyphContourPointForOrigin()
 		{