Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix iOS 11 text rendering issue #364

Merged
merged 2 commits into from
Mar 12, 2018
Merged

Fix iOS 11 text rendering issue #364

merged 2 commits into from
Mar 12, 2018

Conversation

0xpablo
Copy link
Contributor

@0xpablo 0xpablo commented Sep 27, 2017

It looks like on iOS 11, when a text bubble has a carriage return, it renders blank like this:

It looks like Chatto is setting allowsNonContiguousLayout to true on the text view. This option is for large documents when you only want to render part of it (you can read more about this on the "Noncontiguous Layout" section of NSLayoutManager.

It is stated that if you use that option, you are in charge of telling the layout manager to generate the glyphs for a given range.

We have 2 options:

  • Use one of the ensureGlyph(...) functions.
  • Stop using allowsNonContiguousLayout.

I think it makes no sense to use the ensureGlyph option as this is for only generating a given range. Bubbles in general are needed to be fully rendered and determining the range to render a given large bubble is complex and I believe out of the scope of ChattoAdditions. This PR then simply deletes the line that sets allowsNonContiguousLayout to true.

allowsNonContiguousLayout should only be used for large documents, and, if so, the ensureGlyph(...) functions must be called. Since we don't deal with large documents, we don't need this option.
An optimization on iOS 11 made the text not to render on some circumstances (for instance when there is a carriage return).
@codecov-io
Copy link

codecov-io commented Sep 27, 2017

Codecov Report

Merging #364 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #364      +/-   ##
==========================================
+ Coverage   63.62%   63.64%   +0.01%     
==========================================
  Files          77       77              
  Lines        3929     3928       -1     
==========================================
  Hits         2500     2500              
+ Misses       1429     1428       -1
Impacted Files Coverage Δ
...Chat Items/TextMessages/Views/TextBubbleView.swift 74.15% <ø> (+0.41%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e4b1a7...5a14c29. Read the comment docs.

@AntonPalich
Copy link
Contributor

@0xpablo I'll do performance tests on slow devices for both iOS 10 and 11. I have no idea why it was set to true from the beginning, but I believe it was related to rendering performance. @diegosanchezr Do you remember anything about it?

@diegosanchezr
Copy link
Contributor

I don't recall anything in particular. It's just probably there with hopes of better performance as it was working fine with it at that time.

@AntonPalich AntonPalich merged commit b7f3cd2 into badoo:master Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants