Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

Smooth scrolling! #1058

Closed
wants to merge 6 commits into from
Closed

Conversation

gblazex
Copy link

@gblazex gblazex commented Jun 29, 2015

I made scrolling perfect on my iPhone 5s without removing auto-layout.

I commented on every commit.

Basically the changes in order of their importance:

  • textView.scrollEnabled=NO causes scrolling problems for some reason, but it can be safely disabled because the layout takes care of that
  • NSDateFormatter optimized by having separate Date & Time formatters and not changing the style of a single formatter all the time.
  • make sure we don’t cause unnecessary work with setText=nil upon cell reuse
  • remove forced rasterization to unburden the CPU
  • move default dataDetector setting to textView.awakeFromNib instead of cellForRowAtIndexPath so library users can have better control

The biggest bottleneck left seems to be dataDetectors as they heavily block the main thread.

When testing this PR I advise you to disable them to see the smoothness achieved by these commits (without being affected by dataDetectors).

Maybe later I’ll come up with something to make that part faster as well.

With accurate layout scrolling will not be possible anyway.
When reusing the cell setting the text to nil and then to the correct
value causes unnecessary work (measured) that we don’t have if we want
to stay inside the frame budget.
by separating Date & Time part formatting into separate NSDateFormatter
instances
If anything this hurts performance because janky scrolling is CPU bound
(running out of the frame budget).

shouldRasterize makes the GPU do less work by putting more work on the
CPU with a forced rasterization step. This library doesn’t have any use
for it currently.
This makes sure that we are not enabling dataDetectors in every
cellAtIndexPath method call even if the library user wants to disable
it.

When creating it we set the default setting, then the user can do
whatever he wants with it.

Also there’s no reason to reset the dataDetectors to None on reuse, as
the cell probably wants to use the previous settings. If not the
library user should explicitly change it anyway.
Although not sure why string and attributedString representations of a
timestamp should be different.
@gblazex gblazex changed the title Smooth scrolling Smooth scrolling! Jun 29, 2015
@gblazex
Copy link
Author

gblazex commented Jun 30, 2015

Combined with TTTAttributedLabel (which has way faster rendering and does data detection on a background thread) I managed to eliminate ALL jankiness. Seriouosly, it flies.

I think it's a good call to switch to that for 8.0

@gblazex
Copy link
Author

gblazex commented Jul 1, 2015

Just created a PR for TTTAttributedLabel that will make it fly even more with data detectors.

Just look at these number UITextView (1.662s) vs TTT (0.084s)

datadetector

@jessesquires
Copy link
Owner

thanks @galambalazs!

wow - UITextView 😢

@gblazex
Copy link
Author

gblazex commented Jul 2, 2015

Good news:

I managed to implement a version of TTTLabel which allows the native data detector actions to appear.
(Currently you'd have to implement them yourself, plus localize etc...)

Jesse: there are two quick demo projects and two videos in the smooth scrolling issue thread.
Please do take a look at the performance characteristics show in those.

i'm now 100% sure TTT is the way to go.

I'll probably break this PR up to separate small ones and 1-2 may be merged so 7.x branch gets somewhat faster. But 8.0 is TTT all the way.

juliensaad added a commit to juliensaad/JSQMessagesViewController that referenced this pull request Oct 16, 2015
@benjaminhallock
Copy link

I've taken some of these quick fixes and tried to apply to my project. I would really like to see this go somewhere....

@gblazex
Copy link
Author

gblazex commented Nov 4, 2015

Still waiting for @jessesquires to decide what's the plan for 8.0

The biggest change would come from switching to TTTAttributedLabel

In that case only the date formatter is relevant from here.

Maybe I'll add a separate PR for that's easy to merge.

juliensaad added a commit to TransitApp/JSQMessagesViewController that referenced this pull request Feb 1, 2016
@jessesquires jessesquires added this to the 8.0.0 milestone Jul 13, 2016
@jessesquires
Copy link
Owner

Hello everyone!

I'm sorry to inform the community that I'm officially deprecating this project. 😢 Please read my blog post for details:

http://www.jessesquires.com/blog/officially-deprecating-jsqmessagesviewcontroller/

Thus, I'm closing all issues and pull requests and making the necessary updates to formally deprecate the library. I'm sorry if this is unexpected or disappointing. Please know that this was an extremely difficult decision to make. I'd like to thank everyone here for contributing and making this project so great. It was a fun 4 years. 😊

Thanks for understanding,
— jsq

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants