Skip to content

Bugfix/text trunctaion for textnode2#553

Closed
wsdwsd0829 wants to merge 3 commits intoTextureGroup:masterfrom
wsdwsd0829:bugfix/text-trunctaion-for-textnode2
Closed

Bugfix/text trunctaion for textnode2#553
wsdwsd0829 wants to merge 3 commits intoTextureGroup:masterfrom
wsdwsd0829:bugfix/text-trunctaion-for-textnode2

Conversation

@wsdwsd0829
Copy link
Copy Markdown
Contributor

[ASTextLayout layoutWithContainer:container text:text]; is called for same text with different container size for a complete layout pass.

The existing caching mechanism is over confident.

For example "A long string that happened to fit whole screen” is set as layout result when container is full screen size on first round.

Later another narrower width constraint size is tried for same text. This should trigger an new layout calculation while previous cached is sized returned instead;

This fix will only return cached result only when constraint size are same for different layouts.

Another side notes: the truncations rules are remove the last word that caused truncation and apply “…” to the previous word of that word; (This make sense in case last word is less than 2 chars which will cause “…” overflow.) The downside however is that if the last word is long and removed, the result rendered would look much shorter then expected.

@wsdwsd0829
Copy link
Copy Markdown
Contributor Author

@appleguy

@ghost
Copy link
Copy Markdown

ghost commented Sep 6, 2017

1 Warning
⚠️ Any source code changes should have an entry in CHANGELOG.md or have #trivial in their title.

Generated by 🚫 Danger

Copy link
Copy Markdown
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this is the right fix. Review the comments a few lines up – CoreText will not always return a container size that exactly matches our constrained size, so we need to treat the "gap" there as a range.

Later another narrower width constraint size is tried for same text. This should trigger an new layout calculation while previous cached is sized returned instead

We will only return the previous cached result if the second constrained size falls between the prior constrained size and the value returned by CoreText. So a remeasurement will result in the same value.

What am I not seeing here?

@wsdwsd0829
Copy link
Copy Markdown
Contributor Author

wsdwsd0829 commented Sep 14, 2017

Hi I made a sample to reproduce the problem.
https://github.com/wsdwsd0829/Texture/tree/bugfix/text-node-2-example
examples/TextNodeExample/Sample/TextCellNode.m the yoga layout works for ASTextNode but not ASTextNode2. I printed out cache related size for comparisons.
My fix may not be best, let me know how we can better fix the problem. Thanks.
image
tested on iPhone 7 Plus

@nguyenhuy
Copy link
Copy Markdown
Member

@Adlai-Holler @wsdwsd0829 Please follow up on this. Thanks!

@Adlai-Holler
Copy link
Copy Markdown
Member

@wsdwsd0829 This diff was actually landed incidentally in #918. I've created a followup fix that retains the correctness win but reduces the performance impact. Sorry for taking so long to really dig into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants