Improve letter spacing and line heights of high dpi text.

resolves #1918

src/modules/font/GenericShaper.cpp

@@ -23,6 +23,8 @@
 #include "Rasterizer.h"
 #include "common/Optional.h"
 
+#include <algorithm>
+
 namespace love
 {
 namespace font
@@ -48,7 +50,7 @@ void GenericShaper::computeGlyphPositions(const ColoredCodepoints &codepoints, R
 	// Spacing counter and newline handling.
 	Vector2 curpos = offset;
 
-	int maxwidth = 0;
+	float maxwidth = 0;
 	uint32 prevglyph = 0;
 
 	if (positions)
@@ -86,11 +88,10 @@ void GenericShaper::computeGlyphPositions(const ColoredCodepoints &codepoints, R
 
 		if (g == '\n')
 		{
-			if (curpos.x > maxwidth)
-				maxwidth = (int)curpos.x;
+			maxwidth = std::max(maxwidth, curpos.x);
 
 			// Wrap newline, but do not output a position for it.
-			curpos.y += floorf(getHeight() * getLineHeight() + 0.5f);
+			curpos.y += getCombinedHeight();
 			curpos.x = offset.x;
 			prevglyph = 0;
 			continue;
@@ -114,7 +115,7 @@ void GenericShaper::computeGlyphPositions(const ColoredCodepoints &codepoints, R
 		curpos.x += getKerning(prevglyph, g);
 
 		GlyphIndex glyphindex;
-		int advance = getGlyphAdvance(g, &glyphindex);
+		float advance = getGlyphAdvance(g, &glyphindex);
 
 		if (positions)
 			positions->push_back({ Vector2(curpos.x, curpos.y), glyphindex });
@@ -124,20 +125,19 @@ void GenericShaper::computeGlyphPositions(const ColoredCodepoints &codepoints, R
 
 		// Account for extra spacing given to space characters.
 		if (g == ' ' && extraspacing != 0.0f)
-			curpos.x = floorf(curpos.x + extraspacing);
+			curpos.x += extraspacing;
 
 		prevglyph = g;
 	}
 
-	if (curpos.x > maxwidth)
-		maxwidth = (int)curpos.x;
+	maxwidth = std::max(maxwidth, curpos.x);
 
 	if (info != nullptr)
 	{
 		info->width = maxwidth - offset.x;
 		info->height = curpos.y - offset.y;
 		if (curpos.x > offset.x)
-			info->height += floorf(getHeight() * getLineHeight() + 0.5f);
+			info->height += getCombinedHeight();
 	}
 }
 

src/modules/font/TextShaper.cpp

@@ -86,6 +86,7 @@ TextShaper::TextShaper(Rasterizer *rasterizer)
 	: rasterizers{rasterizer}
 	, dpiScales{rasterizer->getDPIScale()}
 	, height(floorf(rasterizer->getHeight() / rasterizer->getDPIScale() + 0.5f))
+	, pixelHeight(rasterizer->getHeight())
 	, lineHeight(1)
 	, useSpacesForTab(false)
 {
@@ -102,6 +103,16 @@ float TextShaper::getHeight() const
 	return height;
 }
 
+float TextShaper::getPixelHeight() const
+{
+	return pixelHeight;
+}
+
+float TextShaper::getCombinedHeight() const
+{
+	return floorf(pixelHeight * lineHeight + 0.5f) / rasterizers[0]->getDPIScale();
+}
+
 void TextShaper::setLineHeight(float h)
 {
 	lineHeight = h;
@@ -112,14 +123,14 @@ float TextShaper::getLineHeight() const
 	return lineHeight;
 }
 
-int TextShaper::getAscent() const
+float TextShaper::getAscent() const
 {
-	return floorf(rasterizers[0]->getAscent() / rasterizers[0]->getDPIScale() + 0.5f);
+	return rasterizers[0]->getAscent() / rasterizers[0]->getDPIScale();
 }
 
-int TextShaper::getDescent() const
+float TextShaper::getDescent() const
 {
-	return floorf(rasterizers[0]->getDescent() / rasterizers[0]->getDPIScale() + 0.5f);
+	return rasterizers[0]->getDescent() / rasterizers[0]->getDPIScale();
 }
 
 float TextShaper::getBaseline() const
@@ -128,7 +139,7 @@ float TextShaper::getBaseline() const
 	if (ascent != 0.0f)
 		return ascent;
 	else if (rasterizers[0]->getDataType() == font::Rasterizer::DATA_TRUETYPE)
-		return floorf(getHeight() / 1.25f + 0.5f); // 1.25 is magic line height for true type fonts
+		return floorf(getPixelHeight() / 1.25f + 0.5f) / rasterizers[0]->getDPIScale(); // 1.25 is magic line height for true type fonts
 	else
 		return 0.0f;
 }
@@ -186,13 +197,13 @@ float TextShaper::getKerning(uint32 leftglyph, uint32 rightglyph)
 		if (r->hasGlyph(leftglyph) && r->hasGlyph(rightglyph))
 		{
 			found = true;
-			k = floorf(r->getKerning(leftglyph, rightglyph) / r->getDPIScale() + 0.5f);
+			k = r->getKerning(leftglyph, rightglyph) / r->getDPIScale();
 			break;
 		}
 	}
 
 	if (!found)
-		k = floorf(rasterizers[0]->getKerning(leftglyph, rightglyph) / rasterizers[0]->getDPIScale() + 0.5f);
+		k = rasterizers[0]->getKerning(leftglyph, rightglyph) / rasterizers[0]->getDPIScale();
 
 	kerning[packedglyphs] = k;
 	return k;
@@ -216,7 +227,7 @@ float TextShaper::getKerning(const std::string &leftchar, const std::string &rig
 	return getKerning(left, right);
 }
 
-int TextShaper::getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex)
+float TextShaper::getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex)
 {
 	const auto it = glyphAdvances.find(glyph);
 	if (it != glyphAdvances.end())
@@ -242,7 +253,7 @@ int TextShaper::getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex)
 	}
 
 	const auto &r = rasterizers[rasterizeri];
-	int advance = floorf(r->getGlyphSpacing(realglyph) / r->getDPIScale() + 0.5f);
+	float advance = r->getGlyphSpacing(realglyph) / r->getDPIScale();
 
 	if (glyph == '\t' && realglyph == ' ')
 		advance *= SPACES_PER_TAB;
@@ -255,7 +266,7 @@ int TextShaper::getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex)
 	return advance;
 }
 
-int TextShaper::getWidth(const std::string &str)
+float TextShaper::getWidth(const std::string& str)
 {
 	if (str.size() == 0) return 0;
 
@@ -281,7 +292,7 @@ static size_t findNewline(const ColoredCodepoints &codepoints, size_t start)
 	return codepoints.cps.size();
 }
 
-void TextShaper::getWrap(const ColoredCodepoints &codepoints, float wraplimit, std::vector<Range> &lineranges, std::vector<int> *linewidths)
+void TextShaper::getWrap(const ColoredCodepoints &codepoints, float wraplimit, std::vector<Range> &lineranges, std::vector<float> *linewidths)
 {
 	size_t nextnewline = findNewline(codepoints, 0);
 
@@ -325,7 +336,7 @@ void TextShaper::getWrap(const ColoredCodepoints &codepoints, float wraplimit, s
 	}
 }
 
-void TextShaper::getWrap(const std::vector<ColoredString> &text, float wraplimit, std::vector<std::string> &lines, std::vector<int> *linewidths)
+void TextShaper::getWrap(const std::vector<ColoredString> &text, float wraplimit, std::vector<std::string> &lines, std::vector<float> *linewidths)
 {
 	ColoredCodepoints cps;
 	getCodepointsFromString(text, cps);

src/modules/font/TextShaper.h

@@ -77,8 +77,8 @@ public:
 
 	struct TextInfo
 	{
-		int width;
-		int height;
+		float width;
+		float height;
 	};
 
 	// This will be used if the Rasterizer doesn't have a tab character itself.
@@ -92,6 +92,9 @@ public:
 	bool isUsingSpacesForTab() const { return useSpacesForTab; }
 
 	float getHeight() const;
+	float getPixelHeight() const;
+
+	float getCombinedHeight() const;
 
 	/**
 	 * Sets the line height (which should be a number to multiply the font size by,
@@ -106,8 +109,8 @@ public:
 	float getLineHeight() const;
 
 	// Extra font metrics
-	int getAscent() const;
-	int getDescent() const;
+	float getAscent() const;
+	float getDescent() const;
 	float getBaseline() const;
 
 	bool hasGlyph(uint32 glyph) const;
@@ -116,12 +119,12 @@ public:
 	float getKerning(uint32 leftglyph, uint32 rightglyph);
 	float getKerning(const std::string &leftchar, const std::string &rightchar);
 
-	int getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex = nullptr);
+	float getGlyphAdvance(uint32 glyph, GlyphIndex *glyphindex = nullptr);
 
-	int getWidth(const std::string &str);
+	float getWidth(const std::string &str);
 
-	void getWrap(const std::vector<ColoredString> &text, float wraplimit, std::vector<std::string> &lines, std::vector<int> *linewidths = nullptr);
-	void getWrap(const ColoredCodepoints &codepoints, float wraplimit, std::vector<Range> &lineranges, std::vector<int> *linewidths = nullptr);
+	void getWrap(const std::vector<ColoredString> &text, float wraplimit, std::vector<std::string> &lines, std::vector<float> *linewidths = nullptr);
+	void getWrap(const ColoredCodepoints &codepoints, float wraplimit, std::vector<Range> &lineranges, std::vector<float> *linewidths = nullptr);
 
 	virtual void setFallbacks(const std::vector<Rasterizer *> &fallbacks);
 
@@ -140,12 +143,13 @@ protected:
 private:
 
 	int height;
+	int pixelHeight;
 	float lineHeight;
 
 	bool useSpacesForTab;
 
 	// maps glyphs to advance and glyph+rasterizer index.
-	std::unordered_map<uint32, std::pair<int, GlyphIndex>> glyphAdvances;
+	std::unordered_map<uint32, std::pair<float, GlyphIndex>> glyphAdvances;
 
 	// map of left/right glyph pairs to horizontal kerning.
 	std::unordered_map<uint64, float> kerning;

src/modules/font/freetype/HarfbuzzShaper.cpp

@@ -27,6 +27,8 @@
 #include <hb.h>
 #include <hb-ft.h>
 
+#include <algorithm>
+
 namespace love
 {
 namespace font
@@ -226,7 +228,7 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 	std::vector<BufferRange> bufferranges;
 	computeBufferRanges(codepoints, range, bufferranges);
 
-	int maxwidth = (int)curpos.x;
+	float maxwidth = curpos.x;
 
 	for (const auto &bufferrange : bufferranges)
 	{
@@ -259,11 +261,10 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 			// the glyph list so we can do it manually.
 			if (clustercodepoint == '\n')
 			{
-				if (curpos.x > maxwidth)
-					maxwidth = (int)curpos.x;
+				maxwidth = std::max(maxwidth, curpos.x);
 
 				// Wrap newline, but do not output a position for it.
-				curpos.y += floorf(getHeight() * getLineHeight() + 0.5f);
+				curpos.y += getCombinedHeight();
 				curpos.x = offset.x;
 				continue;
 			}
@@ -273,7 +274,7 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 				continue;
 
 			// This is a glyph index at this point, despite the name.
-			GlyphIndex gindex = { (int) info.codepoint, (int)bufferrange.index };
+			GlyphIndex gindex = { (int) info.codepoint, (int) bufferrange.index };
 
 			if (clustercodepoint == '\t' && isUsingSpacesForTab())
 			{
@@ -300,30 +301,29 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 
 				// Harfbuzz position coordinate systems are based on the given font.
 				// Freetype uses 26.6 fixed point coordinates, so harfbuzz does too.
-				p.position.x += floorf((glyphpos.x_offset >> 6) / dpiScales[0] + 0.5f);
-				p.position.y += floorf((glyphpos.y_offset >> 6) / dpiScales[0] + 0.5f);
+				p.position.x += (glyphpos.x_offset >> 6) / dpiScales[bufferrange.index];
+				p.position.y += (glyphpos.y_offset >> 6) / dpiScales[bufferrange.index];
 
 				positions->push_back(p);
 			}
 
-			curpos.x += floorf((glyphpos.x_advance >> 6) / dpiScales[0] + 0.5f);
-			curpos.y += floorf((glyphpos.y_advance >> 6) / dpiScales[0] + 0.5f);
+			curpos.x += (glyphpos.x_advance >> 6) / dpiScales[bufferrange.index];
+			curpos.y += (glyphpos.y_advance >> 6) / dpiScales[bufferrange.index];
 
 			// Account for extra spacing given to space characters.
 			if (clustercodepoint == ' ' && extraspacing != 0.0f)
-				curpos.x = floorf(curpos.x + extraspacing);
+				curpos.x += extraspacing;
 		}
 	}
 
-	if (curpos.x > maxwidth)
-		maxwidth = (int)curpos.x;
+	maxwidth = std::max(maxwidth, curpos.x);
 
 	if (info != nullptr)
 	{
 		info->width = maxwidth - offset.x;
 		info->height = curpos.y - offset.y;
 		if (curpos.x > offset.x)
-			info->height += floorf(getHeight() * getLineHeight() + 0.5f);
+			info->height += getCombinedHeight();
 	}
 }
 
@@ -373,7 +373,7 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints &codepoints, Ra
 				glyphpos.y_advance = HB_DIRECTION_IS_VERTICAL(direction) ? tabSpacesAdvanceY : 0;
 			}
 
-			float newwidth = w + floorf((glyphpos.x_advance >> 6) / dpiScales[0] + 0.5f);
+			float newwidth = w + (glyphpos.x_advance >> 6) / dpiScales[bufferrange.index];
 
 			// Don't count trailing spaces in the output width.
 			if (isWhitespace(clustercodepoint))

src/modules/graphics/Font.cpp

@@ -470,7 +470,7 @@ std::vector<Font::DrawCommand> Font::generateVerticesFormatted(const love::font:
 	vertices.reserve(text.cps.size() * 4);
 
 	std::vector<Range> ranges;
-	std::vector<int> widths;
+	std::vector<float> widths;
 	shaper->getWrap(text, wrap, ranges, &widths);
 
 	float y = 0.0f;
@@ -478,15 +478,15 @@ std::vector<Font::DrawCommand> Font::generateVerticesFormatted(const love::font:
 
 	for (int i = 0; i < (int)ranges.size(); i++)
 	{
-		const auto& range = ranges[i];
+		const auto &range = ranges[i];
 
 		if (!range.isValid())
 		{
-			y += getHeight() * getLineHeight();
+			y += shaper->getCombinedHeight();
 			continue;
 		}
 
-		float width = (float) widths[i];
+		float width = widths[i];
 		love::Vector2 offset(0.0f, floorf(y));
 		float extraspacing = 0.0f;
 
@@ -506,7 +506,7 @@ std::vector<Font::DrawCommand> Font::generateVerticesFormatted(const love::font:
 				auto end = start + range.getSize();
 				float numspaces = std::count(start, end, ' ');
 				if (width < wrap && numspaces >= 1)
-					extraspacing = (wrap - width) / numspaces;
+					extraspacing = floorf((wrap - width) / numspaces);
 				else
 					extraspacing = 0.0f;
 				break;
@@ -539,7 +539,7 @@ std::vector<Font::DrawCommand> Font::generateVerticesFormatted(const love::font:
 			drawcommands.insert(drawcommands.end(), firstcmd, newcommands.end());
 		}
 
-		y += getHeight() * getLineHeight();
+		y += shaper->getCombinedHeight();
 	}
 
 	if (info != nullptr)
@@ -612,12 +612,12 @@ int Font::getWidth(uint32 glyph)
 	return shaper->getGlyphAdvance(glyph);
 }
 
-void Font::getWrap(const love::font::ColoredCodepoints &codepoints, float wraplimit, std::vector<Range> &ranges, std::vector<int> *linewidths)
+void Font::getWrap(const love::font::ColoredCodepoints &codepoints, float wraplimit, std::vector<Range> &ranges, std::vector<float> *linewidths)
 {
 	shaper->getWrap(codepoints, wraplimit, ranges, linewidths);
 }
 
-void Font::getWrap(const std::vector<love::font::ColoredString> &text, float wraplimit, std::vector<std::string> &lines, std::vector<int> *linewidths)
+void Font::getWrap(const std::vector<love::font::ColoredString> &text, float wraplimit, std::vector<std::string> &lines, std::vector<float> *linewidths)
 {
 	shaper->getWrap(text, wraplimit, lines, linewidths);
 }

src/modules/graphics/Font.h

@@ -115,8 +115,8 @@ public:
 	 * @param max_width Optional output of the maximum width
 	 * Returns a vector with the lines.
 	 **/
-	void getWrap(const std::vector<love::font::ColoredString> &text, float wraplimit, std::vector<std::string> &lines, std::vector<int> *line_widths = nullptr);
-	void getWrap(const love::font::ColoredCodepoints &codepoints, float wraplimit, std::vector<Range> &ranges, std::vector<int> *line_widths = nullptr);
+	void getWrap(const std::vector<love::font::ColoredString> &text, float wraplimit, std::vector<std::string> &lines, std::vector<float> *line_widths = nullptr);
+	void getWrap(const love::font::ColoredCodepoints &codepoints, float wraplimit, std::vector<Range> &ranges, std::vector<float> *line_widths = nullptr);
 
 	/**
 	 * Sets the line height (which should be a number to multiply the font size by,

src/modules/graphics/wrap_Font.cpp

@@ -107,16 +107,16 @@ int w_Font_getWrap(lua_State *L)
 	luax_checkcoloredstring(L, 2, text);
 
 	float wrap = (float) luaL_checknumber(L, 3);
-	int max_width = 0;
+	float max_width = 0;
 	std::vector<std::string> lines;
-	std::vector<int> widths;
+	std::vector<float> widths;
 
 	luax_catchexcept(L, [&]() { t->getWrap(text, wrap, lines, &widths); });
 
-	for (int width : widths)
+	for (float width : widths)
 		max_width = std::max(max_width, width);
 
-	lua_pushinteger(L, max_width);
+	lua_pushnumber(L, max_width);
 	lua_createtable(L, (int) lines.size(), 0);
 
 	for (int i = 0; i < (int) lines.size(); i++)