font rendering: reimplement tab-as-spaces support.

src/modules/font/GenericShaper.cpp

@@ -167,7 +167,7 @@ int GenericShaper::computeWordWrapIndex(const ColoredCodepoints& codepoints, Ran
 		float newwidth = w + getKerning(prevglyph, g) + getGlyphAdvance(g);
 
 		// Only wrap when there's a non-space character.
-		if (newwidth > wraplimit && g != ' ')
+		if (newwidth > wraplimit && !isWhitespace(g))
 		{
 			// Rewind to the last seen space when wrapping.
 			if (lastspaceindex != -1)
@@ -179,10 +179,10 @@ int GenericShaper::computeWordWrapIndex(const ColoredCodepoints& codepoints, Ran
 		}
 
 		// Don't count trailing spaces in the output width.
-		if (g == ' ')
+		if (isWhitespace(g))
 		{
 			lastspaceindex = i;
-			if (prevglyph != ' ')
+			if (!isWhitespace(prevglyph))
 				widthbeforelastspace = w;
 		}
 		else

src/modules/font/TextShaper.cpp

@@ -87,7 +87,10 @@ TextShaper::TextShaper(Rasterizer *rasterizer)
 	, dpiScales{rasterizer->getDPIScale()}
 	, height(floorf(rasterizer->getHeight() / rasterizer->getDPIScale() + 0.5f))
 	, lineHeight(1)
+	, useSpacesForTab(false)
 {
+	if (!rasterizer->hasGlyph('\t'))
+		useSpacesForTab = true;
 }
 
 TextShaper::~TextShaper()
@@ -215,8 +218,6 @@ float TextShaper::getKerning(const std::string& leftchar, const std::string& rig
 
 int TextShaper::getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex)
 {
-	// TODO: use spaces for tab
-
 	const auto it = glyphAdvances.find(glyph);
 	if (it != glyphAdvances.end())
 	{
@@ -226,10 +227,14 @@ int TextShaper::getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex)
 	}
 
 	int rasterizeri = 0;
+	uint32 realglyph = glyph;
+
+	if (glyph == '\t' && isUsingSpacesForTab())
+		realglyph = ' ';
 
 	for (size_t i = 0; i < rasterizers.size(); i++)
 	{
-		if (rasterizers[i]->hasGlyph(glyph))
+		if (rasterizers[i]->hasGlyph(realglyph))
 		{
 			rasterizeri = (int) i;
 			break;
@@ -237,9 +242,12 @@ int TextShaper::getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex)
 	}
 
 	const auto &r = rasterizers[rasterizeri];
-	int advance = floorf(r->getGlyphSpacing(glyph) / r->getDPIScale() + 0.5f);
+	int advance = floorf(r->getGlyphSpacing(realglyph) / r->getDPIScale() + 0.5f);
 
-	GlyphIndex glyphi = {r->getGlyphIndex(glyph), rasterizeri};
+	if (glyph == '\t' && realglyph == ' ')
+		advance *= SPACES_PER_TAB;
+
+	GlyphIndex glyphi = {r->getGlyphIndex(realglyph), rasterizeri};
 
 	glyphAdvances[glyph] = std::make_pair(advance, glyphi);
 	if (glyphindex)

src/modules/font/TextShaper.h

@@ -81,11 +81,15 @@ public:
 		int height;
 	};
 
+	// This will be used if the Rasterizer doesn't have a tab character itself.
+	static const int SPACES_PER_TAB = 4;
+
 	static love::Type type;
 
 	virtual ~TextShaper();
 
 	const std::vector<StrongRef<Rasterizer>> &getRasterizers() const { return rasterizers; }
+	bool isUsingSpacesForTab() const { return useSpacesForTab; }
 
 	float getHeight() const;
 
@@ -128,6 +132,8 @@ protected:
 
 	TextShaper(Rasterizer *rasterizer);
 
+	static inline bool isWhitespace(uint32 codepoint) { return codepoint == ' ' || codepoint == '\t'; }
+
 	std::vector<StrongRef<Rasterizer>> rasterizers;
 	std::vector<float> dpiScales;
 
@@ -136,6 +142,8 @@ private:
 	int height;
 	float lineHeight;
 
+	bool useSpacesForTab;
+
 	// maps glyphs to advance and glyph+rasterizer index.
 	std::unordered_map<uint32, std::pair<int, GlyphIndex>> glyphAdvances;
 

src/modules/font/freetype/HarfbuzzShaper.cpp

@@ -36,30 +36,70 @@ namespace freetype
 
 HarfbuzzShaper::HarfbuzzShaper(TrueTypeRasterizer *rasterizer)
 	: TextShaper(rasterizer)
+	, spaceGlyphIndex()
+	, tabSpacesAdvanceX(0)
+	, tabSpacesAdvanceY(0)
 {
 	hbFonts.push_back(hb_ft_font_create_referenced((FT_Face)rasterizer->getHandle()));
+	hbBuffers.push_back(hb_buffer_create());
 
 	if (hbFonts[0] == nullptr || hbFonts[0] == hb_font_get_empty())
-		throw love::Exception("");
+		throw love::Exception("Could not create Harfbuzz font object.");
+
+	if (hbBuffers[0] == nullptr || hbBuffers[0] == hb_buffer_get_empty())
+		throw love::Exception("Could not create Harfbuzz buffer object.");
+
+	updateSpacesForTabInfo();
 }
 
 HarfbuzzShaper::~HarfbuzzShaper()
 {
+	for (hb_buffer_t *buffer : hbBuffers)
+		hb_buffer_destroy(buffer);
 	for (hb_font_t *font : hbFonts)
 		hb_font_destroy(font);
 }
 
 void HarfbuzzShaper::setFallbacks(const std::vector<Rasterizer*> &fallbacks)
 {
-	for (size_t i = 1; i < hbFonts.size(); i++)
+	for (size_t i = 1; i < rasterizers.size(); i++)
+	{
+		hb_buffer_destroy(hbBuffers[i]);
 		hb_font_destroy(hbFonts[i]);
+	}
 
 	TextShaper::setFallbacks(fallbacks);
 
 	hbFonts.resize(rasterizers.size());
+	hbBuffers.resize(rasterizers.size());
 
-	for (size_t i = 1; i < hbFonts.size(); i++)
+	for (size_t i = 1; i < rasterizers.size(); i++)
+	{
 		hbFonts[i] = hb_ft_font_create_referenced((FT_Face)rasterizers[i]->getHandle());
+		hbBuffers[i] = hb_buffer_create();
+	}
+
+	updateSpacesForTabInfo();
+}
+
+void HarfbuzzShaper::updateSpacesForTabInfo()
+{
+	if (!isUsingSpacesForTab())
+		return;
+
+	hb_codepoint_t glyphid = 0;
+	for (size_t i = 0; i < hbFonts.size(); i++)
+	{
+		hb_font_t *hbfont = hbFonts[i];
+		if (hb_font_get_glyph(hbfont, ' ', 0, &glyphid))
+		{
+			spaceGlyphIndex.index = glyphid;
+			spaceGlyphIndex.rasterizerIndex = i;
+			tabSpacesAdvanceX = hb_font_get_glyph_h_advance(hbfont, glyphid) * SPACES_PER_TAB;
+			tabSpacesAdvanceY = hb_font_get_glyph_v_advance(hbfont, glyphid) * SPACES_PER_TAB;
+			break;
+		}
+	}
 }
 
 void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints, Range range, Vector2 offset, float extraspacing, std::vector<GlyphPosition> *positions, std::vector<IndexedColor> *colors, TextInfo *info)
@@ -70,13 +110,16 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 	offset.y += getBaseline();
 	Vector2 curpos = offset;
 
-	hb_buffer_t *hbbuffer = hb_buffer_create();
+	hb_buffer_t *hbbuffer = hbBuffers[0];
+	hb_buffer_reset(hbbuffer);
 
 	hb_buffer_add_codepoints(hbbuffer, codepoints.cps.data(), codepoints.cps.size(), (unsigned int) range.getOffset(), (int) range.getSize());
 
 	// TODO: Expose APIs for direction and script?
 	hb_buffer_guess_segment_properties(hbbuffer);
 
+	hb_direction_t direction = hb_buffer_get_direction(hbbuffer);
+
 	hb_shape(hbFonts[0], hbbuffer, nullptr, 0);
 
 	int glyphcount = (int) hb_buffer_get_length(hbbuffer);
@@ -93,7 +136,7 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 		for (int i = 0; i < glyphcount; i++)
 		{
 			bool adding = false;
-			if (glyphinfos[i].codepoint == 0)
+			if (glyphinfos[i].codepoint == 0 && (codepoints.cps[glyphinfos[i].cluster] != '\t' || !isUsingSpacesForTab()))
 			{
 				if (fallbackranges.empty() || fallbackranges.back().getMax() != glyphinfos[i-1].cluster)
 					fallbackranges.push_back(Range(glyphinfos[i].cluster, 1));
@@ -106,9 +149,7 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 		{
 			for (size_t rasti = 1; rasti < rasterizers.size(); rasti++)
 			{
-				hb_buffer_t* hbbuffer = hb_buffer_create();
 
-				hb_buffer_destroy(hbbuffer);
 			}
 		}
 	}
@@ -132,11 +173,10 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 	int maxwidth = (int)curpos.x;
 
 	// TODO: fallbacks
-	// TODO: use spaces for tab
 	for (int i = 0; i < glyphcount; i++)
 	{
 		const hb_glyph_info_t &info = glyphinfos[i];
-		const hb_glyph_position_t& glyphpos = glyphpositions[i];
+		hb_glyph_position_t &glyphpos = glyphpositions[i];
 
 		// TODO: this doesn't handle situations where the user inserted a color
 		// change in the middle of some characters that get combined into a single
@@ -166,6 +206,21 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 		if (clustercodepoint == '\r')
 			continue;
 
+		// This is a glyph index at this point, despite the name.
+		GlyphIndex gindex = { info.codepoint, 0 };
+
+		if (clustercodepoint == '\t' && isUsingSpacesForTab())
+		{
+			gindex = spaceGlyphIndex;
+
+			// This should be safe to overwrite.
+			// TODO: RTL support?
+			glyphpos.x_offset = 0;
+			glyphpos.y_offset = 0;
+			glyphpos.x_advance = HB_DIRECTION_IS_HORIZONTAL(direction) ? tabSpacesAdvanceX : 0;
+			glyphpos.y_advance = HB_DIRECTION_IS_VERTICAL(direction) ? tabSpacesAdvanceY : 0;
+		}
+
 		if (colorToAdd.hasValue && colors && positions)
 		{
 			IndexedColor c = {colorToAdd.value, positions->size()};
@@ -175,8 +230,6 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 
 		if (positions)
 		{
-			// Despite the name this is a glyph index at this point.
-			GlyphIndex gindex = { info.codepoint, 0 };
 			GlyphPosition p = { curpos, gindex };
 
 			// Harfbuzz position coordinate systems are based on the given font.
@@ -195,8 +248,6 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 			curpos.x = floorf(curpos.x + extraspacing);
 	}
 
-	hb_buffer_destroy(hbbuffer);
-
 	if (curpos.x > maxwidth)
 		maxwidth = (int)curpos.x;
 
@@ -214,13 +265,16 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints& codepoints, Ra
 	if (!range.isValid())
 		range = Range(0, codepoints.cps.size());
 
-	hb_buffer_t* hbbuffer = hb_buffer_create();
+	hb_buffer_t *hbbuffer = hbBuffers[0];
+	hb_buffer_reset(hbbuffer);
 
 	hb_buffer_add_codepoints(hbbuffer, codepoints.cps.data(), codepoints.cps.size(), (unsigned int)range.getOffset(), (int)range.getSize());
 
 	// TODO: Expose APIs for direction and script?
 	hb_buffer_guess_segment_properties(hbbuffer);
 
+	hb_direction_t direction = hb_buffer_get_direction(hbbuffer);
+
 	hb_shape(hbFonts[0], hbbuffer, nullptr, 0);
 
 	int glyphcount = (int)hb_buffer_get_length(hbbuffer);
@@ -238,7 +292,7 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints& codepoints, Ra
 	for (int i = 0; i < glyphcount; i++)
 	{
 		const hb_glyph_info_t &info = glyphinfos[i];
-		const hb_glyph_position_t& glyphpos = glyphpositions[i];
+		hb_glyph_position_t &glyphpos = glyphpositions[i];
 
 		uint32 clustercodepoint = codepoints.cps[info.cluster];
 
@@ -248,10 +302,20 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints& codepoints, Ra
 			continue;
 		}
 
+		if (clustercodepoint == '\t' && isUsingSpacesForTab())
+		{
+			// This should be safe to overwrite.
+			// TODO: RTL support?
+			glyphpos.x_offset = 0;
+			glyphpos.y_offset = 0;
+			glyphpos.x_advance = HB_DIRECTION_IS_HORIZONTAL(direction) ? tabSpacesAdvanceX : 0;
+			glyphpos.y_advance = HB_DIRECTION_IS_VERTICAL(direction) ? tabSpacesAdvanceY : 0;
+		}
+
 		float newwidth = w + floorf((glyphpos.x_advance >> 6) / dpiScales[0] + 0.5f);
 
 		// Only wrap when there's a non-space character.
-		if (newwidth > wraplimit && clustercodepoint != ' ')
+		if (newwidth > wraplimit && !isWhitespace(clustercodepoint))
 		{
 			// Rewind to the last seen space when wrapping.
 			if (lastspaceindex != -1)
@@ -263,10 +327,10 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints& codepoints, Ra
 		}
 
 		// Don't count trailing spaces in the output width.
-		if (clustercodepoint == ' ')
+		if (isWhitespace(clustercodepoint))
 		{
 			lastspaceindex = info.cluster;
-			if (prevcodepoint != ' ')
+			if (!isWhitespace(prevcodepoint))
 				widthbeforelastspace = w;
 		}
 		else
@@ -277,8 +341,6 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints& codepoints, Ra
 		wrapindex = info.cluster;
 	}
 
-	hb_buffer_destroy(hbbuffer);
-
 	if (width)
 		*width = outwidth;
 

src/modules/font/freetype/HarfbuzzShaper.h

@@ -26,6 +26,7 @@
 extern "C"
 {
 typedef struct hb_font_t hb_font_t;
+typedef struct hb_buffer_t hb_buffer_t;
 }
 
 namespace love
@@ -50,7 +51,14 @@ public:
 
 private:
 
+	void updateSpacesForTabInfo();
+
 	std::vector<hb_font_t *> hbFonts;
+	std::vector<hb_buffer_t *> hbBuffers;
+
+	GlyphIndex spaceGlyphIndex;
+	int tabSpacesAdvanceX;
+	int tabSpacesAdvanceY;
 
 }; // HarfbuzzShaper
 

src/modules/graphics/Font.cpp

@@ -61,7 +61,6 @@ Font::Font(love::font::Rasterizer *r, const SamplerState &s)
 	, textureHeight(128)
 	, samplerState()
 	, dpiScale(r->getDPIScale())
-	, useSpacesAsTab(false)
 	, textureCacheID(0)
 {
 	samplerState.minFilter = s.minFilter;
@@ -92,10 +91,6 @@ Font::Font(love::font::Rasterizer *r, const SamplerState &s)
 	if (pixelFormat == PIXELFORMAT_LA8_UNORM && !gfx->isPixelFormatSupported(pixelFormat, PIXELFORMATUSAGEFLAGS_SAMPLE))
 		pixelFormat = PIXELFORMAT_RGBA8_UNORM;
 
-	uint32 tab = '\t';
-	if (!r->hasGlyph(tab)) // No tab character in the Rasterizer.
-		useSpacesAsTab = true;
-
 	loadVolatile();
 	++fontCount;
 }
@@ -232,24 +227,6 @@ void Font::unloadVolatile()
 love::font::GlyphData *Font::getRasterizerGlyphData(love::font::TextShaper::GlyphIndex glyphindex, float &dpiscale)
 {
 	const auto &r = shaper->getRasterizers()[glyphindex.rasterizerIndex];
-
-	// Use spaces for the tab 'glyph'. FIXME
-	if (/*glyph == '\t' &&*/ false && useSpacesAsTab)
-	{
-		love::font::GlyphData *spacegd = r->getGlyphData(32);
-		PixelFormat fmt = spacegd->getFormat();
-
-		love::font::GlyphMetrics gm = {};
-		gm.advance = spacegd->getAdvance() * SPACES_PER_TAB;
-		gm.bearingX = spacegd->getBearingX();
-		gm.bearingY = spacegd->getBearingY();
-
-		spacegd->release();
-
-		dpiscale = r->getDPIScale();
-		return new love::font::GlyphData('\t', gm, fmt);
-	}
-
 	dpiscale = r->getDPIScale();
 	return r->getGlyphDataForIndex(glyphindex.index);
 }

src/modules/graphics/Font.h

@@ -201,8 +201,6 @@ private:
 	int textureX, textureY;
 	int rowHeight;
 
-	bool useSpacesAsTab;
-
 	// ID which is incremented when the texture cache is invalidated.
 	uint32 textureCacheID;
 
@@ -211,9 +209,6 @@ private:
 	// use, for edge antialiasing.
 	static const int TEXTURE_PADDING = 2;
 
-	// This will be used if the Rasterizer doesn't have a tab character itself.
-	static const int SPACES_PER_TAB = 4;
-
 	static StringMap<AlignMode, ALIGN_MAX_ENUM>::Entry alignModeEntries[];
 	static StringMap<AlignMode, ALIGN_MAX_ENUM> alignModes;