Improve truetype font text positioning with fractional glyph advances and DPI-scaled coordinates.

Disallow Font:setFallbacks for fonts that have different DPI scales.
It's still too easy to render text at a non-integer pixel when fractional DPI scales are used, so maybe a more thorough redesign would be useful.

Fixes #2318.
Fixes #2168.

src/modules/font/TextShaper.cpp

@@ -84,7 +84,7 @@ love::Type TextShaper::type("TextShaper", &Object::type);
 
 TextShaper::TextShaper(Rasterizer *rasterizer)
 	: rasterizers{rasterizer}
-	, dpiScales{rasterizer->getDPIScale()}
+	, dpiScale(rasterizer->getDPIScale())
 	, height(floorf(rasterizer->getHeight() / rasterizer->getDPIScale() + 0.5f))
 	, pixelHeight(rasterizer->getHeight())
 	, lineHeight(1)
@@ -371,7 +371,10 @@ void TextShaper::setFallbacks(const std::vector<Rasterizer*> &fallbacks)
 	for (Rasterizer *r : fallbacks)
 	{
 		if (r->getDataType() != rasterizers[0]->getDataType())
-			throw love::Exception("Font fallbacks must be of the same font type.");
+			throw love::Exception("Font fallbacks must be of the same font type as the primary Font.");
+
+		if (r->getDPIScale() != rasterizers[0]->getDPIScale())
+			throw love::Exception("Font fallbacks must have the same DPI scales as the primary Font.");
 	}
 
 	// Clear caches.
@@ -379,13 +382,9 @@ void TextShaper::setFallbacks(const std::vector<Rasterizer*> &fallbacks)
 	glyphAdvances.clear();
 
 	rasterizers.resize(1);
-	dpiScales.resize(1);
 
 	for (Rasterizer *r : fallbacks)
-	{
 		rasterizers.push_back(r);
-		dpiScales.push_back(r->getDPIScale());
-	}
 }
 
 } // font

src/modules/font/TextShaper.h

@@ -138,7 +138,7 @@ protected:
 	static inline bool isWhitespace(uint32 codepoint) { return codepoint == ' ' || codepoint == '\t'; }
 
 	std::vector<StrongRef<Rasterizer>> rasterizers;
-	std::vector<float> dpiScales;
+	float dpiScale;
 
 private:
 

src/modules/font/freetype/HarfbuzzShaper.cpp

@@ -36,6 +36,24 @@ namespace font
 namespace freetype
 {
 
+static inline float fixed26_6ToFloat(int32 v)
+{
+	return (v >> 6) + (v & 63) / 64.0f;
+}
+
+static int32 floatToFixed26_6(float v)
+{
+	float integer = 0;
+	float frac = modf(v, &integer);
+
+	return (int32)(((uint32)integer << 6) | (uint32)(frac * 64));
+}
+
+static float snapToPixel(float v, float dpiScale, float dpiScaleInverse)
+{
+	return roundf(v * dpiScale) * dpiScaleInverse;
+}
+
 HarfbuzzShaper::HarfbuzzShaper(TrueTypeRasterizer *rasterizer)
 	: TextShaper(rasterizer)
 	, spaceGlyphIndex()
@@ -97,8 +115,8 @@ void HarfbuzzShaper::updateSpacesForTabInfo()
 		{
 			spaceGlyphIndex.index = glyphid;
 			spaceGlyphIndex.rasterizerIndex = i;
-			tabSpacesAdvanceX = hb_font_get_glyph_h_advance(hbfont, glyphid) * SPACES_PER_TAB;
+			tabSpacesAdvanceX = floatToFixed26_6(fixed26_6ToFloat(hb_font_get_glyph_h_advance(hbfont, glyphid)) * SPACES_PER_TAB);
-			tabSpacesAdvanceY = hb_font_get_glyph_v_advance(hbfont, glyphid) * SPACES_PER_TAB;
+			tabSpacesAdvanceY = floatToFixed26_6(fixed26_6ToFloat(hb_font_get_glyph_v_advance(hbfont, glyphid)) * SPACES_PER_TAB);
 			break;
 		}
 	}
@@ -241,6 +259,8 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 		}
 	}
 
+	const float dpiScaleInverse = 1.0f / dpiScale;
+
 	std::vector<BufferRange> bufferranges;
 	computeBufferRanges(codepoints, range, bufferranges);
 
@@ -317,14 +337,18 @@ 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 += (glyphpos.x_offset >> 6) / dpiScales[bufferrange.index];
+				p.position.x += fixed26_6ToFloat(glyphpos.x_offset) * dpiScaleInverse;
-				p.position.y += (glyphpos.y_offset >> 6) / dpiScales[bufferrange.index];
+				p.position.y += fixed26_6ToFloat(glyphpos.y_offset) * dpiScaleInverse;
+
+				// Snap position to the current font's source pixel grid. 
+				p.position.x = snapToPixel(p.position.x, dpiScale, dpiScaleInverse);
+				p.position.y = snapToPixel(p.position.y, dpiScale, dpiScaleInverse);
 
 				positions->push_back(p);
 			}
 
-			curpos.x += (glyphpos.x_advance >> 6) / dpiScales[bufferrange.index];
+			curpos.x += fixed26_6ToFloat(glyphpos.x_advance) * dpiScaleInverse;
-			curpos.y += (glyphpos.y_advance >> 6) / dpiScales[bufferrange.index];
+			curpos.y += fixed26_6ToFloat(glyphpos.y_advance) * dpiScaleInverse;
 
 			// Account for extra spacing given to space characters.
 			if (clustercodepoint == ' ' && extraspacing != 0.0f)
@@ -340,8 +364,8 @@ void HarfbuzzShaper::computeGlyphPositions(const ColoredCodepoints &codepoints,
 
 	if (info != nullptr)
 	{
-		info->width = maxwidth - offset.x;
+		info->width = snapToPixel(maxwidth - offset.x, dpiScale, dpiScaleInverse);
-		info->height = curpos.y - offset.y;
+		info->height = snapToPixel(curpos.y - offset.y, dpiScale, dpiScaleInverse);
 		if (curpos.x > offset.x)
 			info->height += getCombinedHeight();
 	}
@@ -359,6 +383,8 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints &codepoints, Ra
 
 	uint32 prevcodepoint = 0;
 
+	const float dpiScaleInverse = 1.0f / dpiScale;
+
 	std::vector<BufferRange> bufferranges;
 	computeBufferRanges(codepoints, range, bufferranges);
 
@@ -393,7 +419,7 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints &codepoints, Ra
 				glyphpos.y_advance = HB_DIRECTION_IS_VERTICAL(direction) ? tabSpacesAdvanceY : 0;
 			}
 
-			float newwidth = w + (glyphpos.x_advance >> 6) / dpiScales[bufferrange.index];
+			float newwidth = w + fixed26_6ToFloat(glyphpos.x_advance) * dpiScaleInverse;
 
 			// Don't count trailing spaces in the output width.
 			if (isWhitespace(clustercodepoint))
@@ -407,7 +433,7 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints &codepoints, Ra
 					firstindexafterspace = info.cluster;
 
 				// Only wrap when there's a non-space character.
-				if (newwidth > wraplimit)
+				if (snapToPixel(newwidth, dpiScale, dpiScaleInverse) > wraplimit)
 				{
 					// If this is the first character, wrap from the next one instead of this one.
 					int wrapindex = (int)info.cluster > (int)range.first ? (int)info.cluster : (int)range.first + 1;
@@ -420,7 +446,7 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints &codepoints, Ra
 					}
 
 					if (width)
-						*width = outwidth;
+						*width = snapToPixel(outwidth, dpiScale, dpiScaleInverse);
 
 					return wrapindex;
 				}
@@ -434,7 +460,7 @@ int HarfbuzzShaper::computeWordWrapIndex(const ColoredCodepoints &codepoints, Ra
 	}
 
 	if (width)
-		*width = outwidth;
+		*width = snapToPixel(outwidth, dpiScale, dpiScaleInverse);
 
 	// There wasn't any wrap in the middle of the range.
 	return (int) range.last + 1;

src/modules/font/freetype/HarfbuzzShaper.h

@@ -66,8 +66,8 @@ private:
 	std::vector<hb_buffer_t *> hbBuffers;
 
 	GlyphIndex spaceGlyphIndex;
-	int tabSpacesAdvanceX;
+	int32 tabSpacesAdvanceX;
-	int tabSpacesAdvanceY;
+	int32 tabSpacesAdvanceY;
 
 }; // HarfbuzzShaper
 

src/modules/graphics/Font.cpp

@@ -608,12 +608,12 @@ void Font::printf(graphics::Graphics *gfx, const std::vector<love::font::Colored
 	printv(gfx, m, drawcommands, vertices);
 }
 
-int Font::getWidth(const std::string &str)
+float Font::getWidth(const std::string &str)
 {
 	return shaper->getWidth(str);
 }
 
-int Font::getWidth(uint32 glyph)
+float Font::getWidth(uint32 glyph)
 {
 	return shaper->getGlyphAdvance(glyph);
 }

src/modules/graphics/Font.h

@@ -99,12 +99,12 @@ public:
 	 *
 	 * @param str A string of text.
 	 **/
-	int getWidth(const std::string &str);
+	float getWidth(const std::string &str);
 
 	/**
 	 * Returns the width of the passed glyph.
 	 **/
-	int getWidth(uint32 glyph);
+	float getWidth(uint32 glyph);
 
 	/**
 	 * Returns the maximal width of a wrapped string

src/modules/graphics/wrap_Font.cpp

@@ -93,12 +93,12 @@ int w_Font_getWidth(lua_State *L)
 	if (lua_type(L, 2) == LUA_TSTRING)
 	{
 		const char *str = luaL_checkstring(L, 2);
-		luax_catchexcept(L, [&](){ lua_pushinteger(L, t->getWidth(str)); });
+		luax_catchexcept(L, [&](){ lua_pushnumber(L, t->getWidth(str)); });
 	}
 	else
 	{
 		uint32 glyph = (uint32) luaL_checknumber(L, 2);
-		luax_catchexcept(L, [&](){ lua_pushinteger(L, t->getWidth(glyph)); });
+		luax_catchexcept(L, [&](){ lua_pushnumber(L, t->getWidth(glyph)); });
 	}
 	return 1;
 }