feat: Aligned text in the TextBoxComponent #1620

st-pasha · 2022-05-13T06:55:34Z

Description

Added option align in the TextBoxComponent which controls the alignment of text.
Added option for the TextBoxComponent to have a fixed size (before the only mode was for the textbox to automatically expand/shrink to fit the text).

Checklist

The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
I have read the Contributor Guide and followed the process outlined for submitting PRs.
I have updated/added tests for ALL new/updated/fixed functionality.
I have updated/added relevant documentation in docs and added dartdoc comments with ///.
I have updated/added relevant examples in examples.

Breaking Change

[-] Yes, this is a breaking change.
No, this is not a breaking change.

Related Issues

Closes #1088

spydon

Lgtm

doc/flame/rendering/text.md

examples/lib/stories/rendering/text_example.dart

packages/flame/lib/src/components/text_box_component.dart

st-pasha · 2022-05-16T01:36:02Z

@spydon Please take a look at the failing test, I suspect it might be due to differences between Flutter versions on my machine and in the CI pipeline.

spydon · 2022-05-16T07:07:47Z

@spydon Please take a look at the failing test, I suspect it might be due to differences between Flutter versions on my machine and in the CI pipeline.

I would guess that it is due to the classic font rendering problem when using goldens:
https://pub.dev/packages/alchemist#about-platform-tests-vs-ci-tests

erickzanardo

Golden tests involving texts are usually a pitfall because text rendering is influenced by the OS where the test is run, so if we generate a snapshot on linux and then try to run the test on MacOs or windows, there are high possibility that the test will fail.

So IMO we shouldn't use golden tests for these situations.

spydon · 2022-05-16T14:12:20Z

Golden tests involving texts are usually a pitfall because text rendering is influenced by the OS where the test is run, so if we generate a snapshot on linux and then try to run the test on MacOs or windows, there are high possibility that the test will fail.

So IMO we shouldn't use golden tests for these situations.

We can use them, but render boxes instead where the text would have been.
Alchemist for example has a mode for this.
(There are also a few fonts that are specialized for golden testing, but that's more work.)

erickzanardo · 2022-05-16T14:15:02Z

Alchemist for example has a mode for this.
Interesting, would be cool to test this.

st-pasha · 2022-05-16T15:43:48Z

We can use them, but render boxes instead where the text would have been.

Flutter is already doing this for us: packages/flame/test/_goldens/text_box_component_test_1.png.

So the problem here must be something else. There are diff files generated by the test, but I can't see a way to access them from the CI. Could there be an option somewhere to include those files into the archive with the logs?

Another approach could be to run the test on another local machine with a different version of Flutter and see if the test succeeds or not.

Some links still remain broken after flame-engine#1663, as reported by linkcheck. This PR fixes all of them, except these 5: https://examples.flame-engine.org/#/Collision%20Detection_Circles https://examples.flame-engine.org/#/Collision%20Detection_Shapes%20without%20components https://examples.flame-engine.org/#/Collision%20Detection_Multiple%20shapes https://examples.flame-engine.org/#/Rendering_Isometric%20Tile%20Map https://examples.flame-engine.org/#/Controls_Joystick

…ne#1657) As described flame-engine#1546, It would be more convenient and can reduce the game widget updates if adding or removing overlays at once.

…ngine#1659) Make sure that subscription to GameStateChange events is properly updated when the game: property of a GameWidget changes.

changeParent should be allowed even when the current parent is null.

Fix the last remaining broken example links.

This was a planned move once we'd switch to Dart 2.15; and we're already on 2.16 since flame-engine#1617.

…-engine#1675) We were planning to make this change once we upgrade min Dart version to 2.16; which we finally did in flame-engine#1617.

In some places in code we used patterns like fold(0, (a, b) => a + b), which can be replaced with a simple .sum from the Iterable extension.

…lame-engine#1673) These methods can be simplified using the extension Iterable.firstOrNull getter.

spydon · 2022-05-30T06:30:19Z

I hope that after merging #1634 we can switch to sprite fonts here, and there wouldn't be a problem anymore.

Feels a bit like cheating though, since normal fonts aren't tested then (and this will most likely not be the last time we have this problem in the CI).

st-pasha · 2022-05-30T06:52:18Z

I looked at Alchemist documentation, and they say they use Ahem font when rendering in CI, which replaces all glyphs with blocks. However, Flutter itself uses Ahem font for rendering goldens, yet somehow we see a 1-pixel shift. I'm not sure using Alchemist would solve this...

There's also the golden_toolkit package, and the approach they took is to bundle Roboto font together with the tests, and load it directly from there: https://github.com/eBay/flutter_glove_box/blob/9e881929543e1ed634f854897ded903d4bc2f55a/packages/golden_toolkit/lib/src/font_loader.dart I wonder if we can maybe replicate that somehow?

spydon · 2022-05-30T07:46:37Z

I just noticed that all positions have .5 pixels, maybe this wouldn't cause anti-aliasing if the positions were in .0 instead?
(Just to keep the thread straight, this was already tried)

spydon · 2022-06-03T12:48:14Z

@st-pasha Let's put skip: true on the test and open an issue about fixing it, so that this feature can be included in v1.2.0

st-pasha · 2022-06-03T18:06:00Z

So, it looks like the issue is caused by different ways that the fonts are smoothed on different operating systems. I made these zoomed-in screenshots, which show that both OSes perform smoothing, but in a different way:

I've tried to render the text using a paint with antialiasing turned off - but it didn't make any difference, looks like macOS simply doesn't support switching off antialiasing for fonts (except maybe as a global system option, but that would be too disruptive for day-to-day life)...

In view of this, I believe the best we can do is to not rely on system font rendering altogether and use sprite fonts instead (at least for golden files); or wait until such time as Flutter team figures how to make golden tests match if the difference between images is up to anti-aliasing effects.

spydon · 2022-06-03T18:14:48Z

So, it looks like the issue is caused by different ways that the fonts are smoothed on different operating systems. I made these zoomed-in screenshots, which show that both OSes perform smoothing, but in a different way

That is the classic Flutter issue, didn't we post that one here? Otherwise I know @erickzanardo has the link to it somewhere, because he has a workaround for it in that issue.
Alchemist should solve that issue.

spydon · 2022-06-03T18:23:47Z

Here is the issue (Erick digged it up):
flutter/flutter#56383
(apparently the suggested workaround doesn't work all the time, so we should avoid that)

st-pasha · 2022-06-03T19:19:34Z

because he has a workaround for it in that issue.

The suggested "workaround" - to change the font smoothing at system global level - would only work on a computer that is dedicated solely to running golden tests and to nothing else. Because in a day-to-day life font smoothing is a very much desirable feature, there is a reason companies invest money in improving it.

Of course, ideally, Apple would have an API for rendering a specific string without any antialiasing. But apparently they don't have this, and it's not clear that they ever will (even if they do, we don't want tests that run on some version of OS but not on other, and not everyone can upgrade their OS easily).

In view of this, there may be no other option than manual sprite-based fonts. If Alchemist has a work-around, then this is what they must be doing.

spydon · 2022-06-03T19:22:48Z

The suggested "workaround" - to change the font smoothing at system global level - would only work on a computer that is dedicated solely to running golden tests and to nothing else. Because in a day-to-day life font smoothing is a very much desirable feature, there is a reason companies invest money in improving it.

Yeah, sorry I was unclear, I didn't mean that we should use that work around, it was just how I remembered the issue. :)

If Alchemist has a work-around, then this is what they must be doing.

Indeed, let's investigate that post v1.2.0.

erickzanardo · 2022-06-03T19:24:49Z

The suggested "workaround" - to change the font smoothing at system global level - would only work on a computer that is dedicated solely to running golden tests and to nothing else. Because in a day-to-day life font smoothing is a very much desirable feature, there is a reason companies invest money in improving it.

Yeah, that workaround was something that used to work, but since some time isn't accurate anymore. There is only one workaround that I think is to be trusted. Running golden tests on the same environment as the CI will run them. On a multi platform world, docker would be an alternative for that, but that would render a very bad developer experience IMO.

In view of this, there may be no other option than manual sprite-based fonts. If Alchemist has a work-around, then this is what they must be doing.

I am a little bit skeptical about this still, giving the nature of the issue. But I would love to see this in action and would love even more to be proven wrong. Lets try to plan for doing this, even if as a POC for now.

packages/flame_test/lib/src/test_golden.dart

st-pasha and others added 14 commits May 12, 2022 10:25

Added aligned text rendering

98f2b42

docs

3d3405d

update example in docs

dc20318

minor

c089d13

fix: Bug in "tty" TextBoxComponent

041a3cd

_fixedSize property

d46b343

Added a test and an example

fa92c4c

update docs

37ea340

format

7e37444

revert cache.dispose

fc1fe84

golden file

eaaeb5b

Merge branch 'ps/issue-1618' into ps/aligned-text

80ce5d9

Merge branch 'main' into ps/aligned-text

fd71ca9

analysis issue

4186ecb

spydon requested review from spydon, luanpotter, erickzanardo, renancaraujo and wolfenrain May 13, 2022 16:41

spydon approved these changes May 13, 2022

View reviewed changes

doc/flame/rendering/text.md Outdated Show resolved Hide resolved

examples/lib/stories/rendering/text_example.dart Show resolved Hide resolved

packages/flame/lib/src/components/text_box_component.dart Show resolved Hide resolved

st-pasha added 3 commits May 13, 2022 10:03

clean up example

8c9370c

Merge branch 'main' into ps/aligned-text

3da2655

Use testGolden()

210a01f

erickzanardo reviewed May 16, 2022

View reviewed changes

Merge branch 'main' into ps/aligned-text

d3dc79a

st-pasha and others added 11 commits May 29, 2022 15:16

feat: Add ability to add/remove multiple overlays at once (flame-engi…

005ca7b

…ne#1657) As described flame-engine#1546, It would be more convenient and can reduce the game widget updates if adding or removing overlays at once.

fix: Subscription for events when game changes in GameWidget (flame-e…

b52d115

…ngine#1659) Make sure that subscription to GameStateChange events is properly updated when the game: property of a GameWidget changes.

feat: Allow changing parent from null parent (flame-engine#1662)

2750553

changeParent should be allowed even when the current parent is null.

docs: Fix the broken example links (flame-engine#1666)

29dd46c

Fix the last remaining broken example links.

refactor: Deprecate ComponentSet.createDefault() (flame-engine#1676)

70471a7

This was a planned move once we'd switch to Dart 2.15; and we're already on 2.16 since flame-engine#1617.

feat: Keep stacktrace when rethrowing an error from GameWidget (flame…

512c6c6

…-engine#1675) We were planning to make this change once we upgrade min Dart version to 2.16; which we finally did in flame-engine#1617.

refactor: Replace some usages of fold<> with .sum (flame-engine#1670)

94a5717

In some places in code we used patterns like fold(0, (a, b) => a + b), which can be replaced with a simple .sum from the Iterable extension.

refactor: Simplify Component.firstChild, .lastChild, and .findParent (f…

4c59026

…lame-engine#1673) These methods can be simplified using the extension Iterable.firstOrNull getter.

try to update a test

7c1f6c4

Merge branch 'main' into ps/aligned-text

4746ea4

st-pasha added 3 commits June 3, 2022 10:43

remove half-pixels

855ede1

Merge branch 'main' into ps/aligned-text

02da3a9

mark test as skipped

b05ec8a

Merge branch 'main' into ps/aligned-text

1e5c28d

spydon reviewed Jun 3, 2022

View reviewed changes

packages/flame_test/lib/src/test_golden.dart Outdated Show resolved Hide resolved

skip

150de2c

erickzanardo approved these changes Jun 3, 2022

View reviewed changes

spydon merged commit c64aeda into flame-engine:main Jun 3, 2022

st-pasha deleted the ps/aligned-text branch June 3, 2022 21:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Aligned text in the TextBoxComponent #1620

feat: Aligned text in the TextBoxComponent #1620

st-pasha commented May 13, 2022 •

edited

Loading

spydon left a comment

st-pasha commented May 16, 2022

spydon commented May 16, 2022

erickzanardo left a comment

spydon commented May 16, 2022

erickzanardo commented May 16, 2022

st-pasha commented May 16, 2022

spydon commented May 30, 2022

st-pasha commented May 30, 2022

spydon commented May 30, 2022 •

edited

Loading

spydon commented Jun 3, 2022

st-pasha commented Jun 3, 2022

spydon commented Jun 3, 2022

spydon commented Jun 3, 2022

st-pasha commented Jun 3, 2022

spydon commented Jun 3, 2022

erickzanardo commented Jun 3, 2022

feat: Aligned text in the TextBoxComponent #1620

feat: Aligned text in the TextBoxComponent #1620

Conversation

st-pasha commented May 13, 2022 • edited Loading

Description

Checklist

Breaking Change

Related Issues

spydon left a comment

Choose a reason for hiding this comment

st-pasha commented May 16, 2022

spydon commented May 16, 2022

erickzanardo left a comment

Choose a reason for hiding this comment

spydon commented May 16, 2022

erickzanardo commented May 16, 2022

st-pasha commented May 16, 2022

spydon commented May 30, 2022

st-pasha commented May 30, 2022

spydon commented May 30, 2022 • edited Loading

spydon commented Jun 3, 2022

st-pasha commented Jun 3, 2022

spydon commented Jun 3, 2022

spydon commented Jun 3, 2022

st-pasha commented Jun 3, 2022

spydon commented Jun 3, 2022

erickzanardo commented Jun 3, 2022

st-pasha commented May 13, 2022 •

edited

Loading

spydon commented May 30, 2022 •

edited

Loading