From 28fd2a0f0f1ea04872d0c4e8b674c8ce7bca69ee Mon Sep 17 00:00:00 2001 From: Luan Nico Date: Sun, 23 Jun 2024 07:07:45 -0700 Subject: [PATCH] fix: Fix text rendering issue where spaces are missing (#3192) Fix text rendering issue where spaces end up missing. What happens is that if a single paragraph is split into multiple spans, due to having different formatting, for example, the layouting engine will run into an edge case when trying to place the last span. It will: * split it and try to put each word at a time with the preceding space * it will eventually find a word that cannot fit, and return to a partial state * however, the layouting engine then keeps going and try the same element again. that is because there could be more elements, and the engine conflates bits and elements * now the element "starts" at the next word, and suddenly it fits, because a new space is not added. this only happens if the final word is such that it fits by itself but doesn't with the leading space. That is not normally an issue because if it doesn't fit, it will go to the next line and the space becomes unnecessary in that case. But in case we are still trying to populate the same line, we need to make sure we don't "eat up" the space. There could be other similar edge cases where a space could go missing, so I opted to make it explicit whether the initial space should be added or not by adding a parameter to determine if it is the start of a new line or not. --- .github/.cspell/words_dictionary.txt | 1 + .../lib/src/text/nodes/group_text_node.dart | 11 +++-- .../lib/src/text/nodes/inline_text_node.dart | 5 ++- .../lib/src/text/nodes/plain_text_node.dart | 8 +++- .../lib/src/text/nodes/text_block_node.dart | 5 ++- .../flame/test/_goldens/text_layouting_1.png | Bin 0 -> 371 bytes .../flame/test/text/text_layouting_test.dart | 39 ++++++++++++++++++ 7 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 packages/flame/test/_goldens/text_layouting_1.png create mode 100644 packages/flame/test/text/text_layouting_test.dart diff --git a/.github/.cspell/words_dictionary.txt b/.github/.cspell/words_dictionary.txt index 600fdbe2e99..0d203dda131 100644 --- a/.github/.cspell/words_dictionary.txt +++ b/.github/.cspell/words_dictionary.txt @@ -12,6 +12,7 @@ grayscale hoverable hoverables inactives +layouting microtask orientable platformer diff --git a/packages/flame/lib/src/text/nodes/group_text_node.dart b/packages/flame/lib/src/text/nodes/group_text_node.dart index 34fecc298b1..725c149349d 100644 --- a/packages/flame/lib/src/text/nodes/group_text_node.dart +++ b/packages/flame/lib/src/text/nodes/group_text_node.dart @@ -31,7 +31,10 @@ class _GroupTextLayoutBuilder extends TextNodeLayoutBuilder { bool get isDone => _currentChildIndex == node.children.length; @override - InlineTextElement? layOutNextLine(double availableWidth) { + InlineTextElement? layOutNextLine( + double availableWidth, { + required bool isStartOfLine, + }) { assert(!isDone); final out = []; var usedWidth = 0.0; @@ -45,8 +48,10 @@ class _GroupTextLayoutBuilder extends TextNodeLayoutBuilder { } _currentChildBuilder ??= node.children[_currentChildIndex].layoutBuilder; - final maybeLine = - _currentChildBuilder!.layOutNextLine(availableWidth - usedWidth); + final maybeLine = _currentChildBuilder!.layOutNextLine( + availableWidth - usedWidth, + isStartOfLine: isStartOfLine && out.isEmpty, + ); if (maybeLine == null) { break; } else { diff --git a/packages/flame/lib/src/text/nodes/inline_text_node.dart b/packages/flame/lib/src/text/nodes/inline_text_node.dart index c6aa4fe7642..8e49f7842a0 100644 --- a/packages/flame/lib/src/text/nodes/inline_text_node.dart +++ b/packages/flame/lib/src/text/nodes/inline_text_node.dart @@ -21,6 +21,9 @@ abstract class InlineTextNode extends TextNode { } abstract class TextNodeLayoutBuilder { - InlineTextElement? layOutNextLine(double availableWidth); + InlineTextElement? layOutNextLine( + double availableWidth, { + required bool isStartOfLine, + }); bool get isDone; } diff --git a/packages/flame/lib/src/text/nodes/plain_text_node.dart b/packages/flame/lib/src/text/nodes/plain_text_node.dart index a02fcaeee7f..bf319829072 100644 --- a/packages/flame/lib/src/text/nodes/plain_text_node.dart +++ b/packages/flame/lib/src/text/nodes/plain_text_node.dart @@ -31,11 +31,15 @@ class _PlainTextLayoutBuilder extends TextNodeLayoutBuilder { bool get isDone => index1 > words.length; @override - InlineTextElement? layOutNextLine(double availableWidth) { + InlineTextElement? layOutNextLine( + double availableWidth, { + required bool isStartOfLine, + }) { InlineTextElement? tentativeLine; int? tentativeIndex0; while (index1 <= words.length) { - final textPiece = words.sublist(index0, index1).join(' '); + final prependSpace = index0 == 0 || isStartOfLine ? '' : ' '; + final textPiece = prependSpace + words.sublist(index0, index1).join(' '); final formattedPiece = renderer.format(textPiece); if (formattedPiece.metrics.width > availableWidth) { break; diff --git a/packages/flame/lib/src/text/nodes/text_block_node.dart b/packages/flame/lib/src/text/nodes/text_block_node.dart index a663c4a70b1..3ea8dfe5e13 100644 --- a/packages/flame/lib/src/text/nodes/text_block_node.dart +++ b/packages/flame/lib/src/text/nodes/text_block_node.dart @@ -30,7 +30,10 @@ abstract class TextBlockNode extends BlockNode { var verticalOffset = style.padding.top; final textAlign = style.textAlign ?? TextAlign.left; while (!layoutBuilder.isDone) { - final element = layoutBuilder.layOutNextLine(contentWidth); + final element = layoutBuilder.layOutNextLine( + contentWidth, + isStartOfLine: true, + ); if (element == null) { // Not enough horizontal space to lay out. For now we just stop the // layout altogether cutting off the remainder of the content. But is diff --git a/packages/flame/test/_goldens/text_layouting_1.png b/packages/flame/test/_goldens/text_layouting_1.png new file mode 100644 index 0000000000000000000000000000000000000000..e168ba7d72a7adaa2161ba7946f3a0e566327c58 GIT binary patch literal 371 zcmeAS@N?(olHy`uVBq!ia0vp^DImPgg&ebxsLQ04pzgC;$Ke literal 0 HcmV?d00001 diff --git a/packages/flame/test/text/text_layouting_test.dart b/packages/flame/test/text/text_layouting_test.dart new file mode 100644 index 00000000000..dc6220507cc --- /dev/null +++ b/packages/flame/test/text/text_layouting_test.dart @@ -0,0 +1,39 @@ +import 'package:flame/components.dart'; +import 'package:flame/text.dart'; +import 'package:flame_test/flame_test.dart'; +import 'package:flutter/rendering.dart'; +import 'package:test/test.dart'; + +final _size = Vector2(100, 100); + +void main() { + group('text layouting', () { + testGolden( + 'Text is properly laid out across multiple lines', + (game) async { + game.addAll([ + RectangleComponent( + size: _size, + paint: Paint()..color = const Color(0xffcfc6e5), + ), + TextElementComponent.fromDocument( + document: DocumentRoot([ + ParagraphNode.group( + ['012345', '67 89'].map(PlainTextNode.new).toList(), + ), + ]), + style: DocumentStyle( + text: InlineTextStyle( + // using DebugTextRenderer, this will make each char 10px wide + fontSize: 10, + ), + ), + size: _size, + ), + ]); + }, + goldenFile: '../_goldens/text_layouting_1.png', + size: _size, + ); + }); +}