Skip to content

Commit

Permalink
fix: Fix text rendering issue where spaces are missing (#3192)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
luanpotter committed Jun 23, 2024
1 parent abd43de commit 28fd2a0
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 7 deletions.
1 change: 1 addition & 0 deletions .github/.cspell/words_dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ grayscale
hoverable
hoverables
inactives
layouting
microtask
orientable
platformer
Expand Down
11 changes: 8 additions & 3 deletions packages/flame/lib/src/text/nodes/group_text_node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <InlineTextElement>[];
var usedWidth = 0.0;
Expand All @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion packages/flame/lib/src/text/nodes/inline_text_node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ abstract class InlineTextNode extends TextNode<InlineTextStyle> {
}

abstract class TextNodeLayoutBuilder {
InlineTextElement? layOutNextLine(double availableWidth);
InlineTextElement? layOutNextLine(
double availableWidth, {
required bool isStartOfLine,
});
bool get isDone;
}
8 changes: 6 additions & 2 deletions packages/flame/lib/src/text/nodes/plain_text_node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion packages/flame/lib/src/text/nodes/text_block_node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Binary file added packages/flame/test/_goldens/text_layouting_1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
39 changes: 39 additions & 0 deletions packages/flame/test/text/text_layouting_test.dart
Original file line number Diff line number Diff line change
@@ -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,
);
});
}

0 comments on commit 28fd2a0

Please sign in to comment.